From 93a9561b23b782244a7c5d77efe71f57dee8c4a5 Mon Sep 17 00:00:00 2001 From: Changkun Ou Date: Fri, 28 Feb 2020 21:53:38 +0100 Subject: [PATCH] testing: fix data race between parallel subtests This CL fixes a race condition if there are two subtests, and one finishing but the other is panicking. Fixes #37551 Change-Id: Ic33963eb338aec228964b95f7c34a0d207b91e00 Reviewed-on: https://go-review.googlesource.com/c/go/+/221322 Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- .../go/testdata/script/test_main_panic.txt | 30 +++++++++++++++++++ src/testing/testing.go | 11 ++++--- 2 files changed, 35 insertions(+), 6 deletions(-) create mode 100644 src/cmd/go/testdata/script/test_main_panic.txt diff --git a/src/cmd/go/testdata/script/test_main_panic.txt b/src/cmd/go/testdata/script/test_main_panic.txt new file mode 100644 index 00000000000..45887c5c733 --- /dev/null +++ b/src/cmd/go/testdata/script/test_main_panic.txt @@ -0,0 +1,30 @@ +[short] skip +[!race] skip + +! go test -v -race main_panic/testmain_parallel_sub_panic_test.go +! stdout 'DATA RACE' +-- main_panic/testmain_parallel_sub_panic_test.go -- +package testmain_parallel_sub_panic_test + +import "testing" + +func setup() { println("setup()") } +func teardown() { println("teardown()") } +func TestA(t *testing.T) { + t.Run("1", func(t *testing.T) { + t.Run("1", func(t *testing.T) { + t.Parallel() + panic("A/1/1 panics") + }) + t.Run("2", func(t *testing.T) { + t.Parallel() + println("A/1/2 is ok") + }) + }) +} + +func TestMain(m *testing.M) { + setup() + defer teardown() + m.Run() +} \ No newline at end of file diff --git a/src/testing/testing.go b/src/testing/testing.go index 5c78d9b7417..85a92c93848 100644 --- a/src/testing/testing.go +++ b/src/testing/testing.go @@ -928,16 +928,15 @@ func tRunner(t *T, fn func(t *T)) { t.Logf("cleanup panicked with %v", r) } // Flush the output log up to the root before dying. - t.mu.Lock() - root := &t.common - for ; root.parent != nil; root = root.parent { + for root := &t.common; root.parent != nil; root = root.parent { + root.mu.Lock() root.duration += time.Since(root.start) - fmt.Fprintf(root.parent.w, "--- FAIL: %s (%s)\n", root.name, fmtDuration(root.duration)) + d := root.duration + root.mu.Unlock() + root.flushToParent("--- FAIL: %s (%s)\n", root.name, fmtDuration(d)) if r := root.parent.runCleanup(recoverAndReturnPanic); r != nil { fmt.Fprintf(root.parent.w, "cleanup panicked with %v", r) } - root.parent.mu.Lock() - io.Copy(root.parent.w, bytes.NewReader(root.output)) } panic(err) }