testing: gracefully handle subtest failing parent’s T

Don’t panic if a subtest inadvertently calls FailNow
on a parent’s T.  Instead, report the offending subtest
while still reporting the error with the ancestor test and
keep exiting goroutines.

Note that this implementation has a race if parallel
subtests are failing the parent concurrently.
This is fine:
Calling FailNow on a parent is considered an error
in principle, at the moment, and is reported if it is
detected. Having the race allows the race detector
to detect the error as well.

Fixes #22882

Change-Id: Ifa6d5e55bb88f6bcbb562fc8c99f1f77e320015a
Reviewed-on: https://go-review.googlesource.com/97635
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
Reviewed-by: Kunpei Sakai <namusyaka@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

1 files changed, 18 insertions, 3 deletions

diff --git a/src/testing/testing.go b/src/testing/testing.go
index f56dbf8f6d..27d0de7728 100644
--- a/src/testing/testing.go
+++ b/src/testing/testing.go
@@ -718,6 +718,8 @@ type InternalTest struct {
 	F    func(*T)
 }
 
+var errNilPanicOrGoexit = errors.New("test executed panic(nil) or runtime.Goexit")
+
 func tRunner(t *T, fn func(t *T)) {
 	t.runner = callerName(0)
 
@@ -733,8 +735,17 @@ func tRunner(t *T, fn func(t *T)) {
 		t.duration += time.Since(t.start)
 		// If the test panicked, print any test output before dying.
 		err := recover()
+		signal := true
 		if !t.finished && err == nil {
-			err = fmt.Errorf("test executed panic(nil) or runtime.Goexit")
+			err = errNilPanicOrGoexit
+			for p := t.parent; p != nil; p = p.parent {
+				if p.finished {
+					t.Errorf("%v: subtest may have called FailNow on a parent test", err)
+					err = nil
+					signal = false
+					break
+				}
+			}
 		}
 		if err != nil {
 			t.Fail()
@@ -769,7 +780,7 @@ func tRunner(t *T, fn func(t *T)) {
 		if t.parent != nil && atomic.LoadInt32(&t.hasSub) == 0 {
 			t.setRan()
 		}
-		t.signal <- true
+		t.signal <- signal
 	}()
 
 	t.start = time.Now()
@@ -822,7 +833,11 @@ func (t *T) Run(name string, f func(t *T)) bool {
 	// without being preempted, even when their parent is a parallel test. This
 	// may especially reduce surprises if *parallel == 1.
 	go tRunner(t, f)
-	<-t.signal
+	if !<-t.signal {
+		// At this point, it is likely that FailNow was called on one of the
+		// parent tests by one of the subtests. Continue aborting up the chain.
+		runtime.Goexit()
+	}
 	return !t.failed
 }