diff --git a/gopls/internal/regtest/bench/bench_test.go b/gopls/internal/regtest/bench/bench_test.go index 9cbf2f4d92c..61d4ae2cbed 100644 --- a/gopls/internal/regtest/bench/bench_test.go +++ b/gopls/internal/regtest/bench/bench_test.go @@ -10,7 +10,6 @@ import ( "os" "runtime/pprof" "testing" - "time" "golang.org/x/tools/gopls/internal/hooks" "golang.org/x/tools/internal/lsp/fake" @@ -32,9 +31,9 @@ func benchmarkOptions(dir string) []RunOption { SkipLogs(), // The Debug server only makes sense if running in singleton mode. Modes(Singleton), - // Set a generous timeout. Individual tests should control their own - // graceful termination. - Timeout(20 * time.Minute), + // Remove the default timeout. Individual tests should control their + // own graceful termination. + NoDefaultTimeout(), // Use the actual proxy, since we want our builds to succeed. GOPROXY("https://proxy.golang.org"), diff --git a/gopls/internal/regtest/codelens/codelens_test.go b/gopls/internal/regtest/codelens/codelens_test.go index 38721fb2c60..3e15271147e 100644 --- a/gopls/internal/regtest/codelens/codelens_test.go +++ b/gopls/internal/regtest/codelens/codelens_test.go @@ -9,7 +9,6 @@ import ( "runtime" "strings" "testing" - "time" "golang.org/x/tools/gopls/internal/hooks" . "golang.org/x/tools/internal/lsp/regtest" @@ -292,13 +291,6 @@ func TestGCDetails(t *testing.T) { t.Skipf("the gc details code lens doesn't work on Android") } - // TestGCDetails seems to suffer from poor performance on certain builders. - // Give it as long as it needs to complete. - timeout := 60 * time.Second - if d, ok := testenv.Deadline(t); ok { - timeout = time.Until(d) * 19 / 20 // Leave 5% headroom for cleanup. - } - const mod = ` -- go.mod -- module mod.com @@ -318,7 +310,6 @@ func main() { CodeLenses: map[string]bool{ "gc_details": true, }}, - Timeout(timeout), ).Run(t, mod, func(t *testing.T, env *Env) { env.OpenFile("main.go") env.ExecuteCodeLensCommand("main.go", command.GCDetails) diff --git a/internal/lsp/regtest/regtest.go b/internal/lsp/regtest/regtest.go index c1df26d8aa6..31806233c49 100644 --- a/internal/lsp/regtest/regtest.go +++ b/internal/lsp/regtest/regtest.go @@ -23,11 +23,24 @@ import ( var ( runSubprocessTests = flag.Bool("enable_gopls_subprocess_tests", false, "run regtests against a gopls subprocess") goplsBinaryPath = flag.String("gopls_test_binary", "", "path to the gopls binary for use as a remote, for use with the -enable_gopls_subprocess_tests flag") - regtestTimeout = flag.Duration("regtest_timeout", 20*time.Second, "default timeout for each regtest") + regtestTimeout = flag.Duration("regtest_timeout", defaultRegtestTimeout(), "if nonzero, default timeout for each regtest; defaults to GOPLS_REGTEST_TIMEOUT") skipCleanup = flag.Bool("regtest_skip_cleanup", false, "whether to skip cleaning up temp directories") printGoroutinesOnFailure = flag.Bool("regtest_print_goroutines", false, "whether to print goroutines info on failure") ) +func defaultRegtestTimeout() time.Duration { + s := os.Getenv("GOPLS_REGTEST_TIMEOUT") + if s == "" { + return 0 + } + d, err := time.ParseDuration(s) + if err != nil { + fmt.Fprintf(os.Stderr, "invalid GOPLS_REGTEST_TIMEOUT %q: %v\n", s, err) + os.Exit(2) + } + return d +} + var runner *Runner type regtestRunner interface { diff --git a/internal/lsp/regtest/runner.go b/internal/lsp/regtest/runner.go index 05867c4f970..822a5a315cc 100644 --- a/internal/lsp/regtest/runner.go +++ b/internal/lsp/regtest/runner.go @@ -29,6 +29,7 @@ import ( "golang.org/x/tools/internal/lsp/lsprpc" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/testenv" "golang.org/x/tools/internal/xcontext" ) @@ -71,20 +72,19 @@ type Runner struct { } type runConfig struct { - editor fake.EditorConfig - sandbox fake.SandboxConfig - modes Mode - timeout time.Duration - debugAddr string - skipLogs bool - skipHooks bool - optionsHook func(*source.Options) + editor fake.EditorConfig + sandbox fake.SandboxConfig + modes Mode + noDefaultTimeout bool + debugAddr string + skipLogs bool + skipHooks bool + optionsHook func(*source.Options) } func (r *Runner) defaultConfig() *runConfig { return &runConfig{ modes: r.DefaultModes, - timeout: r.Timeout, optionsHook: r.OptionsHook, } } @@ -100,10 +100,12 @@ func (f optionSetter) set(opts *runConfig) { f(opts) } -// Timeout configures a custom timeout for this test run. -func Timeout(d time.Duration) RunOption { +// NoDefaultTimeout removes the timeout set by the -regtest_timeout flag, for +// individual tests that are expected to run longer than is reasonable for +// ordinary regression tests. +func NoDefaultTimeout() RunOption { return optionSetter(func(opts *runConfig) { - opts.timeout = d + opts.noDefaultTimeout = true }) } @@ -257,8 +259,18 @@ func (r *Runner) Run(t *testing.T, files string, test TestFunc, opts ...RunOptio } t.Run(tc.name, func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), config.timeout) - defer cancel() + ctx := context.Background() + if r.Timeout != 0 && !config.noDefaultTimeout { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, r.Timeout) + defer cancel() + } else if d, ok := testenv.Deadline(t); ok { + timeout := time.Until(d) * 19 / 20 // Leave an arbitrary 5% for cleanup. + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, timeout) + defer cancel() + } + ctx = debug.WithInstance(ctx, "", "off") if config.debugAddr != "" { di := debug.GetInstance(ctx)