Skip to content

Commit

Permalink
cmd/go: re-enable build JSON from go test -json, now with GODEBUG
Browse files Browse the repository at this point in the history
This re-enables the behavior of CL 536399 (by effectively reverting CL
628955), so now go test -json again includes build output and failures
as JSON rather than text.

However, since this behavior is clearly enough to trip up some build
systems, this CL includes a GODEBUG=gotestjsonbuildtext that can be
set to 1 to revert to the old behavior.

Fixes #70402.
Updates #62067.

Cq-Include-Trybots: luci.golang.try:gotip-darwin-arm64_13,gotip-linux-amd64-longtest,gotip-windows-amd64-longtest
Change-Id: I84e778cd844783dacfc83433e391b5ccb5925127
Reviewed-on: https://go-review.googlesource.com/c/go/+/629335
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Austin Clements <austin@google.com>
  • Loading branch information
aclements authored and gopherbot committed Nov 19, 2024
1 parent 5432cd9 commit 3e7cb78
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 9 deletions.
8 changes: 8 additions & 0 deletions doc/godebug.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,14 @@ For Go 1.24, it now defaults to multipathtcp="2", thus
enabled by default on listerners. Using multipathtcp="0" reverts to the
pre-Go 1.24 behavior.

Go 1.24 changed the behavior of `go test -json` to emit build errors as JSON
instead of text.
These new JSON events are distinguished by new `Action` values,
but can still cause problems with CI systems that aren't robust to these events.
This behavior can be controlled with the `gotestjsonbuildtext` setting.
Using `gotestjsonbuildtext=1` restores the 1.23 behavior.
This setting will be removed in a future release, Go 1.28 at the earliest.

### Go 1.23

Go 1.23 changed the channels created by package time to be unbuffered
Expand Down
10 changes: 7 additions & 3 deletions doc/next/3-tools.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,13 @@

The `go build` and `go install` commands now accept a `-json` flag that reports
build output and failures as structured JSON output on standard output.
Furthermore, passing `-json` to `go test` now reports build output and failures
in addition to test results in JSON. For details of the reporting format, see
`go help buildjson`.
For details of the reporting format, see `go help buildjson`.

Furthermore, `go test -json` now reports build output and failures in JSON,
interleaved with test result JSON.
These are distinguished by new `Action` types, but if they cause problems in
a test integration system, you can revert to the text build output by setting
`GODEBUG=gotestjsonbuildtext=1`.

### Cgo {#cgo}

Expand Down
12 changes: 9 additions & 3 deletions src/cmd/go/internal/test/testflag.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ package test

import (
"cmd/go/internal/base"
"cmd/go/internal/cfg"
"cmd/go/internal/cmdflag"
"cmd/go/internal/work"
"errors"
"flag"
"fmt"
"internal/godebug"
"os"
"path/filepath"
"strconv"
Expand All @@ -25,6 +27,8 @@ import (
// our command line are for us, and some are for the test binary, and
// some are for both.

var gotestjsonbuildtext = godebug.New("gotestjsonbuildtext")

func init() {
work.AddBuildFlags(CmdTest, work.OmitVFlag|work.OmitJSONFlag)

Expand All @@ -33,7 +37,6 @@ func init() {
cf.StringVar(&testO, "o", "", "")
work.AddCoverFlags(CmdTest, &testCoverProfile)
cf.Var((*base.StringsFlag)(&work.ExecCmd), "exec", "")
// TODO(austin): Make test -json imply build -json.
cf.BoolVar(&testJSON, "json", false, "")
cf.Var(&testVet, "vet", "")

Expand Down Expand Up @@ -354,8 +357,11 @@ func testFlags(args []string) (packageNames, passToTest []string) {
delete(addFromGOFLAGS, "v")
delete(addFromGOFLAGS, "test.v")

// TODO(austin,#70402): Re-enable this once LUCI can handle build JSON in the test stream.
//cfg.BuildJSON = true
if gotestjsonbuildtext.Value() == "1" {
gotestjsonbuildtext.IncNonDefault()
} else {
cfg.BuildJSON = true
}
}

// Inject flags from GOFLAGS before the explicit command-line arguments.
Expand Down
15 changes: 12 additions & 3 deletions src/cmd/go/testdata/script/test_json_build.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
# TODO(austin,#70402)
skip

[short] skip

env GODEBUG=gotestjsonbuildtext=0

# There are several places where paths appear in JSON in regexps here.
# For the path separator we use (/|\\\\).
# Unfortunately, we can't just use ${/} because, while script test automatically
Expand Down Expand Up @@ -52,6 +51,16 @@ stdout '"Action":"output","Package":"m/veterror","Output":"FAIL\\tm/veterror \[b
stdout '"Action":"fail","Package":"m/veterror","Elapsed":.*,"FailedBuild":"m/veterror \[m/veterror.test\]"'
! stderr '.'

# Test that the GODEBUG fallback works.
env GODEBUG=gotestjsonbuildtext=1
! go test -json -o=$devnull ./builderror
stderr '# m/builderror \[m/builderror.test\]\n'
stderr 'builderror'${/}'main_test.go:3:11: undefined: y\n'
stdout '"Action":"start","Package":"m/builderror"'
stdout '"Action":"output","Package":"m/builderror","Output":"FAIL\\tm/builderror \[build failed\]\\n"'
stdout '"Action":"fail","Package":"m/builderror","Elapsed":.*,"FailedBuild":"m/builderror \[m/builderror\.test\]"'


-- go.mod --
module m
go 1.21
Expand Down
1 change: 1 addition & 0 deletions src/internal/godebugs/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var All = []Info{
{Name: "gocachehash", Package: "cmd/go"},
{Name: "gocachetest", Package: "cmd/go"},
{Name: "gocacheverify", Package: "cmd/go"},
{Name: "gotestjsonbuildtext", Package: "cmd/go", Changed: 24, Old: "1"},
{Name: "gotypesalias", Package: "go/types", Changed: 23, Old: "0"},
{Name: "http2client", Package: "net/http"},
{Name: "http2debug", Package: "net/http", Opaque: true},
Expand Down
5 changes: 5 additions & 0 deletions src/runtime/metrics/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,11 @@ Below is the full list of supported metrics, ordered lexicographically.
The number of non-default behaviors executed by the cmd/go
package due to a non-default GODEBUG=gocacheverify=... setting.
/godebug/non-default-behavior/gotestjsonbuildtext:events
The number of non-default behaviors executed by the cmd/go
package due to a non-default GODEBUG=gotestjsonbuildtext=...
setting.
/godebug/non-default-behavior/gotypesalias:events
The number of non-default behaviors executed by the go/types
package due to a non-default GODEBUG=gotypesalias=... setting.
Expand Down

0 comments on commit 3e7cb78

Please sign in to comment.