Skip to content

Commit a47d2b1

Browse files
marshall-leejlou2u
authored andcommitted
assert: collect.FailNow() should not panic (stretchr#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 stretchr#1396 Fixes stretchr#1457
1 parent 3cf5303 commit a47d2b1

3 files changed

Lines changed: 66 additions & 13 deletions

File tree

assert/assertions.go

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1956,6 +1956,9 @@ func Eventually(t TestingT, condition func() bool, waitFor time.Duration, tick t
19561956

19571957
// CollectT implements the TestingT interface and collects all errors.
19581958
type CollectT struct {
1959+
// A slice of errors. Non-nil slice denotes a failure.
1960+
// If it's non-nil but len(c.errors) == 0, this is also a failure
1961+
// obtained by direct c.FailNow() call.
19591962
errors []error
19601963
}
19611964

@@ -1964,9 +1967,10 @@ func (c *CollectT) Errorf(format string, args ...interface{}) {
19641967
c.errors = append(c.errors, fmt.Errorf(format, args...))
19651968
}
19661969

1967-
// FailNow panics.
1968-
func (*CollectT) FailNow() {
1969-
panic("Assertion failed")
1970+
// FailNow stops execution by calling runtime.Goexit.
1971+
func (c *CollectT) FailNow() {
1972+
c.fail()
1973+
runtime.Goexit()
19701974
}
19711975

19721976
// 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) {
19791983
panic("Copy() is deprecated")
19801984
}
19811985

1986+
func (c *CollectT) fail() {
1987+
if !c.failed() {
1988+
c.errors = []error{} // Make it non-nil to mark a failure.
1989+
}
1990+
}
1991+
1992+
func (c *CollectT) failed() bool {
1993+
return c.errors != nil
1994+
}
1995+
19821996
// EventuallyWithT asserts that given condition will be met in waitFor time,
19831997
// periodically checking target function each tick. In contrast to Eventually,
19841998
// 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
20032017
}
20042018

20052019
var lastFinishedTickErrs []error
2006-
ch := make(chan []error, 1)
2020+
ch := make(chan *CollectT, 1)
20072021

20082022
timer := time.NewTimer(waitFor)
20092023
defer timer.Stop()
@@ -2023,16 +2037,16 @@ func EventuallyWithT(t TestingT, condition func(collect *CollectT), waitFor time
20232037
go func() {
20242038
collect := new(CollectT)
20252039
defer func() {
2026-
ch <- collect.errors
2040+
ch <- collect
20272041
}()
20282042
condition(collect)
20292043
}()
2030-
case errs := <-ch:
2031-
if len(errs) == 0 {
2044+
case collect := <-ch:
2045+
if !collect.failed() {
20322046
return true
20332047
}
20342048
// Keep the errors from the last ended condition, so that they can be copied to t if timeout is reached.
2035-
lastFinishedTickErrs = errs
2049+
lastFinishedTickErrs = collect.errors
20362050
tick = ticker.C
20372051
}
20382052
}

assert/assertions_test.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2923,16 +2923,15 @@ func TestEventuallyWithTFalse(t *testing.T) {
29232923
func TestEventuallyWithTTrue(t *testing.T) {
29242924
mockT := new(errorsCapturingT)
29252925

2926-
state := 0
2926+
counter := 0
29272927
condition := func(collect *CollectT) {
2928-
defer func() {
2929-
state += 1
2930-
}()
2931-
True(collect, state == 2)
2928+
counter += 1
2929+
True(collect, counter == 2)
29322930
}
29332931

29342932
True(t, EventuallyWithT(mockT, condition, 100*time.Millisecond, 20*time.Millisecond))
29352933
Len(t, mockT.errors, 0)
2934+
Equal(t, 2, counter, "Condition is expected to be called 2 times")
29362935
}
29372936

29382937
func TestEventuallyWithT_ConcurrencySafe(t *testing.T) {
@@ -2970,6 +2969,17 @@ func TestEventuallyWithT_ReturnsTheLatestFinishedConditionErrors(t *testing.T) {
29702969
Len(t, mockT.errors, 2)
29712970
}
29722971

2972+
func TestEventuallyWithTFailNow(t *testing.T) {
2973+
mockT := new(CollectT)
2974+
2975+
condition := func(collect *CollectT) {
2976+
collect.FailNow()
2977+
}
2978+
2979+
False(t, EventuallyWithT(mockT, condition, 100*time.Millisecond, 20*time.Millisecond))
2980+
Len(t, mockT.errors, 1)
2981+
}
2982+
29732983
func TestNeverFalse(t *testing.T) {
29742984
condition := func() bool {
29752985
return false

require/requirements_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"errors"
66
"testing"
77
"time"
8+
9+
"github.com/stretchr/testify/assert"
810
)
911

1012
// AssertionTesterInterface defines an interface to be used for testing assertion methods
@@ -681,3 +683,30 @@ func TestErrorAssertionFunc(t *testing.T) {
681683
})
682684
}
683685
}
686+
687+
func TestEventuallyWithTFalse(t *testing.T) {
688+
mockT := new(MockT)
689+
690+
condition := func(collect *assert.CollectT) {
691+
True(collect, false)
692+
}
693+
694+
EventuallyWithT(mockT, condition, 100*time.Millisecond, 20*time.Millisecond)
695+
True(t, mockT.Failed, "Check should fail")
696+
}
697+
698+
func TestEventuallyWithTTrue(t *testing.T) {
699+
mockT := new(MockT)
700+
701+
counter := 0
702+
condition := func(collect *assert.CollectT) {
703+
defer func() {
704+
counter += 1
705+
}()
706+
True(collect, counter == 1)
707+
}
708+
709+
EventuallyWithT(mockT, condition, 100*time.Millisecond, 20*time.Millisecond)
710+
False(t, mockT.Failed, "Check should pass")
711+
Equal(t, 2, counter, "Condition is expected to be called 2 times")
712+
}

0 commit comments

Comments
 (0)