testing: don't run Cleanup functions until parallel subtests complete

Fixes #31651

Change-Id: Idbab0c4355fcc58520e210126795223435cf0078
Reviewed-on: https://go-review.googlesource.com/c/go/+/214822
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

1 files changed, 44 insertions, 5 deletions

diff --git a/src/testing/testing.go b/src/testing/testing.go
index 15ff1dd81d..a875fe145f 100644
--- a/src/testing/testing.go
+++ b/src/testing/testing.go
@@ -791,15 +791,34 @@ func (c *common) Cleanup(f func()) {
 	}
 }
 
+// panicHanding is an argument to runCleanup.
+type panicHandling int
+
+const (
+	normalPanic panicHandling = iota
+	recoverAndReturnPanic
+)
+
 // runCleanup is called at the end of the test.
-func (c *common) runCleanup() {
+// If catchPanic is true, this will catch panics, and return the recovered
+// value if any.
+func (c *common) runCleanup(ph panicHandling) (panicVal interface{}) {
 	c.mu.Lock()
 	cleanup := c.cleanup
 	c.cleanup = nil
 	c.mu.Unlock()
-	if cleanup != nil {
-		cleanup()
+	if cleanup == nil {
+		return nil
+	}
+
+	if ph == recoverAndReturnPanic {
+		defer func() {
+			panicVal = recover()
+		}()
 	}
+
+	cleanup()
+	return nil
 }
 
 // callerName gives the function name (qualified with a package path)
@@ -902,19 +921,29 @@ func tRunner(t *T, fn func(t *T)) {
 				}
 			}
 		}
-		if err != nil {
+
+		doPanic := func(err interface{}) {
 			t.Fail()
+			if r := t.runCleanup(recoverAndReturnPanic); r != nil {
+				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 {
 				root.duration += time.Since(root.start)
 				fmt.Fprintf(root.parent.w, "--- FAIL: %s (%s)\n", root.name, fmtDuration(root.duration))
+				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)
 		}
+		if err != nil {
+			doPanic(err)
+		}
 
 		t.duration += time.Since(t.start)
 
@@ -928,6 +957,12 @@ func tRunner(t *T, fn func(t *T)) {
 			for _, sub := range t.sub {
 				<-sub.signal
 			}
+			cleanupStart := time.Now()
+			err := t.runCleanup(recoverAndReturnPanic)
+			t.duration += time.Since(cleanupStart)
+			if err != nil {
+				doPanic(err)
+			}
 			if !t.isParallel {
 				// Reacquire the count for sequential tests. See comment in Run.
 				t.context.waitParallel()
@@ -947,7 +982,11 @@ func tRunner(t *T, fn func(t *T)) {
 		}
 		t.signal <- signal
 	}()
-	defer t.runCleanup()
+	defer func() {
+		if len(t.sub) == 0 {
+			t.runCleanup(normalPanic)
+		}
+	}()
 
 	t.start = time.Now()
 	t.raceErrors = -race.Errors()