Skip to content

Commit

Permalink
internal/testenv: add a Command function that replaces exec.Command
Browse files Browse the repository at this point in the history
The function is derived from the testenv.Command function in the main
repo as of CL 446875, with a couple of modifications to allow it to
build (with more limited functionality) with Go versions as old as
1.16 (currently needed in order to test gopls with such versions).

testenv.Command sets up an exec.Cmd with more useful termination
behavior in the context of a test: namely, it is terminated with
SIGQUIT (to get a goroutine dump from the subprocess) shortly before
the test would otherwise time out.

Assuming that the test logs the output from the command appropriately,
this should make deadlocks and unexpectedly slow operations easier to
diagnose in the builders.

For golang/go#50014.
Updates golang/go#50436.

Change-Id: I872d4b24e63951bf9b7811189e672973d366fb78
Reviewed-on: https://go-review.googlesource.com/c/tools/+/377835
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Joedian Reid <joedian@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
  • Loading branch information
Bryan C. Mills authored and gopherbot committed Nov 21, 2022
1 parent 19fb30d commit 932ec22
Show file tree
Hide file tree
Showing 5 changed files with 193 additions and 36 deletions.
3 changes: 2 additions & 1 deletion go/analysis/analysistest/analysistest.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"sort"
"strconv"
"strings"
"testing"
"text/scanner"

"golang.org/x/tools/go/analysis"
Expand Down Expand Up @@ -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)
}

Expand Down
149 changes: 149 additions & 0 deletions internal/testenv/exec.go
Original file line number Diff line number Diff line change
@@ -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...)
}
49 changes: 14 additions & 35 deletions internal/testenv/testenv.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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=") {
Expand All @@ -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.
Expand Down Expand Up @@ -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)
})
Expand Down
14 changes: 14 additions & 0 deletions internal/testenv/testenv_notunix.go
Original file line number Diff line number Diff line change
@@ -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
14 changes: 14 additions & 0 deletions internal/testenv/testenv_unix.go
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 932ec22

Please sign in to comment.