Skip to content

Commit 09f5542

Browse files
authored
!feat: Call.Release takes context; add MustRelease (#17)
**BREAKING CHANGE** Adds a `context.Context` to `(*Call.).Release()` and a new `MustRelease()`, since releasing a call can be blocking. Use like ``` err := call.Release(ctx) if err != nil { t.Error(err.Error()) } ``` or more succinctly ``` call.MustRelease(ctx) ``` This, combined with a per-test timeout context, should make it much easier to debug issues if you have a call that gets trapped by more than one trap.
1 parent 49f235b commit 09f5542

File tree

4 files changed

+73
-26
lines changed

4 files changed

+73
-26
lines changed

README.md

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ func TestTrap(t *testing.T) {
222222
count++
223223
})
224224
call := trap.MustWait(ctx)
225-
call.Release()
225+
call.MustRelease(ctx)
226226
if call.Duration != time.Hour {
227227
t.Fatal("wrong duration")
228228
}
@@ -268,15 +268,15 @@ func TestTrap2(t *testing.T) {
268268
}(mClock)
269269

270270
// start
271-
trap.MustWait(ctx).Release()
271+
trap.MustWait(ctx).MustRelease(ctx)
272272
// phase 1
273273
call := trap.MustWait(ctx)
274274
mClock.Advance(3*time.Second).MustWait(ctx)
275-
call.Release()
275+
call.MustRelease(ctx)
276276
// phase 2
277277
call = trap.MustWait(ctx)
278278
mClock.Advance(5*time.Second).MustWait(ctx)
279-
call.Release()
279+
call.MustRelease(ctx)
280280

281281
<-done
282282
// Now logs contains []string{"Phase 1 took 3s", "Phase 2 took 5s"}
@@ -302,7 +302,7 @@ go func(){
302302
}()
303303
call := trap.MustWait(ctx)
304304
mClock.Advance(time.Second).MustWait(ctx)
305-
call.Release()
305+
call.MustRelease(ctx)
306306
// call.Tags contains []string{"foo", "bar"}
307307

308308
gotFoo := <-foo // 1s after start
@@ -478,8 +478,8 @@ func TestTicker(t *testing.T) {
478478
trap := mClock.Trap().TickerFunc()
479479
defer trap.Close() // stop trapping at end
480480
go runMyTicker(mClock) // async calls TickerFunc()
481-
call := trap.Wait(context.Background()) // waits for a call and blocks its return
482-
call.Release() // allow the TickerFunc() call to return
481+
call := trap.MustWait(context.Background()) // waits for a call and blocks its return
482+
call.MustRelease(ctx) // allow the TickerFunc() call to return
483483
// optionally check the duration using call.Duration
484484
// Move the clock forward 1 tick
485485
mClock.Advance(time.Second).MustWait(context.Background())
@@ -527,9 +527,9 @@ go func(clock quartz.Clock) {
527527
measurement = clock.Since(start)
528528
}(mClock)
529529

530-
c := trap.Wait(ctx)
530+
c := trap.MustWait(ctx)
531531
mClock.Advance(5*time.Second)
532-
c.Release()
532+
c.MustRelease(ctx)
533533
```
534534

535535
We wait until we trap the `clock.Since()` call, which implies that `clock.Now()` has completed, then
@@ -617,10 +617,10 @@ func TestInactivityTimer_Late(t *testing.T) {
617617

618618
// Trigger the AfterFunc
619619
w := mClock.Advance(10*time.Minute)
620-
c := trap.Wait(ctx)
620+
c := trap.MustWait(ctx)
621621
// Advance the clock a few ms to simulate a busy system
622622
mClock.Advance(3*time.Millisecond)
623-
c.Release() // Until() returns
623+
c.MustRelease(ctx) // Until() returns
624624
w.MustWait(ctx) // Wait for the AfterFunc to wrap up
625625

626626
// Assert that the timeoutLocked() function was called

example_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func TestExampleTickerFunc(t *testing.T) {
6565
// it's good practice to release calls before any possible t.Fatal() calls
6666
// so that we don't leave dangling goroutines waiting for the call to be
6767
// released.
68-
call.Release()
68+
call.MustRelease(ctx)
6969
if call.Duration != time.Hour {
7070
t.Fatal("unexpected duration")
7171
}
@@ -122,7 +122,7 @@ func TestExampleLatencyMeasurer(t *testing.T) {
122122
w := mClock.Advance(10 * time.Second) // triggers first tick
123123
c := trap.MustWait(ctx) // call to Since()
124124
mClock.Advance(33 * time.Millisecond)
125-
c.Release()
125+
c.MustRelease(ctx)
126126
w.MustWait(ctx)
127127

128128
if l := lm.LastLatency(); l != 33*time.Millisecond {
@@ -133,7 +133,7 @@ func TestExampleLatencyMeasurer(t *testing.T) {
133133
d, w2 := mClock.AdvanceNext()
134134
c = trap.MustWait(ctx)
135135
mClock.Advance(17 * time.Millisecond)
136-
c.Release()
136+
c.MustRelease(ctx)
137137
w2.MustWait(ctx)
138138

139139
expectedD := 10*time.Second - 33*time.Millisecond

mock.go

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,7 @@ func (c clockFunction) String() string {
589589
case clockFunctionUntil:
590590
return "Until"
591591
default:
592-
return "?????"
592+
return fmt.Sprintf("Unknown clockFunction(%d)", c)
593593
}
594594
}
595595

@@ -633,7 +633,7 @@ func (a *apiCall) String() string {
633633
case clockFunctionUntil:
634634
return fmt.Sprintf("Until(%s, %v)", a.Time, a.Tags)
635635
default:
636-
return "?????"
636+
return fmt.Sprintf("Unknown clockFunction(%d)", a.fn)
637637
}
638638
}
639639

@@ -643,14 +643,38 @@ type Call struct {
643643
Duration time.Duration
644644
Tags []string
645645

646+
tb testing.TB
646647
apiCall *apiCall
647648
trap *Trap
648649
}
649650

650-
func (c *Call) Release() {
651+
// Release the call and wait for it to complete. If the provided context expires before the call completes, it returns
652+
// an error.
653+
//
654+
// IMPORTANT: If a call is trapped by more than one trap, they all must release the call before it can complete, and
655+
// they must do so from different goroutines.
656+
func (c *Call) Release(ctx context.Context) error {
651657
c.apiCall.releases.Done()
652-
<-c.apiCall.complete
658+
select {
659+
case <-ctx.Done():
660+
return fmt.Errorf("timed out waiting for release; did more than one trap capture the call?: %w", ctx.Err())
661+
case <-c.apiCall.complete:
662+
// OK
663+
}
653664
c.trap.callReleased()
665+
return nil
666+
}
667+
668+
// MustRelease releases the call and waits for it to complete. If the provided context expires before the call
669+
// completes, it fails the test.
670+
//
671+
// IMPORTANT: If a call is trapped by more than one trap, they all must release the call before it can complete, and
672+
// they must do so from different goroutines.
673+
func (c *Call) MustRelease(ctx context.Context) {
674+
if err := c.Release(ctx); err != nil {
675+
c.tb.Helper()
676+
c.tb.Fatal(err.Error())
677+
}
654678
}
655679

656680
func withTime(t time.Time) callArg {
@@ -745,6 +769,7 @@ func (t *Trap) Wait(ctx context.Context) (*Call, error) {
745769
Tags: a.Tags,
746770
apiCall: a,
747771
trap: t,
772+
tb: t.mock.tb,
748773
}
749774
t.mu.Lock()
750775
defer t.mu.Unlock()

mock_test.go

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func TestTimer_NegativeDuration(t *testing.T) {
2424
timers <- mClock.NewTimer(-time.Second)
2525
}()
2626
c := trap.MustWait(ctx)
27-
c.Release()
27+
c.MustRelease(ctx)
2828
// trap returns the actual passed value
2929
if c.Duration != -time.Second {
3030
t.Fatalf("expected -time.Second, got: %v", c.Duration)
@@ -62,7 +62,7 @@ func TestAfterFunc_NegativeDuration(t *testing.T) {
6262
})
6363
}()
6464
c := trap.MustWait(ctx)
65-
c.Release()
65+
c.MustRelease(ctx)
6666
// trap returns the actual passed value
6767
if c.Duration != -time.Second {
6868
t.Fatalf("expected -time.Second, got: %v", c.Duration)
@@ -99,7 +99,7 @@ func TestNewTicker(t *testing.T) {
9999
tickers <- mClock.NewTicker(time.Hour, "new")
100100
}()
101101
c := trapNT.MustWait(ctx)
102-
c.Release()
102+
c.MustRelease(ctx)
103103
if c.Duration != time.Hour {
104104
t.Fatalf("expected time.Hour, got: %v", c.Duration)
105105
}
@@ -123,7 +123,7 @@ func TestNewTicker(t *testing.T) {
123123
go tkr.Reset(time.Minute, "reset")
124124
c = trapReset.MustWait(ctx)
125125
mClock.Advance(time.Second).MustWait(ctx)
126-
c.Release()
126+
c.MustRelease(ctx)
127127
if c.Duration != time.Minute {
128128
t.Fatalf("expected time.Minute, got: %v", c.Duration)
129129
}
@@ -142,7 +142,7 @@ func TestNewTicker(t *testing.T) {
142142
}
143143

144144
go tkr.Stop("stop")
145-
trapStop.MustWait(ctx).Release()
145+
trapStop.MustWait(ctx).MustRelease(ctx)
146146
mClock.Advance(time.Hour).MustWait(ctx)
147147
select {
148148
case <-tkr.C:
@@ -153,7 +153,7 @@ func TestNewTicker(t *testing.T) {
153153

154154
// Resetting after stop
155155
go tkr.Reset(time.Minute, "reset")
156-
trapReset.MustWait(ctx).Release()
156+
trapReset.MustWait(ctx).MustRelease(ctx)
157157
mClock.Advance(time.Minute).MustWait(ctx)
158158
tTime = mClock.Now()
159159
select {
@@ -344,11 +344,11 @@ func Test_MultipleTraps(t *testing.T) {
344344
done := make(chan struct{})
345345
go func() {
346346
defer close(done)
347-
c0.Release()
347+
c0.MustRelease(testCtx)
348348
}()
349349
c1 := trap1.MustWait(testCtx)
350350
mClock.Advance(time.Second)
351-
c1.Release()
351+
c1.MustRelease(testCtx)
352352

353353
select {
354354
case <-done:
@@ -367,6 +367,28 @@ func Test_MultipleTraps(t *testing.T) {
367367
}
368368
}
369369

370+
func Test_MultipleTrapsDeadlock(t *testing.T) {
371+
t.Parallel()
372+
tRunFail(t, func(t testing.TB) {
373+
testCtx, testCancel := context.WithTimeout(context.Background(), 2*time.Second)
374+
defer testCancel()
375+
mClock := quartz.NewMock(t)
376+
377+
trap0 := mClock.Trap().Now("0")
378+
defer trap0.Close()
379+
trap1 := mClock.Trap().Now("1")
380+
defer trap1.Close()
381+
382+
timeCh := make(chan time.Time)
383+
go func() {
384+
timeCh <- mClock.Now("0", "1")
385+
}()
386+
387+
c0 := trap0.MustWait(testCtx)
388+
c0.MustRelease(testCtx) // deadlocks, test failure
389+
})
390+
}
391+
370392
func Test_UnreleasedCalls(t *testing.T) {
371393
t.Parallel()
372394
tRunFail(t, func(t testing.TB) {

0 commit comments

Comments
 (0)