assert: collect.FailNow() should not panic (#1481)

## Summary
`collect.FailNow()` should exit goroutine without a panic to be usable
with `require` package.

## Changes
`collect.FailNow()` just does `runtime.Goexit()` instead of `panic()`.
For example `FailNow()` from `testing` package [behaves
similarly](https://cs.opensource.google/go/go/+/refs/tags/go1.21.2:src/testing/testing.go;l=973).

## Motivation

I just want `require` package to be usable with `EventuallyWithT` e.g. I
want this example to pass but it panics:

```go
package main

import (
	"testing"
	"time"

	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
)

func TestRequireEventuallyWithT(t *testing.T) {
	state := 0
	require.EventuallyWithT(t, func(c *assert.CollectT) {
		defer func() { state += 1 }()
		require.True(c, state == 2)
	}, 100*time.Millisecond, 10*time.Millisecond)
}
```

See https://go.dev/play/p/Oqd95IT7qxQ

## Related issues
Fixes https://github.com/stretchr/testify/issues/1396
Fixes https://github.com/stretchr/testify/issues/1457
pull/1595/head^2
Vladimir Kochnev 2024-06-13 23:09:51 +03:00 committed by GitHub
parent 84619f5c3c
commit b074924938
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 66 additions and 13 deletions

View File

@ -1956,6 +1956,9 @@ func Eventually(t TestingT, condition func() bool, waitFor time.Duration, tick t
// CollectT implements the TestingT interface and collects all errors.
type CollectT struct {
// A slice of errors. Non-nil slice denotes a failure.
// If it's non-nil but len(c.errors) == 0, this is also a failure
// obtained by direct c.FailNow() call.
errors []error
}
@ -1964,9 +1967,10 @@ func (c *CollectT) Errorf(format string, args ...interface{}) {
c.errors = append(c.errors, fmt.Errorf(format, args...))
}
// FailNow panics.
func (*CollectT) FailNow() {
panic("Assertion failed")
// FailNow stops execution by calling runtime.Goexit.
func (c *CollectT) FailNow() {
c.fail()
runtime.Goexit()
}
// Deprecated: That was a method for internal usage that should not have been published. Now just panics.
@ -1979,6 +1983,16 @@ func (*CollectT) Copy(TestingT) {
panic("Copy() is deprecated")
}
func (c *CollectT) fail() {
if !c.failed() {
c.errors = []error{} // Make it non-nil to mark a failure.
}
}
func (c *CollectT) failed() bool {
return c.errors != nil
}
// EventuallyWithT asserts that given condition will be met in waitFor time,
// periodically checking target function each tick. In contrast to Eventually,
// it supplies a CollectT to the condition function, so that the condition
@ -2003,7 +2017,7 @@ func EventuallyWithT(t TestingT, condition func(collect *CollectT), waitFor time
}
var lastFinishedTickErrs []error
ch := make(chan []error, 1)
ch := make(chan *CollectT, 1)
timer := time.NewTimer(waitFor)
defer timer.Stop()
@ -2023,16 +2037,16 @@ func EventuallyWithT(t TestingT, condition func(collect *CollectT), waitFor time
go func() {
collect := new(CollectT)
defer func() {
ch <- collect.errors
ch <- collect
}()
condition(collect)
}()
case errs := <-ch:
if len(errs) == 0 {
case collect := <-ch:
if !collect.failed() {
return true
}
// Keep the errors from the last ended condition, so that they can be copied to t if timeout is reached.
lastFinishedTickErrs = errs
lastFinishedTickErrs = collect.errors
tick = ticker.C
}
}

View File

@ -2923,16 +2923,15 @@ func TestEventuallyWithTFalse(t *testing.T) {
func TestEventuallyWithTTrue(t *testing.T) {
mockT := new(errorsCapturingT)
state := 0
counter := 0
condition := func(collect *CollectT) {
defer func() {
state += 1
}()
True(collect, state == 2)
counter += 1
True(collect, counter == 2)
}
True(t, EventuallyWithT(mockT, condition, 100*time.Millisecond, 20*time.Millisecond))
Len(t, mockT.errors, 0)
Equal(t, 2, counter, "Condition is expected to be called 2 times")
}
func TestEventuallyWithT_ConcurrencySafe(t *testing.T) {
@ -2970,6 +2969,17 @@ func TestEventuallyWithT_ReturnsTheLatestFinishedConditionErrors(t *testing.T) {
Len(t, mockT.errors, 2)
}
func TestEventuallyWithTFailNow(t *testing.T) {
mockT := new(CollectT)
condition := func(collect *CollectT) {
collect.FailNow()
}
False(t, EventuallyWithT(mockT, condition, 100*time.Millisecond, 20*time.Millisecond))
Len(t, mockT.errors, 1)
}
func TestNeverFalse(t *testing.T) {
condition := func() bool {
return false

View File

@ -5,6 +5,8 @@ import (
"errors"
"testing"
"time"
"github.com/stretchr/testify/assert"
)
// AssertionTesterInterface defines an interface to be used for testing assertion methods
@ -681,3 +683,30 @@ func TestErrorAssertionFunc(t *testing.T) {
})
}
}
func TestEventuallyWithTFalse(t *testing.T) {
mockT := new(MockT)
condition := func(collect *assert.CollectT) {
True(collect, false)
}
EventuallyWithT(mockT, condition, 100*time.Millisecond, 20*time.Millisecond)
True(t, mockT.Failed, "Check should fail")
}
func TestEventuallyWithTTrue(t *testing.T) {
mockT := new(MockT)
counter := 0
condition := func(collect *assert.CollectT) {
defer func() {
counter += 1
}()
True(collect, counter == 1)
}
EventuallyWithT(mockT, condition, 100*time.Millisecond, 20*time.Millisecond)
False(t, mockT.Failed, "Check should pass")
Equal(t, 2, counter, "Condition is expected to be called 2 times")
}