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>

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
 	}()