Handle unexpected exits in condition polling

assert/assertion_format.go

@@ -168,6 +168,10 @@ func ErrorIsf(t TestingT, err error, target error, msg string, args ...interface
 // Eventuallyf asserts that given condition will be met in waitFor time,
 // periodically checking target function each tick.
 //
+// If the condition does not return normally, but instead calls [runtime.Goexit],
+// the assertion fails immediately. This usually means that the condition called
+// t.FailNow() on the outer 't'.
+//
 //	assert.Eventuallyf(t, func() bool { return true; }, time.Second, 10*time.Millisecond, "error message %s", "formatted")
 func Eventuallyf(t TestingT, condition func() bool, waitFor time.Duration, tick time.Duration, msg string, args ...interface{}) bool {
 	if h, ok := t.(tHelper); ok {
@@ -185,6 +189,12 @@ func Eventuallyf(t TestingT, condition func() bool, waitFor time.Duration, tick
 // If the condition is not met before waitFor, the collected errors of
 // the last tick are copied to t.
 //
+// If the condition does not return normally, but instead calls [runtime.Goexit],
+// and the exit was not via 'collect.FailNow()', the assertion fails immediately.
+// This usually means that the condition called t.FailNow() on the outer 't'.
+// Use [CollectT.FailNow] or 'require' functions on the provided 'collect' to
+// only fail the current tick.
+//
 //	externalValue := false
 //	go func() {
 //		time.Sleep(8*time.Second)
@@ -525,6 +535,10 @@ func Negativef(t TestingT, e interface{}, msg string, args ...interface{}) bool
 // Neverf asserts that the given condition doesn't satisfy in waitFor time,
 // periodically checking the target function each tick.
 //
+// If the condition does not return normally, but instead calls [runtime.Goexit],
+// the assertion fails immediately. This usually means that the condition called
+// t.FailNow() on the outer 't'.
+//
 //	assert.Neverf(t, func() bool { return false; }, time.Second, 10*time.Millisecond, "error message %s", "formatted")
 func Neverf(t TestingT, condition func() bool, waitFor time.Duration, tick time.Duration, msg string, args ...interface{}) bool {
 	if h, ok := t.(tHelper); ok {

assert/assertion_forward.go

@@ -325,6 +325,10 @@ func (a *Assertions) Errorf(err error, msg string, args ...interface{}) bool {
 // Eventually asserts that given condition will be met in waitFor time,
 // periodically checking target function each tick.
 //
+// If the condition does not return normally, but instead calls [runtime.Goexit],
+// the assertion fails immediately. This usually means that the condition called
+// t.FailNow() on the outer 't'.
+//
 //	a.Eventually(func() bool { return true; }, time.Second, 10*time.Millisecond)
 func (a *Assertions) Eventually(condition func() bool, waitFor time.Duration, tick time.Duration, msgAndArgs ...interface{}) bool {
 	if h, ok := a.t.(tHelper); ok {
@@ -342,6 +346,12 @@ func (a *Assertions) Eventually(condition func() bool, waitFor time.Duration, ti
 // If the condition is not met before waitFor, the collected errors of
 // the last tick are copied to t.
 //
+// If the condition does not return normally, but instead calls [runtime.Goexit],
+// and the exit was not via 'collect.FailNow()', the assertion fails immediately.
+// This usually means that the condition called t.FailNow() on the outer 't'.
+// Use [CollectT.FailNow] or 'require' functions on the provided 'collect' to
+// only fail the current tick.
+//
 //	externalValue := false
 //	go func() {
 //		time.Sleep(8*time.Second)
@@ -367,6 +377,12 @@ func (a *Assertions) EventuallyWithT(condition func(collect *CollectT), waitFor
 // If the condition is not met before waitFor, the collected errors of
 // the last tick are copied to t.
 //
+// If the condition does not return normally, but instead calls [runtime.Goexit],
+// and the exit was not via 'collect.FailNow()', the assertion fails immediately.
+// This usually means that the condition called t.FailNow() on the outer 't'.
+// Use [CollectT.FailNow] or 'require' functions on the provided 'collect' to
+// only fail the current tick.
+//
 //	externalValue := false
 //	go func() {
 //		time.Sleep(8*time.Second)
@@ -386,6 +402,10 @@ func (a *Assertions) EventuallyWithTf(condition func(collect *CollectT), waitFor
 // Eventuallyf asserts that given condition will be met in waitFor time,
 // periodically checking target function each tick.
 //
+// If the condition does not return normally, but instead calls [runtime.Goexit],
+// the assertion fails immediately. This usually means that the condition called
+// t.FailNow() on the outer 't'.
+//
 //	a.Eventuallyf(func() bool { return true; }, time.Second, 10*time.Millisecond, "error message %s", "formatted")
 func (a *Assertions) Eventuallyf(condition func() bool, waitFor time.Duration, tick time.Duration, msg string, args ...interface{}) bool {
 	if h, ok := a.t.(tHelper); ok {
@@ -1039,6 +1059,10 @@ func (a *Assertions) Negativef(e interface{}, msg string, args ...interface{}) b
 // Never asserts that the given condition doesn't satisfy in waitFor time,
 // periodically checking the target function each tick.
 //
+// If the condition does not return normally, but instead calls [runtime.Goexit],
+// the assertion fails immediately. This usually means that the condition called
+// t.FailNow() on the outer 't'.
+//
 //	a.Never(func() bool { return false; }, time.Second, 10*time.Millisecond)
 func (a *Assertions) Never(condition func() bool, waitFor time.Duration, tick time.Duration, msgAndArgs ...interface{}) bool {
 	if h, ok := a.t.(tHelper); ok {
@@ -1050,6 +1074,10 @@ func (a *Assertions) Never(condition func() bool, waitFor time.Duration, tick ti
 // Neverf asserts that the given condition doesn't satisfy in waitFor time,
 // periodically checking the target function each tick.
 //
+// If the condition does not return normally, but instead calls [runtime.Goexit],
+// the assertion fails immediately. This usually means that the condition called
+// t.FailNow() on the outer 't'.
+//
 //	a.Neverf(func() bool { return false; }, time.Second, 10*time.Millisecond, "error message %s", "formatted")
 func (a *Assertions) Neverf(condition func() bool, waitFor time.Duration, tick time.Duration, msg string, args ...interface{}) bool {
 	if h, ok := a.t.(tHelper); ok {

assert/assertions.go

@@ -13,6 +13,7 @@ import (
 	"runtime"
 	"runtime/debug"
 	"strings"
+	"sync/atomic"
 	"time"
 	"unicode"
 	"unicode/utf8"
@@ -2004,14 +2005,34 @@ type tHelper = interface {
 // Eventually asserts that given condition will be met in waitFor time,
 // periodically checking target function each tick.
 //
+// If the condition does not return normally, but instead calls [runtime.Goexit],
+// the assertion fails immediately. This usually means that the condition called
+// t.FailNow() on the outer 't'.
+//
 //	assert.Eventually(t, func() bool { return true; }, time.Second, 10*time.Millisecond)
 func Eventually(t TestingT, condition func() bool, waitFor time.Duration, tick time.Duration, msgAndArgs ...interface{}) bool {
 	if h, ok := t.(tHelper); ok {
 		h.Helper()
 	}
 
-	ch := make(chan bool, 1)
-	checkCond := func() { ch <- condition() }
+	const (
+		conditionExitedUnexpectedly = iota
+		conditionReturnedTrue
+		conditionReturnedFalse
+	)
+
+	resultCh := make(chan int, 1)
+	checkCond := func() {
+		result := conditionExitedUnexpectedly
+		defer func() {
+			resultCh <- result
+		}()
+		if condition() {
+			result = conditionReturnedTrue
+		} else {
+			result = conditionReturnedFalse
+		}
+	}
 
 	timer := time.NewTimer(waitFor)
 	defer timer.Stop()
@@ -2031,11 +2052,20 @@ func Eventually(t TestingT, condition func() bool, waitFor time.Duration, tick t
 		case <-tickC:
 			tickC = nil
 			go checkCond()
-		case v := <-ch:
-			if v {
+		case result := <-resultCh:
+			switch result {
+			case conditionExitedUnexpectedly:
+				// Condition exited via [runtime.Goexit]. This usually means
+				// that the condition called t.FailNow() on the outer 't'.
+				return Fail(t, "Condition exited unexpectedly", msgAndArgs...)
+			case conditionReturnedTrue:
 				return true
+			case conditionReturnedFalse:
+				// All good, continue checking.
+				fallthrough
+			default:
+				tickC = ticker.C
 			}
-			tickC = ticker.C
 		}
 	}
 }
@@ -2046,6 +2076,10 @@ type CollectT struct {
 	// If it's non-nil but len(c.errors) == 0, this is also a failure
 	// obtained by direct c.FailNow() call.
 	errors []error
+
+	// exited is set to true if FailNow was called to indicate that the test
+	// exited correctly via runtime.Goexit.
+	exited bool
 }
 
 // Helper is like [testing.T.Helper] but does nothing.
@@ -2059,6 +2093,7 @@ func (c *CollectT) Errorf(format string, args ...interface{}) {
 // FailNow stops execution by calling runtime.Goexit.
 func (c *CollectT) FailNow() {
 	c.fail()
+	c.exited = true
 	runtime.Goexit()
 }
 
@@ -2091,6 +2126,12 @@ func (c *CollectT) failed() bool {
 // If the condition is not met before waitFor, the collected errors of
 // the last tick are copied to t.
 //
+// If the condition does not return normally, but instead calls [runtime.Goexit],
+// and the exit was not via 'collect.FailNow()', the assertion fails immediately.
+// This usually means that the condition called t.FailNow() on the outer 't'.
+// Use [CollectT.FailNow] or 'require' functions on the provided 'collect' to
+// only fail the current tick.
+//
 //	externalValue := false
 //	go func() {
 //		time.Sleep(8*time.Second)
@@ -2105,15 +2146,51 @@ func EventuallyWithT(t TestingT, condition func(collect *CollectT), waitFor time
 		h.Helper()
 	}
 
-	var lastFinishedTickErrs []error
-	ch := make(chan *CollectT, 1)
+	const (
+		conditionExitedUnexpectedly = iota
+		conditionFailed
+		conditionSucceeded
+	)
+
+	var lastFinishedTickErrs atomic.Value // of []error
+	ch := make(chan int, 1)
 
 	checkCond := func() {
+		result := conditionExitedUnexpectedly
 		collect := new(CollectT)
 		defer func() {
-			ch <- collect
+			// At this point, the condition has returned or exited. It is safe
+			// to check collect.errors and collect.exited, unless the user has
+			// created additional goroutines that access 'collect', which would
+			// be a misuse and is not supported.
+			if collect.exited {
+				// Condition exited via [CollectT.FailNow], which is a regular
+				// way to fail the condition early and exit the goroutine.
+				result = conditionFailed
+			}
+			// Keep the collected tick errors, so that they can be copied to 't'
+			// when timeout is reached or there is an unexpected exit.
+			// Always store the actual value of collect.errors, even if nil
+			lastFinishedTickErrs.Store(collect.errors)
+
+			ch <- result
 		}()
 		condition(collect)
+		if collect.failed() {
+			result = conditionFailed
+		} else {
+			result = conditionSucceeded
+		}
+	}
+
+	copyLastFinishedTickErrs := func() {
+		errs, ok := lastFinishedTickErrs.Load().([]error)
+		if !ok {
+			return
+		}
+		for _, err := range errs {
+			t.Errorf("%v", err)
+		}
 	}
 
 	timer := time.NewTimer(waitFor)
@@ -2130,35 +2207,61 @@ func EventuallyWithT(t TestingT, condition func(collect *CollectT), waitFor time
 	for {
 		select {
 		case <-timer.C:
-			for _, err := range lastFinishedTickErrs {
-				t.Errorf("%v", err)
-			}
+			copyLastFinishedTickErrs()
 			return Fail(t, "Condition never satisfied", msgAndArgs...)
 		case <-tickC:
 			tickC = nil
 			go checkCond()
-		case collect := <-ch:
-			if !collect.failed() {
+		case result := <-ch:
+			switch result {
+			case conditionExitedUnexpectedly:
+				// Condition exited via [runtime.Goexit]. This usually means
+				// that the condition called t.FailNow() on the outer 't'.
+				copyLastFinishedTickErrs()
+				return Fail(t, "Condition exited unexpectedly", msgAndArgs...)
+			case conditionSucceeded:
 				return true
+			case conditionFailed:
+				// All good, continue checking.
+				fallthrough
+			default:
+				tickC = ticker.C
 			}
-			// Keep the errors from the last ended condition, so that they can be copied to t if timeout is reached.
-			lastFinishedTickErrs = collect.errors
-			tickC = ticker.C
 		}
 	}
 }
 
 // Never asserts that the given condition doesn't satisfy in waitFor time,
 // periodically checking the target function each tick.
 //
+// If the condition does not return normally, but instead calls [runtime.Goexit],
+// the assertion fails immediately. This usually means that the condition called
+// t.FailNow() on the outer 't'.
+//
 //	assert.Never(t, func() bool { return false; }, time.Second, 10*time.Millisecond)
 func Never(t TestingT, condition func() bool, waitFor time.Duration, tick time.Duration, msgAndArgs ...interface{}) bool {
 	if h, ok := t.(tHelper); ok {
 		h.Helper()
 	}
 
-	ch := make(chan bool, 1)
-	checkCond := func() { ch <- condition() }
+	const (
+		conditionExitedUnexpectedly = iota
+		conditionReturnedTrue
+		conditionReturnedFalse
+	)
+
+	ch := make(chan int, 1)
+	checkCond := func() {
+		result := conditionExitedUnexpectedly
+		defer func() {
+			ch <- result
+		}()
+		if condition() {
+			result = conditionReturnedTrue
+		} else {
+			result = conditionReturnedFalse
+		}
+	}
 
 	timer := time.NewTimer(waitFor)
 	defer timer.Stop()
@@ -2178,11 +2281,20 @@ func Never(t TestingT, condition func() bool, waitFor time.Duration, tick time.D
 		case <-tickC:
 			tickC = nil
 			go checkCond()
-		case v := <-ch:
-			if v {
+		case result := <-ch:
+			switch result {
+			case conditionExitedUnexpectedly:
+				// Condition exited via [runtime.Goexit]. This usually means
+				// that the condition called t.FailNow() on the outer 't'.
+				return Fail(t, "Condition exited unexpectedly", msgAndArgs...)
+			case conditionReturnedTrue:
 				return Fail(t, "Condition satisfied", msgAndArgs...)
+			case conditionReturnedFalse:
+				// All good, continue checking.
+				fallthrough
+			default:
+				tickC = ticker.C
 			}
-			tickC = ticker.C
 		}
 	}
 }