assert: collect.FailNow() should not panic

[marshall-lee]

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.

Motivation

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

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 #1396
Fixes #1457

[dolmen]

@marshall-lee Your review of #1395 would be welcome.

[dolmen]

#1395 is now merged.

@marshall-lee Please rebase.

@czeslavo Please review.

[czeslavo]

I'm wondering whether we could document this behavior somewhere. It would be cool to have an example of using EventuallyWithT with a series of require and assert checks with an explanation of what happens underneath (i.e. if one of the require checks is not satisfied, this single condition run is aborted, but EventuallyWithT will retry if the timeout hasn't passed yet, etc.)

[dolmen]

@cszczepaniak Could you also help with a review of this MR?

[dolmen]

I think that the CollectT.failed field is not necessary to fix the issue.

If we ensure c.errors is a non nil slice when c.FailNow is called, the patch can be much smaller.

[marshall-lee]

@czeslavo @cszczepaniak @dolmen Hi! Any plans on merging this?

[czeslavo]

LGTM, but I'm just an ordinary contributor here, @dolmen decides 🙂

[dolmen]

LGTM.

Rebase needed.

[ccoVeille]

It's a good thing this issue has been addressed. Thank you guys

[adilansari]

when will this be released?

[HaraldNordgren]

@marshall-lee This seems to have broken the tests for 1.18.