From 294d8f46a81242ac6473c61faf7ca14fac6f8f15 Mon Sep 17 00:00:00 2001 From: Andrea Nodari Date: Tue, 29 May 2018 18:16:28 +0100 Subject: [PATCH] Clarify asynchronous test behaviour The tests was occasionally failing due to a race between the goroutine waiting for a timeout and the goroutine setting the test state. This commit changes the test so that it does not close the channel in time, so that the chance of flakiness is significantely reduced. Fixes #465 Co-authored-by: William Martin --- internal/leafnodes/runner.go | 3 +++ internal/leafnodes/shared_runner_test.go | 7 ++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/leafnodes/runner.go b/internal/leafnodes/runner.go index 8b6518b5c..16cb66c3e 100644 --- a/internal/leafnodes/runner.go +++ b/internal/leafnodes/runner.go @@ -87,6 +87,9 @@ func (r *runner) runAsync() (outcome types.SpecState, failure types.SpecFailure) finished = true }() + // If this goroutine gets no CPU time before the select block, + // the <-done case may complete even if the test took longer than the timeoutThreshold. + // This can cause flaky behaviour, but we haven't seen it in the wild. select { case <-done: case <-time.After(r.timeoutThreshold): diff --git a/internal/leafnodes/shared_runner_test.go b/internal/leafnodes/shared_runner_test.go index 6f750ff61..0897836cb 100644 --- a/internal/leafnodes/shared_runner_test.go +++ b/internal/leafnodes/shared_runner_test.go @@ -199,21 +199,18 @@ func AsynchronousSharedRunnerBehaviors(build func(body interface{}, timeout time }) }) - Context("when the function times out", func() { + Context("when the function doesn't close the done channel in time", func() { var guard chan struct{} BeforeEach(func() { guard = make(chan struct{}) outcome, failure = build(func(done Done) { didRun = true - time.Sleep(20 * time.Millisecond) close(guard) - defer close(done) - panic("doesn't matter") }, 10*time.Millisecond, failer, componentCodeLocation).Run() }) - It("should return the timeout", func() { + It("should return a timeout", func() { <-guard Ω(didRun).Should(BeTrue())