diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go index abef7b4472..a3a53ba9f2 100644 --- a/go/analysis/analysistest/analysistest.go +++ b/go/analysis/analysistest/analysistest.go @@ -19,6 +19,7 @@ import ( "sort" "strconv" "strings" + "testing" "text/scanner" "golang.org/x/tools/go/analysis" @@ -278,7 +279,7 @@ func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns // attempted, even if unsuccessful. It is safe for a test to ignore all // the results, but a test may use it to perform additional checks. func Run(t Testing, dir string, a *analysis.Analyzer, patterns ...string) []*Result { - if t, ok := t.(testenv.Testing); ok { + if t, ok := t.(testing.TB); ok { testenv.NeedsGoPackages(t) } diff --git a/internal/testenv/exec.go b/internal/testenv/exec.go new file mode 100644 index 0000000000..f103ad9d82 --- /dev/null +++ b/internal/testenv/exec.go @@ -0,0 +1,149 @@ +// Copyright 2015 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package testenv + +import ( + "context" + "os" + "os/exec" + "reflect" + "runtime" + "strconv" + "testing" + "time" +) + +// HasExec reports whether the current system can start new processes +// using os.StartProcess or (more commonly) exec.Command. +func HasExec() bool { + switch runtime.GOOS { + case "js", "ios": + return false + } + return true +} + +// NeedsExec checks that the current system can start new processes +// using os.StartProcess or (more commonly) exec.Command. +// If not, NeedsExec calls t.Skip with an explanation. +func NeedsExec(t testing.TB) { + if !HasExec() { + t.Skipf("skipping test: cannot exec subprocess on %s/%s", runtime.GOOS, runtime.GOARCH) + } +} + +// CommandContext is like exec.CommandContext, but: +// - skips t if the platform does not support os/exec, +// - if supported, sends SIGQUIT instead of SIGKILL in its Cancel function +// - if the test has a deadline, adds a Context timeout and (if supported) WaitDelay +// for an arbitrary grace period before the test's deadline expires, +// - if Cmd has the Cancel field, fails the test if the command is canceled +// due to the test's deadline, and +// - if supported, sets a Cleanup function that verifies that the test did not +// leak a subprocess. +func CommandContext(t testing.TB, ctx context.Context, name string, args ...string) *exec.Cmd { + t.Helper() + NeedsExec(t) + + var ( + cancelCtx context.CancelFunc + gracePeriod time.Duration // unlimited unless the test has a deadline (to allow for interactive debugging) + ) + + if td, ok := Deadline(t); ok { + // Start with a minimum grace period, just long enough to consume the + // output of a reasonable program after it terminates. + gracePeriod = 100 * time.Millisecond + if s := os.Getenv("GO_TEST_TIMEOUT_SCALE"); s != "" { + scale, err := strconv.Atoi(s) + if err != nil { + t.Fatalf("invalid GO_TEST_TIMEOUT_SCALE: %v", err) + } + gracePeriod *= time.Duration(scale) + } + + // If time allows, increase the termination grace period to 5% of the + // test's remaining time. + testTimeout := time.Until(td) + if gp := testTimeout / 20; gp > gracePeriod { + gracePeriod = gp + } + + // When we run commands that execute subprocesses, we want to reserve two + // grace periods to clean up: one for the delay between the first + // termination signal being sent (via the Cancel callback when the Context + // expires) and the process being forcibly terminated (via the WaitDelay + // field), and a second one for the delay becween the process being + // terminated and and the test logging its output for debugging. + // + // (We want to ensure that the test process itself has enough time to + // log the output before it is also terminated.) + cmdTimeout := testTimeout - 2*gracePeriod + + if cd, ok := ctx.Deadline(); !ok || time.Until(cd) > cmdTimeout { + // Either ctx doesn't have a deadline, or its deadline would expire + // after (or too close before) the test has already timed out. + // Add a shorter timeout so that the test will produce useful output. + ctx, cancelCtx = context.WithTimeout(ctx, cmdTimeout) + } + } + + cmd := exec.CommandContext(ctx, name, args...) + + // Use reflection to set the Cancel and WaitDelay fields, if present. + // TODO(bcmills): When we no longer support Go versions below 1.20, + // remove the use of reflect and assume that the fields are always present. + rc := reflect.ValueOf(cmd).Elem() + + if rCancel := rc.FieldByName("Cancel"); rCancel.IsValid() { + rCancel.Set(reflect.ValueOf(func() error { + if cancelCtx != nil && ctx.Err() == context.DeadlineExceeded { + // The command timed out due to running too close to the test's deadline + // (because we specifically set a shorter Context deadline for that + // above). There is no way the test did that intentionally — it's too + // close to the wire! — so mark it as a test failure. That way, if the + // test expects the command to fail for some other reason, it doesn't + // have to distinguish between that reason and a timeout. + t.Errorf("test timed out while running command: %v", cmd) + } else { + // The command is being terminated due to ctx being canceled, but + // apparently not due to an explicit test deadline that we added. + // Log that information in case it is useful for diagnosing a failure, + // but don't actually fail the test because of it. + t.Logf("%v: terminating command: %v", ctx.Err(), cmd) + } + return cmd.Process.Signal(Sigquit) + })) + } + + if rWaitDelay := rc.FieldByName("WaitDelay"); rWaitDelay.IsValid() { + rWaitDelay.Set(reflect.ValueOf(gracePeriod)) + } + + // t.Cleanup was added in Go 1.14; for earlier Go versions, + // we just let the Context leak. + type Cleanupper interface { + Cleanup(func()) + } + if ct, ok := t.(Cleanupper); ok { + ct.Cleanup(func() { + if cancelCtx != nil { + cancelCtx() + } + if cmd.Process != nil && cmd.ProcessState == nil { + t.Errorf("command was started, but test did not wait for it to complete: %v", cmd) + } + }) + } + + return cmd +} + +// Command is like exec.Command, but applies the same changes as +// testenv.CommandContext (with a default Context). +func Command(t testing.TB, name string, args ...string) *exec.Cmd { + t.Helper() + return CommandContext(t, context.Background(), name, args...) +} diff --git a/internal/testenv/testenv.go b/internal/testenv/testenv.go index 22df96a858..ff38498e56 100644 --- a/internal/testenv/testenv.go +++ b/internal/testenv/testenv.go @@ -24,16 +24,6 @@ import ( exec "golang.org/x/sys/execabs" ) -// Testing is an abstraction of a *testing.T. -type Testing interface { - Skipf(format string, args ...interface{}) - Fatalf(format string, args ...interface{}) -} - -type helperer interface { - Helper() -} - // packageMainIsDevel reports whether the module containing package main // is a development version (if module information is available). func packageMainIsDevel() bool { @@ -192,14 +182,13 @@ func allowMissingTool(tool string) bool { // NeedsTool skips t if the named tool is not present in the path. // As a special case, "cgo" means "go" is present and can compile cgo programs. -func NeedsTool(t Testing, tool string) { - if t, ok := t.(helperer); ok { - t.Helper() - } +func NeedsTool(t testing.TB, tool string) { err := hasTool(tool) if err == nil { return } + + t.Helper() if allowMissingTool(tool) { t.Skipf("skipping because %s tool not available: %v", tool, err) } else { @@ -209,10 +198,8 @@ func NeedsTool(t Testing, tool string) { // NeedsGoPackages skips t if the go/packages driver (or 'go' tool) implied by // the current process environment is not present in the path. -func NeedsGoPackages(t Testing) { - if t, ok := t.(helperer); ok { - t.Helper() - } +func NeedsGoPackages(t testing.TB) { + t.Helper() tool := os.Getenv("GOPACKAGESDRIVER") switch tool { @@ -232,10 +219,8 @@ func NeedsGoPackages(t Testing) { // NeedsGoPackagesEnv skips t if the go/packages driver (or 'go' tool) implied // by env is not present in the path. -func NeedsGoPackagesEnv(t Testing, env []string) { - if t, ok := t.(helperer); ok { - t.Helper() - } +func NeedsGoPackagesEnv(t testing.TB, env []string) { + t.Helper() for _, v := range env { if strings.HasPrefix(v, "GOPACKAGESDRIVER=") { @@ -256,10 +241,8 @@ func NeedsGoPackagesEnv(t Testing, env []string) { // and then run them with os.StartProcess or exec.Command. // Android doesn't have the userspace go build needs to run, // and js/wasm doesn't support running subprocesses. -func NeedsGoBuild(t Testing) { - if t, ok := t.(helperer); ok { - t.Helper() - } +func NeedsGoBuild(t testing.TB) { + t.Helper() // This logic was derived from internal/testing.HasGoBuild and // may need to be updated as that function evolves. @@ -318,29 +301,25 @@ func Go1Point() int { // NeedsGo1Point skips t if the Go version used to run the test is older than // 1.x. -func NeedsGo1Point(t Testing, x int) { - if t, ok := t.(helperer); ok { - t.Helper() - } +func NeedsGo1Point(t testing.TB, x int) { if Go1Point() < x { + t.Helper() t.Skipf("running Go version %q is version 1.%d, older than required 1.%d", runtime.Version(), Go1Point(), x) } } // SkipAfterGo1Point skips t if the Go version used to run the test is newer than // 1.x. -func SkipAfterGo1Point(t Testing, x int) { - if t, ok := t.(helperer); ok { - t.Helper() - } +func SkipAfterGo1Point(t testing.TB, x int) { if Go1Point() > x { + t.Helper() t.Skipf("running Go version %q is version 1.%d, newer than maximum 1.%d", runtime.Version(), Go1Point(), x) } } // Deadline returns the deadline of t, if known, // using the Deadline method added in Go 1.15. -func Deadline(t Testing) (time.Time, bool) { +func Deadline(t testing.TB) (time.Time, bool) { td, ok := t.(interface { Deadline() (time.Time, bool) }) diff --git a/internal/testenv/testenv_notunix.go b/internal/testenv/testenv_notunix.go new file mode 100644 index 0000000000..74de6f0a8e --- /dev/null +++ b/internal/testenv/testenv_notunix.go @@ -0,0 +1,14 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build windows || plan9 || (js && wasm) +// +build windows plan9 js,wasm + +package testenv + +import "os" + +// Sigquit is the signal to send to kill a hanging subprocess. +// On Unix we send SIGQUIT, but on non-Unix we only have os.Kill. +var Sigquit = os.Kill diff --git a/internal/testenv/testenv_unix.go b/internal/testenv/testenv_unix.go new file mode 100644 index 0000000000..bc6af1ff81 --- /dev/null +++ b/internal/testenv/testenv_unix.go @@ -0,0 +1,14 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build unix || aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris +// +build unix aix darwin dragonfly freebsd linux netbsd openbsd solaris + +package testenv + +import "syscall" + +// Sigquit is the signal to send to kill a hanging subprocess. +// Send SIGQUIT to get a stack trace. +var Sigquit = syscall.SIGQUIT