aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBryan C. Mills <bcmills@google.com>2023-06-15 17:41:37 -0400
committerGopher Robot <gobot@golang.org>2023-06-16 20:01:06 +0000
commit60876717b402f0dd6b4f585827779a9e435400c8 (patch)
tree44d0b1c79ec9c25ca62e8d5f06cf20f293dd8b76
parentc1bc44642db321c2f125fd043c220cac08877e95 (diff)
downloadgo-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.go86
-rw-r--r--src/cmd/go/internal/test/test.go10
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.