From 7b9d3ff4cbd93c0b9e69c78cbb2cb891839c7fb3 Mon Sep 17 00:00:00 2001
From: Marcel van Lohuizen <mpvl@golang.org>
Date: Sat, 21 May 2016 14:37:29 +0200
Subject: [PATCH] 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/sub_test.go | 21 +++++++++++++++++++++
 src/testing/testing.go  | 12 ++++++++++--
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/src/testing/sub_test.go b/src/testing/sub_test.go
index 2804550737429b..2a24aaacfd72a0 100644
--- 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 3a7a135a3c6842..9943fa6b4d8c21 100644
--- 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
 	}()