From 1fc30e1f4ccc2986b7ad007d40afcf96be78a429 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Mon, 23 Mar 2020 17:26:05 -0400 Subject: [PATCH] internal/lsp/regtest: remove redundant T and ctx params from regtest funcs In an effort to be idiomatic, I made the regtest func signature func(context.Context, testing.T, *Env), despite the fact that Env already has a Context and a T. This just ended up causing more confusion, as it's not clear which Context or T to use. Remove this and just use the fields on Env. Change-Id: I54da1fdfe6ce17a67601b2af8640d4d2ea676e8c Reviewed-on: https://go-review.googlesource.com/c/tools/+/225157 Run-TryBot: Robert Findley Reviewed-by: Rebecca Stambler Reviewed-by: Heschi Kreinick TryBot-Result: Gobot Gobot --- internal/lsp/regtest/definition_test.go | 5 +- internal/lsp/regtest/diagnostics_test.go | 13 ++-- internal/lsp/regtest/env.go | 20 +++--- internal/lsp/regtest/formatting_test.go | 7 +- internal/lsp/regtest/serialization_test.go | 5 +- internal/lsp/regtest/shared_test.go | 13 ++-- internal/lsp/regtest/wrappers.go | 78 +++++++++++----------- 7 files changed, 68 insertions(+), 73 deletions(-) diff --git a/internal/lsp/regtest/definition_test.go b/internal/lsp/regtest/definition_test.go index 09084ac829..9f175b55ab 100644 --- a/internal/lsp/regtest/definition_test.go +++ b/internal/lsp/regtest/definition_test.go @@ -5,7 +5,6 @@ package regtest import ( - "context" "path" "testing" ) @@ -30,7 +29,7 @@ const message = "Hello World." ` func TestGoToInternalDefinition(t *testing.T) { - runner.Run(t, internalDefinition, func(ctx context.Context, t *testing.T, env *Env) { + runner.Run(t, internalDefinition, func(env *Env) { env.OpenFile("main.go") name, pos := env.GoToDefinition("main.go", env.RegexpSearch("main.go", "message")) if want := "const.go"; name != want { @@ -60,7 +59,7 @@ func main() { }` func TestGoToStdlibDefinition(t *testing.T) { - runner.Run(t, stdlibDefinition, func(ctx context.Context, t *testing.T, env *Env) { + runner.Run(t, stdlibDefinition, func(env *Env) { env.OpenFile("main.go") name, pos := env.GoToDefinition("main.go", env.RegexpSearch("main.go", "Now")) if got, want := path.Base(name), "time.go"; got != want { diff --git a/internal/lsp/regtest/diagnostics_test.go b/internal/lsp/regtest/diagnostics_test.go index 48a3fdc3a5..609b375fc9 100644 --- a/internal/lsp/regtest/diagnostics_test.go +++ b/internal/lsp/regtest/diagnostics_test.go @@ -5,7 +5,6 @@ package regtest import ( - "context" "testing" ) @@ -24,7 +23,7 @@ func main() { }` func TestDiagnosticErrorInEditedFile(t *testing.T) { - runner.Run(t, exampleProgram, func(ctx context.Context, t *testing.T, env *Env) { + runner.Run(t, exampleProgram, func(env *Env) { // Deleting the 'n' at the end of Println should generate a single error // diagnostic. env.OpenFile("main.go") @@ -43,7 +42,7 @@ go 1.12 func TestMissingImportDiagsClearOnFirstFile(t *testing.T) { t.Skip("skipping due to golang.org/issues/37195") t.Parallel() - runner.Run(t, onlyMod, func(ctx context.Context, t *testing.T, env *Env) { + runner.Run(t, onlyMod, func(env *Env) { env.CreateBuffer("main.go", "package main\n\nfunc m() {\nlog.Println()\n}") env.SaveBuffer("main.go") // TODO: this shouldn't actually happen @@ -57,7 +56,7 @@ const Foo = "abc ` func TestDiagnosticErrorInNewFile(t *testing.T) { - runner.Run(t, brokenFile, func(ctx context.Context, t *testing.T, env *Env) { + runner.Run(t, brokenFile, func(env *Env) { env.CreateBuffer("broken.go", brokenFile) env.Await(env.DiagnosticAtRegexp("broken.go", "\"abc")) }) @@ -80,7 +79,7 @@ const a = 2 ` func TestDiagnosticClearingOnEdit(t *testing.T) { - runner.Run(t, badPackage, func(ctx context.Context, t *testing.T, env *Env) { + runner.Run(t, badPackage, func(env *Env) { env.OpenFile("b.go") env.Await(env.DiagnosticAtRegexp("a.go", "a = 1"), env.DiagnosticAtRegexp("b.go", "a = 2")) @@ -91,7 +90,7 @@ func TestDiagnosticClearingOnEdit(t *testing.T) { } func TestDiagnosticClearingOnDelete(t *testing.T) { - runner.Run(t, badPackage, func(ctx context.Context, t *testing.T, env *Env) { + runner.Run(t, badPackage, func(env *Env) { env.OpenFile("a.go") env.Await(env.DiagnosticAtRegexp("a.go", "a = 1"), env.DiagnosticAtRegexp("b.go", "a = 2")) env.RemoveFileFromWorkspace("b.go") @@ -101,7 +100,7 @@ func TestDiagnosticClearingOnDelete(t *testing.T) { } func TestDiagnosticClearingOnClose(t *testing.T) { - runner.Run(t, badPackage, func(ctx context.Context, t *testing.T, env *Env) { + runner.Run(t, badPackage, func(env *Env) { env.CreateBuffer("c.go", `package consts const a = 3`) diff --git a/internal/lsp/regtest/env.go b/internal/lsp/regtest/env.go index 8ba39f9f68..2b372c0265 100644 --- a/internal/lsp/regtest/env.go +++ b/internal/lsp/regtest/env.go @@ -141,13 +141,13 @@ func (r *Runner) Close() error { // Run executes the test function in the default configured gopls execution // modes. For each a test run, a new workspace is created containing the // un-txtared files specified by filedata. -func (r *Runner) Run(t *testing.T, filedata string, test func(context.Context, *testing.T, *Env)) { +func (r *Runner) Run(t *testing.T, filedata string, test func(e *Env)) { t.Helper() r.RunInMode(r.defaultModes, t, filedata, test) } // RunInMode runs the test in the execution modes specified by the modes bitmask. -func (r *Runner) RunInMode(modes EnvMode, t *testing.T, filedata string, test func(ctx context.Context, t *testing.T, e *Env)) { +func (r *Runner) RunInMode(modes EnvMode, t *testing.T, filedata string, test func(e *Env)) { t.Helper() tests := []struct { name string @@ -182,7 +182,7 @@ func (r *Runner) RunInMode(modes EnvMode, t *testing.T, filedata string, test fu panic(err) } }() - test(ctx, t, env) + test(env) }) } } @@ -230,8 +230,8 @@ func (r *Runner) separateProcessEnv(ctx context.Context, t *testing.T) (serverte // on any error, so that tests for the happy path may be written without // checking errors. type Env struct { - t *testing.T - ctx context.Context + T *testing.T + Ctx context.Context // Most tests should not need to access the workspace, editor, server, or // connection, but they are available if needed. @@ -266,8 +266,8 @@ func NewEnv(ctx context.Context, t *testing.T, ws *fake.Workspace, ts servertest t.Fatal(err) } env := &Env{ - t: t, - ctx: ctx, + T: t, + Ctx: ctx, W: ws, E: editor, Server: ts, @@ -362,7 +362,7 @@ func (e *Env) Await(expectations ...DiagnosticExpectation) { // require careful checking of conditions around every state change, so for // now we just limit the scope to diagnostic conditions. - e.t.Helper() + e.T.Helper() e.mu.Lock() // Before adding the waiter, we check if the condition is currently met to // avoid a race where the condition was realized before Await was called. @@ -379,7 +379,7 @@ func (e *Env) Await(expectations ...DiagnosticExpectation) { e.mu.Unlock() select { - case <-e.ctx.Done(): + case <-e.Ctx.Done(): // Debugging an unmet expectation can be tricky, so we put some effort into // nicely formatting the failure. var descs []string @@ -389,7 +389,7 @@ func (e *Env) Await(expectations ...DiagnosticExpectation) { e.mu.Lock() diagString := formatDiagnostics(e.lastDiagnostics) e.mu.Unlock() - e.t.Fatalf("waiting on [%s]:\nerr:%v\ndiagnostics:\n%s", strings.Join(descs, ", "), e.ctx.Err(), diagString) + e.T.Fatalf("waiting on [%s]:\nerr:%v\ndiagnostics:\n%s", strings.Join(descs, ", "), e.Ctx.Err(), diagString) case <-met: } } diff --git a/internal/lsp/regtest/formatting_test.go b/internal/lsp/regtest/formatting_test.go index d1b43bb9a5..202f4d49ee 100644 --- a/internal/lsp/regtest/formatting_test.go +++ b/internal/lsp/regtest/formatting_test.go @@ -1,7 +1,6 @@ package regtest import ( - "context" "testing" ) @@ -23,7 +22,7 @@ func main() { ` func TestFormatting(t *testing.T) { - runner.Run(t, unformattedProgram, func(ctx context.Context, t *testing.T, env *Env) { + runner.Run(t, unformattedProgram, func(env *Env) { env.OpenFile("main.go") env.FormatBuffer("main.go") got := env.E.BufferText("main.go") @@ -69,7 +68,7 @@ func main() { ` func TestOrganizeImports(t *testing.T) { - runner.Run(t, disorganizedProgram, func(ctx context.Context, t *testing.T, env *Env) { + runner.Run(t, disorganizedProgram, func(env *Env) { env.OpenFile("main.go") env.OrganizeImports("main.go") got := env.E.BufferText("main.go") @@ -81,7 +80,7 @@ func TestOrganizeImports(t *testing.T) { } func TestFormattingOnSave(t *testing.T) { - runner.Run(t, disorganizedProgram, func(ctx context.Context, t *testing.T, env *Env) { + runner.Run(t, disorganizedProgram, func(env *Env) { env.OpenFile("main.go") env.SaveBuffer("main.go") got := env.E.BufferText("main.go") diff --git a/internal/lsp/regtest/serialization_test.go b/internal/lsp/regtest/serialization_test.go index 6b7bef0107..2ac90d95f1 100644 --- a/internal/lsp/regtest/serialization_test.go +++ b/internal/lsp/regtest/serialization_test.go @@ -5,7 +5,6 @@ package regtest import ( - "context" "encoding/json" "testing" @@ -27,14 +26,14 @@ func main() { }` func TestHoverSerialization(t *testing.T) { - runner.Run(t, simpleProgram, func(ctx context.Context, t *testing.T, env *Env) { + runner.Run(t, simpleProgram, func(env *Env) { // Hover on an empty line. params := protocol.HoverParams{} params.TextDocument.URI = env.W.URI("main.go") params.Position.Line = 3 params.Position.Character = 0 var resp json.RawMessage - if err := env.Conn.Call(ctx, "textDocument/hover", ¶ms, &resp); err != nil { + if err := env.Conn.Call(env.Ctx, "textDocument/hover", ¶ms, &resp); err != nil { t.Fatal(err) } if len(string(resp)) > 0 { diff --git a/internal/lsp/regtest/shared_test.go b/internal/lsp/regtest/shared_test.go index dd465aef1c..8cf5e9c138 100644 --- a/internal/lsp/regtest/shared_test.go +++ b/internal/lsp/regtest/shared_test.go @@ -5,7 +5,6 @@ package regtest import ( - "context" "testing" ) @@ -23,19 +22,19 @@ func main() { fmt.Println("Hello World.") }` -func runShared(t *testing.T, program string, testFunc func(ctx context.Context, t *testing.T, env1 *Env, env2 *Env)) { +func runShared(t *testing.T, program string, testFunc func(env1 *Env, env2 *Env)) { // Only run these tests in forwarded modes. modes := runner.Modes() & (Forwarded | SeparateProcess) - runner.RunInMode(modes, t, sharedProgram, func(ctx context.Context, t *testing.T, env1 *Env) { + runner.RunInMode(modes, t, sharedProgram, func(env1 *Env) { // Create a second test session connected to the same workspace and server // as the first. - env2 := NewEnv(ctx, t, env1.W, env1.Server) - testFunc(ctx, t, env1, env2) + env2 := NewEnv(env1.Ctx, t, env1.W, env1.Server) + testFunc(env1, env2) }) } func TestSimultaneousEdits(t *testing.T) { - runShared(t, exampleProgram, func(ctx context.Context, t *testing.T, env1 *Env, env2 *Env) { + runShared(t, exampleProgram, func(env1 *Env, env2 *Env) { // In editor #1, break fmt.Println as before. env1.OpenFile("main.go") env1.RegexpReplace("main.go", "Printl(n)", "") @@ -50,7 +49,7 @@ func TestSimultaneousEdits(t *testing.T) { } func TestShutdown(t *testing.T) { - runShared(t, sharedProgram, func(ctx context.Context, t *testing.T, env1 *Env, env2 *Env) { + runShared(t, sharedProgram, func(env1 *Env, env2 *Env) { env1.CloseEditor() // Now make an edit in editor #2 to trigger diagnostics. env2.OpenFile("main.go") diff --git a/internal/lsp/regtest/wrappers.go b/internal/lsp/regtest/wrappers.go index a374b57186..ece6063eb7 100644 --- a/internal/lsp/regtest/wrappers.go +++ b/internal/lsp/regtest/wrappers.go @@ -11,53 +11,53 @@ import ( // RemoveFileFromWorkspace deletes a file on disk but does nothing in the // editor. It calls t.Fatal on any error. func (e *Env) RemoveFileFromWorkspace(name string) { - e.t.Helper() - if err := e.W.RemoveFile(e.ctx, name); err != nil { - e.t.Fatal(err) + e.T.Helper() + if err := e.W.RemoveFile(e.Ctx, name); err != nil { + e.T.Fatal(err) } } // ReadWorkspaceFile reads a file from the workspace, calling t.Fatal on any // error. func (e *Env) ReadWorkspaceFile(name string) string { - e.t.Helper() + e.T.Helper() content, err := e.W.ReadFile(name) if err != nil { - e.t.Fatal(err) + e.T.Fatal(err) } return content } // OpenFile opens a file in the editor, calling t.Fatal on any error. func (e *Env) OpenFile(name string) { - e.t.Helper() - if err := e.E.OpenFile(e.ctx, name); err != nil { - e.t.Fatal(err) + e.T.Helper() + if err := e.E.OpenFile(e.Ctx, name); err != nil { + e.T.Fatal(err) } } // CreateBuffer creates a buffer in the editor, calling t.Fatal on any error. func (e *Env) CreateBuffer(name string, content string) { - e.t.Helper() - if err := e.E.CreateBuffer(e.ctx, name, content); err != nil { - e.t.Fatal(err) + e.T.Helper() + if err := e.E.CreateBuffer(e.Ctx, name, content); err != nil { + e.T.Fatal(err) } } // CloseBuffer closes an editor buffer without saving, calling t.Fatal on any // error. func (e *Env) CloseBuffer(name string) { - e.t.Helper() - if err := e.E.CloseBuffer(e.ctx, name); err != nil { - e.t.Fatal(err) + e.T.Helper() + if err := e.E.CloseBuffer(e.Ctx, name); err != nil { + e.T.Fatal(err) } } // EditBuffer applies edits to an editor buffer, calling t.Fatal on any error. func (e *Env) EditBuffer(name string, edits ...fake.Edit) { - e.t.Helper() - if err := e.E.EditBuffer(e.ctx, name, edits); err != nil { - e.t.Fatal(err) + e.T.Helper() + if err := e.E.EditBuffer(e.Ctx, name, edits); err != nil { + e.T.Fatal(err) } } @@ -65,13 +65,13 @@ func (e *Env) EditBuffer(name string, edits ...fake.Edit) { // buffer specified by name, calling t.Fatal on any error. It first searches // for the position in open buffers, then in workspace files. func (e *Env) RegexpSearch(name, re string) fake.Pos { - e.t.Helper() + e.T.Helper() pos, err := e.E.RegexpSearch(name, re) if err == fake.ErrUnknownBuffer { pos, err = e.W.RegexpSearch(name, re) } if err != nil { - e.t.Fatalf("RegexpSearch: %v", err) + e.T.Fatalf("RegexpSearch: %v", err) } return pos } @@ -79,55 +79,55 @@ func (e *Env) RegexpSearch(name, re string) fake.Pos { // RegexpReplace replaces the first group in the first match of regexpStr with // the replace text, calling t.Fatal on any error. func (e *Env) RegexpReplace(name, regexpStr, replace string) { - e.t.Helper() - if err := e.E.RegexpReplace(e.ctx, name, regexpStr, replace); err != nil { - e.t.Fatalf("RegexpReplace: %v", err) + e.T.Helper() + if err := e.E.RegexpReplace(e.Ctx, name, regexpStr, replace); err != nil { + e.T.Fatalf("RegexpReplace: %v", err) } } // SaveBuffer saves an editor buffer, calling t.Fatal on any error. func (e *Env) SaveBuffer(name string) { - e.t.Helper() - if err := e.E.SaveBuffer(e.ctx, name); err != nil { - e.t.Fatal(err) + e.T.Helper() + if err := e.E.SaveBuffer(e.Ctx, name); err != nil { + e.T.Fatal(err) } } // GoToDefinition goes to definition in the editor, calling t.Fatal on any // error. func (e *Env) GoToDefinition(name string, pos fake.Pos) (string, fake.Pos) { - e.t.Helper() - n, p, err := e.E.GoToDefinition(e.ctx, name, pos) + e.T.Helper() + n, p, err := e.E.GoToDefinition(e.Ctx, name, pos) if err != nil { - e.t.Fatal(err) + e.T.Fatal(err) } return n, p } // FormatBuffer formats the editor buffer, calling t.Fatal on any error. func (e *Env) FormatBuffer(name string) { - e.t.Helper() - if err := e.E.FormatBuffer(e.ctx, name); err != nil { - e.t.Fatal(err) + e.T.Helper() + if err := e.E.FormatBuffer(e.Ctx, name); err != nil { + e.T.Fatal(err) } } // OrganizeImports processes the source.organizeImports codeAction, calling // t.Fatal on any error. func (e *Env) OrganizeImports(name string) { - e.t.Helper() - if err := e.E.OrganizeImports(e.ctx, name); err != nil { - e.t.Fatal(err) + e.T.Helper() + if err := e.E.OrganizeImports(e.Ctx, name); err != nil { + e.T.Fatal(err) } } // CloseEditor shuts down the editor, calling t.Fatal on any error. func (e *Env) CloseEditor() { - e.t.Helper() - if err := e.E.Shutdown(e.ctx); err != nil { - e.t.Fatal(err) + e.T.Helper() + if err := e.E.Shutdown(e.Ctx); err != nil { + e.T.Fatal(err) } - if err := e.E.Exit(e.ctx); err != nil { - e.t.Fatal(err) + if err := e.E.Exit(e.Ctx); err != nil { + e.T.Fatal(err) } }