diff options
author | Bryan C. Mills <bcmills@google.com> | 2023-06-15 17:41:37 -0400 |
---|---|---|
committer | Gopher Robot <gobot@golang.org> | 2023-06-16 20:01:06 +0000 |
commit | 60876717b402f0dd6b4f585827779a9e435400c8 (patch) | |
tree | 44d0b1c79ec9c25ca62e8d5f06cf20f293dd8b76 | |
parent | c1bc44642db321c2f125fd043c220cac08877e95 (diff) | |
download | go-60876717b402f0dd6b4f585827779a9e435400c8.tar.gz go-60876717b402f0dd6b4f585827779a9e435400c8.zip |
cmd/go/internal/test: don't wait for previous test actions when interrupted
Fixes #60203.
Change-Id: I59a3320ede1eb3cf4443d7ea37b8cb39d01f222a
Reviewed-on: https://go-review.googlesource.com/c/go/+/503936
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
-rw-r--r-- | src/cmd/go/go_unix_test.go | 86 | ||||
-rw-r--r-- | src/cmd/go/internal/test/test.go | 10 |
2 files changed, 94 insertions, 2 deletions
diff --git a/src/cmd/go/go_unix_test.go b/src/cmd/go/go_unix_test.go index bab9494401..d04e496778 100644 --- a/src/cmd/go/go_unix_test.go +++ b/src/cmd/go/go_unix_test.go @@ -2,12 +2,19 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris +//go:build unix package main_test import ( + "bufio" + "context" + "internal/testenv" + "io" "os" + "os/exec" + "slices" + "strings" "syscall" "testing" ) @@ -33,3 +40,80 @@ func TestGoBuildUmask(t *testing.T) { t.Fatalf("wrote x with mode=%v, wanted no 0077 bits", mode) } } + +// TestTestInterrupt verifies the fix for issue #60203. +// +// If the whole process group for a 'go test' invocation receives +// SIGINT (as would be sent by pressing ^C on a console), +// it should return quickly, not deadlock. +func TestTestInterrupt(t *testing.T) { + if testing.Short() { + t.Skipf("skipping in short mode: test executes many subprocesses") + } + // Don't run this test in parallel, for the same reason. + + tg := testgo(t) + defer tg.cleanup() + tg.setenv("GOROOT", testGOROOT) + + ctx, cancel := context.WithCancel(context.Background()) + cmd := testenv.CommandContext(t, ctx, tg.goTool(), "test", "std", "-short", "-count=1") + cmd.Dir = tg.execDir + + // Override $TMPDIR when running the tests: since we're terminating the tests + // with a signal they might fail to clean up some temp files, and we don't + // want that to cause an "unexpected files" failure at the end of the run. + cmd.Env = append(slices.Clip(tg.env), tempEnvName()+"="+t.TempDir()) + + cmd.SysProcAttr = &syscall.SysProcAttr{ + Setpgid: true, + } + cmd.Cancel = func() error { + pgid := cmd.Process.Pid + return syscall.Kill(-pgid, syscall.SIGINT) + } + + pipe, err := cmd.StdoutPipe() + if err != nil { + t.Fatal(err) + } + + t.Logf("running %v", cmd) + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + + stdout := new(strings.Builder) + r := bufio.NewReader(pipe) + line, err := r.ReadString('\n') + if err != nil { + t.Fatal(err) + } + stdout.WriteString(line) + + // The output line for some test was written, so we know things are in progress. + // + // Cancel the rest of the run by sending SIGINT to the process group: + // it should finish up and exit with a nonzero status, + // not have to be killed with SIGKILL. + cancel() + + io.Copy(stdout, r) + if stdout.Len() > 0 { + t.Logf("stdout:\n%s", stdout) + } + err = cmd.Wait() + + ee, _ := err.(*exec.ExitError) + if ee == nil { + t.Fatalf("unexpectedly finished with nonzero status") + } + if len(ee.Stderr) > 0 { + t.Logf("stderr:\n%s", ee.Stderr) + } + if !ee.Exited() { + t.Fatalf("'go test' did not exit after interrupt: %v", err) + } + + t.Logf("interrupted tests without deadlocking") +} diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go index 2ce4c1a28e..995da15c90 100644 --- a/src/cmd/go/internal/test/test.go +++ b/src/cmd/go/internal/test/test.go @@ -1217,7 +1217,15 @@ func (lockedStdout) Write(b []byte) (int, error) { func (r *runTestActor) Act(b *work.Builder, ctx context.Context, a *work.Action) error { // Wait for previous test to get started and print its first json line. - <-r.prev + select { + case <-r.prev: + case <-base.Interrupted: + // We can't wait for the previous test action to complete: we don't start + // new actions after an interrupt, so if that action wasn't already running + // it might never happen. Instead, just don't log anything for this action. + base.SetExitStatus(1) + return nil + } if a.Failed { // We were unable to build the binary. |