Skip to content

Commit b015609

Browse files
fix(test): replace time.Sleep synchronization with deterministic polling
- ScheduleRetransmissions: register onTick handler synchronously instead of in a goroutine; onTick is a fast mutex+map-insert and has no reason to be async, and the goroutine created a registration race when tests sent ticks before the handler was installed - retransmission_test: drop pre-tick yield sleeps (no longer needed) and replace 50ms post-tick sleeps with polling loops bounded by 2s deadline - node_test: add waitForDispatcherIdle helper that polls until the wallet dispatcher is idle; replace all time.Sleep(200ms/50ms) wait-for-goroutine patterns with the helper - signing_done: replace IIFE Lock/defer-Unlock pattern in write path with explicit Lock/Unlock -- single-statement body has no early return, so the closure adds indirection without benefit
1 parent 6065ba6 commit b015609

4 files changed

Lines changed: 45 additions & 36 deletions

File tree

pkg/net/retransmission/retransmission.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,13 @@ func ScheduleRetransmissions(
2929
retransmit RetransmitFn,
3030
strategy Strategy,
3131
) {
32-
go func() {
33-
ticker.onTick(ctx, func() {
34-
go func() {
35-
if err := strategy.Tick(retransmit); err != nil {
36-
logger.Errorf("could not retransmit message: [%v]", err)
37-
}
38-
}()
39-
})
40-
}()
32+
ticker.onTick(ctx, func() {
33+
go func() {
34+
if err := strategy.Tick(retransmit); err != nil {
35+
logger.Errorf("could not retransmit message: [%v]", err)
36+
}
37+
}()
38+
})
4139
}
4240

4341
// WithRetransmissionSupport takes the standard network message handler and

pkg/net/retransmission/retransmission_test.go

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -129,15 +129,15 @@ func TestScheduleRetransmissions_WithBackoffStrategy(t *testing.T) {
129129
WithBackoffStrategy(),
130130
)
131131

132-
// ScheduleRetransmissions registers its onTick handler in a goroutine;
133-
// yield briefly so that goroutine runs before we start sending ticks.
134-
time.Sleep(10 * time.Millisecond)
135-
136132
// BackoffStrategy fires at ticks 1, 3, 6, 11, 20 -- 5 fires in 20 ticks.
137133
for i := uint64(1); i <= 20; i++ {
138134
ticks <- i
139135
}
140-
time.Sleep(50 * time.Millisecond)
136+
137+
deadline := time.Now().Add(2 * time.Second)
138+
for atomic.LoadUint64(&retransmissions) < 5 && time.Now().Before(deadline) {
139+
time.Sleep(time.Millisecond)
140+
}
141141

142142
got := atomic.LoadUint64(&retransmissions)
143143
if got != 5 {
@@ -167,12 +167,18 @@ func TestScheduleRetransmissions_LogsRetransmitError(t *testing.T) {
167167
WithStandardStrategy(),
168168
)
169169

170-
// Allow the registration goroutine inside ScheduleRetransmissions to call
171-
// onTick before we send the first tick.
172-
time.Sleep(10 * time.Millisecond)
173-
174170
ticks <- 1
175-
time.Sleep(50 * time.Millisecond)
171+
172+
deadline := time.Now().Add(2 * time.Second)
173+
for time.Now().Before(deadline) {
174+
logger.mu.Lock()
175+
n := len(logger.errors)
176+
logger.mu.Unlock()
177+
if n > 0 {
178+
break
179+
}
180+
time.Sleep(time.Millisecond)
181+
}
176182

177183
logger.mu.Lock()
178184
errs := logger.errors

pkg/tbtc/node_test.go

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -520,8 +520,7 @@ func TestNode_HandleHeartbeatProposal_DispatchesAction(t *testing.T) {
520520

521521
n.handleHeartbeatProposal(signer.wallet, &HeartbeatProposal{Message: [16]byte{0x03}}, 10, 100)
522522

523-
// Allow the dispatched goroutine to run and clean up.
524-
time.Sleep(200 * time.Millisecond)
523+
waitForDispatcherIdle(t, n)
525524

526525
if count := dispatchedActionsCount(n); count != 0 {
527526
t.Errorf(
@@ -587,8 +586,7 @@ func TestNode_HandleDepositSweepProposal_DispatchesAction(t *testing.T) {
587586
100,
588587
)
589588

590-
// Allow the dispatched goroutine to run and clean up.
591-
time.Sleep(200 * time.Millisecond)
589+
waitForDispatcherIdle(t, n)
592590

593591
if count := dispatchedActionsCount(n); count != 0 {
594592
t.Errorf(
@@ -654,8 +652,7 @@ func TestNode_HandleRedemptionProposal_DispatchesAction(t *testing.T) {
654652
100,
655653
)
656654

657-
// Allow the dispatched goroutine to run and clean up.
658-
time.Sleep(200 * time.Millisecond)
655+
waitForDispatcherIdle(t, n)
659656

660657
if count := dispatchedActionsCount(n); count != 0 {
661658
t.Errorf(
@@ -716,8 +713,7 @@ func TestNode_HandleMovingFundsProposal_DispatchesAction(t *testing.T) {
716713

717714
n.handleMovingFundsProposal(signer.wallet, &MovingFundsProposal{}, 10, 100)
718715

719-
// Allow the dispatched goroutine to run and clean up.
720-
time.Sleep(200 * time.Millisecond)
716+
waitForDispatcherIdle(t, n)
721717

722718
if count := dispatchedActionsCount(n); count != 0 {
723719
t.Errorf(
@@ -783,8 +779,7 @@ func TestNode_HandleMovedFundsSweepProposal_DispatchesAction(t *testing.T) {
783779
100,
784780
)
785781

786-
// Allow the dispatched goroutine to run and clean up.
787-
time.Sleep(200 * time.Millisecond)
782+
waitForDispatcherIdle(t, n)
788783

789784
if count := dispatchedActionsCount(n); count != 0 {
790785
t.Errorf(
@@ -831,8 +826,7 @@ func TestProcessCoordinationResult_HeartbeatRoutesToHandler(t *testing.T) {
831826

832827
processCoordinationResult(n, result)
833828

834-
// Allow dispatched goroutine to complete.
835-
time.Sleep(50 * time.Millisecond)
829+
waitForDispatcherIdle(t, n)
836830

837831
// Dispatcher should be idle; a panicking handler would have made this fail.
838832
if count := dispatchedActionsCount(n); count != 0 {
@@ -1242,6 +1236,19 @@ func dispatchedActionsCount(n *node) int {
12421236
return len(n.walletDispatcher.actions)
12431237
}
12441238

1239+
// waitForDispatcherIdle polls until walletDispatcher has no active actions or
1240+
// the 2-second deadline is exceeded.
1241+
func waitForDispatcherIdle(t *testing.T, n *node) {
1242+
t.Helper()
1243+
deadline := time.Now().Add(2 * time.Second)
1244+
for dispatchedActionsCount(n) > 0 {
1245+
if time.Now().After(deadline) {
1246+
t.Fatal("timed out waiting for walletDispatcher to become idle")
1247+
}
1248+
time.Sleep(time.Millisecond)
1249+
}
1250+
}
1251+
12451252
// createMockSigner creates a mock signer instance that can be used for
12461253
// test cases that needs a placeholder signer. The produced signer cannot
12471254
// be used to test actual signing scenarios.

pkg/tbtc/signing_done.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -117,11 +117,9 @@ func (sdc *signingDoneCheck) listen(
117117
continue
118118
}
119119

120-
func() {
121-
sdc.doneSignersMutex.Lock()
122-
defer sdc.doneSignersMutex.Unlock()
123-
sdc.doneSigners[doneMessage.senderID] = doneMessage
124-
}()
120+
sdc.doneSignersMutex.Lock()
121+
sdc.doneSigners[doneMessage.senderID] = doneMessage
122+
sdc.doneSignersMutex.Unlock()
125123

126124
case <-sdc.receiveCtx.Done():
127125
return

0 commit comments

Comments
 (0)