gopls/internal/lsp/cmd/test: signpost future cleanups
Also, remove dead code.
Updates golang/go#54845
Change-Id: Ida96b91b878bfb0598dbe59e4d704771f83e0c0a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/462817
Auto-Submit: Alan Donovan <adonovan@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/gopls/internal/lsp/cmd/test/cmdtest.go b/gopls/internal/lsp/cmd/test/cmdtest.go
index a95875d..cc0ea86 100644
--- a/gopls/internal/lsp/cmd/test/cmdtest.go
+++ b/gopls/internal/lsp/cmd/test/cmdtest.go
@@ -30,7 +30,7 @@
// TestCommandLine runs the marker tests in files beneath testdata/ using
// implementations of each of the marker operations (e.g. @hover) that
-// fork+exec the gopls command.
+// call the main function of the gopls command within this process.
func TestCommandLine(t *testing.T, testdata string, options func(*source.Options)) {
// On Android, the testdata directory is not copied to the runner.
if runtime.GOOS == "android" {
@@ -39,7 +39,7 @@
tests.RunTests(t, testdata, false, func(t *testing.T, datum *tests.Data) {
ctx := tests.Context(t)
ts := newTestServer(ctx, options)
- tests.Run(t, NewRunner(datum, ctx, ts.Addr, options), datum)
+ tests.Run(t, newRunner(datum, ctx, ts.Addr, options), datum)
cmd.CloseTestConnections(ctx)
})
}
@@ -51,7 +51,29 @@
return servertest.NewTCPServer(ctx, ss, nil)
}
-// runner implements tests.Tests by fork+execing the gopls command.
+// runner implements tests.Tests by invoking the gopls command.
+//
+// TODO(golang/go#54845): We don't plan to implement all the methods
+// of tests.Test. Indeed, we'd like to delete the methods that are
+// implemented because the two problems they solve are best addressed
+// in other ways:
+//
+// 1. They provide coverage of the behavior of the server, but this
+// coverage is almost identical to the coverage provided by
+// executing the same tests by making LSP RPCs directly (see
+// lsp_test), and the latter is more efficient. When they do
+// differ, it is a pain for maintainers.
+//
+// 2. They provide coverage of the client-side code of the
+// command-line tool, which turns arguments into an LSP request and
+// prints the results. But this coverage could be more directly and
+// efficiently achieved by running a small number of tests tailored
+// to exercise the client-side code, not the server behavior.
+//
+// Once that's done, tests.Tests would have only a single
+// implementation (LSP), and we could refactor the marker tests
+// so that they more closely resemble self-contained regtests,
+// as described in #54845.
type runner struct {
data *tests.Data
ctx context.Context
@@ -60,7 +82,7 @@
remote string
}
-func NewRunner(data *tests.Data, ctx context.Context, remote string, options func(*source.Options)) *runner {
+func newRunner(data *tests.Data, ctx context.Context, remote string, options func(*source.Options)) *runner {
return &runner{
data: data,
ctx: ctx,
@@ -70,60 +92,15 @@
}
}
-func (r *runner) CodeLens(t *testing.T, uri span.URI, want []protocol.CodeLens) {
- //TODO: add command line completions tests when it works
-}
-
-func (r *runner) Completion(t *testing.T, src span.Span, test tests.Completion, items tests.CompletionItems) {
- //TODO: add command line completions tests when it works
-}
-
-func (r *runner) CompletionSnippet(t *testing.T, src span.Span, expected tests.CompletionSnippet, placeholders bool, items tests.CompletionItems) {
- //TODO: add command line completions tests when it works
-}
-
-func (r *runner) UnimportedCompletion(t *testing.T, src span.Span, test tests.Completion, items tests.CompletionItems) {
- //TODO: add command line completions tests when it works
-}
-
-func (r *runner) DeepCompletion(t *testing.T, src span.Span, test tests.Completion, items tests.CompletionItems) {
- //TODO: add command line completions tests when it works
-}
-
-func (r *runner) FuzzyCompletion(t *testing.T, src span.Span, test tests.Completion, items tests.CompletionItems) {
- //TODO: add command line completions tests when it works
-}
-
-func (r *runner) CaseSensitiveCompletion(t *testing.T, src span.Span, test tests.Completion, items tests.CompletionItems) {
- //TODO: add command line completions tests when it works
-}
-
-func (r *runner) RankCompletion(t *testing.T, src span.Span, test tests.Completion, items tests.CompletionItems) {
- //TODO: add command line completions tests when it works
-}
-
-func (r *runner) FunctionExtraction(t *testing.T, start span.Span, end span.Span) {
- //TODO: function extraction not supported on command line
-}
-
-func (r *runner) MethodExtraction(t *testing.T, start span.Span, end span.Span) {
- //TODO: function extraction not supported on command line
-}
-
-func (r *runner) AddImport(t *testing.T, uri span.URI, expectedImport string) {
- //TODO: import addition not supported on command line
-}
-
-func (r *runner) Hover(t *testing.T, spn span.Span, info string) {
- //TODO: hovering not supported on command line
-}
-
-func (r *runner) InlayHints(t *testing.T, spn span.Span) {
- // TODO: inlayHints not supported on command line
-}
-
-func (r *runner) SelectionRanges(t *testing.T, spn span.Span) {}
-
+// runGoplsCmd returns the stdout and stderr of a gopls command.
+//
+// It does not fork+exec gopls, but in effect calls its main function,
+// and thus the subcommand's Application.Run method, within this process.
+//
+// Stdout and stderr are temporarily redirected: not concurrency-safe!
+//
+// The "exit code" is printed to stderr but not returned.
+// Invalid flags cause process exit.
func (r *runner) runGoplsCmd(t testing.TB, args ...string) (string, string) {
rStdout, wStdout, err := os.Pipe()
if err != nil {
@@ -175,6 +152,26 @@
return tests.Normalize(s, r.normalizers)
}
-func (r *runner) NormalizePrefix(s string) string {
- return tests.NormalizePrefix(s, r.normalizers)
+// Unimplemented methods of tests.Tests (see comment at runner):
+
+func (*runner) CodeLens(t *testing.T, uri span.URI, want []protocol.CodeLens) {}
+func (*runner) Completion(t *testing.T, src span.Span, test tests.Completion, items tests.CompletionItems) {
}
+func (*runner) CompletionSnippet(t *testing.T, src span.Span, expected tests.CompletionSnippet, placeholders bool, items tests.CompletionItems) {
+}
+func (*runner) UnimportedCompletion(t *testing.T, src span.Span, test tests.Completion, items tests.CompletionItems) {
+}
+func (*runner) DeepCompletion(t *testing.T, src span.Span, test tests.Completion, items tests.CompletionItems) {
+}
+func (*runner) FuzzyCompletion(t *testing.T, src span.Span, test tests.Completion, items tests.CompletionItems) {
+}
+func (*runner) CaseSensitiveCompletion(t *testing.T, src span.Span, test tests.Completion, items tests.CompletionItems) {
+}
+func (*runner) RankCompletion(t *testing.T, src span.Span, test tests.Completion, items tests.CompletionItems) {
+}
+func (*runner) FunctionExtraction(t *testing.T, start span.Span, end span.Span) {}
+func (*runner) MethodExtraction(t *testing.T, start span.Span, end span.Span) {}
+func (*runner) AddImport(t *testing.T, uri span.URI, expectedImport string) {}
+func (*runner) Hover(t *testing.T, spn span.Span, info string) {}
+func (*runner) InlayHints(t *testing.T, spn span.Span) {}
+func (*runner) SelectionRanges(t *testing.T, spn span.Span) {}
diff --git a/gopls/internal/lsp/completion_test.go b/gopls/internal/lsp/completion_test.go
index 2257846..8211cc5 100644
--- a/gopls/internal/lsp/completion_test.go
+++ b/gopls/internal/lsp/completion_test.go
@@ -5,11 +5,13 @@
package lsp
import (
+ "fmt"
"strings"
"testing"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/source"
+ "golang.org/x/tools/gopls/internal/lsp/source/completion"
"golang.org/x/tools/gopls/internal/lsp/tests"
"golang.org/x/tools/gopls/internal/span"
)
@@ -111,10 +113,28 @@
func expected(t *testing.T, test tests.Completion, items tests.CompletionItems) []protocol.CompletionItem {
t.Helper()
+ toProtocolCompletionItem := func(item *completion.CompletionItem) protocol.CompletionItem {
+ pItem := protocol.CompletionItem{
+ Label: item.Label,
+ Kind: item.Kind,
+ Detail: item.Detail,
+ Documentation: item.Documentation,
+ InsertText: item.InsertText,
+ TextEdit: &protocol.TextEdit{
+ NewText: item.Snippet(),
+ },
+ // Negate score so best score has lowest sort text like real API.
+ SortText: fmt.Sprint(-item.Score),
+ }
+ if pItem.InsertText == "" {
+ pItem.InsertText = pItem.Label
+ }
+ return pItem
+ }
+
var want []protocol.CompletionItem
for _, pos := range test.CompletionItems {
- item := items[pos]
- want = append(want, tests.ToProtocolCompletionItem(*item))
+ want = append(want, toProtocolCompletionItem(items[pos]))
}
return want
}
diff --git a/gopls/internal/lsp/tests/normalizer.go b/gopls/internal/lsp/tests/normalizer.go
index 77d9e66..9c5d7b9 100644
--- a/gopls/internal/lsp/tests/normalizer.go
+++ b/gopls/internal/lsp/tests/normalizer.go
@@ -41,22 +41,6 @@
return normalizers
}
-// NormalizePrefix normalizes a single path at the front of the input string.
-func NormalizePrefix(s string, normalizers []Normalizer) string {
- for _, n := range normalizers {
- if t := strings.TrimPrefix(s, n.path); t != s {
- return n.fragment + t
- }
- if t := strings.TrimPrefix(s, n.slashed); t != s {
- return n.fragment + t
- }
- if t := strings.TrimPrefix(s, n.escaped); t != s {
- return n.fragment + t
- }
- }
- return s
-}
-
// Normalize replaces all paths present in s with just the fragment portion
// this is used to make golden files not depend on the temporary paths of the files
func Normalize(s string, normalizers []Normalizer) string {
diff --git a/gopls/internal/lsp/tests/util.go b/gopls/internal/lsp/tests/util.go
index ec7f80c..cdbd12e 100644
--- a/gopls/internal/lsp/tests/util.go
+++ b/gopls/internal/lsp/tests/util.go
@@ -275,25 +275,6 @@
return ""
}
-func ToProtocolCompletionItem(item completion.CompletionItem) protocol.CompletionItem {
- pItem := protocol.CompletionItem{
- Label: item.Label,
- Kind: item.Kind,
- Detail: item.Detail,
- Documentation: item.Documentation,
- InsertText: item.InsertText,
- TextEdit: &protocol.TextEdit{
- NewText: item.Snippet(),
- },
- // Negate score so best score has lowest sort text like real API.
- SortText: fmt.Sprint(-item.Score),
- }
- if pItem.InsertText == "" {
- pItem.InsertText = pItem.Label
- }
- return pItem
-}
-
func FilterBuiltins(src span.Span, items []protocol.CompletionItem) []protocol.CompletionItem {
var (
got []protocol.CompletionItem
