Skip to content

Commit 88dcfb8

Browse files
Fix race condition in CLI runner eventually assertion (#3193)
## Why This should fix panics that happen in sync integration tests, which happen because of the orphaned go routines that hang around even after a test is left over.
1 parent 4194784 commit 88dcfb8

File tree

1 file changed

+18
-2
lines changed

1 file changed

+18
-2
lines changed

internal/testcli/runner.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,20 @@ func (r *Runner) Eventually(condition func() bool, waitFor, tick time.Duration,
241241
ticker := time.NewTicker(tick)
242242
defer ticker.Stop()
243243

244+
// Ensure all the goroutines created by this function are cleaned up.
245+
// If we do not have this check it is possible that multiple goroutines are created,
246+
// one of them returns and the test terminates. In that scenario if any of the other
247+
// goroutines use the *testing.T interface, the resulting panic will bring down the
248+
// entire test runner.
249+
var wg sync.WaitGroup
250+
defer wg.Wait()
251+
244252
// Kick off condition check immediately.
245-
go func() { ch <- condition() }()
253+
wg.Add(1)
254+
go func() {
255+
defer wg.Done()
256+
ch <- condition()
257+
}()
246258

247259
for tick := ticker.C; ; {
248260
select {
@@ -254,7 +266,11 @@ func (r *Runner) Eventually(condition func() bool, waitFor, tick time.Duration,
254266
return
255267
case <-tick:
256268
tick = nil
257-
go func() { ch <- condition() }()
269+
wg.Add(1)
270+
go func() {
271+
defer wg.Done()
272+
ch <- condition()
273+
}()
258274
case v := <-ch:
259275
if v {
260276
return

0 commit comments

Comments
 (0)