]> Cypherpunks repositories - gostls13.git/commitdiff
testing: fix data race between parallel subtests
authorChangkun Ou <hi@changkun.us>
Fri, 28 Feb 2020 20:53:38 +0000 (21:53 +0100)
committerIan Lance Taylor <iant@golang.org>
Thu, 19 Mar 2020 21:07:59 +0000 (21:07 +0000)
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 <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
src/cmd/go/testdata/script/test_main_panic.txt [new file with mode: 0644]
src/testing/testing.go

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 (file)
index 0000000..45887c5
--- /dev/null
@@ -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
index 5c78d9b7417d4fee90371c595037f5578c93dcc7..85a92c93848d67a6c06644cd7d82d6a92b90635e 100644 (file)
@@ -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)
                }