From: Bryan C. Mills <bcmills@google.com>
Date: Mon, 18 Sep 2023 19:50:27 +0000 (-0400)
Subject: cmd/go: avoid writing non-JSON "build failed" errors from 'go test -json'
X-Git-Tag: go1.22rc1~801
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=5b123aeaf5af3044cefeaaeac496443f85344983;p=gostls13.git

cmd/go: avoid writing non-JSON "build failed" errors from 'go test -json'

In 'go test -json' we expect stdout to contain only JSON events,
not unstructured text. Unstructured text should either go to stderr
or be wrapped in a JSON event.

(If we add structured build output in #62067, we can emit this output
as a build event instead of a test event.)

Fixes #35169.
For #54378.

Change-Id: Ibedd28e79b5adf8d6ae56165b9f0393b14ece9aa
Reviewed-on: https://go-review.googlesource.com/c/go/+/529120
Reviewed-by: Austin Clements <austin@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
---

diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go
index 11d1aefc19..4c181dbcd2 100644
--- a/src/cmd/go/internal/test/test.go
+++ b/src/cmd/go/internal/test/test.go
@@ -1233,18 +1233,6 @@ func (r *runTestActor) Act(b *work.Builder, ctx context.Context, a *work.Action)
 		return nil
 	}
 
-	if a.Failed {
-		// We were unable to build the binary.
-		a.Failed = false
-		a.TestOutput = new(bytes.Buffer)
-		fmt.Fprintf(a.TestOutput, "FAIL\t%s [build failed]\n", a.Package.ImportPath)
-		base.SetExitStatus(1)
-
-		// release next test to start
-		close(r.next)
-		return nil
-	}
-
 	var stdout io.Writer = os.Stdout
 	var err error
 	if testJSON {
@@ -1259,6 +1247,16 @@ func (r *runTestActor) Act(b *work.Builder, ctx context.Context, a *work.Action)
 	// Release next test to start (test2json.NewConverter writes the start event).
 	close(r.next)
 
+	if a.Failed {
+		// We were unable to build the binary.
+		a.Failed = false
+		fmt.Fprintf(stdout, "FAIL\t%s [build failed]\n", a.Package.ImportPath)
+		// Tell the JSON converter that this was a failure, not a passing run.
+		err = errors.New("build failed")
+		base.SetExitStatus(1)
+		return nil
+	}
+
 	if p := a.Package; len(p.TestGoFiles)+len(p.XTestGoFiles) == 0 {
 		fmt.Fprintf(stdout, "?   \t%s\t[no test files]\n", p.ImportPath)
 		return nil
diff --git a/src/cmd/go/testdata/script/test_json_issue35169.txt b/src/cmd/go/testdata/script/test_json_issue35169.txt
new file mode 100644
index 0000000000..fdb57556bd
--- /dev/null
+++ b/src/cmd/go/testdata/script/test_json_issue35169.txt
@@ -0,0 +1,28 @@
+! go test -json .
+
+	# We should see only JSON output on stdout, no non-JSON.
+	# To simplify the check, we just look for non-curly-braces, since
+	# every JSON entry has them and they're unlikely to occur
+	# in other error messages.
+! stdout '^[^{]'
+! stdout '[^}]\n$'
+
+	# Since the only test we requested failed to build, we should
+	# not see any "pass" actions in the JSON stream.
+! stdout '\{.*"Action":"pass".*\}'
+
+	# TODO(#62067): emit this as a build event instead of a test event.
+stdout '\{.*"Action":"output","Package":"example","Output":"FAIL\\texample \[build failed\]\\n"\}'
+stdout '\{.*"Action":"fail","Package":"example",.*\}'
+
+-- go.mod --
+module example
+go 1.19
+-- example.go --
+package example
+
+This is not valid Go source.
+-- example_test.go --
+package  example
+
+func Test(*testing.T) {}