Skip to content

Commit

Permalink
Clarify asynchronous test behaviour
Browse files Browse the repository at this point in the history
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 <wmartin@pivotal.io>
  • Loading branch information
Andrea Nodari and williammartin committed May 29, 2018
1 parent 26d2143 commit 294d8f4
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 5 deletions.
3 changes: 3 additions & 0 deletions internal/leafnodes/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
7 changes: 2 additions & 5 deletions internal/leafnodes/shared_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down

0 comments on commit 294d8f4

Please sign in to comment.