]> Cypherpunks repositories - gostls13.git/commitdiff
testing: prepare for the introduction of Run methods
authorMarcel van Lohuizen <mpvl@golang.org>
Tue, 19 Jan 2016 21:43:52 +0000 (22:43 +0100)
committerMarcel van Lohuizen <mpvl@golang.org>
Fri, 18 Mar 2016 11:25:54 +0000 (11:25 +0000)
The biggest change is that each test is now responsible for managing
the starting and stopping of its parallel subtests.

The "Main" test could be run as a tRunner as well. This shows that
the introduction of subtests is merely a generalization of and
consistent with the current semantics.

Change-Id: Ibf8388c08f85d4b2c0df69c069326762ed36a72e
Reviewed-on: https://go-review.googlesource.com/18893
Reviewed-by: Russ Cox <rsc@golang.org>
src/testing/benchmark.go
src/testing/sub_test.go [new file with mode: 0644]
src/testing/testing.go
test/fixedbugs/bug229.go

index a5b163201e0958971314ebd11bc1a4224c7615ac..b092a9d9e2f3d1b0224ae6d1c4e1f710a17f5433 100644 (file)
@@ -49,7 +49,7 @@ type B struct {
        N                int
        previousN        int           // number of iterations in the previous run
        previousDuration time.Duration // total duration of the previous run
-       benchmark        InternalBenchmark
+       benchFunc        func(b *B)
        bytes            int64
        timerOn          bool
        showAllocResult  bool
@@ -132,7 +132,7 @@ func (b *B) runN(n int) {
        b.parallelism = 1
        b.ResetTimer()
        b.StartTimer()
-       b.benchmark.F(b)
+       b.benchFunc(b)
        b.StopTimer()
        b.previousN = n
        b.previousDuration = b.duration
@@ -204,7 +204,7 @@ func (b *B) launch() {
        // Signal that we're done whether we return normally
        // or by FailNow's runtime.Goexit.
        defer func() {
-               b.signal <- b
+               b.signal <- true
        }()
 
        b.runN(n)
@@ -339,9 +339,10 @@ func runBenchmarksInternal(matchString func(pat, str string) (bool, error), benc
                        runtime.GOMAXPROCS(procs)
                        b := &B{
                                common: common{
-                                       signal: make(chan interface{}),
+                                       signal: make(chan bool),
+                                       name:   Benchmark.Name,
                                },
-                               benchmark: Benchmark,
+                               benchFunc: Benchmark.F,
                        }
                        benchName := benchmarkName(Benchmark.Name, procs)
                        fmt.Printf("%-*s\t", maxlen, benchName)
@@ -476,9 +477,9 @@ func (b *B) SetParallelism(p int) {
 func Benchmark(f func(b *B)) BenchmarkResult {
        b := &B{
                common: common{
-                       signal: make(chan interface{}),
+                       signal: make(chan bool),
                },
-               benchmark: InternalBenchmark{"", f},
+               benchFunc: f,
        }
        return b.run()
 }
diff --git a/src/testing/sub_test.go b/src/testing/sub_test.go
new file mode 100644 (file)
index 0000000..8cb13ee
--- /dev/null
@@ -0,0 +1,101 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package testing
+
+func TestTestContext(t *T) {
+       const (
+               add1 = 0
+               done = 1
+       )
+       // After each of the calls are applied to the context, the
+       type call struct {
+               typ int // run or done
+               // result from applying the call
+               running int
+               waiting int
+               started bool
+       }
+       testCases := []struct {
+               max int
+               run []call
+       }{{
+               max: 1,
+               run: []call{
+                       {typ: add1, running: 1, waiting: 0, started: true},
+                       {typ: done, running: 0, waiting: 0, started: false},
+               },
+       }, {
+               max: 1,
+               run: []call{
+                       {typ: add1, running: 1, waiting: 0, started: true},
+                       {typ: add1, running: 1, waiting: 1, started: false},
+                       {typ: done, running: 1, waiting: 0, started: true},
+                       {typ: done, running: 0, waiting: 0, started: false},
+                       {typ: add1, running: 1, waiting: 0, started: true},
+               },
+       }, {
+               max: 3,
+               run: []call{
+                       {typ: add1, running: 1, waiting: 0, started: true},
+                       {typ: add1, running: 2, waiting: 0, started: true},
+                       {typ: add1, running: 3, waiting: 0, started: true},
+                       {typ: add1, running: 3, waiting: 1, started: false},
+                       {typ: add1, running: 3, waiting: 2, started: false},
+                       {typ: add1, running: 3, waiting: 3, started: false},
+                       {typ: done, running: 3, waiting: 2, started: true},
+                       {typ: add1, running: 3, waiting: 3, started: false},
+                       {typ: done, running: 3, waiting: 2, started: true},
+                       {typ: done, running: 3, waiting: 1, started: true},
+                       {typ: done, running: 3, waiting: 0, started: true},
+                       {typ: done, running: 2, waiting: 0, started: false},
+                       {typ: done, running: 1, waiting: 0, started: false},
+                       {typ: done, running: 0, waiting: 0, started: false},
+               },
+       }}
+       for i, tc := range testCases {
+               ctx := &testContext{
+                       startParallel: make(chan bool),
+                       maxParallel:   tc.max,
+               }
+               for j, call := range tc.run {
+                       doCall := func(f func()) chan bool {
+                               done := make(chan bool)
+                               go func() {
+                                       f()
+                                       done <- true
+                               }()
+                               return done
+                       }
+                       started := false
+                       switch call.typ {
+                       case add1:
+                               signal := doCall(ctx.waitParallel)
+                               select {
+                               case <-signal:
+                                       started = true
+                               case ctx.startParallel <- true:
+                                       <-signal
+                               }
+                       case done:
+                               signal := doCall(ctx.release)
+                               select {
+                               case <-signal:
+                               case <-ctx.startParallel:
+                                       started = true
+                                       <-signal
+                               }
+                       }
+                       if started != call.started {
+                               t.Errorf("%d:%d:started: got %v; want %v", i, j, started, call.started)
+                       }
+                       if ctx.running != call.running {
+                               t.Errorf("%d:%d:running: got %v; want %v", i, j, ctx.running, call.running)
+                       }
+                       if ctx.numWaiting != call.waiting {
+                               t.Errorf("%d:%d:waiting: got %v; want %v", i, j, ctx.numWaiting, call.waiting)
+                       }
+               }
+       }
+}
index 981883e07afea0da1f0ca8f7efd9fdffdcc7ae8f..edd6c4fe749c859d9188de37608d6ba60fd567f5 100644 (file)
@@ -147,6 +147,7 @@ import (
        "bytes"
        "flag"
        "fmt"
+       "io"
        "os"
        "runtime"
        "runtime/debug"
@@ -197,14 +198,18 @@ var (
 type common struct {
        mu       sync.RWMutex // guards output and failed
        output   []byte       // Output generated by test or benchmark.
+       w        io.Writer    // For flushToParent.
        failed   bool         // Test or benchmark has failed.
        skipped  bool         // Test of benchmark has been skipped.
        finished bool
 
+       parent   *common
+       name     string    // Name of test or benchmark.
        start    time.Time // Time test or benchmark started
        duration time.Duration
-       self     interface{}      // To be sent on signal channel when done.
-       signal   chan interface{} // Output for serial tests.
+       barrier  chan bool // To signal parallel subtests they may start.
+       signal   chan bool // To signal a test is done.
+       sub      []*T      // Queue of subtests to be run in parallel.
 }
 
 // Short reports whether the -test.short flag is set.
@@ -251,6 +256,22 @@ func decorate(s string) string {
        return buf.String()
 }
 
+// flushToParent writes c.output to the parent after first writing the header
+// with the given format and arguments.
+func (c *common) flushToParent(format string, args ...interface{}) {
+       p := c.parent
+       p.mu.Lock()
+       defer p.mu.Unlock()
+
+       fmt.Fprintf(p.w, format, args...)
+       fmt.Fprintln(p.w)
+
+       c.mu.Lock()
+       defer c.mu.Unlock()
+       io.Copy(p.w, bytes.NewReader(c.output))
+       c.output = c.output[:0]
+}
+
 // fmtDuration returns a string representing d in the form "87.00s".
 func fmtDuration(d time.Duration) string {
        return fmt.Sprintf("%.2fs", d.Seconds())
@@ -293,15 +314,17 @@ var _ TB = (*B)(nil)
 // may be called simultaneously from multiple goroutines.
 type T struct {
        common
-       name          string // Name of test.
-       isParallel    bool
-       startParallel chan bool // Parallel tests will wait on this.
+       isParallel bool
+       context    *testContext // For running tests and subtests.
 }
 
 func (c *common) private() {}
 
 // Fail marks the function as having failed but continues execution.
 func (c *common) Fail() {
+       if c.parent != nil {
+               c.parent.Fail()
+       }
        c.mu.Lock()
        defer c.mu.Unlock()
        c.failed = true
@@ -437,8 +460,13 @@ func (t *T) Parallel() {
        // in the test duration. Record the elapsed time thus far and reset the
        // timer afterwards.
        t.duration += time.Since(t.start)
-       t.signal <- (*T)(nil) // Release main testing loop
-       <-t.startParallel     // Wait for serial tests to finish
+
+       // Add to the list of tests to be released by the parent.
+       t.parent.sub = append(t.parent.sub, t)
+
+       t.signal <- true   // Release calling test.
+       <-t.parent.barrier // Wait for the parent test to complete.
+       t.context.waitParallel()
        t.start = time.Now()
 }
 
@@ -449,8 +477,8 @@ type InternalTest struct {
        F    func(*T)
 }
 
-func tRunner(t *T, test *InternalTest) {
-       // When this goroutine is done, either because test.F(t)
+func tRunner(t *T, fn func(t *T)) {
+       // When this goroutine is done, either because fn(t)
        // returned normally or because a test failure triggered
        // a call to runtime.Goexit, record the duration and send
        // a signal saying that the test is done.
@@ -466,14 +494,87 @@ func tRunner(t *T, test *InternalTest) {
                        t.report()
                        panic(err)
                }
-               t.signal <- t
+
+               if len(t.sub) > 0 {
+                       // Run parallel subtests.
+                       // Decrease the running count for this test.
+                       t.context.release()
+                       // Release the parallel subtests.
+                       close(t.barrier)
+                       // Wait for subtests to complete.
+                       for _, sub := range t.sub {
+                               <-sub.signal
+                       }
+                       if !t.isParallel {
+                               // Reacquire the count for sequential tests. See comment in Run.
+                               t.context.waitParallel()
+                       }
+               } else if t.isParallel {
+                       // Only release the count for this test if it was run as a parallel
+                       // test. See comment in Run method.
+                       t.context.release()
+               }
+               t.report() // Report after all subtests have finished.
+
+               t.signal <- true
        }()
 
        t.start = time.Now()
-       test.F(t)
+       fn(t)
        t.finished = true
 }
 
+// testContext holds all fields that are common to all tests. This includes
+// synchronization primitives to run at most *parallel tests.
+type testContext struct {
+       mu sync.Mutex
+
+       // Channel used to signal tests that are ready to be run in parallel.
+       startParallel chan bool
+
+       // running is the number of tests currently running in parallel.
+       // This does not include tests that are waiting for subtests to complete.
+       running int
+
+       // numWaiting is the number tests waiting to be run in parallel.
+       numWaiting int
+
+       // maxParallel is a copy of the parallel flag.
+       maxParallel int
+}
+
+func newTestContext(maxParallel int) *testContext {
+       return &testContext{
+               startParallel: make(chan bool),
+               maxParallel:   *parallel,
+               running:       1, // Set the count to 1 for the main (sequential) test.
+       }
+}
+
+func (c *testContext) waitParallel() {
+       c.mu.Lock()
+       if c.running < c.maxParallel {
+               c.running++
+               c.mu.Unlock()
+               return
+       }
+       c.numWaiting++
+       c.mu.Unlock()
+       <-c.startParallel
+}
+
+func (c *testContext) release() {
+       c.mu.Lock()
+       if c.numWaiting == 0 {
+               c.running--
+               c.mu.Unlock()
+               return
+       }
+       c.numWaiting--
+       c.mu.Unlock()
+       c.startParallel <- true // Pick a waiting test to be run.
+}
+
 // An internal function but exported because it is cross-package; part of the implementation
 // of the "go test" command.
 func Main(matchString func(pat, str string) (bool, error), tests []InternalTest, benchmarks []InternalBenchmark, examples []InternalExample) {
@@ -526,15 +627,18 @@ func (m *M) Run() int {
 }
 
 func (t *T) report() {
+       if t.parent == nil {
+               return
+       }
        dstr := fmtDuration(t.duration)
-       format := "--- %s: %s (%s)\n%s"
+       format := "--- %s: %s (%s)"
        if t.Failed() {
-               fmt.Printf(format, "FAIL", t.name, dstr, t.output)
+               t.flushToParent(format, "FAIL", t.name, dstr)
        } else if *chatty {
                if t.Skipped() {
-                       fmt.Printf(format, "SKIP", t.name, dstr, t.output)
+                       t.flushToParent(format, "SKIP", t.name, dstr)
                } else {
-                       fmt.Printf(format, "PASS", t.name, dstr, t.output)
+                       t.flushToParent(format, "PASS", t.name, dstr)
                }
        }
 }
@@ -547,63 +651,55 @@ func RunTests(matchString func(pat, str string) (bool, error), tests []InternalT
        }
        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 test
-               // kicks off a goroutine that Fails, yet the test still delivers a completion signal,
-               // which skews the counting.
-               var collector = make(chan interface{})
-
-               numParallel := 0
-               startParallel := make(chan bool)
-
-               for i := 0; i < len(tests); i++ {
-                       matched, err := matchString(*match, tests[i].Name)
-                       if err != nil {
-                               fmt.Fprintf(os.Stderr, "testing: invalid regexp for -test.run: %s\n", err)
-                               os.Exit(1)
-                       }
-                       if !matched {
-                               continue
-                       }
-                       testName := tests[i].Name
-                       t := &T{
-                               common: common{
-                                       signal: make(chan interface{}),
-                               },
-                               name:          testName,
-                               startParallel: startParallel,
-                       }
-                       t.self = t
-                       if *chatty {
-                               fmt.Printf("=== RUN   %s\n", t.name)
-                       }
-                       go tRunner(t, &tests[i])
-                       out := (<-t.signal).(*T)
-                       if out == nil { // Parallel run.
-                               go func() {
-                                       collector <- <-t.signal
-                               }()
-                               numParallel++
-                               continue
-                       }
-                       t.report()
-                       ok = ok && !out.Failed()
+               ctx := newTestContext(*parallel)
+               t := &T{
+                       common: common{
+                               signal:  make(chan bool),
+                               barrier: make(chan bool),
+                               w:       os.Stdout,
+                       },
+                       context: ctx,
                }
 
-               running := 0
-               for numParallel+running > 0 {
-                       if running < *parallel && numParallel > 0 {
-                               startParallel <- true
-                               running++
-                               numParallel--
-                               continue
+               tRunner(t, func(t *T) {
+                       for i := 0; i < len(tests); i++ {
+                               // TODO: a version of this will be the Run method.
+                               matched, err := matchString(*match, tests[i].Name)
+                               if err != nil {
+                                       fmt.Fprintf(os.Stderr, "testing: invalid regexp for -test.run: %s\n", err)
+                                       os.Exit(1)
+                               }
+                               if !matched {
+                                       continue
+                               }
+                               testName := tests[i].Name
+                               t := &T{
+                                       common: common{
+                                               barrier: make(chan bool),
+                                               signal:  make(chan bool),
+                                               name:    testName,
+                                               parent:  &t.common,
+                                       },
+                                       context: t.context,
+                               }
+
+                               if *chatty {
+                                       fmt.Printf("=== RUN   %s\n", t.name)
+                               }
+                               // Instead of reducing the running count of this test before calling the
+                               // tRunner and increasing it afterwards, we rely on tRunner keeping the
+                               // count correct. This ensures that a sequence of sequential tests runs
+                               // without being preempted, even when their parent is a parallel test. This
+                               // may especially reduce surprises if *parallel == 1.
+                               go tRunner(t, tests[i].F)
+                               <-t.signal
                        }
-                       t := (<-collector).(*T)
-                       t.report()
-                       ok = ok && !t.Failed()
-                       running--
-               }
+                       // Run catching the signal rather than the tRunner as a separate
+                       // goroutine to avoid adding a goroutine during the sequential
+                       // phase as this pollutes the stacktrace output when aborting.
+                       go func() { <-t.signal }()
+               })
+               ok = ok && !t.Failed()
        }
        return
 }
index 19776881d170f3263b73c1a43fb44d9665cdac75..5cc3988555504e31740e67fb2035f436570b7fb5 100644 (file)
@@ -14,7 +14,7 @@ func main() {
        // make sure error mentions that
        // name is unexported, not just "name not found".
 
-       t.name = nil    // ERROR "unexported"
+       t.common.name = nil     // ERROR "unexported"
        
        println(testing.anyLowercaseName("asdf"))       // ERROR "unexported" "undefined: testing.anyLowercaseName"
 }