gopls/internal/regtest: follow-ups to review comments from earlier CLs
For convenience, this CL addresses review comments from earlier CLs in
the stack.
Change-Id: I28581c77170aa5e3978b28c4c7c7c85e6cbf506f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/461943
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
diff --git a/gopls/internal/lsp/fake/edit.go b/gopls/internal/lsp/fake/edit.go
index 0b688e7..5fd65b0 100644
--- a/gopls/internal/lsp/fake/edit.go
+++ b/gopls/internal/lsp/fake/edit.go
@@ -9,8 +9,10 @@
"golang.org/x/tools/internal/diff"
)
-// NewEdit creates an edit replacing all content between
+// NewEdit creates an edit replacing all content between the 0-based
// (startLine, startColumn) and (endLine, endColumn) with text.
+//
+// Columns measure UTF-16 codes.
func NewEdit(startLine, startColumn, endLine, endColumn uint32, text string) protocol.TextEdit {
return protocol.TextEdit{
Range: protocol.Range{
diff --git a/gopls/internal/lsp/fake/editor.go b/gopls/internal/lsp/fake/editor.go
index 7a61785..c9214a8 100644
--- a/gopls/internal/lsp/fake/editor.go
+++ b/gopls/internal/lsp/fake/editor.go
@@ -816,9 +816,7 @@
// Symbol performs a workspace symbol search using query
func (e *Editor) Symbol(ctx context.Context, query string) ([]protocol.SymbolInformation, error) {
- params := &protocol.WorkspaceSymbolParams{}
- params.Query = query
-
+ params := &protocol.WorkspaceSymbolParams{Query: query}
return e.Server.Symbol(ctx, params)
}
diff --git a/gopls/internal/lsp/regtest/env.go b/gopls/internal/lsp/regtest/env.go
index ad60658..67e7207 100644
--- a/gopls/internal/lsp/regtest/env.go
+++ b/gopls/internal/lsp/regtest/env.go
@@ -297,17 +297,17 @@
if v > finalVerdict {
finalVerdict = v
}
- summary.WriteString(fmt.Sprintf("%v: %s\n", v, e.Description))
+ fmt.Fprintf(&summary, "%v: %s\n", v, e.Description)
}
return finalVerdict, summary.String()
}
// Await blocks until the given expectations are all simultaneously met.
//
-// Generally speaking Await should be avoided because it can block indefinitely
-// if gopls ends up in a state where the expectations are never going to be
-// met. Use AfterChange or OnceMet instead, so that the runner knows when to
-// stop waiting.
+// Generally speaking Await should be avoided because it blocks indefinitely if
+// gopls ends up in a state where the expectations are never going to be met.
+// Use AfterChange or OnceMet instead, so that the runner knows when to stop
+// waiting.
func (e *Env) Await(expectations ...Expectation) {
e.T.Helper()
if err := e.Awaiter.Await(e.Ctx, expectations...); err != nil {
@@ -315,8 +315,9 @@
}
}
-// OnceMet blocks until precondition is met or unmeetable; if the precondition
-// is met, it atomically checks that all expectations in mustMeets are met.
+// OnceMet blocks until the precondition is met by the state or becomes
+// unmeetable. If it was met, OnceMet checks that the state meets all
+// expectations in mustMeets.
func (e *Env) OnceMet(precondition Expectation, mustMeets ...Expectation) {
e.Await(OnceMet(precondition, mustMeets...))
}
diff --git a/gopls/internal/lsp/regtest/expectation.go b/gopls/internal/lsp/regtest/expectation.go
index d0896ff..6d3f334 100644
--- a/gopls/internal/lsp/regtest/expectation.go
+++ b/gopls/internal/lsp/regtest/expectation.go
@@ -49,9 +49,25 @@
return fmt.Sprintf("unrecognized verdict %d", v)
}
-// Expectation holds an arbitrary check func, and implements the Expectation interface.
+// An Expectation is an expected property of the state of the LSP client.
+// The Check function reports whether the property is met.
+//
+// Expectations are combinators. By composing them, tests may express
+// complex expectations in terms of simpler ones.
+//
+// TODO(rfindley): as expectations are combined, it becomes harder to identify
+// why they failed. A better signature for Check would be
+//
+// func(State) (Verdict, string)
+//
+// returning a reason for the verdict that can be composed similarly to
+// descriptions.
type Expectation struct {
- Check func(State) Verdict
+ Check func(State) Verdict
+
+ // Description holds a noun-phrase identifying what the expectation checks.
+ //
+ // TODO(rfindley): revisit existing descriptions to ensure they compose nicely.
Description string
}
@@ -135,8 +151,11 @@
}
}
-// ReadDiagnostics is an 'expectation' that is used to read diagnostics
-// atomically. It is intended to be used with 'OnceMet'.
+// ReadDiagnostics is an Expectation that stores the current diagnostics for
+// fileName in into, whenever it is evaluated.
+//
+// It can be used in combination with OnceMet or AfterChange to capture the
+// state of diagnostics when other expectations are satisfied.
func ReadDiagnostics(fileName string, into *protocol.PublishDiagnosticsParams) Expectation {
check := func(s State) Verdict {
diags, ok := s.diagnostics[fileName]
@@ -695,7 +714,7 @@
func (e *Env) AtRegexp(name, pattern string) DiagnosticFilter {
pos := e.RegexpSearch(name, pattern)
return DiagnosticFilter{
- desc: fmt.Sprintf("at the first position matching %q in %q", pattern, name),
+ desc: fmt.Sprintf("at the first position matching %#q in %q", pattern, name),
check: func(diagName string, d protocol.Diagnostic) bool {
return diagName == name && d.Range.Start == pos
},
@@ -704,6 +723,10 @@
// AtPosition filters to diagnostics at location name:line:character, for a
// sandbox-relative path name.
+//
+// Line and character are 0-based, and character measures UTF-16 codes.
+//
+// Note: prefer the more readable AtRegexp.
func AtPosition(name string, line, character uint32) DiagnosticFilter {
pos := protocol.Position{Line: line, Character: character}
return DiagnosticFilter{
diff --git a/gopls/internal/regtest/misc/workspace_symbol_test.go b/gopls/internal/regtest/misc/workspace_symbol_test.go
index 33b8147..a492e1d 100644
--- a/gopls/internal/regtest/misc/workspace_symbol_test.go
+++ b/gopls/internal/regtest/misc/workspace_symbol_test.go
@@ -79,7 +79,7 @@
"Fooest",
}
got := env.Symbol("Foo")
- compareSymbols(t, got, want)
+ compareSymbols(t, got, want...)
})
}
@@ -102,15 +102,15 @@
WithOptions(
Settings{"symbolMatcher": symbolMatcher},
).Run(t, files, func(t *testing.T, env *Env) {
- compareSymbols(t, env.Symbol("ABC"), []string{"ABC", "AxxBxxCxx"})
- compareSymbols(t, env.Symbol("'ABC"), []string{"ABC"})
- compareSymbols(t, env.Symbol("^mod.com"), []string{"mod.com/a.ABC", "mod.com/a.AxxBxxCxx"})
- compareSymbols(t, env.Symbol("^mod.com Axx"), []string{"mod.com/a.AxxBxxCxx"})
- compareSymbols(t, env.Symbol("C$"), []string{"ABC"})
+ compareSymbols(t, env.Symbol("ABC"), "ABC", "AxxBxxCxx")
+ compareSymbols(t, env.Symbol("'ABC"), "ABC")
+ compareSymbols(t, env.Symbol("^mod.com"), "mod.com/a.ABC", "mod.com/a.AxxBxxCxx")
+ compareSymbols(t, env.Symbol("^mod.com Axx"), "mod.com/a.AxxBxxCxx")
+ compareSymbols(t, env.Symbol("C$"), "ABC")
})
}
-func compareSymbols(t *testing.T, got []protocol.SymbolInformation, want []string) {
+func compareSymbols(t *testing.T, got []protocol.SymbolInformation, want ...string) {
t.Helper()
if len(got) != len(want) {
t.Errorf("got %d symbols, want %d", len(got), len(want))
