[go] testing: expose subtest and subbenchmark functionality

[Marcel van Lohuizen (Gerrit)]

Reviewers: Russ Cox

Marcel van Lohuizen uploaded a change:
https://go-review.googlesource.com/18898

testing: expose subtest and subbenchmark functionality

Fixes #12166

Change-Id: Ie62cba2c39beb5732447ba3688c93c08ef12abb5
---
M src/testing/benchmark.go
M src/testing/sub_test.go
M src/testing/testing.go
3 files changed, 39 insertions(+), 17 deletions(-)



diff --git a/src/testing/benchmark.go b/src/testing/benchmark.go
index fdf7758..73c3d08 100644
--- a/src/testing/benchmark.go
+++ b/src/testing/benchmark.go
@@ -14,7 +14,7 @@
"time"
)

-var matchBenchmarks = flag.String("test.bench", "", "regular expression to
select benchmarks to run")
+var matchBenchmarks = flag.String("test.bench", "", "regular expression
per path component to select benchmarks to run")
var benchTime = flag.Duration("test.benchtime",
1*time.Second, "approximate run time for each benchmark")
var benchmarkMemory = flag.Bool("test.benchmem", false, "print memory
allocations for benchmarks")

@@ -366,7 +366,7 @@
common: common{name: "Main"},
benchFunc: func(b *B) {
for _, Benchmark := range bs {
- b.runBench(Benchmark.Name, Benchmark.F)
+ b.Run(Benchmark.Name, Benchmark.F)
}
},
benchTime: *benchTime,
@@ -422,12 +422,12 @@
}
}

-// runBench benchmarks f as a subbenchmark with the given name. It reports
+// Run benchmarks f as a subbenchmark with the given name. It reports
// whether there were any failures.
//
// A subbenchmark is like any other benchmark. A benchmark that calls Run
at
// least once will not be measured itself and will only run for one
iteration.
-func (b *B) runBench(name string, f func(b *B)) bool {
+func (b *B) Run(name string, f func(b *B)) bool {
// b is no longer to be treated as a normal benchmark. Release the lock
and
// acquire it on exit to ensure locks stay paired.
b.hasSub = true
@@ -577,6 +577,9 @@

// Benchmark benchmarks a single function. Useful for creating
// custom benchmarks that do not use the "go test" command.
+//
+// If f calls Run, the result will be an estimate of running all its
+// subbenchmarks that don't call Run in sequence in a single benchmark.
func Benchmark(f func(b *B)) BenchmarkResult {
b := &B{
common: common{
diff --git a/src/testing/sub_test.go b/src/testing/sub_test.go
index 64fea4f..45dc4d1 100644
--- a/src/testing/sub_test.go
+++ b/src/testing/sub_test.go
@@ -90,9 +90,6 @@
}
}

-// TODO: remove this stub when API is exposed
-func (t *T) Run(name string, f func(t *T)) bool { return t.run(name, f) }
-
func TestTRun(t *T) {
realTest := t
ctx := &testContext{
@@ -228,9 +225,6 @@
}
}
}
-
-// TODO: remove this stub when API is exposed
-func (b *B) Run(name string, f func(b *B)) bool { return b.runBench(name,
f) }

func TestBRun(t *T) {
work := func(b *B) {
diff --git a/src/testing/testing.go b/src/testing/testing.go
index 4834f82..a18bea4 100644
--- a/src/testing/testing.go
+++ b/src/testing/testing.go
@@ -118,6 +118,28 @@
// example function, at least one other function, type, variable, or
constant
// declaration, and no test or benchmark functions.
//
+// Subunits
+//
+// The Run methods of T and B allow running subunits for the respective
types.
+// A subtest or subbenchmark is just like any other test or benchmark.
Subunits
+// enable many things, such as more control over parallelism, clustered
failure,
+// and dynamically generating tests or benchmarks.
+//
+// Each subunit has a unique identifier which is obtained from joining its
name
+// with that of its ancestors using '/' as path separator. The name for a
+// subunit that is given by the user is sanitized and uniqued before doing
so.
+// Spaces are replaced by underscores and duplicated names are appended
with a
+// unique suffix.
+//
+// An identifier can be passed to -test.run or -test.bench flag as is to
select
+// the specific test or benchmark. Each path component (the part between
the
+// separators) is a regular expression that needs to match that component.
+//
+// Example filters
+// go test -run=Foo # Run top-level tests that contain "Foo"
and their subtests.
+// go test -run=Foo/A:3 # Further constrain its subtests with names
containing "A:3".
+// go test -run="/A:1 B:2" # Run all direct subtests
containing "A:1_B:2".
+//
// Main
//
// It is sometimes necessary for a test program to do extra setup or
teardown
@@ -177,7 +199,7 @@
chatty = flag.Bool("test.v", false, "verbose: print additional
output")
count = flag.Uint("test.count", 1, "run tests and benchmarks
`n` times")
coverProfile = flag.String("test.coverprofile", "", "write a coverage
profile to the named file after execution")
- match = flag.String("test.run", "", "regular expression to
select tests and examples to run")
+ match = flag.String("test.run", "", "regular expression per
path component to select tests and examples to run")
memProfile = flag.String("test.memprofile", "", "write a memory
profile to the named file after execution")
memProfileRate = flag.Int("test.memprofilerate", 0, "if >=0, sets
runtime.MemProfileRate")
cpuProfile = flag.String("test.cpuprofile", "", "write a cpu
profile to the named file during execution")
@@ -344,7 +366,8 @@

func (c *common) private() {}

-// Fail marks the function as having failed but continues execution.
+// Fail marks the function and its calling functions as having failed but
+// continues execution.
func (c *common) Fail() {
if c.parent != nil {
c.parent.Fail()
@@ -361,7 +384,8 @@
return c.failed
}

-// FailNow marks the function as having failed and stops its execution.
+// FailNow marks the function as having failed, stops its execution
+// and aborts pending parallel subtests.
// Execution will continue at the next test or benchmark.
// FailNow must be called from the goroutine running the
// test or benchmark function, not from other goroutines
@@ -453,7 +477,8 @@
c.SkipNow()
}

-// SkipNow marks the test as having been skipped and stops its execution.
+// SkipNow marks the test as having been skipped, stops its execution
+// and aborts pending parallel subtests.
// Execution will continue at the next test or benchmark. See also FailNow.
// SkipNow must be called from the goroutine running the test, not from
// other goroutines created during the test. Calling SkipNow does not stop
@@ -558,9 +583,9 @@
t.finished = true
}

-// run runs f as a subtest of t called name. It reports whether f
succeeded.
+// Run runs f as a subtest of t called name. It reports whether f
succeeded.
// Run will block until all its parallel subtests have completed.
-func (t *T) run(name string, f func(t *T)) bool {
+func (t *T) Run(name string, f func(t *T)) bool {
testName, ok := t.context.match.fullName(&t.common, name)
if !ok {
return true
@@ -723,7 +748,7 @@
// be one more thing for users to analyze when investigating stack
// traces.
for _, test := range tests {
- t.run(test.Name, test.F)
+ t.Run(test.Name, test.F)
}
// These two loops are copied from tRunner.
for _, sub := range t.sub {

--
https://go-review.googlesource.com/18898
Gerrit-Reviewer: Russ Cox <r...@golang.org>

[Russ Cox (Gerrit)]

Russ Cox has posted comments on this change.

testing: expose subtest and subbenchmark functionality

Patch Set 1:

(6 comments)

https://go-review.googlesource.com/#/c/18898/1/src/testing/testing.go
File src/testing/testing.go:

Line 121: // Subunits
Subtests is fine. We don't need yet another name (units).
More generally this section will need some work.
But I think the details should probably all be here, not scattered below.
Hence the 'undo' comments in the individual places below.


Line 124: // A subtest or subbenchmark is just like any other test or
benchmark. Subunits
And then here just say

A subtest (or subbenchmark)


Line 202: match = flag.String("test.run", "", "regular

expression per path component to select tests and examples to run")

undo
drop "per path component "
no one will understand what that means without reading the full docs anyway.


Line 369: // Fail marks the function and its calling functions as having
failed but
undo


Line 387: // FailNow marks the function as having failed, stops its
execution
undo


Line 480: // SkipNow marks the test as having been skipped, stops its
execution
undo

--
https://go-review.googlesource.com/18898
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: Yes

[Russ Cox (Gerrit)]

Russ Cox has posted comments on this change.

testing: expose subtest and subbenchmark functionality

Patch Set 1:

(2 comments)

https://go-review.googlesource.com/#/c/18898/1/src/testing/testing.go
File src/testing/testing.go:

Line 388: // and aborts pending parallel subtests.
FWIW, I am not convinced that this should abort pending parallel subtests,
but I also don't see code implementing that claim. So maybe it's okay just
to remove the text.


Line 481: // and aborts pending parallel subtests.
FWIW, I am not convinced that this should abort pending parallel subtests,
but I also don't see code implementing that claim. So maybe it's okay just
to remove the text.

--
https://go-review.googlesource.com/18898
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: Yes

[Russ Cox (Gerrit)]

Russ Cox has posted comments on this change.

testing: expose subtest and subbenchmark functionality

Patch Set 1:

R=go1.7

--
https://go-review.googlesource.com/18898
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: No

[Marcel van Lohuizen (Gerrit)]

Marcel van Lohuizen uploaded a new patch set:

https://go-review.googlesource.com/18898

testing: expose subtest and subbenchmark functionality

Fixes #12166

Change-Id: Ie62cba2c39beb5732447ba3688c93c08ef12abb5
---
M src/testing/benchmark.go
M src/testing/sub_test.go
M src/testing/testing.go

3 files changed, 10 insertions(+), 13 deletions(-)

[Marcel van Lohuizen (Gerrit)]

Marcel van Lohuizen has posted comments on this change.

testing: expose subtest and subbenchmark functionality

Patch Set 2:

(7 comments)

https://go-review.googlesource.com/#/c/18898/1/src/testing/testing.go
File src/testing/testing.go:

Line 121: // Main

> Subtests is fine. We don't need yet another name (units).

removed this section for after the run.test and run.bnech flags allow
seelecting subtest or subbenchmarks.


Line 202: failed bool // Test or benchmark has failed.
> undo
Done


Line 369: // those other goroutines.
> undo
Done


Line 387: // before the test exited. If a test deferred an important
cleanup
> undo
Done


Line 388: // function (like removing temporary files), there was no
guarantee

> FWIW, I am not convinced that this should abort pending parallel subtests,

The code was there, but you're right. Removed it.


Line 480: }
> undo
Done


Line 481: t.isParallel = true

> FWIW, I am not convinced that this should abort pending parallel subtests,

Done


--
https://go-review.googlesource.com/18898
Gerrit-Reviewer: Marcel van Lohuizen <mp...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>
Gerrit-HasComments: Yes

[Russ Cox (Gerrit)]

Russ Cox has posted comments on this change.

testing: expose subtest and subbenchmark functionality

Patch Set 2:

R=go1.7
(For next cycle I will make that tag not cleared by new CLs.)

--
https://go-review.googlesource.com/18898
Gerrit-Reviewer: Marcel van Lohuizen <mp...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: No

[Russ Cox (Gerrit)]

Russ Cox has posted comments on this change.

testing: expose subtest and subbenchmark functionality

Patch Set 3: Code-Review+2

[Marcel van Lohuizen (Gerrit)]

Reviewers: Russ Cox

Marcel van Lohuizen uploaded a new patch set:

https://go-review.googlesource.com/18898

testing: expose subtest and subbenchmark functionality

Fixes #12166

Change-Id: Ie62cba2c39beb5732447ba3688c93c08ef12abb5
---
M src/testing/benchmark.go
M src/testing/sub_test.go
M src/testing/testing.go

3 files changed, 10 insertions(+), 13 deletions(-)

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

testing: expose subtest and subbenchmark functionality

Patch Set 6:

TryBots beginning. Status page: http://farmer.golang.org/try?commit=fe06449b

--
https://go-review.googlesource.com/18898
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

Gerrit-Reviewer: Marcel van Lohuizen <mp...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: No

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

testing: expose subtest and subbenchmark functionality

Patch Set 6: TryBot-Result+1

TryBots are happy.

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

testing: expose subtest and subbenchmark functionality

Patch Set 7:

TryBots beginning. Status page: http://farmer.golang.org/try?commit=b2430f75

[Marcel van Lohuizen (Gerrit)]

Marcel van Lohuizen has submitted this change and it was merged.

testing: expose subtest and subbenchmark functionality

Fixes #12166

Change-Id: Ie62cba2c39beb5732447ba3688c93c08ef12abb5

Reviewed-on: https://go-review.googlesource.com/18898
Reviewed-by: Russ Cox <r...@golang.org>
Run-TryBot: Marcel van Lohuizen <mp...@golang.org>

---
M src/testing/benchmark.go
M src/testing/sub_test.go
M src/testing/testing.go

3 files changed, 10 insertions(+), 13 deletions(-)

Approvals:
Russ Cox: Looks good to me, approved
Marcel van Lohuizen: Run TryBots