testing: don't be silent if a test's goroutine fails a test after test exits

Fixes #15654

Change-Id: I9bdaa9b76d480d75f24d95f0235efd4a79e3593e
Reviewed-on: https://go-review.googlesource.com/23320
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>

diff --git a/src/testing/sub_test.go b/src/testing/sub_test.go
index 2804550737429b23cd09a16584d458c84ff52579..2a24aaacfd72a031deb98832f89fcf99d1c0d099 100644 (file)
--- a/src/testing/sub_test.go
+++ b/src/testing/sub_test.go
@@ -307,6 +307,27 @@ func TestTRun(t *T) {
                f: func(t *T) {
                        t.Skip()
                },
+       }, {
+               desc:   "panic on goroutine fail after test exit",
+               ok:     false,
+               maxPar: 4,
+               f: func(t *T) {
+                       ch := make(chan bool)
+                       t.Run("", func(t *T) {
+                               go func() {
+                                       <-ch
+                                       defer func() {
+                                               if r := recover(); r == nil {
+                                                       realTest.Errorf("expected panic")
+                                               }
+                                               ch <- true
+                                       }()
+                                       t.Errorf("failed after success")
+                               }()
+                       })
+                       ch <- true
+                       <-ch
+               },
        }}
        for _, tc := range testCases {
                ctx := newTestContext(tc.maxPar, newMatcher(regexp.MatchString, "", ""))

diff --git a/src/testing/testing.go b/src/testing/testing.go
index 3a7a135a3c6842e6d5a7fc967ebd4d745e6bc1fe..9943fa6b4d8c21e4a6fdf2ff7495f87f3c4d4b0e 100644 (file)
--- a/src/testing/testing.go
+++ b/src/testing/testing.go
@@ -196,13 +196,14 @@ var (
 // common holds the elements common between T and B and
 // captures common methods such as Errorf.
 type common struct {
-       mu       sync.RWMutex // guards output and failed
+       mu       sync.RWMutex // guards output, failed, and done.
        output   []byte       // Output generated by test or benchmark.
        w        io.Writer    // For flushToParent.
        chatty   bool         // A copy of the chatty flag.
        failed   bool         // Test or benchmark has failed.
        skipped  bool         // Test of benchmark has been skipped.
-       finished bool
+       finished bool         // Test function has completed.
+       done     bool         // Test is finished and all subtests have completed.
 
        parent   *common
        level    int       // Nesting depth of test or benchmark.
@@ -351,6 +352,10 @@ func (c *common) Fail() {
        }
        c.mu.Lock()
        defer c.mu.Unlock()
+       // c.done needs to be locked to synchronize checks to c.done in parent tests.
+       if c.done {
+               panic("Fail in goroutine after " + c.name + " has completed")
+       }
        c.failed = true
 }
 
@@ -540,6 +545,9 @@ func tRunner(t *T, fn func(t *T)) {
                }
                t.report() // Report after all subtests have finished.
 
+               // Do not lock t.done to allow race detector to detect race in case
+               // the user does not appropriately synchronizes a goroutine.
+               t.done = true
                t.signal <- true
        }()