Skip to content

Commit

Permalink
cmd/go/internal/test: wrap os.Stdout always
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
adg authored and gopherbot committed Apr 21, 2022
1 parent ebe1435 commit ab9d31d
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/cmd/go/internal/test/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
35 changes: 35 additions & 0 deletions src/cmd/go/testdata/script/test_output_wait.txt
Original file line number Diff line number Diff line change
@@ -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)
}
}

0 comments on commit ab9d31d

Please sign in to comment.