From 192c18ca685f5400ad0e22a26b4b4df4df12dfeb Mon Sep 17 00:00:00 2001 From: Uwe Jugel Date: Tue, 14 Oct 2025 09:12:31 +0200 Subject: [PATCH 1/6] add test for failed condition hang --- require/requirements_test.go | 107 +++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/require/requirements_test.go b/require/requirements_test.go index 7cb63a554..afd1dba31 100644 --- a/require/requirements_test.go +++ b/require/requirements_test.go @@ -3,6 +3,10 @@ package require import ( "encoding/json" "errors" + "fmt" + "os" + "os/exec" + "strings" "testing" "time" @@ -788,3 +792,106 @@ func TestEventuallyWithTTrue(t *testing.T) { False(t, mockT.Failed, "Check should pass") Equal(t, 2, counter, "Condition is expected to be called 2 times") } + +func TestFailInsideEventuallyViaCommandLine(t *testing.T) { + t.Setenv("TestFailInsideEventually", "1") + cmd := exec.Command("go", "test", "-race", "-count=1", "-run", "^TestFailInsideEventually$") + out, err := cmd.CombinedOutput() + Error(t, err) + for _, line := range strings.Split(string(out), "\n") { + if strings.Contains(line, "ℹ️") { + fmt.Println(line) + } + NotContains(t, line, "❌") + } +} + +func TestFailInsideEventually(t *testing.T) { + if os.Getenv("TestFailInsideEventually") == "" { + t.Skip("Skipping failing test that calls Fail inside Eventually, set TestFailInsideEventually to run it") + } + // Using testing.T: + // Enable this test temporarily to manually check that the issue is fixed. + // Note that MockT does not reproduce the issue, so we have to use the real *testing.T. + // TODO: Enhance MockT to reproduce the issue, then we can remove the t.Skip above. + + // The Bug: + // Calling require.Fail (or similar) inside require.Eventually will prevent the 'condition' + // to exit with a result. The channel assignment in the assert.Eventually will block + // and hang the test until the timeout is reached. There was is not other way to wait + // for the unclean exit. The changes to assert.Eventually committed with this tis test + // fix this issue, by also waiting for an unclean exit of the condition. + + // How to read the test results: + // - See [TestFailInsideEventuallyViaCommandLine], which automates this + // - The test will always fail, because it calls Fail or assert.Fail + // - The test is "successful" if it fails quickly and cleanly, i.e. without + // multiple calls to the eventually function, except if expected. + // - The test logs should only contain INFO messages (see ℹ️ emoji) + // - The test must not log any UNCLEAN EXIT or MISSED ASSERTIONS messages or any errors ❌. + + type test struct { + Name string + Return bool + FailFunc func(t *testing.T) + MustStop bool // after the FailFunc is called + } + + const stopAfterFail = true + const noStopAfterFail = false + const mustStop = true + const mustNotStop = false + + RequireFail := func(t *testing.T) { t.Helper(); Fail(t, "fail now") } + AssertFail := func(t *testing.T) { t.Helper(); assert.Fail(t, "mark as failed") } + + for _, tt := range []test{ + {"require.Fail must stop", stopAfterFail, RequireFail, mustStop}, + {"require.Fail must stop even if told not to", noStopAfterFail, RequireFail, mustStop}, + {"assert.Fail must stop if told to", stopAfterFail, AssertFail, mustStop}, + {"assert.Fail must not stop if told not to", noStopAfterFail, AssertFail, mustNotStop}, + } { + count := 0 + start := time.Now() + timeout := time.Second * 1 + tick := time.Second / 3 + + ok := t.Run(tt.Name, func(t *testing.T) { + // Cannot use a MockT here, because it does reproduce the issue. + Eventually(t, func() bool { + count++ + t.Log("ℹ️ eventually call number:", count, "calling FailNow! 💥") + tt.FailFunc(t) // any case that calls Fail or assert.Fail should stop retrying + return tt.Return // indicate whether to stop retrying + }, timeout, tick) + }) + + dur := time.Since(start) + t.Log("DEBUG: test duration:", dur) + + // TODO: Replace with plain t with MockT once it can reproduce the issue. + // Until can only indicate that the test should have failed using stdout. + + if ok { + t.Logf("❌ Test should have failed, but did not") + } + + if tt.MustStop { + if count != 1 { + t.Logf("❌ UNCLEAN EXIT: eventually func should be called exactly once, but was called %d times", count) + } + if dur >= tick { + t.Logf("❌ UNCLEAN EXIT: eventually func should be called only once, but total duration was %s", dur) + } + } else { + if count <= 1 { + t.Logf("❌ MISSED ASSERTIONS: eventually func should be called multiple times, but was called %d times", count) + } + if dur < tick { + t.Logf("❌ MISSED ASSERTIONS: eventually func should be called multiple times over time, but total duration was only %s", dur) + } + } + } + + t.SkipNow() +} From 59520eed996259b09935ad262bdd7021cc24eff9 Mon Sep 17 00:00:00 2001 From: Uwe Jugel Date: Tue, 14 Oct 2025 13:20:38 +0200 Subject: [PATCH 2/6] add panic test --- require/requirements_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/require/requirements_test.go b/require/requirements_test.go index afd1dba31..c955e5439 100644 --- a/require/requirements_test.go +++ b/require/requirements_test.go @@ -795,7 +795,7 @@ func TestEventuallyWithTTrue(t *testing.T) { func TestFailInsideEventuallyViaCommandLine(t *testing.T) { t.Setenv("TestFailInsideEventually", "1") - cmd := exec.Command("go", "test", "-race", "-count=1", "-run", "^TestFailInsideEventually$") + cmd := exec.Command("go", "test", "-v", "-race", "-count=1", "-run", "^TestFailInsideEventually$") out, err := cmd.CombinedOutput() Error(t, err) for _, line := range strings.Split(string(out), "\n") { @@ -844,12 +844,14 @@ func TestFailInsideEventually(t *testing.T) { RequireFail := func(t *testing.T) { t.Helper(); Fail(t, "fail now") } AssertFail := func(t *testing.T) { t.Helper(); assert.Fail(t, "mark as failed") } + Panic := func(t *testing.T) { t.Helper(); panic("panicking now") } for _, tt := range []test{ {"require.Fail must stop", stopAfterFail, RequireFail, mustStop}, {"require.Fail must stop even if told not to", noStopAfterFail, RequireFail, mustStop}, {"assert.Fail must stop if told to", stopAfterFail, AssertFail, mustStop}, {"assert.Fail must not stop if told not to", noStopAfterFail, AssertFail, mustNotStop}, + {"panic must stop", stopAfterFail, Panic, mustStop}, } { count := 0 start := time.Now() From 7bbc95ed5e1f768ed867f39edd3092bb4b62b3c6 Mon Sep 17 00:00:00 2001 From: Uwe Jugel Date: Tue, 14 Oct 2025 21:25:24 +0200 Subject: [PATCH 3/6] recover on condition panic --- assert/assertions.go | 22 ++++---- require/require.go | 5 +- require/requirements_test.go | 102 ++++++++++++++++++++++------------- 3 files changed, 81 insertions(+), 48 deletions(-) diff --git a/assert/assertions.go b/assert/assertions.go index 558201ff5..4c596bc35 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -2011,13 +2011,17 @@ func Eventually(t TestingT, condition func() bool, waitFor time.Duration, tick t } const failed = 0 - const stop = 1 - const noStop = 2 + const panic = 1 + const stop = 2 + const noStop = 3 resultCh := make(chan int, 1) checkCond := func() { result := failed defer func() { + if r := recover(); r != nil { + result = panic + } resultCh <- result }() if condition() { @@ -2048,17 +2052,17 @@ func Eventually(t TestingT, condition func() bool, waitFor time.Duration, tick t go checkCond() // Schedule the next check. case v := <-resultCh: switch v { - case failed: + case failed, panic: // Condition panicked or test failed and finished. // Cannot determine correct result. // Cannot decide if we should continue gracefully or not. // We can stop here and now, and mark test as failed with // the same error message as the timeout case. - return Fail(t, "Condition never satisfied", msgAndArgs...) + return FailNow(t, "Condition never satisfied", msgAndArgs...) case stop: - return true + return true // Condition satisfied. case noStop: - fallthrough + fallthrough // Condition not satisfied yet, continue waiting. default: tickC = ticker.C // Enable ticks to check again. } @@ -2099,12 +2103,12 @@ func (*CollectT) Copy(TestingT) { } func (c *CollectT) fail() { - if !c.failed() { + if !c.Failed() { c.errors = []error{} // Make it non-nil to mark a failure. } } -func (c *CollectT) failed() bool { +func (c *CollectT) Failed() bool { return c.errors != nil } @@ -2164,7 +2168,7 @@ func EventuallyWithT(t TestingT, condition func(collect *CollectT), waitFor time tickC = nil go checkCond() case collect := <-ch: - if !collect.failed() { + 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. diff --git a/require/require.go b/require/require.go index 23a3be780..2a9e58b47 100644 --- a/require/require.go +++ b/require/require.go @@ -3,10 +3,11 @@ package require import ( - assert "github.com/stretchr/testify/assert" http "net/http" url "net/url" time "time" + + assert "github.com/stretchr/testify/assert" ) // Condition uses a Comparison to assert a complex condition. @@ -864,7 +865,7 @@ func Implements(t TestingT, interfaceObject interface{}, object interface{}, msg t.FailNow() } -// Implementsf asserts that an object is implemented by the specified interface. +// Implements asserts that an object is implemented by the specified interface. // // require.Implementsf(t, (*MyInterface)(nil), new(MyObject), "error message %s", "formatted") func Implementsf(t TestingT, interfaceObject interface{}, object interface{}, msg string, args ...interface{}) { diff --git a/require/requirements_test.go b/require/requirements_test.go index c955e5439..9fde9fdf5 100644 --- a/require/requirements_test.go +++ b/require/requirements_test.go @@ -3,7 +3,6 @@ package require import ( "encoding/json" "errors" - "fmt" "os" "os/exec" "strings" @@ -30,7 +29,11 @@ type AssertionTesterNonConformingObject struct { } type MockT struct { + // Failed marks the test as failed. Failed bool + // finished marks the test as finished, indicating that FailNow was called + // and no further code should be executed after that. + finished bool } // Helper is like [testing.T.Helper] but does nothing. @@ -38,8 +41,11 @@ func (MockT) Helper() {} func (t *MockT) FailNow() { t.Failed = true + t.finished = true } +func (t *MockT) Finished() bool { return t.finished } + func (t *MockT) Errorf(format string, args ...interface{}) { _, _ = format, args } @@ -797,23 +803,46 @@ func TestFailInsideEventuallyViaCommandLine(t *testing.T) { t.Setenv("TestFailInsideEventually", "1") cmd := exec.Command("go", "test", "-v", "-race", "-count=1", "-run", "^TestFailInsideEventually$") out, err := cmd.CombinedOutput() - Error(t, err) + assert.Error(t, err, "go test for TestFailInsideEventually must fail") + finishedTests := 0 + failedAssertions := 0 + expectedFailures := 0 for _, line := range strings.Split(string(out), "\n") { - if strings.Contains(line, "ℹ️") { - fmt.Println(line) + if strings.Contains(line, "❌") { + t.Log(strings.TrimSpace(line), " (this is NOT expected)") + failedAssertions++ + } + if strings.Contains(line, "✅ FINISHED") { + t.Log(strings.TrimSpace(line)) + finishedTests++ + } + if strings.Contains(line, "Error:") && strings.Contains(line, "Condition") { + t.Log(strings.TrimSpace(line), "(expected 'Condition …' message)") + expectedFailures++ } - NotContains(t, line, "❌") } + assert.Equal(t, 0, failedAssertions, "Unexpected errors detected, see output") + // There are 6 tests that are expected to fail. + // If you change the number of tests in TestFailInsideEventually, please update this number accordingly. + assert.Equal(t, 6, finishedTests, "Expected number of finished tests not found") + // There are 6 tests that are expected to fail, but one of them uses assert.Fail and return true. + // For this case the condition will be called once and then returns true. + // The "Condition ..." message is not printed in that case. Therefore we expect 5 failures here. + // If you change the number of tests in TestFailInsideEventually, please update this number accordingly. + assert.Equal(t, 5, expectedFailures, "Missed expected panics or final failures, see output") } func TestFailInsideEventually(t *testing.T) { if os.Getenv("TestFailInsideEventually") == "" { - t.Skip("Skipping failing test that calls Fail inside Eventually, set TestFailInsideEventually to run it") + t.Skip("Skipping test, run via TestFailInsideEventuallyViaCommandLine") } // Using testing.T: // Enable this test temporarily to manually check that the issue is fixed. // Note that MockT does not reproduce the issue, so we have to use the real *testing.T. // TODO: Enhance MockT to reproduce the issue, then we can remove the t.Skip above. + // UPDATE: Tried to enhance MockT, but it still does not reproduce the issue. + // require.MockT does not play well when assert.Fail is called. + // The test will not be marked as failed, even though Fail is called. // The Bug: // Calling require.Fail (or similar) inside require.Eventually will prevent the 'condition' @@ -833,67 +862,66 @@ func TestFailInsideEventually(t *testing.T) { type test struct { Name string Return bool - FailFunc func(t *testing.T) + FailFunc func(t TestingT) MustStop bool // after the FailFunc is called } - const stopAfterFail = true - const noStopAfterFail = false + const returnStop = true + const returnNoStop = false const mustStop = true const mustNotStop = false - RequireFail := func(t *testing.T) { t.Helper(); Fail(t, "fail now") } - AssertFail := func(t *testing.T) { t.Helper(); assert.Fail(t, "mark as failed") } - Panic := func(t *testing.T) { t.Helper(); panic("panicking now") } + RequireFail := func(t TestingT) { Fail(t, "💥 fail now") } + AssertFail := func(t TestingT) { assert.Fail(t, "💥 mark as failed") } + Panic := func(_ TestingT) { panic("💥 panicking now") } for _, tt := range []test{ - {"require.Fail must stop", stopAfterFail, RequireFail, mustStop}, - {"require.Fail must stop even if told not to", noStopAfterFail, RequireFail, mustStop}, - {"assert.Fail must stop if told to", stopAfterFail, AssertFail, mustStop}, - {"assert.Fail must not stop if told not to", noStopAfterFail, AssertFail, mustNotStop}, - {"panic must stop", stopAfterFail, Panic, mustStop}, + {"require.Fail must stop", returnStop, RequireFail, mustStop}, + {"require.Fail must stop even if told not to", returnNoStop, RequireFail, mustStop}, + {"assert.Fail must not stop if told not to", returnNoStop, AssertFail, mustNotStop}, + {"assert.Fail must stop if told to", returnStop, AssertFail, mustStop}, + {"panic must stop", returnStop, Panic, mustStop}, + {"panic must stop even if told not to", returnNoStop, Panic, mustStop}, + // Make sure to update the assertions in TestFailInsideEventuallyViaCommandLine + // accordingly if you change the number of tests here. } { count := 0 start := time.Now() timeout := time.Second * 1 tick := time.Second / 3 + ok := false - ok := t.Run(tt.Name, func(t *testing.T) { + ok = t.Run(tt.Name, func(t *testing.T) { // Cannot use a MockT here, because it does reproduce the issue. Eventually(t, func() bool { count++ - t.Log("ℹ️ eventually call number:", count, "calling FailNow! 💥") + t.Log("🪲 eventually call number:", count, "calling FailNow!") tt.FailFunc(t) // any case that calls Fail or assert.Fail should stop retrying return tt.Return // indicate whether to stop retrying }, timeout, tick) }) dur := time.Since(start) - t.Log("DEBUG: test duration:", dur) + t.Log("🪲 test duration:", dur) // TODO: Replace with plain t with MockT once it can reproduce the issue. // Until can only indicate that the test should have failed using stdout. - if ok { - t.Logf("❌ Test should have failed, but did not") - } + c := new(assert.CollectT) + assert.True(c, !ok, "❌ UNCLEAN EXIT: test was expected to fail, but passed") if tt.MustStop { - if count != 1 { - t.Logf("❌ UNCLEAN EXIT: eventually func should be called exactly once, but was called %d times", count) - } - if dur >= tick { - t.Logf("❌ UNCLEAN EXIT: eventually func should be called only once, but total duration was %s", dur) - } + assert.Equal(c, 1, count, "❌ UNCLEAN EXIT: eventually func should be called exactly once") + assert.Less(c, dur, tick, "❌ UNCLEAN EXIT: eventually func should be called only once, but took too long") } else { - if count <= 1 { - t.Logf("❌ MISSED ASSERTIONS: eventually func should be called multiple times, but was called %d times", count) - } - if dur < tick { - t.Logf("❌ MISSED ASSERTIONS: eventually func should be called multiple times over time, but total duration was only %s", dur) - } + assert.Greater(c, count, 1, "❌ MISSED ASSERTIONS: eventually func should be called multiple times, but was called only once") + assert.Greater(c, dur, tick, "❌ MISSED ASSERTIONS: eventually func should be called multiple times over time, but total duration was too short") } - } - t.SkipNow() + if c.Failed() { + t.Log("❌ TEST FAILED") + } else { + t.Log("✅ FINISHED", tt.Name) + } + } } From 27ea0dc195416199ce4b2e72e1aba038544efa66 Mon Sep 17 00:00:00 2001 From: Uwe Jugel Date: Tue, 14 Oct 2025 22:42:59 +0200 Subject: [PATCH 4/6] alternative way of handling failed and panicked conditions --- assert/assertions.go | 31 +++++++++++----- require/requirements_test.go | 72 +++++++++++++++++++++++++----------- 2 files changed, 72 insertions(+), 31 deletions(-) diff --git a/assert/assertions.go b/assert/assertions.go index 4c596bc35..59dca380b 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -2010,17 +2010,28 @@ func Eventually(t TestingT, condition func() bool, waitFor time.Duration, tick t h.Helper() } - const failed = 0 - const panic = 1 - const stop = 2 - const noStop = 3 - - resultCh := make(chan int, 1) + // "never satisfied" is the original message when a timeout happens. + const timeout = "never satisfied" + // "failed" means the condition function called require.Fail or similar. + const failed = "failed" + // "panicked" means the condition function panicked. + const panicked = "panicked" + + // "start" and "stop" are non-error result values from the condition function + const stop = "stop" + const noStop = "noStop" + + resultCh := make(chan string, 1) checkCond := func() { result := failed defer func() { + // A panic goes a different route that a failed test. + // We can distinguish them here and add the recover() result as detailed info. + // This is similar to what happens with a real panic in a test but allows us + // to stop the test gracefully. if r := recover(); r != nil { - result = panic + t.Errorf("Panic in condition: %v\n%s", r, debug.Stack()) + result = panicked } resultCh <- result }() @@ -2052,13 +2063,13 @@ func Eventually(t TestingT, condition func() bool, waitFor time.Duration, tick t go checkCond() // Schedule the next check. case v := <-resultCh: switch v { - case failed, panic: + case failed, panicked: // Condition panicked or test failed and finished. // Cannot determine correct result. // Cannot decide if we should continue gracefully or not. - // We can stop here and now, and mark test as failed with + // We can stop here and now, and mark test as finally failed with // the same error message as the timeout case. - return FailNow(t, "Condition never satisfied", msgAndArgs...) + return FailNow(t, "Condition "+v, msgAndArgs...) case stop: return true // Condition satisfied. case noStop: diff --git a/require/requirements_test.go b/require/requirements_test.go index 9fde9fdf5..382bcdcee 100644 --- a/require/requirements_test.go +++ b/require/requirements_test.go @@ -3,8 +3,10 @@ package require import ( "encoding/json" "errors" + "fmt" "os" "os/exec" + "regexp" "strings" "testing" "time" @@ -805,58 +807,79 @@ func TestFailInsideEventuallyViaCommandLine(t *testing.T) { out, err := cmd.CombinedOutput() assert.Error(t, err, "go test for TestFailInsideEventually must fail") finishedTests := 0 - failedAssertions := 0 - expectedFailures := 0 + observedErrors := 0 + observedConditionFailures := 0 + observedPanics := 0 + extractTestNameExp := regexp.MustCompile(`TestFailInsideEventuallyViaCommandLine[^ ]*`) + name := "" for _, line := range strings.Split(string(out), "\n") { + line = strings.TrimSpace(line) + if strings.HasPrefix(line, "=== RUN ") { + name = extractTestNameExp.FindString(line) + fmt.Println(line) + } + if strings.Contains(line, "❌") { - t.Log(strings.TrimSpace(line), " (this is NOT expected)") - failedAssertions++ + fmt.Println(name, line, "<-- unexpected error") + observedErrors++ } if strings.Contains(line, "✅ FINISHED") { - t.Log(strings.TrimSpace(line)) + fmt.Println(name, line, "<-- expected successful test") finishedTests++ } if strings.Contains(line, "Error:") && strings.Contains(line, "Condition") { - t.Log(strings.TrimSpace(line), "(expected 'Condition …' message)") - expectedFailures++ + fmt.Println(name, line, "<-- expected 'Condition' message") + observedConditionFailures++ + } + if strings.Contains(line, "Panic in condition:") { + fmt.Println(name, line, "<-- expected 'Condition' panic message") + observedPanics++ } } - assert.Equal(t, 0, failedAssertions, "Unexpected errors detected, see output") + + assert.Equal(t, 0, observedErrors, "Unexpected errors detected, see output") + // There are 6 tests that are expected to fail. // If you change the number of tests in TestFailInsideEventually, please update this number accordingly. - assert.Equal(t, 6, finishedTests, "Expected number of finished tests not found") - // There are 6 tests that are expected to fail, but one of them uses assert.Fail and return true. - // For this case the condition will be called once and then returns true. - // The "Condition ..." message is not printed in that case. Therefore we expect 5 failures here. + assert.Equal(t, 6, finishedTests, "Expected number of FINISHED tests not found") + + // There are 5 tests where the condition is never satisfied. + // One test uses assert.Fail but eventually returns true and thus satisfies the condition. // If you change the number of tests in TestFailInsideEventually, please update this number accordingly. - assert.Equal(t, 5, expectedFailures, "Missed expected panics or final failures, see output") + assert.Equal(t, 5, observedConditionFailures, "Expected number of 'Condition' messages not found, see output") + + // There are 2 tests that panic, so we expect 2 panic messages. + assert.Equal(t, 2, observedPanics, "Missed expected panics, see output") } func TestFailInsideEventually(t *testing.T) { if os.Getenv("TestFailInsideEventually") == "" { t.Skip("Skipping test, run via TestFailInsideEventuallyViaCommandLine") } + // Using testing.T: // Enable this test temporarily to manually check that the issue is fixed. // Note that MockT does not reproduce the issue, so we have to use the real *testing.T. // TODO: Enhance MockT to reproduce the issue, then we can remove the t.Skip above. - // UPDATE: Tried to enhance MockT, but it still does not reproduce the issue. - // require.MockT does not play well when assert.Fail is called. + // UPDATE: I tried to enhance MockT, but it still does not reproduce the issue or fails + // in a different way. Therefore I keep using *testing.T for now. + // In general, require.MockT does not play well when assert.Fail is called. // The test will not be marked as failed, even though Fail is called. // The Bug: // Calling require.Fail (or similar) inside require.Eventually will prevent the 'condition' // to exit with a result. The channel assignment in the assert.Eventually will block - // and hang the test until the timeout is reached. There was is not other way to wait - // for the unclean exit. The changes to assert.Eventually committed with this tis test - // fix this issue, by also waiting for an unclean exit of the condition. + // and hang the test until the timeout is reached. There was is no other way to wait + // for the unclean exit. The changes to assert.Eventually committed with this test + // fix this issue, by also waiting for an unclean exit of the condition and moreover + // by handling panics inside the condition gracefully. // How to read the test results: // - See [TestFailInsideEventuallyViaCommandLine], which automates this - // - The test will always fail, because it calls Fail or assert.Fail + // - The test will always fail, because it calls Fail or assert.Fail or panics // - The test is "successful" if it fails quickly and cleanly, i.e. without // multiple calls to the eventually function, except if expected. - // - The test logs should only contain INFO messages (see ℹ️ emoji) + // - The test logs should contain specific messages, see below. // - The test must not log any UNCLEAN EXIT or MISSED ASSERTIONS messages or any errors ❌. type test struct { @@ -876,12 +899,19 @@ func TestFailInsideEventually(t *testing.T) { Panic := func(_ TestingT) { panic("💥 panicking now") } for _, tt := range []test{ + // Test cases that must exit immediately after the first call to the condition. {"require.Fail must stop", returnStop, RequireFail, mustStop}, {"require.Fail must stop even if told not to", returnNoStop, RequireFail, mustStop}, - {"assert.Fail must not stop if told not to", returnNoStop, AssertFail, mustNotStop}, {"assert.Fail must stop if told to", returnStop, AssertFail, mustStop}, + + // The following test case is the only one that must not stop and where multiple calls + // to the condition are expected, because assert.Fail does not stop the execution of the condition. + {"assert.Fail must not stop if told not to", returnNoStop, AssertFail, mustNotStop}, + + // Panics must always stop, because they are not expected and indicate a bug in the code. {"panic must stop", returnStop, Panic, mustStop}, {"panic must stop even if told not to", returnNoStop, Panic, mustStop}, + // Make sure to update the assertions in TestFailInsideEventuallyViaCommandLine // accordingly if you change the number of tests here. } { From b9276033b772155ac653fa542e31ea89ff72d52c Mon Sep 17 00:00:00 2001 From: Uwe Jugel Date: Tue, 14 Oct 2025 22:46:18 +0200 Subject: [PATCH 5/6] remove mockT experiments --- assert/assertions.go | 26 +++++++++++++------------- require/requirements_test.go | 8 +------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/assert/assertions.go b/assert/assertions.go index 59dca380b..de7e72a2a 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -2010,20 +2010,20 @@ func Eventually(t TestingT, condition func() bool, waitFor time.Duration, tick t h.Helper() } - // "never satisfied" is the original message when a timeout happens. - const timeout = "never satisfied" - // "failed" means the condition function called require.Fail or similar. - const failed = "failed" - // "panicked" means the condition function panicked. - const panicked = "panicked" + // ResultTimeout is the original message when a timeout happens. + const ResultTimeout = "Condition never satisfied" + // ResultFailed means the condition function called require.Fail or similar. + const ResultFailed = "Condition failed" + // ResultPanic means the condition function panicked. + const ResultPanic = "Condition panicked" // "start" and "stop" are non-error result values from the condition function - const stop = "stop" - const noStop = "noStop" + const stop = "Condition satisfied" + const noStop = "Condition not satisfied" resultCh := make(chan string, 1) checkCond := func() { - result := failed + result := ResultFailed defer func() { // A panic goes a different route that a failed test. // We can distinguish them here and add the recover() result as detailed info. @@ -2031,7 +2031,7 @@ func Eventually(t TestingT, condition func() bool, waitFor time.Duration, tick t // to stop the test gracefully. if r := recover(); r != nil { t.Errorf("Panic in condition: %v\n%s", r, debug.Stack()) - result = panicked + result = ResultPanic } resultCh <- result }() @@ -2057,19 +2057,19 @@ func Eventually(t TestingT, condition func() bool, waitFor time.Duration, tick t for { select { case <-timer.C: - return Fail(t, "Condition never satisfied", msgAndArgs...) + return Fail(t, ResultTimeout, msgAndArgs...) case <-tickC: tickC = nil // Do not check again until we get a result. go checkCond() // Schedule the next check. case v := <-resultCh: switch v { - case failed, panicked: + case ResultFailed, ResultPanic: // Condition panicked or test failed and finished. // Cannot determine correct result. // Cannot decide if we should continue gracefully or not. // We can stop here and now, and mark test as finally failed with // the same error message as the timeout case. - return FailNow(t, "Condition "+v, msgAndArgs...) + return FailNow(t, v, msgAndArgs...) case stop: return true // Condition satisfied. case noStop: diff --git a/require/requirements_test.go b/require/requirements_test.go index 382bcdcee..e1c10fb35 100644 --- a/require/requirements_test.go +++ b/require/requirements_test.go @@ -33,9 +33,6 @@ type AssertionTesterNonConformingObject struct { type MockT struct { // Failed marks the test as failed. Failed bool - // finished marks the test as finished, indicating that FailNow was called - // and no further code should be executed after that. - finished bool } // Helper is like [testing.T.Helper] but does nothing. @@ -43,11 +40,8 @@ func (MockT) Helper() {} func (t *MockT) FailNow() { t.Failed = true - t.finished = true } -func (t *MockT) Finished() bool { return t.finished } - func (t *MockT) Errorf(format string, args ...interface{}) { _, _ = format, args } @@ -832,7 +826,7 @@ func TestFailInsideEventuallyViaCommandLine(t *testing.T) { observedConditionFailures++ } if strings.Contains(line, "Panic in condition:") { - fmt.Println(name, line, "<-- expected 'Condition' panic message") + fmt.Println(name, line, "<-- expected panic message") observedPanics++ } } From 80bd5f00945a4f77d29e758c7ee190fd8836bcbe Mon Sep 17 00:00:00 2001 From: Uwe Jugel Date: Thu, 16 Oct 2025 12:46:05 +0200 Subject: [PATCH 6/6] revert auto fmt --- require/require.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/require/require.go b/require/require.go index 2a9e58b47..23a3be780 100644 --- a/require/require.go +++ b/require/require.go @@ -3,11 +3,10 @@ package require import ( + assert "github.com/stretchr/testify/assert" http "net/http" url "net/url" time "time" - - assert "github.com/stretchr/testify/assert" ) // Condition uses a Comparison to assert a complex condition. @@ -865,7 +864,7 @@ func Implements(t TestingT, interfaceObject interface{}, object interface{}, msg t.FailNow() } -// Implements asserts that an object is implemented by the specified interface. +// Implementsf asserts that an object is implemented by the specified interface. // // require.Implementsf(t, (*MyInterface)(nil), new(MyObject), "error message %s", "formatted") func Implementsf(t TestingT, interfaceObject interface{}, object interface{}, msg string, args ...interface{}) {