feat(condition): EventuallyWith now supports require in condition (#90)

* fix(condition): in async testing, allow condition goroutine to exit
  * allows condition to Goexit without exiting the whole assertion
* feat(condition): CollectT now distinguishes between FailNow() and Cancel()
  * FailNow() exits the condition, Cancel() interrupts the whole
    assertion
* regenerated code with examples and doc strings

* fixes #90

Follow-up PRs:

1. Update documentation and code examples
2. Enhance / better report to user in migration tool

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>

Author: fredbi

assert/assert_assertions.go

@@ -798,13 +798,23 @@ The supplied [CollectT](https://pkg.go.dev/github.com/go-openapi/testify/v2/asse
 If the condition is not met before the timeout, the collected errors from the
 last tick are copied to t.
 
-Calling [CollectT.FailNow](https://pkg.go.dev/CollectT#FailNow) cancels the condition immediately and causes the assertion to fail.
+Calling [CollectT.FailNow](https://pkg.go.dev/CollectT#FailNow) (directly, or transitively through [require](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#require) assertions)
+fails the current tick only: the poller will retry on the next tick. This means
+[require](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#require)-style assertions inside [EventuallyWith](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#EventuallyWith) behave naturally — they abort
+the current evaluation and let the polling loop converge.
+
+To abort the whole assertion immediately (e.g. when the condition can no longer
+be expected to succeed), call [CollectT.Cancel](https://pkg.go.dev/CollectT#Cancel).
 
 #### Concurrency
 
 The condition function is never executed in parallel: only one goroutine executes it.
 It may write to variables outside its scope without triggering race conditions.
 
+The condition is wrapped in its own goroutine, so a call to [runtime.Goexit](https://pkg.go.dev/runtime#Goexit)
+(e.g. via [require](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#require) assertions or [CollectT.FailNow](https://pkg.go.dev/CollectT#FailNow)) cleanly aborts only the
+current tick.
+
 {{% expand title="Examples" %}}
 {{< tabs >}}
 {{% tab title="Usage" %}}
@@ -824,6 +834,7 @@ It may write to variables outside its scope without triggering race conditions.
 	)
 	success: func(c *CollectT) { True(c,true) }, 100*time.Millisecond, 20*time.Millisecond
 	failure: func(c *CollectT) { False(c,true) }, 100*time.Millisecond, 20*time.Millisecond
+	failure: func(c *CollectT) { c.Cancel() }, 100*time.Millisecond, 20*time.Millisecond
 ```
 {{< /tab >}}
 {{% tab title="Testable Examples (assert)" %}}
@@ -924,7 +935,7 @@ func main() {
 |--|--|
 | [`assertions.EventuallyWith[C CollectibleConditioner](t T, condition C, timeout time.Duration, tick time.Duration, msgAndArgs ...any) bool`](https://pkg.go.dev/github.com/go-openapi/testify/v2/internal/assertions#EventuallyWith) | internal implementation |
 
-**Source:** [github.com/go-openapi/testify/v2/internal/assertions#EventuallyWith](https://github.com/go-openapi/testify/blob/master/internal/assertions/condition.go#L253)
+**Source:** [github.com/go-openapi/testify/v2/internal/assertions#EventuallyWith](https://github.com/go-openapi/testify/blob/master/internal/assertions/condition.go#L264)
 {{% /tab %}}
 {{< /tabs >}}
 

assert/assert_assertions_test.go

@@ -222,7 +222,13 @@ func Consistently[C Conditioner](t T, condition C, timeout time.Duration, tick t
 // If the condition is not met before the timeout, the collected errors from the
 // last tick are copied to t.
 //
-// Calling [CollectT.FailNow] cancels the condition immediately and causes the assertion to fail.
+// Calling [CollectT.FailNow] (directly, or transitively through [require] assertions)
+// fails the current tick only: the poller will retry on the next tick. This means
+// [require]-style assertions inside [EventuallyWith] behave naturally — they abort
+// the current evaluation and let the polling loop converge.
+//
+// To abort the whole assertion immediately (e.g. when the condition can no longer
+// be expected to succeed), call [CollectT.Cancel].
 //
 // # Usage
 //
@@ -246,10 +252,15 @@ func Consistently[C Conditioner](t T, condition C, timeout time.Duration, tick t
 // The condition function is never executed in parallel: only one goroutine executes it.
 // It may write to variables outside its scope without triggering race conditions.
 //
+// The condition is wrapped in its own goroutine, so a call to [runtime.Goexit]
+// (e.g. via [require] assertions or [CollectT.FailNow]) cleanly aborts only the
+// current tick.
+//
 // # Examples
 //
 //	success: func(c *CollectT) { True(c,true) }, 100*time.Millisecond, 20*time.Millisecond
 //	failure: func(c *CollectT) { False(c,true) }, 100*time.Millisecond, 20*time.Millisecond
+//	failure: func(c *CollectT) { c.Cancel() }, 100*time.Millisecond, 20*time.Millisecond
 func EventuallyWith[C CollectibleConditioner](t T, condition C, timeout time.Duration, tick time.Duration, msgAndArgs ...any) bool {
 	// Domain: condition
 	if h, ok := t.(H); ok {
@@ -558,9 +569,19 @@ func (p *conditionPoller) executeCondition(parentCtx, ctx context.Context, failF
 				case <-ctx.Done():
 					return // timeout = success
 				case fn := <-p.conditionChan:
-					if err := fn(ctx); err != nil {
-						close(p.doneChan) // (condition true <=> returns error) = failure for Never and Consistently
+					var conditionWg sync.WaitGroup
+					conditionWg.Go(func() { // guards against the condition issue an early GoExit
+
+						if err := fn(ctx); err != nil {
+							close(p.doneChan) // (condition true <=> returns error) = failure for Never and Consistently
+						}
+					})
+					conditionWg.Wait()
+
+					select {
+					case <-p.doneChan: // done: early exit
 						return
+					default:
 					}
 				}
 			}
@@ -577,9 +598,19 @@ func (p *conditionPoller) executeCondition(parentCtx, ctx context.Context, failF
 				failFunc(ctx.Err().Error())
 				return
 			case fn := <-p.conditionChan:
-				if err := fn(ctx); err == nil {
-					close(p.doneChan) // (condition true <=> err == nil) = success for Eventually
+				var conditionWg sync.WaitGroup
+				conditionWg.Go(func() { // guards against the condition issue an early GoExit
+
+					if err := fn(ctx); err == nil {
+						close(p.doneChan) // (condition true <=> err == nil) = success for Eventually
+					}
+				})
+				conditionWg.Wait()
+
+				select {
+				case <-p.doneChan: // done: early exit
 					return
+				default:
 				}
 			}
 		}
@@ -660,6 +691,15 @@ func (p *conditionPoller) cancellableContext(parentCtx context.Context, timeout
 	return ctx, cancel
 }
 
+// Sentinel errors recorded by [CollectT.FailNow] and [CollectT.Cancel].
+// Kept package-private: callers should rely on observable behavior, not on
+// the marker shape. They are distinguishable so future tooling can tell apart
+// "tick aborted by require" from "user explicitly cancelled the assertion".
+var (
+	errFailNow   = errors.New("collect: failed now (tick aborted)")
+	errCancelled = errors.New("collect: cancelled (assertion aborted)")
+)
+
 // CollectT implements the [T] interface and collects all errors.
 //
 // [CollectT] is specifically intended to be used with [EventuallyWith] and
@@ -668,16 +708,20 @@ type CollectT struct {
 	// Domain: condition
 	//
 	// Maintainer:
-	//   1. FailNow() no longer just exits the go routine, but cancels the context of the caller instead before exiting.
-	//   2. We no longer establish the distinction between c.error nil or empty. Non-empty is an error, full stop.
-	//   2. Deprecated methods have been removed.
+	//   1. FailNow() exits the current tick goroutine via runtime.Goexit (matching
+	//      stretchr/testify semantics): require-style assertions abort the current
+	//      evaluation and the poller retries on the next tick. It does NOT cancel
+	//      the EventuallyWith context.
+	//   2. Cancel() is the explicit escape hatch: it cancels the EventuallyWith
+	//      context before exiting via runtime.Goexit, aborting the whole assertion.
+	//   3. We no longer establish the distinction between c.errors nil or empty.
+	//      Non-empty is an error, full stop.
+	//   4. Deprecated methods have been removed.
 
 	// A slice of errors. Non-empty slice denotes a failure.
-	// NOTE: When c.FailNow() is called, it cancels the context and exits the goroutine.
-	// The "failed now" error is appended but may be lost if the goroutine exits before collection.
 	errors []error
 
-	// cancelContext cancels the parent context on FailNow()
+	// cancelContext cancels the parent EventuallyWith context on Cancel().
 	cancelContext func()
 }
 
@@ -689,13 +733,33 @@ func (c *CollectT) Errorf(format string, args ...any) {
 	c.errors = append(c.errors, fmt.Errorf(format, args...))
 }
 
-// FailNow records a failure and cancels the parent [EventuallyWith] context,
-// before exiting the current go routine with [runtime.Goexit].
+// FailNow records a failure for the current tick and exits the condition
+// goroutine via [runtime.Goexit].
 //
-// This causes the assertion to fail immediately without waiting for a timeout.
+// It does NOT cancel the [EventuallyWith] context: the poller will retry on
+// the next tick. If a later tick succeeds, the assertion succeeds. If no tick
+// ever succeeds before the timeout, the errors collected during the LAST tick
+// (the one which most recently called FailNow) are reported on the parent t.
+//
+// To abort the whole assertion immediately, use [CollectT.Cancel].
 func (c *CollectT) FailNow() {
+	c.errors = append(c.errors, errFailNow)
+	runtime.Goexit()
+}
+
+// Cancel records a failure, cancels the [EventuallyWith] context, then exits
+// the condition goroutine via [runtime.Goexit].
+//
+// This aborts the whole assertion immediately, without waiting for the timeout.
+// The errors collected during the cancelled tick are reported on the parent t.
+//
+// Use this when the condition can no longer be expected to succeed (e.g. an
+// upstream resource has been observed in an unrecoverable state). For ordinary
+// per-tick failures (e.g. "value not yet ready"), use [CollectT.FailNow]
+// directly or transitively through [require] assertions.
+func (c *CollectT) Cancel() {
+	c.errors = append(c.errors, errCancelled)
 	c.cancelContext()
-	c.errors = append(c.errors, errors.New("failed now")) // so c.failed() is true (currently lost as not owned by another go routine)
 	runtime.Goexit()
 }
 

assert/assert_format_test.go

@@ -294,6 +294,7 @@ func TestConditionEventuallyNoLeak(t *testing.T) {
 	})
 }
 
+//nolint:gocognit,gocyclo,cyclop // subtests are actually not complex
 func TestConditionEventuallyWith(t *testing.T) {
 	t.Parallel()
 
@@ -406,28 +407,88 @@ func TestConditionEventuallyWith(t *testing.T) {
 		}
 	})
 
-	t.Run("should fail with a call to collect.FailNow", func(t *testing.T) {
+	t.Run("collect.FailNow only fails the current tick (poller retries)", func(t *testing.T) {
 		t.Parallel()
 
 		mock := new(errorsCapturingT)
-		counter := 0
+		var counter int
+		var mu sync.Mutex
 
-		// The call to FailNow cancels the execution context of EventuallyWith.
-		// so we don't have to wait for the timeout.
+		// FailNow on every tick: the poller must keep retrying until the timeout.
 		condition := func(collect *CollectT) {
+			mu.Lock()
 			counter++
+			mu.Unlock()
 			collect.FailNow()
 		}
 
+		if EventuallyWith(mock, condition, testTimeout, testTick) {
+			t.Error("expected EventuallyWith to return false")
+		}
+		mu.Lock()
+		got := counter
+		mu.Unlock()
+		if got < 2 {
+			t.Errorf("expected the condition to be retried multiple times, got %d call(s)", got)
+		}
+	})
+
+	t.Run("collect.FailNow allows convergence on a later tick", func(t *testing.T) {
+		t.Parallel()
+
+		mock := new(errorsCapturingT)
+		var counter int
+		var mu sync.Mutex
+
+		// First few ticks fail via FailNow, then converge.
+		condition := func(collect *CollectT) {
+			mu.Lock()
+			counter++
+			n := counter
+			mu.Unlock()
+			if n < 3 {
+				collect.FailNow()
+			}
+		}
+
+		if !EventuallyWith(mock, condition, testTimeout, testTick) {
+			t.Error("expected EventuallyWith to eventually return true")
+		}
+		if len(mock.errors) != 0 {
+			t.Errorf("expected no errors reported on parent t after success, got %d: %v", len(mock.errors), mock.errors)
+		}
+	})
+
+	t.Run("collect.Cancel aborts the whole assertion immediately", func(t *testing.T) {
+		t.Parallel()
+
+		mock := new(errorsCapturingT)
+		var counter int
+		var mu sync.Mutex
+
+		// Cancel must short-circuit: a 30-minute timeout must NOT be waited on.
+		condition := func(collect *CollectT) {
+			mu.Lock()
+			counter++
+			mu.Unlock()
+			collect.Cancel()
+		}
+
+		start := time.Now()
 		if EventuallyWith(mock, condition, 30*time.Minute, testTick) {
 			t.Error("expected EventuallyWith to return false")
 		}
-		const expectedErrors = 2
-		if len(mock.errors) != expectedErrors {
-			t.Errorf("expected %d errors (0 accumulated + 2 from EventuallyWith), got %d", expectedErrors, len(mock.errors))
+		if elapsed := time.Since(start); elapsed > 5*time.Second {
+			t.Errorf("expected Cancel to short-circuit, but EventuallyWith took %s", elapsed)
+		}
+		mu.Lock()
+		got := counter
+		mu.Unlock()
+		if got != 1 {
+			t.Errorf("expected the condition function to have been called only once, but got: %d", got)
 		}
-		if counter != 1 {
-			t.Errorf("expected the condition function to have been called only once, but got: %d", counter)
+		if len(mock.errors) == 0 {
+			t.Error("expected at least one error reported on parent t after Cancel")
 		}
 	})
 }