-
Notifications
You must be signed in to change notification settings - Fork 4
fix: fix mock TickerFunc to act like real #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ package quartz_test | |
|
||
import ( | ||
"context" | ||
"errors" | ||
"testing" | ||
"time" | ||
|
||
|
@@ -210,3 +211,107 @@ func TestPeek(t *testing.T) { | |
t.Fatal("expected Peek() to return false") | ||
} | ||
} | ||
|
||
// TestTickerFunc_ContextDoneDuringTick tests that TickerFunc.Wait() can't return while the tick | ||
// function callback is in progress. | ||
func TestTickerFunc_ContextDoneDuringTick(t *testing.T) { | ||
t.Parallel() | ||
testCtx, testCancel := context.WithTimeout(context.Background(), 10*time.Second) | ||
defer testCancel() | ||
mClock := quartz.NewMock(t) | ||
|
||
tickStart := make(chan struct{}) | ||
tickDone := make(chan struct{}) | ||
ctx, cancel := context.WithCancel(testCtx) | ||
defer cancel() | ||
tkr := mClock.TickerFunc(ctx, time.Second, func() error { | ||
close(tickStart) | ||
select { | ||
case <-tickDone: | ||
case <-testCtx.Done(): | ||
t.Error("timeout waiting for tickDone") | ||
} | ||
return nil | ||
}) | ||
w := mClock.Advance(time.Second) | ||
select { | ||
case <-tickStart: | ||
// OK | ||
case <-testCtx.Done(): | ||
t.Fatal("timeout waiting for tickStart") | ||
} | ||
waitErr := make(chan error, 1) | ||
go func() { | ||
waitErr <- tkr.Wait() | ||
}() | ||
cancel() | ||
select { | ||
case <-waitErr: | ||
t.Fatal("wait should not return while tick callback in progress") | ||
case <-time.After(time.Millisecond * 100): | ||
// OK | ||
} | ||
close(tickDone) | ||
select { | ||
case err := <-waitErr: | ||
if !errors.Is(err, context.Canceled) { | ||
t.Fatal("expected context.Canceled error") | ||
} | ||
case <-testCtx.Done(): | ||
t.Fatal("timed out waiting for wait to finish") | ||
} | ||
w.MustWait(testCtx) | ||
} | ||
|
||
// TestTickerFunc_LongCallback tests that we don't call the ticker func a second time while the | ||
// first is still executing. | ||
func TestTickerFunc_LongCallback(t *testing.T) { | ||
t.Parallel() | ||
testCtx, testCancel := context.WithTimeout(context.Background(), 10*time.Second) | ||
defer testCancel() | ||
mClock := quartz.NewMock(t) | ||
|
||
expectedErr := errors.New("callback error") | ||
tickStart := make(chan struct{}) | ||
tickDone := make(chan struct{}) | ||
ctx, cancel := context.WithCancel(testCtx) | ||
defer cancel() | ||
tkr := mClock.TickerFunc(ctx, time.Second, func() error { | ||
close(tickStart) | ||
select { | ||
case <-tickDone: | ||
case <-testCtx.Done(): | ||
t.Error("timeout waiting for tickDone") | ||
} | ||
return expectedErr | ||
}) | ||
w := mClock.Advance(time.Second) | ||
select { | ||
case <-tickStart: | ||
// OK | ||
case <-testCtx.Done(): | ||
t.Fatal("timeout waiting for tickStart") | ||
} | ||
// second tick completes immediately, since it doesn't actually call the | ||
// ticker function. | ||
mClock.Advance(time.Second).MustWait(testCtx) | ||
|
||
waitErr := make(chan error, 1) | ||
go func() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Do we need a waitgroup to ensure this goroutine is completely finished by the time the text exits? Ditto above on L244. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We read from Generally speaking, we mainly care about whether the code under test leaks goroutines. Leaking a test goroutine almost never matters, since the test is finite (the exception is if the leaked goroutine interferes with other tests, which is rare). |
||
waitErr <- tkr.Wait() | ||
}() | ||
cancel() | ||
close(tickDone) | ||
|
||
select { | ||
case err := <-waitErr: | ||
// we should get the function error, not the context error, since context was canceled while | ||
// we were calling the function, and it returned an error. | ||
if !errors.Is(err, expectedErr) { | ||
t.Fatalf("wrong error: %s", err) | ||
} | ||
case <-testCtx.Done(): | ||
t.Fatal("timed out waiting for wait to finish") | ||
} | ||
w.MustWait(testCtx) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: this is the canonical usage from https://pkg.go.dev/sync#Cond.Wait