From 95d874e9b4ffd995e638887aa9220ea7fe69af1c Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 19 May 2026 18:07:03 +0000 Subject: [PATCH] cmd/testwrapper: surface race reports and skip retries when detected A data race in a package matters more than any individual test result. Two related problems: 1. Where go test's race detector text ("WARNING: DATA RACE" plus the goroutine stack traces) lands in JSON output is timing- dependent: it can be attributed to a test that ends up reporting PASS (e.g. when the racing goroutines outlive the test that spawned them and TSan prints during a different test's window). testwrapper's main loop only flushes the logs of failed tests, so the race report ends up stuck in a passing test's buffer and is silently dropped. The race builders just see a bare "FAIL\nFAIL\tpkg\ttime". 2. If the failing test in such a package happens to be marked flaky, testwrapper retries it. That is the worst possible response to a race: the flaky test might not even be the racy code, and a second run without the racy goroutines could "succeed" while hiding the real bug. Address both: scan every output line for the race detector's first- line marker. Track whether the package observed a race at all, on the pkgFinished testAttempt. When a race was seen, fold every per- test log buffer into the package-level logs (so the full report surfaces from the existing pkg-fail flush path), and drop any flaky-test retry plans for that package so we fail immediately instead of running another attempt. Two new tests: - TestRaceSuppressesFlakyRetry verifies that a flaky test alongside a racy test does NOT get retried. - TestRaceAttributedToPassingTest verifies that a race attributed by test2json to a passing test still surfaces in the output. Also add a corpus of captured raw test binary outputs under cmd/testwrapper/testdata/, with one subdirectory per scenario, documenting the six representative shapes that go test -race can emit (race in test body, race in goroutines that outlive a test, race forced into a later test, race in TestMain post-m.Run, and a parallel-tests split-attribution case via a "=== NAME" redirect line). See its README.md for details. Fixes #19603 Change-Id: Ifbfcd67fb3b1882c4907bd9cb2d68a8b5a91dd54 Signed-off-by: Brad Fitzpatrick --- cmd/testwrapper/testdata/A_baseline/raw.txt | 6 + cmd/testwrapper/testdata/A_baseline/src.go | 13 ++ cmd/testwrapper/testdata/B_inbody/raw.txt | 30 ++++ cmd/testwrapper/testdata/B_inbody/src.go | 19 +++ .../testdata/C_spawnwait/caught.raw.txt | 32 +++++ .../testdata/C_spawnwait/pass.raw.txt | 31 ++++ cmd/testwrapper/testdata/C_spawnwait/src.go | 22 +++ cmd/testwrapper/testdata/D_delayed/raw.txt | 32 +++++ cmd/testwrapper/testdata/D_delayed/src.go | 28 ++++ cmd/testwrapper/testdata/E_testmain/raw.txt | 26 ++++ cmd/testwrapper/testdata/E_testmain/src.go | 24 ++++ .../testdata/F_parallel/split.raw.txt | 57 ++++++++ cmd/testwrapper/testdata/F_parallel/src.go | 22 +++ cmd/testwrapper/testdata/README.md | 133 ++++++++++++++++++ cmd/testwrapper/testwrapper.go | 77 +++++++++- cmd/testwrapper/testwrapper_test.go | 112 +++++++++++++++ 16 files changed, 657 insertions(+), 7 deletions(-) create mode 100644 cmd/testwrapper/testdata/A_baseline/raw.txt create mode 100644 cmd/testwrapper/testdata/A_baseline/src.go create mode 100644 cmd/testwrapper/testdata/B_inbody/raw.txt create mode 100644 cmd/testwrapper/testdata/B_inbody/src.go create mode 100644 cmd/testwrapper/testdata/C_spawnwait/caught.raw.txt create mode 100644 cmd/testwrapper/testdata/C_spawnwait/pass.raw.txt create mode 100644 cmd/testwrapper/testdata/C_spawnwait/src.go create mode 100644 cmd/testwrapper/testdata/D_delayed/raw.txt create mode 100644 cmd/testwrapper/testdata/D_delayed/src.go create mode 100644 cmd/testwrapper/testdata/E_testmain/raw.txt create mode 100644 cmd/testwrapper/testdata/E_testmain/src.go create mode 100644 cmd/testwrapper/testdata/F_parallel/split.raw.txt create mode 100644 cmd/testwrapper/testdata/F_parallel/src.go create mode 100644 cmd/testwrapper/testdata/README.md diff --git a/cmd/testwrapper/testdata/A_baseline/raw.txt b/cmd/testwrapper/testdata/A_baseline/raw.txt new file mode 100644 index 000000000..36bfad518 --- /dev/null +++ b/cmd/testwrapper/testdata/A_baseline/raw.txt @@ -0,0 +1,6 @@ +=== RUN TestOne + a_test.go:6: hello from one +--- PASS: TestOne (0.00s) +=== RUN TestTwo +--- PASS: TestTwo (0.00s) +PASS diff --git a/cmd/testwrapper/testdata/A_baseline/src.go b/cmd/testwrapper/testdata/A_baseline/src.go new file mode 100644 index 000000000..5f90db1dd --- /dev/null +++ b/cmd/testwrapper/testdata/A_baseline/src.go @@ -0,0 +1,13 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package baseline + +import "testing" + +func TestOne(t *testing.T) { + t.Log("hello from one") +} + +func TestTwo(t *testing.T) { +} diff --git a/cmd/testwrapper/testdata/B_inbody/raw.txt b/cmd/testwrapper/testdata/B_inbody/raw.txt new file mode 100644 index 000000000..108a0ec59 --- /dev/null +++ b/cmd/testwrapper/testdata/B_inbody/raw.txt @@ -0,0 +1,30 @@ +=== RUN TestRace +================== +WARNING: DATA RACE +Read at 0x0000007f12a8 by goroutine 8: + racesurvey/B_inbody.TestRace.func1() + /tmp/racesurvey/B_inbody/b_test.go:13 +0x74 + +Previous write at 0x0000007f12a8 by goroutine 9: + racesurvey/B_inbody.TestRace.func2() + /tmp/racesurvey/B_inbody/b_test.go:14 +0x8c + +Goroutine 8 (running) created at: + racesurvey/B_inbody.TestRace() + /tmp/racesurvey/B_inbody/b_test.go:13 +0xbe + testing.tRunner() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2036 +0x21c + testing.(*T).Run.gowrap1() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2101 +0x38 + +Goroutine 9 (finished) created at: + racesurvey/B_inbody.TestRace() + /tmp/racesurvey/B_inbody/b_test.go:14 +0x124 + testing.tRunner() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2036 +0x21c + testing.(*T).Run.gowrap1() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2101 +0x38 +================== + testing.go:1712: race detected during execution of test +--- FAIL: TestRace (0.00s) +FAIL diff --git a/cmd/testwrapper/testdata/B_inbody/src.go b/cmd/testwrapper/testdata/B_inbody/src.go new file mode 100644 index 000000000..f0abebb99 --- /dev/null +++ b/cmd/testwrapper/testdata/B_inbody/src.go @@ -0,0 +1,19 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package inbody + +import ( + "sync" + "testing" +) + +var counter int + +func TestRace(t *testing.T) { + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); counter++ }() + go func() { defer wg.Done(); counter++ }() + wg.Wait() +} diff --git a/cmd/testwrapper/testdata/C_spawnwait/caught.raw.txt b/cmd/testwrapper/testdata/C_spawnwait/caught.raw.txt new file mode 100644 index 000000000..c2dcf3655 --- /dev/null +++ b/cmd/testwrapper/testdata/C_spawnwait/caught.raw.txt @@ -0,0 +1,32 @@ +=== RUN TestSpawn +================== +WARNING: DATA RACE +Read at 0x0000007f12a8 by goroutine 9: + racesurvey/C_spawnwait.TestSpawn.func2() + /tmp/racesurvey/C_spawnwait/c_test.go:14 +0x70 + +Previous write at 0x0000007f12a8 by goroutine 8: + racesurvey/C_spawnwait.TestSpawn.func1() + /tmp/racesurvey/C_spawnwait/c_test.go:13 +0x88 + +Goroutine 9 (running) created at: + racesurvey/C_spawnwait.TestSpawn() + /tmp/racesurvey/C_spawnwait/c_test.go:14 +0x44 + testing.tRunner() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2036 +0x21c + testing.(*T).Run.gowrap1() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2101 +0x38 + +Goroutine 8 (finished) created at: + racesurvey/C_spawnwait.TestSpawn() + /tmp/racesurvey/C_spawnwait/c_test.go:13 +0x34 + testing.tRunner() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2036 +0x21c + testing.(*T).Run.gowrap1() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2101 +0x38 +================== + testing.go:1712: race detected during execution of test +--- FAIL: TestSpawn (0.00s) +=== RUN TestWait +--- PASS: TestWait (0.00s) +FAIL diff --git a/cmd/testwrapper/testdata/C_spawnwait/pass.raw.txt b/cmd/testwrapper/testdata/C_spawnwait/pass.raw.txt new file mode 100644 index 000000000..c7ba6ebf3 --- /dev/null +++ b/cmd/testwrapper/testdata/C_spawnwait/pass.raw.txt @@ -0,0 +1,31 @@ +=== RUN TestSpawn +--- PASS: TestSpawn (0.00s) +=== RUN TestWait +================== +WARNING: DATA RACE +Read at 0x0000007f12a8 by goroutine 8: + racesurvey/C_spawnwait.TestSpawn.func1() + /tmp/racesurvey/C_spawnwait/c_test.go:13 +0x70 + +Previous write at 0x0000007f12a8 by goroutine 9: + racesurvey/C_spawnwait.TestSpawn.func2() + /tmp/racesurvey/C_spawnwait/c_test.go:14 +0x88 + +Goroutine 8 (running) created at: + racesurvey/C_spawnwait.TestSpawn() + /tmp/racesurvey/C_spawnwait/c_test.go:13 +0x34 + testing.tRunner() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2036 +0x21c + testing.(*T).Run.gowrap1() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2101 +0x38 + +Goroutine 9 (finished) created at: + racesurvey/C_spawnwait.TestSpawn() + /tmp/racesurvey/C_spawnwait/c_test.go:14 +0x44 + testing.tRunner() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2036 +0x21c + testing.(*T).Run.gowrap1() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2101 +0x38 +================== +--- PASS: TestWait (0.00s) +FAIL diff --git a/cmd/testwrapper/testdata/C_spawnwait/src.go b/cmd/testwrapper/testdata/C_spawnwait/src.go new file mode 100644 index 000000000..42e13fef1 --- /dev/null +++ b/cmd/testwrapper/testdata/C_spawnwait/src.go @@ -0,0 +1,22 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package spawnwait + +import ( + "sync" + "testing" +) + +var counter int +var wg sync.WaitGroup + +func TestSpawn(t *testing.T) { + wg.Add(2) + go func() { defer wg.Done(); counter++ }() + go func() { defer wg.Done(); counter++ }() +} + +func TestWait(t *testing.T) { + wg.Wait() +} diff --git a/cmd/testwrapper/testdata/D_delayed/raw.txt b/cmd/testwrapper/testdata/D_delayed/raw.txt new file mode 100644 index 000000000..d7d22f21e --- /dev/null +++ b/cmd/testwrapper/testdata/D_delayed/raw.txt @@ -0,0 +1,32 @@ +=== RUN TestA +--- PASS: TestA (0.00s) +=== RUN TestSleep +================== +WARNING: DATA RACE +Read at 0x0000007f12a8 by goroutine 9: + racesurvey/D_postterminal.TestA.func2() + /tmp/racesurvey/D_postterminal/d_test.go:16 +0x8a + +Previous write at 0x0000007f12a8 by goroutine 8: + racesurvey/D_postterminal.TestA.func1() + /tmp/racesurvey/D_postterminal/d_test.go:15 +0xa4 + +Goroutine 9 (running) created at: + racesurvey/D_postterminal.TestA() + /tmp/racesurvey/D_postterminal/d_test.go:16 +0x44 + testing.tRunner() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2036 +0x21c + testing.(*T).Run.gowrap1() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2101 +0x38 + +Goroutine 8 (finished) created at: + racesurvey/D_postterminal.TestA() + /tmp/racesurvey/D_postterminal/d_test.go:15 +0x34 + testing.tRunner() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2036 +0x21c + testing.(*T).Run.gowrap1() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2101 +0x38 +================== + testing.go:1712: race detected during execution of test +--- FAIL: TestSleep (0.05s) +FAIL diff --git a/cmd/testwrapper/testdata/D_delayed/src.go b/cmd/testwrapper/testdata/D_delayed/src.go new file mode 100644 index 000000000..f0fed3cfc --- /dev/null +++ b/cmd/testwrapper/testdata/D_delayed/src.go @@ -0,0 +1,28 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package delayed + +import ( + "sync" + "testing" + "time" +) + +var counter int +var wg sync.WaitGroup +var trigger = make(chan struct{}) + +func TestA(t *testing.T) { + wg.Add(2) + go func() { defer wg.Done(); <-trigger; counter++ }() + go func() { defer wg.Done(); <-trigger; counter++ }() +} + +func TestSleep(t *testing.T) { + close(trigger) + // Sleep long enough that the goroutines race during this sleep, + // but TestSleep itself doesn't write to counter. + time.Sleep(50 * time.Millisecond) + wg.Wait() +} diff --git a/cmd/testwrapper/testdata/E_testmain/raw.txt b/cmd/testwrapper/testdata/E_testmain/raw.txt new file mode 100644 index 000000000..df96dbe1f --- /dev/null +++ b/cmd/testwrapper/testdata/E_testmain/raw.txt @@ -0,0 +1,26 @@ +=== RUN TestPass +--- PASS: TestPass (0.00s) +PASS +================== +WARNING: DATA RACE +Read at 0x0000007f52a8 by goroutine 9: + racesurvey/E_testmain.TestMain.func2() + /tmp/racesurvey/E_testmain/e_test.go:18 +0x74 + +Previous write at 0x0000007f52a8 by goroutine 8: + racesurvey/E_testmain.TestMain.func1() + /tmp/racesurvey/E_testmain/e_test.go:17 +0x8c + +Goroutine 9 (running) created at: + racesurvey/E_testmain.TestMain() + /tmp/racesurvey/E_testmain/e_test.go:18 +0x139 + main.main() + _testmain.go:48 +0x171 + +Goroutine 8 (finished) created at: + racesurvey/E_testmain.TestMain() + /tmp/racesurvey/E_testmain/e_test.go:17 +0xcc + main.main() + _testmain.go:48 +0x171 +================== +Found 1 data race(s) diff --git a/cmd/testwrapper/testdata/E_testmain/src.go b/cmd/testwrapper/testdata/E_testmain/src.go new file mode 100644 index 000000000..ec15be22b --- /dev/null +++ b/cmd/testwrapper/testdata/E_testmain/src.go @@ -0,0 +1,24 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package testmain + +import ( + "sync" + "testing" +) + +var counter int + +func TestPass(t *testing.T) { +} + +func TestMain(m *testing.M) { + code := m.Run() + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); counter++ }() + go func() { defer wg.Done(); counter++ }() + wg.Wait() + _ = code +} diff --git a/cmd/testwrapper/testdata/F_parallel/split.raw.txt b/cmd/testwrapper/testdata/F_parallel/split.raw.txt new file mode 100644 index 000000000..6c8e1137d --- /dev/null +++ b/cmd/testwrapper/testdata/F_parallel/split.raw.txt @@ -0,0 +1,57 @@ +=== RUN TestParA +=== PAUSE TestParA +=== RUN TestParB +=== PAUSE TestParB +=== CONT TestParA +=== CONT TestParB +--- PASS: TestParA (0.00s) +================== +WARNING: DATA RACE +Read at 0x0000007f02b0 by goroutine 8: + racesurvey/F_parallel.TestParB() + /tmp/racesurvey/F_parallel/f_test.go:17 +0x3b + testing.tRunner() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2036 +0x21c + testing.(*T).Run.gowrap1() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2101 +0x38 + +Previous write at 0x0000007f02b0 by goroutine 7: + racesurvey/F_parallel.TestParA() + /tmp/racesurvey/F_parallel/f_test.go:10 +0x53 + testing.tRunner() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2036 +0x21c + testing.(*T).Run.gowrap1() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2101 +0x38 + +Goroutine 8 (running) created at: + testing.(*T).Run() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2101 +0xb12 + testing.runTests.func1() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2585 +0x84 + testing.tRunner() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2036 +0x21c + testing.runTests() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2583 +0x9e9 + testing.(*M).Run() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2443 +0xf4b + main.main() + _testmain.go:48 +0x164 + +Goroutine 7 (running) created at: + testing.(*T).Run() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2101 +0xb12 + testing.runTests.func1() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2585 +0x84 + testing.tRunner() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2036 +0x21c + testing.runTests() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2583 +0x9e9 + testing.(*M).Run() + /home/ubuntu/sdk/go1.26.3/src/testing/testing.go:2443 +0xf4b + main.main() + _testmain.go:48 +0x164 +================== +=== NAME TestParB + testing.go:1712: race detected during execution of test +--- FAIL: TestParB (0.00s) +FAIL diff --git a/cmd/testwrapper/testdata/F_parallel/src.go b/cmd/testwrapper/testdata/F_parallel/src.go new file mode 100644 index 000000000..18cd968ff --- /dev/null +++ b/cmd/testwrapper/testdata/F_parallel/src.go @@ -0,0 +1,22 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package parallel + +import "testing" + +var counter int + +func TestParA(t *testing.T) { + t.Parallel() + for i := 0; i < 100; i++ { + counter++ + } +} + +func TestParB(t *testing.T) { + t.Parallel() + for i := 0; i < 100; i++ { + counter++ + } +} diff --git a/cmd/testwrapper/testdata/README.md b/cmd/testwrapper/testdata/README.md new file mode 100644 index 000000000..d15dc64c3 --- /dev/null +++ b/cmd/testwrapper/testdata/README.md @@ -0,0 +1,133 @@ +# Race-output test corpus + +This directory is a corpus of captured Go test binary outputs that +exercise the various ways the `-race` detector's `WARNING: DATA RACE` +text can land relative to `=== RUN` / `--- PASS:` / `--- FAIL:` / +`=== NAME` lines, and how `cmd/internal/test2json` attributes that +output to tests. + +Each scenario subdirectory contains: + +- `src.go` — the Go source code that was compiled and run to produce + the captured output. Reproduce via + `go test -race -c -o /tmp/scenario.test .//`. +- `raw.txt` (or scenario-specific name) — the raw stdout+stderr of + the resulting test binary when run as `./scenario.test -test.v`. + This is the byte stream that `go test -json` feeds to + `go tool test2json` in production. + +`go test -json` adds two things on top of what `test2json` sees, +which are NOT in these captures: a `FAIL\t\t