-
Notifications
You must be signed in to change notification settings - Fork 1.7k
assert: check early in Eventually, EventuallyWithT, and Never #1427
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
68f35d2
cd4dc28
f963164
e4e93dd
ab114f8
bf2c747
bae586f
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 |
|---|---|---|
|
|
@@ -1931,25 +1931,31 @@ func Eventually(t TestingT, condition func() bool, waitFor time.Duration, tick t | |
| } | ||
|
|
||
| ch := make(chan bool, 1) | ||
| checkCond := func() { ch <- condition() } | ||
|
|
||
| timer := time.NewTimer(waitFor) | ||
| defer timer.Stop() | ||
|
|
||
| ticker := time.NewTicker(tick) | ||
| defer ticker.Stop() | ||
|
|
||
| for tick := ticker.C; ; { | ||
| var tickC <-chan time.Time | ||
|
|
||
| // Check the condition once first on the initial call. | ||
| go checkCond() | ||
|
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. @brackendawson I just got impacted in some of our test cases, because the behaviour changes a little here: the (in our case your change pinpointed a race condition in some kubernetes ingress / contour / envoy reconfiguration that we were not aware of 😅 ) |
||
|
|
||
| for { | ||
| select { | ||
| case <-timer.C: | ||
| return Fail(t, "Condition never satisfied", msgAndArgs...) | ||
| case <-tick: | ||
| tick = nil | ||
| go func() { ch <- condition() }() | ||
| case <-tickC: | ||
| tickC = nil | ||
| go checkCond() | ||
| case v := <-ch: | ||
| if v { | ||
| return true | ||
| } | ||
| tick = ticker.C | ||
| tickC = ticker.C | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -2019,35 +2025,42 @@ func EventuallyWithT(t TestingT, condition func(collect *CollectT), waitFor time | |
| var lastFinishedTickErrs []error | ||
| ch := make(chan *CollectT, 1) | ||
|
|
||
| checkCond := func() { | ||
| collect := new(CollectT) | ||
| defer func() { | ||
| ch <- collect | ||
| }() | ||
| condition(collect) | ||
| } | ||
|
|
||
| timer := time.NewTimer(waitFor) | ||
| defer timer.Stop() | ||
|
|
||
| ticker := time.NewTicker(tick) | ||
| defer ticker.Stop() | ||
|
|
||
| for tick := ticker.C; ; { | ||
| var tickC <-chan time.Time | ||
|
|
||
| // Check the condition once first on the initial call. | ||
| go checkCond() | ||
|
|
||
| for { | ||
| select { | ||
| case <-timer.C: | ||
| for _, err := range lastFinishedTickErrs { | ||
| t.Errorf("%v", err) | ||
| } | ||
| return Fail(t, "Condition never satisfied", msgAndArgs...) | ||
| case <-tick: | ||
| tick = nil | ||
| go func() { | ||
| collect := new(CollectT) | ||
| defer func() { | ||
| ch <- collect | ||
| }() | ||
| condition(collect) | ||
| }() | ||
| case <-tickC: | ||
| tickC = nil | ||
| go checkCond() | ||
| case collect := <-ch: | ||
| if !collect.failed() { | ||
| return true | ||
| } | ||
| // Keep the errors from the last ended condition, so that they can be copied to t if timeout is reached. | ||
| lastFinishedTickErrs = collect.errors | ||
| tick = ticker.C | ||
| tickC = ticker.C | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -2062,25 +2075,31 @@ func Never(t TestingT, condition func() bool, waitFor time.Duration, tick time.D | |
| } | ||
|
|
||
| ch := make(chan bool, 1) | ||
| checkCond := func() { ch <- condition() } | ||
|
|
||
| timer := time.NewTimer(waitFor) | ||
| defer timer.Stop() | ||
|
|
||
| ticker := time.NewTicker(tick) | ||
| defer ticker.Stop() | ||
|
|
||
| for tick := ticker.C; ; { | ||
| var tickC <-chan time.Time | ||
|
|
||
| // Check the condition once first on the initial call. | ||
| go checkCond() | ||
|
|
||
| for { | ||
| select { | ||
| case <-timer.C: | ||
| return true | ||
| case <-tick: | ||
| tick = nil | ||
| go func() { ch <- condition() }() | ||
| case <-tickC: | ||
| tickC = nil | ||
| go checkCond() | ||
| case v := <-ch: | ||
| if v { | ||
| return Fail(t, "Condition satisfied", msgAndArgs...) | ||
| } | ||
| tick = ticker.C | ||
| tickC = ticker.C | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2980,6 +2980,49 @@ func TestEventuallyWithTFailNow(t *testing.T) { | |
| Len(t, mockT.errors, 1) | ||
| } | ||
|
|
||
| // Check that a long running condition doesn't block Eventually. | ||
| // See issue 805 (and its long tail of following issues) | ||
| func TestEventuallyTimeout(t *testing.T) { | ||
|
Comment on lines
+2983
to
+2985
Contributor
Author
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. This test isn't new; I just moved it so that all of the |
||
| mockT := new(testing.T) | ||
|
|
||
| NotPanics(t, func() { | ||
| done, done2 := make(chan struct{}), make(chan struct{}) | ||
|
|
||
| // A condition function that returns after the Eventually timeout | ||
| condition := func() bool { | ||
| // Wait until Eventually times out and terminates | ||
| <-done | ||
| close(done2) | ||
| return true | ||
| } | ||
|
|
||
| False(t, Eventually(mockT, condition, time.Millisecond, time.Microsecond)) | ||
|
|
||
| close(done) | ||
| <-done2 | ||
| }) | ||
| } | ||
|
|
||
| func TestEventuallySucceedQuickly(t *testing.T) { | ||
| mockT := new(testing.T) | ||
|
|
||
| condition := func() bool { return true } | ||
|
|
||
| // By making the tick longer than the total duration, we expect that this test would fail if | ||
| // we didn't check the condition before the first tick elapses. | ||
| True(t, Eventually(mockT, condition, 100*time.Millisecond, time.Second)) | ||
| } | ||
|
|
||
| func TestEventuallyWithTSucceedQuickly(t *testing.T) { | ||
| mockT := new(testing.T) | ||
|
|
||
| condition := func(t *CollectT) {} | ||
|
|
||
| // By making the tick longer than the total duration, we expect that this test would fail if | ||
| // we didn't check the condition before the first tick elapses. | ||
| True(t, EventuallyWithT(mockT, condition, 100*time.Millisecond, time.Second)) | ||
| } | ||
|
|
||
| func TestNeverFalse(t *testing.T) { | ||
| condition := func() bool { | ||
| return false | ||
|
|
@@ -3007,27 +3050,13 @@ func TestNeverTrue(t *testing.T) { | |
| False(t, Never(mockT, condition, 100*time.Millisecond, 20*time.Millisecond)) | ||
| } | ||
|
|
||
| // Check that a long running condition doesn't block Eventually. | ||
| // See issue 805 (and its long tail of following issues) | ||
| func TestEventuallyTimeout(t *testing.T) { | ||
| func TestNeverFailQuickly(t *testing.T) { | ||
| mockT := new(testing.T) | ||
|
|
||
| NotPanics(t, func() { | ||
| done, done2 := make(chan struct{}), make(chan struct{}) | ||
|
|
||
| // A condition function that returns after the Eventually timeout | ||
| condition := func() bool { | ||
| // Wait until Eventually times out and terminates | ||
| <-done | ||
| close(done2) | ||
| return true | ||
| } | ||
|
|
||
| False(t, Eventually(mockT, condition, time.Millisecond, time.Microsecond)) | ||
|
|
||
| close(done) | ||
| <-done2 | ||
| }) | ||
| // By making the tick longer than the total duration, we expect that this test would fail if | ||
| // we didn't check the condition before the first tick elapses. | ||
| condition := func() bool { return true } | ||
| False(t, Never(mockT, condition, 100*time.Millisecond, time.Second)) | ||
| } | ||
|
|
||
| func Test_validateEqualArgs(t *testing.T) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.