diff --git a/src/cmd/go/testdata/script/test_fail_fast.txt b/src/cmd/go/testdata/script/test_fail_fast.txt index 1f169d6da8717b..89a304941e2739 100644 --- a/src/cmd/go/testdata/script/test_fail_fast.txt +++ b/src/cmd/go/testdata/script/test_fail_fast.txt @@ -32,6 +32,13 @@ stdout -count=5 'FAIL - ' ! go test ./failfast_test.go -run='TestParallelFailingSubtestsA' -failfast=true stdout -count=1 'FAIL - ' +# a failure in a cleanup of a test with a parallel subtest must stop failfast +# (go.dev/issue/61034) +! go test ./failfast_test.go -run='TestCleanupFailingWithParallelSubtestA|TestFailingB' -failfast=true +stdout -count=1 'FAIL - ' +! go test ./failfast_test.go -run='TestCleanupFailingWithParallelSubtestA|TestFailingB' -failfast=false +stdout -count=2 'FAIL - ' + # only parallels ! go test ./failfast_test.go -run='TestParallelFailing[AB]' -failfast=false stdout -count=2 'FAIL - ' @@ -109,6 +116,18 @@ func TestFailingSubtestsA(t *testing.T) { }) } +func TestCleanupFailingWithParallelSubtestA(t *testing.T) { + // Regression test for go.dev/issue/61034: a failure in a Cleanup + // function must be counted for -failfast even when the test has a + // parallel subtest. + t.Cleanup(func() { + t.Errorf("FAIL - %s", t.Name()) + }) + t.Run("sub", func(t *testing.T) { + t.Parallel() + }) +} + func TestFailingB(t *testing.T) { t.Errorf("FAIL - %s", t.Name()) } diff --git a/src/testing/testing.go b/src/testing/testing.go index 8b7742d85edd9a..832d9e597fb4a2 100644 --- a/src/testing/testing.go +++ b/src/testing/testing.go @@ -2037,11 +2037,6 @@ func tRunner(t *T, fn func(t *T)) { defer func() { t.checkRaces() - // TODO(#61034): This is the wrong place for this check. - if t.Failed() { - numFailed.Add(1) - } - // Check if the test panicked or Goexited inappropriately. // // If this happens in a normal test, print output but continue panicking. @@ -2170,6 +2165,14 @@ func tRunner(t *T, fn func(t *T)) { for root := &t.common; root.parent != nil; root = root.parent { root.flushPartial() } + + // Record this test's failure for -failfast. This must happen after the + // test's cleanup functions have run, since a cleanup may call t.Fail. + // See go.dev/issue/61034. + if t.Failed() { + numFailed.Add(1) + } + t.report() // Report after all subtests have finished. // Do not lock t.done to allow race detector to detect race in case