From: Andrew Gerrand <adg@golang.org>
Date: Tue, 19 Apr 2022 05:11:37 +0000 (+1000)
Subject: cmd/go/internal/test: wrap os.Stdout always
X-Git-Tag: go1.19beta1~613
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=ab9d31da9e088a271e656120a3d99cd3b1103ab6;p=gostls13.git

cmd/go/internal/test: wrap os.Stdout always

There is an issue where 'go test' will hang after the tests complete if
a test starts a sub-process that does not exit (see #24050).

However, go test only exhibits that behavior when a package name is
explicitly passed as an argument. If 'go test' is invoked without any
package arguments then the package in the working directory is assumed,
however in that case (and only that case) os.Stdout is used as the test
process's cmd.Stdout, which does *not* cause 'go test' wait for the
sub-process to exit (see #23019).

This change wraps os.Stdout in an io.Writer struct in this case, hiding
the *os.File from the os/exec package, causing cmd.Wait to always wait
for the full output from the test process and any of its sub-processes.

In other words, this makes 'go test' exhibit the same behavior as
'go test .' (or 'go test ./...' and so on).

Update #23019
Update #24050

Change-Id: Ica09bf156f3b017f9a31aad91ed0f16a7837195b
Reviewed-on: https://go-review.googlesource.com/c/go/+/400877
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Andrew Gerrand <adg@golang.org>
Auto-Submit: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
---

diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go
index 50e6d5201b..f2d2290dbb 100644
--- a/src/cmd/go/internal/test/test.go
+++ b/src/cmd/go/internal/test/test.go
@@ -1261,7 +1261,11 @@ func (c *runCache) builderRunTest(b *work.Builder, ctx context.Context, a *work.
 		return nil
 	}
 
-	var stdout io.Writer = os.Stdout
+	// The os/exec package treats an *os.File differently to an io.Writer.
+	// Embed os.Stdout in an io.Writer struct so that we get the same
+	// behavior regardless of whether we wrap it below.
+	// See golang.org/issue/24050
+	var stdout io.Writer = struct{ io.Writer }{os.Stdout}
 	var err error
 	if testJSON {
 		json := test2json.NewConverter(lockedStdout{}, a.Package.ImportPath, test2json.Timestamp)
diff --git a/src/cmd/go/testdata/script/test_output_wait.txt b/src/cmd/go/testdata/script/test_output_wait.txt
new file mode 100644
index 0000000000..878b108612
--- /dev/null
+++ b/src/cmd/go/testdata/script/test_output_wait.txt
@@ -0,0 +1,35 @@
+# Wait for test output from sub-processes whether or not the package name is
+# provided on the command-line.
+go test -v
+stdout 'PASS\s+WAIT\s+ok'
+go test -v .
+stdout 'PASS\s+WAIT\s+ok'
+
+-- go.mod --
+module x
+
+-- x_test.go --
+package x
+
+import (
+	"os"
+	"os/exec"
+	"testing"
+)
+
+func TestMain(m *testing.M) {
+	if os.Getenv("WAIT") == "true" {
+		os.Stdout.Write([]byte("WAIT\n"))
+		return
+	}
+	m.Run()
+}
+
+func TestWait(t *testing.T) {
+	cmd := exec.Command(os.Args[0])
+	cmd.Env = []string{"WAIT=true"}
+	cmd.Stdout = os.Stdout
+	if err := cmd.Start(); err != nil {
+		t.Fatal(err)
+	}
+}