internal/lsp/regtest: fix a panic TestResolveImportCycle
CL 333289 introduced a panic, which was subsequently suppressed in test
error output due to the deferred t.Fatal (an interesting gotcha that I
honestly wasn't aware of).
Fix both the panic, and the suppression of regtest panics.
Also fix the regtest editor shutdown to run on a detached context, so
that shutdown doesn't fail for tests that have timed out.
For golang/go#46773
Change-Id: I080a713ae4cd4651476d8b4aab1d2291754a4f5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333510
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/gopls/internal/regtest/misc/shared_test.go b/gopls/internal/regtest/misc/shared_test.go
index e6dc0ac..6861743 100644
--- a/gopls/internal/regtest/misc/shared_test.go
+++ b/gopls/internal/regtest/misc/shared_test.go
@@ -53,7 +53,9 @@
func TestShutdown(t *testing.T) {
runShared(t, func(env1 *Env, env2 *Env) {
- env1.CloseEditor()
+ if err := env1.Editor.Close(env1.Ctx); err != nil {
+ t.Errorf("closing first editor: %v", err)
+ }
// Now make an edit in editor #2 to trigger diagnostics.
env2.OpenFile("main.go")
env2.RegexpReplace("main.go", "\\)\n(})", "")
diff --git a/internal/lsp/regtest/expectation.go b/internal/lsp/regtest/expectation.go
index eb24955..8fb6afb 100644
--- a/internal/lsp/regtest/expectation.go
+++ b/internal/lsp/regtest/expectation.go
@@ -539,7 +539,7 @@
// send at least one diagnostic set for open files.
func EmptyOrNoDiagnostics(name string) Expectation {
check := func(s State) Verdict {
- if diags := s.diagnostics[name]; len(diags.Diagnostics) == 0 {
+ if diags := s.diagnostics[name]; diags == nil || len(diags.Diagnostics) == 0 {
return Met
}
return Unmet
diff --git a/internal/lsp/regtest/runner.go b/internal/lsp/regtest/runner.go
index c245fa7..fb17b49 100644
--- a/internal/lsp/regtest/runner.go
+++ b/internal/lsp/regtest/runner.go
@@ -29,6 +29,7 @@
"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/xcontext"
)
// Mode is a bitmask that defines for which execution modes a test should run.
@@ -307,7 +308,13 @@
if t.Failed() || testing.Verbose() {
ls.printBuffers(t.Name(), os.Stderr)
}
- env.CloseEditor()
+ // For tests that failed due to a timeout, don't fail to shutdown
+ // because ctx is done.
+ closeCtx, cancel := context.WithTimeout(xcontext.Detach(ctx), 5*time.Second)
+ defer cancel()
+ if err := env.Editor.Close(closeCtx); err != nil {
+ t.Errorf("closing editor: %v", err)
+ }
}()
// Always await the initial workspace load.
env.Await(InitialWorkspaceLoad)
diff --git a/internal/lsp/regtest/wrappers.go b/internal/lsp/regtest/wrappers.go
index 68e1b74..5677ab0 100644
--- a/internal/lsp/regtest/wrappers.go
+++ b/internal/lsp/regtest/wrappers.go
@@ -6,14 +6,12 @@
import (
"encoding/json"
- "io"
"path"
"testing"
"golang.org/x/tools/internal/lsp/command"
"golang.org/x/tools/internal/lsp/fake"
"golang.org/x/tools/internal/lsp/protocol"
- errors "golang.org/x/xerrors"
)
func (e *Env) ChangeFilesOnDisk(events []fake.FileEvent) {
@@ -247,19 +245,6 @@
return highlights
}
-func checkIsFatal(t testing.TB, err error) {
- t.Helper()
- if err != nil && !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrClosedPipe) {
- t.Fatal(err)
- }
-}
-
-// CloseEditor shuts down the editor, calling t.Fatal on any error.
-func (e *Env) CloseEditor() {
- e.T.Helper()
- checkIsFatal(e.T, e.Editor.Close(e.Ctx))
-}
-
// RunGenerate runs go:generate on the given dir, calling t.Fatal on any error.
// It waits for the generate command to complete and checks for file changes
// before returning.
