Skip to content

Commit

Permalink
assert: collect.FailNow() should not panic
Browse files Browse the repository at this point in the history
Fixes #1396
Fixes #1457
  • Loading branch information
marshall-lee committed Nov 25, 2023
1 parent db8608e commit cbb508a
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 13 deletions.
30 changes: 22 additions & 8 deletions assert/assertions.go
Original file line number Diff line number Diff line change
Expand Up @@ -1874,6 +1874,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
}

Expand All @@ -1882,9 +1885,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.
Expand All @@ -1897,6 +1901,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
Expand All @@ -1921,7 +1935,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()
Expand All @@ -1941,16 +1955,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
}
}
Expand Down
20 changes: 15 additions & 5 deletions assert/assertions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2796,16 +2796,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)
Equalf(t, 2, counter, "Condition is expected to be called 2 times but got %v", counter)
}

func TestEventuallyWithT_ConcurrencySafe(t *testing.T) {
Expand Down Expand Up @@ -2843,6 +2842,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
Expand Down
29 changes: 29 additions & 0 deletions require/requirements_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"errors"
"testing"
"time"

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

// AssertionTesterInterface defines an interface to be used for testing assertion methods
Expand Down Expand Up @@ -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")
Equalf(t, 2, counter, "Condition is expected to be called 2 times but got %v", counter)
}

0 comments on commit cbb508a

Please sign in to comment.