internal/testenv: remove RunWithTimout

For most tests, the test's deadline itself is more appropriate than an
arbitrary timeout layered atop of it (especially once #48157 is
implemented), and testenv.Command already adds cleaner timeout
behavior when a command would run too close to the test's deadline.

That makes RunWithTimeout something of an attractive nuisance. For
now, migrate the two existing uses of it to testenv.CommandContext,
with a shorter timeout implemented using context.WithTimeout.

As a followup, we may want to drop the extra timeouts from these
invocations entirely.

Updates #50436.
Updates #37405.

Change-Id: I16840fd36c0137b6da87ec54012b3e44661f0d08
Reviewed-on: https://go-review.googlesource.com/c/go/+/445597
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/internal/testenv/testenv.go b/src/internal/testenv/testenv.go
index f71f5cdd41f540f0b0730446cadf2f2824fcc980..b61a44c8558c7aead6b930e4c0b45f164003d600 100644 (file)
--- a/src/internal/testenv/testenv.go
+++ b/src/internal/testenv/testenv.go
@@ -11,7 +11,6 @@
 package testenv
 
 import (
-       "bytes"
        "context"
        "errors"
        "flag"
@@ -505,62 +504,6 @@ func SkipIfOptimizationOff(t testing.TB) {
        }
 }
 
-// RunWithTimeout runs cmd and returns its combined output. If the
-// subprocess exits with a non-zero status, it will log that status
-// and return a non-nil error, but this is not considered fatal.
-func RunWithTimeout(t testing.TB, cmd *exec.Cmd) ([]byte, error) {
-       args := cmd.Args
-       if args == nil {
-               args = []string{cmd.Path}
-       }
-
-       var b bytes.Buffer
-       cmd.Stdout = &b
-       cmd.Stderr = &b
-       if err := cmd.Start(); err != nil {
-               t.Fatalf("starting %s: %v", args, err)
-       }
-
-       // If the process doesn't complete within 1 minute,
-       // assume it is hanging and kill it to get a stack trace.
-       p := cmd.Process
-       done := make(chan bool)
-       go func() {
-               scale := 1
-               // This GOARCH/GOOS test is copied from cmd/dist/test.go.
-               // TODO(iant): Have cmd/dist update the environment variable.
-               if runtime.GOARCH == "arm" || runtime.GOOS == "windows" {
-                       scale = 2
-               }
-               if s := os.Getenv("GO_TEST_TIMEOUT_SCALE"); s != "" {
-                       if sc, err := strconv.Atoi(s); err == nil {
-                               scale = sc
-                       }
-               }
-
-               select {
-               case <-done:
-               case <-time.After(time.Duration(scale) * time.Minute):
-                       p.Signal(Sigquit)
-                       // If SIGQUIT doesn't do it after a little
-                       // while, kill the process.
-                       select {
-                       case <-done:
-                       case <-time.After(time.Duration(scale) * 30 * time.Second):
-                               p.Signal(os.Kill)
-                       }
-               }
-       }()
-
-       err := cmd.Wait()
-       if err != nil {
-               t.Logf("%s exit status: %v", args, err)
-       }
-       close(done)
-
-       return b.Bytes(), err
-}
-
 // WriteImportcfg writes an importcfg file used by the compiler or linker to
 // dstPath containing entries for the packages in std and cmd in addition
 // to the package to package file mappings in additionalPackageFiles.

diff --git a/src/runtime/crash_test.go b/src/runtime/crash_test.go
index d5f755296bad098fed6c8d049281038c53302bcf..6e00489b49563a5ddc0c61d6a795c77f18a6b2ad 100644 (file)
--- a/src/runtime/crash_test.go
+++ b/src/runtime/crash_test.go
@@ -6,6 +6,7 @@ package runtime_test
 
 import (
        "bytes"
+       "context"
        "errors"
        "flag"
        "fmt"
@@ -18,6 +19,7 @@ import (
        "strings"
        "sync"
        "testing"
+       "time"
 )
 
 var toRemove []string
@@ -58,18 +60,27 @@ func runTestProg(t *testing.T, binary, name string, env ...string) string {
 }
 
 func runBuiltTestProg(t *testing.T, exe, name string, env ...string) string {
+       t.Helper()
+
        if *flagQuick {
                t.Skip("-quick")
        }
 
-       testenv.MustHaveGoBuild(t)
-
-       cmd := testenv.CleanCmdEnv(exec.Command(exe, name))
+       ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+       defer cancel()
+       cmd := testenv.CleanCmdEnv(testenv.CommandContext(t, ctx, exe, name))
        cmd.Env = append(cmd.Env, env...)
        if testing.Short() {
                cmd.Env = append(cmd.Env, "RUNTIME_TEST_SHORT=1")
        }
-       out, _ := testenv.RunWithTimeout(t, cmd)
+       out, err := cmd.CombinedOutput()
+       if err != nil {
+               if _, ok := err.(*exec.ExitError); ok {
+                       t.Logf("%v: %v", cmd, err)
+               } else {
+                       t.Fatalf("%v failed to start: %v", cmd, err)
+               }
+       }
        return string(out)
 }
 

diff --git a/src/runtime/runtime-gdb_test.go b/src/runtime/runtime-gdb_test.go
index efc09c67e4534928baf7f22b0053cec3bdd37270..d3a30870c18004bef35a3c71aa82b8a7b959b792 100644 (file)
--- a/src/runtime/runtime-gdb_test.go
+++ b/src/runtime/runtime-gdb_test.go
@@ -6,6 +6,7 @@ package runtime_test
 
 import (
        "bytes"
+       "context"
        "fmt"
        "internal/testenv"
        "os"
@@ -16,6 +17,7 @@ import (
        "strconv"
        "strings"
        "testing"
+       "time"
 )
 
 // NOTE: In some configurations, GDB will segfault when sent a SIGWINCH signal.
@@ -428,7 +430,9 @@ func TestGdbBacktrace(t *testing.T) {
                "-ex", "continue",
                filepath.Join(dir, "a.exe"),
        }
-       got, err := testenv.RunWithTimeout(t, exec.Command("gdb", args...))
+       ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+       defer cancel()
+       got, err := testenv.CommandContext(t, ctx, "gdb", args...).CombinedOutput()
        t.Logf("gdb output:\n%s", got)
        if err != nil {
                if bytes.Contains(got, []byte("internal-error: wait returned unexpected status 0x0")) {