From 66155134a7daa2a28bf0ecd55bcf36be3b21e473 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Thu, 22 Dec 2011 10:43:54 -0800 Subject: [PATCH] testing: make signalling safer for parallel tests Each test gets a private signal channel. Also fix a bug that prevented parallel tests from running. R=r, r CC=golang-dev https://golang.org/cl/5505061 --- src/pkg/testing/testing.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/pkg/testing/testing.go b/src/pkg/testing/testing.go index ded48d9e21c..b1fb8dd3de6 100644 --- a/src/pkg/testing/testing.go +++ b/src/pkg/testing/testing.go @@ -182,8 +182,8 @@ func (c *common) Fatalf(format string, args ...interface{}) { // Parallel signals that this test is to be run in parallel with (and only with) // other parallel tests in this CPU group. func (t *T) Parallel() { - t.signal <- nil // Release main testing loop - <-t.startParallel // Wait for serial tests to finish + t.signal <- (*T)(nil) // Release main testing loop + <-t.startParallel // Wait for serial tests to finish } // An internal type but exported because it is cross-package; part of the implementation @@ -236,11 +236,14 @@ func RunTests(matchString func(pat, str string) (bool, error), tests []InternalT fmt.Fprintln(os.Stderr, "testing: warning: no tests to run") return } - // TODO: each test should have its own channel, although that means - // keeping track of the channels when we're running parallel tests. - signal := make(chan interface{}) for _, procs := range cpuList { runtime.GOMAXPROCS(procs) + // We build a new channel tree for each run of the loop. + // collector merges in one channel all the upstream signals from parallel tests. + // If all tests pump to the same channel, a bug can occur where a goroutine + // kicks off a test, fails, and still delivers a completion signal, which skews the + // counting. + var collector = make(chan interface{}) numParallel := 0 startParallel := make(chan bool) @@ -260,7 +263,7 @@ func RunTests(matchString func(pat, str string) (bool, error), tests []InternalT } t := &T{ common: common{ - signal: signal, + signal: make(chan interface{}), }, name: testName, startParallel: startParallel, @@ -272,6 +275,9 @@ func RunTests(matchString func(pat, str string) (bool, error), tests []InternalT go tRunner(t, &tests[i]) out := (<-t.signal).(*T) if out == nil { // Parallel run. + go func() { + collector <- <-t.signal + }() numParallel++ continue } @@ -287,7 +293,7 @@ func RunTests(matchString func(pat, str string) (bool, error), tests []InternalT numParallel-- continue } - t := (<-signal).(*T) + t := (<-collector).(*T) t.report() ok = ok && !t.failed running--