diff --git a/assert/assert_assertions.go b/assert/assert_assertions.go index 70f22c9c3..c6a641775 100644 --- a/assert/assert_assertions.go +++ b/assert/assert_assertions.go @@ -45,7 +45,7 @@ func Condition(t T, comp func() bool, msgAndArgs ...any) bool { // // assertions.Consistently(t, func() bool { return true }, time.Second, 10*time.Millisecond) // -// See also [Eventually] for details about using context and concurrency. +// See also [Eventually] for details about using context, concurrency, and panic recovery. // // # Alternative condition signature // @@ -66,6 +66,11 @@ func Condition(t T, comp func() bool, msgAndArgs ...any) bool { // It will be executed with the context of the assertion, which inherits the [testing.T.Context] and // is cancelled on timeout. // +// # Panic recovery +// +// A panicking condition is treated as an error, causing [Consistently] to fail immediately. +// See [Eventually] for details. +// // # Concurrency // // See [Eventually]. @@ -483,6 +488,17 @@ func ErrorIs(t T, err error, target error, msgAndArgs ...any) bool { // // Notice that time ticks may be skipped if the condition takes longer than the tick interval. // +// # Panic recovery +// +// If the condition panics, the panic is recovered and treated as a failed tick +// (equivalent to returning false or a non-nil error). For [Eventually], this means +// the poller retries on the next tick — if a later tick succeeds, the assertion +// succeeds. For [Never] and [Consistently], a panic is treated as the condition +// erroring, which causes immediate failure. +// +// The recovered panic is wrapped as an error with the sentinel [errConditionPanicked], +// detectable with [errors.Is]. +// // # Attention point // // Time-based tests may be flaky in a resource-constrained environment such as a CI runner and may produce @@ -550,6 +566,15 @@ func Eventually[C Conditioner](t T, condition C, timeout time.Duration, tick tim // (e.g. via [require] assertions or [CollectT.FailNow]) cleanly aborts only the // current tick. // +// # Panic recovery +// +// If the condition panics, the panic is recovered and recorded as an error in the +// [CollectT] for that tick. The poller treats it as a failed tick and retries on the +// next one. If the assertion times out, the panic error is included in the collected +// errors reported on the parent t. +// +// See [Eventually] for the general panic recovery semantics. +// // # Examples // // success: func(c *CollectT) { True(c,true) }, 100*time.Millisecond, 20*time.Millisecond @@ -1868,7 +1893,7 @@ func NegativeT[SignedNumber SignedNumeric](t T, e SignedNumber, msgAndArgs ...an // // assertions.Never(t, func() bool { return false }, time.Second, 10*time.Millisecond) // -// See also [Eventually] for details about using context and concurrency. +// See also [Eventually] for details about using context, concurrency, and panic recovery. // // # Alternative condition signature // @@ -1878,6 +1903,11 @@ func NegativeT[SignedNumber SignedNumeric](t T, e SignedNumber, msgAndArgs ...an // // Use [Consistently] instead if you want to use a condition returning an error. // +// # Panic recovery +// +// A panicking condition is treated as an error, causing [Never] to fail immediately. +// See [Eventually] for details. +// // # Concurrency // // See [Eventually]. diff --git a/docs/doc-site/api/condition.md b/docs/doc-site/api/condition.md index bad61b566..998ac2dd8 100644 --- a/docs/doc-site/api/condition.md +++ b/docs/doc-site/api/condition.md @@ -179,6 +179,11 @@ This is consistent with [Eventually](https://pkg.go.dev/github.com/go-openapi/te It will be executed with the context of the assertion, which inherits the [testing.T.Context](https://pkg.go.dev/testing#T.Context) and is cancelled on timeout. +#### Panic recovery + +A panicking condition is treated as an error, causing [Consistently](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Consistently) to fail immediately. +See [Eventually](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Eventually) for details. + #### Concurrency See [Eventually](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Eventually). @@ -192,7 +197,7 @@ See [Eventually](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Even {{% tab title="Usage" %}} ```go assertions.Consistently(t, func() bool { return true }, time.Second, 10*time.Millisecond) -See also [Eventually](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Eventually) for details about using context and concurrency. +See also [Eventually](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Eventually) for details about using context, concurrency, and panic recovery. success: func() bool { return true }, 100*time.Millisecond, 20*time.Millisecond failure: func() bool { return false }, 100*time.Millisecond, 20*time.Millisecond ``` @@ -444,7 +449,7 @@ func main() { |--|--| | [`assertions.Consistently[C Conditioner](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#Consistently) | internal implementation | -**Source:** [github.com/go-openapi/testify/v2/internal/assertions#Consistently](https://github.com/go-openapi/testify/blob/master/internal/assertions/condition.go#L204) +**Source:** [github.com/go-openapi/testify/v2/internal/assertions#Consistently](https://github.com/go-openapi/testify/blob/master/internal/assertions/condition.go#L225) {{% /tab %}} {{< /tabs >}} @@ -501,6 +506,17 @@ A blocking condition will cause [Eventually](https://pkg.go.dev/github.com/go-op Notice that time ticks may be skipped if the condition takes longer than the tick interval. +#### Panic recovery + +If the condition panics, the panic is recovered and treated as a failed tick +(equivalent to returning false or a non-nil error). For [Eventually](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Eventually), this means +the poller retries on the next tick — if a later tick succeeds, the assertion +succeeds. For [Never](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Never) and [Consistently](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Consistently), a panic is treated as the condition +erroring, which causes immediate failure. + +The recovered panic is wrapped as an error with the sentinel [errConditionPanicked](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#errConditionPanicked), +detectable with [errors.Is](https://pkg.go.dev/errors#Is). + #### Attention point Time-based tests may be flaky in a resource-constrained environment such as a CI runner and may produce @@ -781,7 +797,7 @@ func main() { |--|--| | [`assertions.Eventually[C Conditioner](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#Eventually) | internal implementation | -**Source:** [github.com/go-openapi/testify/v2/internal/assertions#Eventually](https://github.com/go-openapi/testify/blob/master/internal/assertions/condition.go#L108) +**Source:** [github.com/go-openapi/testify/v2/internal/assertions#Eventually](https://github.com/go-openapi/testify/blob/master/internal/assertions/condition.go#L119) {{% /tab %}} {{< /tabs >}} @@ -815,6 +831,15 @@ The condition is wrapped in its own goroutine, so a call to [runtime.Goexit](htt (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. +#### Panic recovery + +If the condition panics, the panic is recovered and recorded as an error in the +[CollectT](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#CollectT) for that tick. The poller treats it as a failed tick and retries on the +next one. If the assertion times out, the panic error is included in the collected +errors reported on the parent t. + +See [Eventually](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Eventually) for the general panic recovery semantics. + {{% expand title="Examples" %}} {{< tabs >}} {{% tab title="Usage" %}} @@ -935,7 +960,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#L264) +**Source:** [github.com/go-openapi/testify/v2/internal/assertions#EventuallyWith](https://github.com/go-openapi/testify/blob/master/internal/assertions/condition.go#L294) {{% /tab %}} {{< /tabs >}} @@ -956,6 +981,11 @@ The simplest form of condition is: Use [Consistently](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Consistently) instead if you want to use a condition returning an error. +#### Panic recovery + +A panicking condition is treated as an error, causing [Never](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Never) to fail immediately. +See [Eventually](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Eventually) for details. + #### Concurrency See [Eventually](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Eventually). @@ -969,7 +999,7 @@ See [Eventually](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Even {{% tab title="Usage" %}} ```go assertions.Never(t, func() bool { return false }, time.Second, 10*time.Millisecond) -See also [Eventually](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Eventually) for details about using context and concurrency. +See also [Eventually](https://pkg.go.dev/github.com/go-openapi/testify/v2/assert#Eventually) for details about using context, concurrency, and panic recovery. success: func() bool { return false }, 100*time.Millisecond, 20*time.Millisecond failure: func() bool { return true }, 100*time.Millisecond, 20*time.Millisecond ``` @@ -1157,7 +1187,7 @@ func main() { |--|--| | [`assertions.Never(t T, condition func() bool, timeout time.Duration, tick time.Duration, msgAndArgs ...any) bool`](https://pkg.go.dev/github.com/go-openapi/testify/v2/internal/assertions#Never) | internal implementation | -**Source:** [github.com/go-openapi/testify/v2/internal/assertions#Never](https://github.com/go-openapi/testify/blob/master/internal/assertions/condition.go#L151) +**Source:** [github.com/go-openapi/testify/v2/internal/assertions#Never](https://github.com/go-openapi/testify/blob/master/internal/assertions/condition.go#L167) {{% /tab %}} {{< /tabs >}} diff --git a/internal/assertions/condition.go b/internal/assertions/condition.go index c6ac8d8cd..a6099f203 100644 --- a/internal/assertions/condition.go +++ b/internal/assertions/condition.go @@ -93,6 +93,17 @@ func Condition(t T, comp func() bool, msgAndArgs ...any) bool { // // Notice that time ticks may be skipped if the condition takes longer than the tick interval. // +// # Panic recovery +// +// If the condition panics, the panic is recovered and treated as a failed tick +// (equivalent to returning false or a non-nil error). For [Eventually], this means +// the poller retries on the next tick — if a later tick succeeds, the assertion +// succeeds. For [Never] and [Consistently], a panic is treated as the condition +// erroring, which causes immediate failure. +// +// The recovered panic is wrapped as an error with the sentinel [errConditionPanicked], +// detectable with [errors.Is]. +// // # Attention point // // Time-based tests may be flaky in a resource-constrained environment such as a CI runner and may produce @@ -126,7 +137,7 @@ func Eventually[C Conditioner](t T, condition C, timeout time.Duration, tick tim // // assertions.Never(t, func() bool { return false }, time.Second, 10*time.Millisecond) // -// See also [Eventually] for details about using context and concurrency. +// See also [Eventually] for details about using context, concurrency, and panic recovery. // // # Alternative condition signature // @@ -136,6 +147,11 @@ func Eventually[C Conditioner](t T, condition C, timeout time.Duration, tick tim // // Use [Consistently] instead if you want to use a condition returning an error. // +// # Panic recovery +// +// A panicking condition is treated as an error, causing [Never] to fail immediately. +// See [Eventually] for details. +// // # Concurrency // // See [Eventually]. @@ -168,7 +184,7 @@ func Never(t T, condition func() bool, timeout time.Duration, tick time.Duration // // assertions.Consistently(t, func() bool { return true }, time.Second, 10*time.Millisecond) // -// See also [Eventually] for details about using context and concurrency. +// See also [Eventually] for details about using context, concurrency, and panic recovery. // // # Alternative condition signature // @@ -189,6 +205,11 @@ func Never(t T, condition func() bool, timeout time.Duration, tick time.Duration // It will be executed with the context of the assertion, which inherits the [testing.T.Context] and // is cancelled on timeout. // +// # Panic recovery +// +// A panicking condition is treated as an error, causing [Consistently] to fail immediately. +// See [Eventually] for details. +// // # Concurrency // // See [Eventually]. @@ -256,6 +277,15 @@ func Consistently[C Conditioner](t T, condition C, timeout time.Duration, tick t // (e.g. via [require] assertions or [CollectT.FailNow]) cleanly aborts only the // current tick. // +// # Panic recovery +// +// If the condition panics, the panic is recovered and recorded as an error in the +// [CollectT] for that tick. The poller treats it as a failed tick and retries on the +// next one. If the assertion times out, the panic error is included in the collected +// errors reported on the parent t. +// +// See [Eventually] for the general panic recovery semantics. +// // # Examples // // success: func(c *CollectT) { True(c,true) }, 100*time.Millisecond, 20*time.Millisecond @@ -318,13 +348,21 @@ func eventuallyWithT[C CollectibleConditioner](t T, collectCondition C, timeout var cancelFunc func() // will be set by pollCondition via onSetup fn := makeCollectibleCondition(collectCondition) - condition := func(ctx context.Context) error { + condition := func(ctx context.Context) (err error) { collector := new(CollectT).withCancelFunc(cancelFunc) + + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("%w: %v", errConditionPanicked, r) + collector.errors = append(collector.errors, err) + } + if collector.failed() { + lastCollectedErrors = collector.collected() + err = collector.last() + } + }() + fn(ctx, collector) - if collector.failed() { - lastCollectedErrors = collector.collected() - return collector.last() - } return nil } @@ -408,6 +446,18 @@ func makeCollectibleCondition[C CollectibleConditioner](condition C) func(contex } } +func recoverCondition(fn func(context.Context) error) func(context.Context) error { + return func(ctx context.Context) (err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("%w: %v", errConditionPanicked, r) + } + }() + + return fn(ctx) + } +} + type conditionPoller struct { pollOptions @@ -462,6 +512,8 @@ func (p *conditionPoller) pollCondition(t T, condition func(context.Context) err p.onSetup(cancel) } + condition = recoverCondition(condition) + p.ticker = time.NewTicker(tick) defer p.ticker.Stop() @@ -691,13 +743,15 @@ func (p *conditionPoller) cancellableContext(parentCtx context.Context, timeout return ctx, cancel } -// Sentinel errors recorded by [CollectT.FailNow] and [CollectT.Cancel]. +// Sentinel errors recorded by async condition assertions. // 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". +// "tick aborted by require", "user explicitly cancelled the assertion", +// and "condition panicked". var ( - errFailNow = errors.New("collect: failed now (tick aborted)") - errCancelled = errors.New("collect: cancelled (assertion aborted)") + errFailNow = errors.New("collect: failed now (tick aborted)") + errCancelled = errors.New("collect: cancelled (assertion aborted)") + errConditionPanicked = errors.New("condition panicked") ) // CollectT implements the [T] interface and collects all errors. diff --git a/internal/assertions/condition_test.go b/internal/assertions/condition_test.go index 1a43919af..3e70d58f6 100644 --- a/internal/assertions/condition_test.go +++ b/internal/assertions/condition_test.go @@ -6,6 +6,7 @@ package assertions import ( "context" "errors" + "fmt" "iter" "slices" "sort" @@ -712,3 +713,152 @@ func pollUntilTimeoutCases() iter.Seq[pollUntilTimeoutCase] { }, }) } + +func TestConditionPanicRecovery(t *testing.T) { + t.Parallel() + + t.Run("Eventually survives a panicking condition and retries", func(t *testing.T) { + t.Parallel() + + mock := new(errorsCapturingT) + var counter int + var mu sync.Mutex + + condition := func() bool { + mu.Lock() + counter++ + n := counter + mu.Unlock() + if n < 3 { + panic("boom") + } + + return true + } + + if !Eventually(mock, condition, testTimeout, testTick) { + t.Error("expected Eventually to return true after recovering from panics") + } + mu.Lock() + got := counter + mu.Unlock() + if got < 3 { + t.Errorf("expected at least 3 calls, got %d", got) + } + }) + + t.Run("Eventually fails when condition always panics", func(t *testing.T) { + t.Parallel() + + mock := new(errorsCapturingT) + condition := func() bool { + panic("persistent failure") + } + + if Eventually(mock, condition, testTimeout, testTick) { + t.Error("expected Eventually to return false when condition always panics") + } + }) + + t.Run("Never fails when condition panics", func(t *testing.T) { + t.Parallel() + + mock := new(errorsCapturingT) + condition := func() bool { + panic("unexpected") + } + + if Never(mock, condition, testTimeout, testTick) { + t.Error("expected Never to return false when condition panics") + } + }) + + t.Run("Consistently fails when condition panics", func(t *testing.T) { + t.Parallel() + + mock := new(errorsCapturingT) + condition := func() bool { + panic("unexpected") + } + + if Consistently(mock, condition, testTimeout, testTick) { + t.Error("expected Consistently to return false when condition panics") + } + }) + + t.Run("EventuallyWith survives a panicking condition and retries", func(t *testing.T) { + t.Parallel() + + mock := new(errorsCapturingT) + var counter int + var mu sync.Mutex + + condition := func(_ *CollectT) { + mu.Lock() + counter++ + n := counter + mu.Unlock() + if n < 3 { + panic("boom in collect") + } + } + + if !EventuallyWith(mock, condition, testTimeout, testTick) { + t.Error("expected EventuallyWith to return true after recovering from panics") + } + mu.Lock() + got := counter + mu.Unlock() + if got < 3 { + t.Errorf("expected at least 3 calls, got %d", got) + } + }) + + t.Run("EventuallyWith fails when condition always panics", func(t *testing.T) { + t.Parallel() + + mock := new(errorsCapturingT) + condition := func(_ *CollectT) { + panic("always panics") + } + + if EventuallyWith(mock, condition, testTimeout, testTick) { + t.Error("expected EventuallyWith to return false when condition always panics") + } + }) + + t.Run("EventuallyWith collects panic error via sentinel", func(t *testing.T) { + t.Parallel() + + mock := new(errorsCapturingT) + var counter int + var mu sync.Mutex + + condition := func(collect *CollectT) { + mu.Lock() + counter++ + n := counter + mu.Unlock() + + if n == 1 { + panic("boom on first tick") + } + // Subsequent ticks fail normally, preserving the panic error + // from the first tick in lastCollectedErrors. + Fail(collect, "still failing") + } + + if EventuallyWith(mock, condition, testTimeout, testTick) { + t.Error("expected EventuallyWith to return false") + } + }) + + t.Run("errConditionPanicked sentinel is detectable with errors.Is", func(t *testing.T) { + t.Parallel() + + err := fmt.Errorf("%w: %v", errConditionPanicked, "test panic") + if !errors.Is(err, errConditionPanicked) { + t.Error("expected errors.Is to detect errConditionPanicked sentinel") + } + }) +} diff --git a/require/require_assertions.go b/require/require_assertions.go index 3025ae25d..e90b28c06 100644 --- a/require/require_assertions.go +++ b/require/require_assertions.go @@ -49,7 +49,7 @@ func Condition(t T, comp func() bool, msgAndArgs ...any) { // // assertions.Consistently(t, func() bool { return true }, time.Second, 10*time.Millisecond) // -// See also [Eventually] for details about using context and concurrency. +// See also [Eventually] for details about using context, concurrency, and panic recovery. // // # Alternative condition signature // @@ -70,6 +70,11 @@ func Condition(t T, comp func() bool, msgAndArgs ...any) { // It will be executed with the context of the assertion, which inherits the [testing.T.Context] and // is cancelled on timeout. // +// # Panic recovery +// +// A panicking condition is treated as an error, causing [Consistently] to fail immediately. +// See [Eventually] for details. +// // # Concurrency // // See [Eventually]. @@ -551,6 +556,17 @@ func ErrorIs(t T, err error, target error, msgAndArgs ...any) { // // Notice that time ticks may be skipped if the condition takes longer than the tick interval. // +// # Panic recovery +// +// If the condition panics, the panic is recovered and treated as a failed tick +// (equivalent to returning false or a non-nil error). For [Eventually], this means +// the poller retries on the next tick — if a later tick succeeds, the assertion +// succeeds. For [Never] and [Consistently], a panic is treated as the condition +// erroring, which causes immediate failure. +// +// The recovered panic is wrapped as an error with the sentinel [errConditionPanicked], +// detectable with [errors.Is]. +// // # Attention point // // Time-based tests may be flaky in a resource-constrained environment such as a CI runner and may produce @@ -622,6 +638,15 @@ func Eventually[C Conditioner](t T, condition C, timeout time.Duration, tick tim // (e.g. via [require] assertions or [CollectT.FailNow]) cleanly aborts only the // current tick. // +// # Panic recovery +// +// If the condition panics, the panic is recovered and recorded as an error in the +// [CollectT] for that tick. The poller treats it as a failed tick and retries on the +// next one. If the assertion times out, the panic error is included in the collected +// errors reported on the parent t. +// +// See [Eventually] for the general panic recovery semantics. +// // # Examples // // success: func(c *CollectT) { True(c,true) }, 100*time.Millisecond, 20*time.Millisecond @@ -2164,7 +2189,7 @@ func NegativeT[SignedNumber SignedNumeric](t T, e SignedNumber, msgAndArgs ...an // // assertions.Never(t, func() bool { return false }, time.Second, 10*time.Millisecond) // -// See also [Eventually] for details about using context and concurrency. +// See also [Eventually] for details about using context, concurrency, and panic recovery. // // # Alternative condition signature // @@ -2174,6 +2199,11 @@ func NegativeT[SignedNumber SignedNumeric](t T, e SignedNumber, msgAndArgs ...an // // Use [Consistently] instead if you want to use a condition returning an error. // +// # Panic recovery +// +// A panicking condition is treated as an error, causing [Never] to fail immediately. +// See [Eventually] for details. +// // # Concurrency // // See [Eventually].