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>
This commit is contained in:
Rob Findley 2021-07-09 12:14:13 -04:00 committed by Robert Findley
parent 2583041a9d
commit 8e32e9f144
4 changed files with 12 additions and 18 deletions

View File

@ -53,7 +53,9 @@ func TestSimultaneousEdits(t *testing.T) {
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(})", "")

View File

@ -539,7 +539,7 @@ func EmptyDiagnostics(name string) Expectation {
// 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

View File

@ -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/xcontext"
)
// Mode is a bitmask that defines for which execution modes a test should run.
@ -307,7 +308,13 @@ func (r *Runner) Run(t *testing.T, files string, test TestFunc, opts ...RunOptio
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)

View File

@ -6,14 +6,12 @@ package regtest
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 @@ func (e *Env) DocumentHighlight(name string, pos fake.Pos) []protocol.DocumentHi
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.