design/2981: simplify design
Address comments by adg and aclements
Change-Id: I7a1d8f640a06804f7809e977758d301198839f04
Reviewed-on: https://go-review.googlesource.com/29157
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/design/2981-go-test-json.md b/design/2981-go-test-json.md
index 7393bda..d37720a 100644
--- a/design/2981-go-test-json.md
+++ b/design/2981-go-test-json.md
@@ -4,7 +4,7 @@
_With initial input by Russ Cox, Caleb Spare, Andrew Gerrand and Minux Ma._
-Last updated: 2015-10-16
+Last updated: 2016-09-14
Discussion at https://golang.org/issue/2981.
@@ -12,7 +12,7 @@
* [Background](#background)
* [Proposal](#proposal)
* [`testing` package](#testing-package)
- * [`go test`](#go-test)
+ * [Example output](#example-output)
* [Rationale](#rationale)
* [Compatibility](#compatibility)
* [Implementation](#implementation)
@@ -32,37 +32,29 @@
Currently, under certain conditions, `go test` streams test/benchmark results
so a user can see them as they happen.
+Also streaming prevents losing data if `go test` crashes.
This proposal attempts to preserve streaming capability in the `-json` mode, so
third party tools interpreting `go test` output can stream results too.
`-json` flag was originally proposed by Russ Cox in
-https://golang.org/issue/2981 in 2012.
-This proposal differs from the original:
-
-* supports streaming
-* `go test` JSON output contains unrecognized test binary output.
-* minimal changes in `testing` package.
-* output is not indented
+https://golang.org/issue/2981 in 2012. This proposal has several differences.
## Proposal
I propose the following user-visible changes:
* add `-json` flag to `go test`
- * `-json`: all `go test` stdout is JSON objects containing
- test binary artifacts, separated by newline.
+ * `-json`: `go test` stdout is a valid [JSON Text Sequence][rfc7464]
+ of JSON objects containing test binary artifacts.
Format below.
- * `-json -v`: verbose messages are printed to stderr,
- so stdout contains only JSON.
+ * `-json -v`: verbose messages are printed to stderr, so stdout contains
+ only JSON.
* `-json -n`: not supported
* `-json -x`: not supported
* In `testing` package
- * Add `type State` which is enum
- * Add `Name`, `Log`, `State` and `Procs` fields to `BenchmarkResult`.
- * Add `type CoverageResult`.
- * In type `Cover`, change `CoveredPackages` field type from `string` to
- `[]string`. This type is not covered by Go 1 compatibility guidelines.
- * Add `type JSONResult` for JSON output.
+ * Add `type State int` with constants to describe test/benchmark states.
+ * Add type `JSONResult` for JSON output.
+ * Change `Cover.CoveredPackages` field type from `string` to `[]string`.
Type definitions and details below.
@@ -75,267 +67,277 @@
type State int
const (
- RUN State = iota
+ // RUN means a test/benchmark execution has started
+ RUN State = iota + 1
PASS
FAIL
SKIP
)
-type BenchmarkResult struct {
- Name string
- State State
- Procs int // The value of runtime.GOMAXPROCS for this benchmark run.
- Log string // The log created by calling (*B).Log and (*B).Logf.
-
- // existing fields
- // make them `json:",omitempty"`
-}
-
-// CoverageResult is aggregated code coverage info.
-// It is used for `go test` JSON output.
-// To get full coverage info, use -coverprofile flag in go test.
-type CoverageResult struct {
- Mode string
- TotalStatements int64
- ActiveStatements int64
- CoveredPackages []string
-}
-
-// JSONResult is used for test binary JSON output format.
-//
-// Each time a test/benchmark completes, the test binary emits one result
-// in JSON format to stdout, surrounded by '\n'.
+// JSONResult structs encoded in JSON are emitted by `go test` if -json flag is
+// specified.
type JSONResult struct {
- // BenchmarkResult contains fields used by both benchmarks and tests,
- // such as Name and State.
- BenchmarkResult
+ // Configuration is metadata produced by test/benchmark infrastructure.
+ // The interpretation of a key/value pair is up to tooling, but the key/value
+ // pair describes all test/benchmark results that follow,
+ // until overwritten by a JSONResult with a non-empty Configuration field.
+ //
+ // The key begins with a lowercase character (as defined by unicode.IsLower),
+ // contains no space characters (as defined by unicode.IsSpace)
+ // nor upper case characters (as defined by unicode.IsUpper).
+ // Conventionally, multiword keys are written with the words separated by hyphens,
+ // as in cpu-speed.
+ Configuration map[string]string
- Coverage *CoverageResult `json:",omitempty"`
+ // Package is a full name of the package containing the test/benchmark.
+ // It is zero iff Name is zero.
+ Package string `json:",omitempty"
+ // Name is the name of the test/benchmark that this JSONResult is about.
+ // It can be empty if JSONResult describes global state, such as
+ // Configuration or Stdout/Stderr.
+ Name string `json:",omitempty"
+ // State is the current state of the test/benchmark.
+ // It is non-zero iff Name is non-zero.
+ State State `json:",omitempty"
+ // Procs is the value of runtime.GOMAXPROCS for this test/benchmark run.
+ // It is specified only in the first JSONResult of a test/benchmark.
+ Procs int `json:",omitempty"
+ // Log is log created by calling Log or Logf functions of *T or *B.
+ // A JSONResult with Log is emitted by go test as soon as possible.
+ // First occurrence of test/benchmark does not contain logs.
+ Log string `json:",omitempty"
+
+ // Benchmark contains benchmark-specific details.
+ // It is emitted in the final JSONResult of a benchmark with a terminal
+ // State if the benchmark does not have sub-benchmarks.
+ Benchmark *BenchmarkResult `json:",omitempty"
+
+ // CoverageMode is coverage mode that was used to run these tests.
+ CoverageMode string `json:",omitempty"
+ // TotalStatements is the number of statements checked for coverage.
+ TotalStatements int64 `json:",omitempty"
+ // ActiveStatements is the number of statements covered by tests, examples
+ // or benchmarks.
+ ActiveStatements int64 `json:",omitempty"
+ // CoveragedPackages is full names of packages included in coverage.
+ CoveredPackages []string `json:",omitempty"
+
+ // Stdout is text written by the test binary directly to os.Stdout.
+ // If this field is non-zero, all others are zero.
+ Stdout string `json:",omitempty"
+ // Stderr is text written by test binary directly to os.Stderr.
+ // If this field is non-zero, all others are zero.
+ Stderr string `json:",omitempty"
}
```
-Example of a test binary stdout (JSON output is made indented for the
-convenience of the reader. It will be unindented in fact):
+### Example output
+
+Here is an example of `go test -json` output.
+It is simplified and commented for the convenience of the reader;
+in practice it will be unindented and will contain JSON Text Sequence separators
+and no comments.
```json
-
+// go test emits environment configuration
{
+ "Configuration": {
+ "commit": "7cd9055",
+ "commit-time": "2016-02-11T13:25:45-0500",
+ "goos": "darwin",
+ "goarch": "amd64",
+ "cpu": "Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz",
+ "cpu-count": "8",
+ "cpu-physical-count": "4",
+ "os": "Mac OS X 10.11.3",
+ "mem": "16 GB"
+ }
+}
+// TestFoo started
+{
+ "Package": "github.com/user/repo",
"Name": "TestFoo",
"State": "RUN",
+ "Procs": 4
}
-Random string written directly to os.Stdout.
+// A line was written directly to os.Stdout
{
+ "Package": "github.com/user/repo",
+ "Stderr": "Random string written directly to os.Stdout\n"
+}
+// TestFoo passed
+{
+ "Package": "github.com/user/repo",
"Name": "TestFoo",
"State": "PASS",
- "T": 1000000
}
-
+// TestBar started
{
+ "Package": "github.com/user/repo",
"Name": "TestBar",
"State": "RUN",
+ "Procs": 4
}
-
+// TestBar logged a line
{
+ "Package": "github.com/user/repo",
"Name": "TestBar",
- "State": "PASS",
- "T": 1000000,
- "Log": "some test output\n"
+ "State": "RUN",
+ "Log": "some test output"
}
-
+// TestBar failed
{
+ "Package": "github.com/user/repo",
+ "Name": "TestBar",
+ "State": "FAIL"
+}
+// TestQux started
+{
+ "Package": "github.com/user/repo",
+ "Name": "TestQux",
+ "State": "RUN",
+ "Procs": 4
+}
+// TestQux calls T.Fatal("bug")
+{
+ "Package": "github.com/user/repo",
+ "Name": "TestBar",
+ "State": "RUN",
+ "Log": "bug"
+}
+{
+ "Package": "github.com/user/repo",
+ "Name": "TestQux",
+ "State": "FAIL"
+}
+// TestComposite started
+{
+ "Package": "github.com/user/repo",
+ "Name": "TestComposite",
+ "State": "RUN",
+ "Procs": 4
+}
+// TestComposite/A=1 subtest started
+{
+ "Package": "github.com/user/repo",
+ "Name": "TestComposite/A=1",
+ "State": "RUN",
+ "Procs": 4
+}
+// TestComposite/A=1 passed
+{
+ "Package": "github.com/user/repo",
+ "Name": "TestComposite/A=1",
+ "State": "PASS",
+}
+// TestComposite passed
+{
+ "Package": "github.com/user/repo",
+ "Name": "TestComposite",
+ "State": "PASS",
+}
+// Example1 started
+{
+ "Package": "github.com/user/repo",
"Name": "Example1",
"State": "RUN",
+ "Procs": 4
}
-
+// Example1 passed
{
+ "Package": "github.com/user/repo",
"Name": "Example1",
- "State": "PASS",
- "T": 1000000,
+ "State": "PASS"
}
-
+// BenchmarkRun started
{
+ "Package": "github.com/user/repo",
"Name": "BenchmarkBar",
"State": "RUN",
+ "Procs": 4
}
-
+// BenchmarkRun passed
{
+ "Package": "github.com/user/repo",
"Name": "BenchmarkBar",
"State": "PASS",
- "T": 1000000,
- "N": 1000,
- "Bytes": 100,
- "MemAllocs": 10,
- "MemBytes": 10
-}
-
-{
- "Coverage": {
- "Mode": "set",
- "TotalStatements": 1000,
- "ActiveStatements": 900,
- "CoveredPackages": [
- "example.com/foobar"
- ]
- }
-}
-```
-
-### `go test`
-
-`go test` JSON output format:
-
-```go
-// TestResult contains one output line of a test binary.
-type TestResult struct {
- Package string // package of the test binary.
-
- Result *testing.JSONResult `json:",omitempty"`
- Stdout string `json:",omitempty"` // Unrecognized stdout of the test binary.
- Stderr string `json:",omitempty"` // Stderr output line of the test binary.
-}
-```
-
-Example `go test -json` output:
-
-```json
-{
- "Package": "example.com/foobar",
- "Result": {
- "Name": "TestFoo",
- "State": "Run",
- }
-}
-{
- "Package": "example.com/foobar",
- "Stdout": "Random string written directly to os.Stdout."
-}
-{
- "Package": "example.com/foobar",
- "Result": {
- "Name": "TestFoo",
- "State": "PASS",
- "T": 1000000
- }
-}
-{
- "Package": "example.com/foobar",
- "Result": {
- "Name": "TestBar",
- "State": "Run",
- }
-}
-{
- "Package": "example.com/foobar",
- "Result": {
- "Name": "TestBar",
- "State": "PASS",
- "T": 1000000,
- "Log": "some test output\n"
- }
-}
-{
- "Package": "example.com/foobar",
- "Result": {
- "Name": "Example1",
- "State": "Run",
- }
-}
-{
- "Package": "example.com/foobar",
- "Result": {
- "Name": "Example1",
- "State": "PASS",
- "T": 1000000
- }
-}
-{
- "Package": "example.com/foobar",
- "Result": {
- "Name": "BenchmarkBar",
- "State": "Run",
- }
-}
-{
- "Package": "example.com/foobar",
- "Result": {
- "Name": "BenchmarkBar",
- "State": "PASS",
- "Procs": 8,
+ "Benchmark": {
"T": 1000000,
"N": 1000,
+ "Bytes": 100,
+ "MemAllocs": 10,
+ "MemBytes": 10
}
}
+// BenchmarkComposite started
{
- "Package": "example.com/foobar",
- "Result": {
- "Coverage": {
- "Mode": "set",
- "TotalStatements": 1000,
- "ActiveStatements": 900,
- "CoveredPackages": [
- "example.com/foobar"
- ]
- }
+ "Package": "github.com/user/repo",
+ "Name": "BenchmarkComposite",
+ "State": "RUN",
+ "Procs": 4
+}
+// BenchmarkComposite/A=1 started
+{
+ "Package": "github.com/user/repo",
+ "Name": "BenchmarkComposite/A=1",
+ "State": "RUN",
+ "Procs": 4
+}
+// BenchmarkComposite/A=1 passed
+{
+ "Package": "github.com/user/repo",
+ "Name": "BenchmarkComposite/A=1",
+ "State": "PASS",
+ "Benchmark": {
+ "T": 1000000,
+ "N": 1000,
+ "Bytes": 100,
+ "MemAllocs": 10,
+ "MemBytes": 10
}
}
-
+// BenchmarComposite passed
+{
+ "Package": "github.com/user/repo",
+ "Name": "BenchmarComposite",
+ "State": "PASS"
+}
+// Total coverage information in the end.
+{
+ "CoverageMode": "set",
+ "TotalStatements": 1000,
+ "ActiveStatements": 900,
+ "CoveredPackages": [
+ "github.com/user/repo"
+ ]
+}
```
## Rationale
-* A test binary surrounds `testing.JSONResult` JSON with `\n` to handle the
- situation when a string without a trailing `\n` is printed directly to
- `os.Stdout`.
-* A test binary always streams results so we don't loose them if the binary
- panics.
-
Alternatives:
* Add `-format` and `-benchformat` flags proposed in
- https://github.com/golang/go/issues/12826. This is simpler to implement
- by moving the burden of output parsing to third party programs.
+ https://github.com/golang/go/issues/12826.
+ While this is simpler to implement, users will have to do more work to
+ specify format and then parse it.
Trade offs:
-* `testing.JSONResult` is used for tests, examples and benchmarks.
- A third party tool would have to determine the type of the result by the
- prefix of `"Name"` property, e.g. tests always start with `"Test"`.
- This is a trade off for simplicity of `testing` package API.
-
- Alternatives:
-
- * add `type TestResult`, which together with
- `BenchmarkResult` would have duplicated fields, such as `Name`,
- `State`, `Procs`, `T`, `Log`.
- We cannot add `type CommonResult` with common fields and embed it in
- `TestResult` and `BenchmarkResult` because it would break backwards
- compatibility of `BenchmarkResult`.
- * Duplicate fields in `TestResult` but make `TestResult` and any other
- JSON-output-related types internal.
- The problem is that third party tool authors would have to write structs
- for JSON parsing themselves.
-
* I propose to make `-json` mutually exclusive with `-n` and `-x` flags.
- This is a trade off for `go test` output format simplicity.
- Supporting `-json` with `-n`/`-x` flags would require a new field in
- `TestResult` that would contain commands that have been run. Note that
- we cannot print commands to stdout because stdout must be valid JSON.
+ These flags belong to `go build` subcommand while this proposal is scoped
+ to `go test`.
+ Supporting the flags would require adding JSON output knowledge to
+ `go/build.go`.
+* `JSONResult.Benchmark.T` provides duration of a benchmark run, but there is
+ not an equivalent for a test run.
+ This is a trade off for `JSONResult` simplicity.
+ We don't have to define `TestResult` because `JSONResult` is enough to
+ describe a test result.
- Supporting `-json` with `-n`/`-x` flags would also raise the question
- whether the field must be specific to commands or it should contain anything
- `build.go` prints to stdout.
- At this time `-n` and `-x` are the only flags that cause `build.go`
- to print to stdout, so we can avoid the problem for now.
+ Currently `go test` does not provide test timing info, so the proposal is
+ consistent with the current `go test` output.
- If we add more output to `build.go` in future, we can add
- `BuildOutput string` field to `TestResult` for arbitrary `build.go` output.
-
- I propose not to add `BuildOutput` now because `-n` affects `go test` too.
- For example, `go test -n` prints a command to run the test binary, which
- should not be a part of `BuildOutput` (because it is not build).
-* `go test` always streams and does not aggregate results into one JSON
- object.
- This is a trade off for `go test -json` output format simplicity.
## Compatibility
@@ -347,22 +349,8 @@
Most of the work would be done by the author of this proposal.
-Implementation steps:
-
-1. Add `type State` and add new fields to `testing.BenchmarkResult`.
- Modify `testing.(*B).launch` to fill the new fields.
-1. Add `-test.json` flag, `type CoverageResult` and `type JSONResult` to the
- `testing` package.
- Modify `(*T).report`, `RunBenchmarks`, `coverReport` and `runExample`
- functions to print JSON if `-test.json` is specified.
- If `-test.verbose` is specified, print verbose messages to stderr.
-1. Add `-json` flag to `go test`.
- If specified, pass `-test.json` to test binaries.
-
- For each line in a test binary output, try to parse it as
- `testing.JSONResult`, and print a `TestResult`.
-
The goal is to get agreement on this proposal and to complete the work
-before the 1.6 freeze date.
+before the 1.8 freeze date.
[testStreamOutput]: https://github.com/golang/go/blob/0b248cea169a261cd0c2db8c014269cca5a170c4/src/cmd/go/test.go#L361-L369
+[rfc7464]: https://tools.ietf.org/html/rfc7464
