gopls/internal/lsp: add missing comments on 3x tests.Test impls
Also, use top-down order for some decls.
Updates golang/go#54845
Change-Id: I5d732593fb709c02300b46a444360c4909b92773
Reviewed-on: https://go-review.googlesource.com/c/tools/+/462659
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
diff --git a/gopls/internal/lsp/cmd/test/cmdtest.go b/gopls/internal/lsp/cmd/test/cmdtest.go
index 1649709..a95875d 100644
--- a/gopls/internal/lsp/cmd/test/cmdtest.go
+++ b/gopls/internal/lsp/cmd/test/cmdtest.go
@@ -28,14 +28,9 @@
"golang.org/x/tools/internal/tool"
)
-type runner struct {
- data *tests.Data
- ctx context.Context
- options func(*source.Options)
- normalizers []tests.Normalizer
- remote string
-}
-
+// 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.
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" {
@@ -43,19 +38,28 @@
}
tests.RunTests(t, testdata, false, func(t *testing.T, datum *tests.Data) {
ctx := tests.Context(t)
- ts := NewTestServer(ctx, options)
+ ts := newTestServer(ctx, options)
tests.Run(t, NewRunner(datum, ctx, ts.Addr, options), datum)
cmd.CloseTestConnections(ctx)
})
}
-func NewTestServer(ctx context.Context, options func(*source.Options)) *servertest.TCPServer {
+func newTestServer(ctx context.Context, options func(*source.Options)) *servertest.TCPServer {
ctx = debug.WithInstance(ctx, "", "")
cache := cache.New(nil, nil)
ss := lsprpc.NewStreamServer(cache, false, options)
return servertest.NewTCPServer(ctx, ss, nil)
}
+// runner implements tests.Tests by fork+execing the gopls command.
+type runner struct {
+ data *tests.Data
+ ctx context.Context
+ options func(*source.Options)
+ normalizers []tests.Normalizer
+ remote string
+}
+
func NewRunner(data *tests.Data, ctx context.Context, remote string, options func(*source.Options)) *runner {
return &runner{
data: data,
diff --git a/gopls/internal/lsp/lsp_test.go b/gopls/internal/lsp/lsp_test.go
index d7acc1c..efb10be 100644
--- a/gopls/internal/lsp/lsp_test.go
+++ b/gopls/internal/lsp/lsp_test.go
@@ -43,19 +43,13 @@
os.Exit(m.Run())
}
+// TestLSP runs the marker tests in files beneath testdata/ using
+// implementations of each of the marker operations (e.g. @hover) that
+// make LSP RPCs (e.g. textDocument/hover) to a gopls server.
func TestLSP(t *testing.T) {
tests.RunTests(t, "testdata", true, testLSP)
}
-type runner struct {
- server *Server
- data *tests.Data
- diagnostics map[span.URI][]*source.Diagnostic
- ctx context.Context
- normalizers []tests.Normalizer
- editRecv chan map[span.URI]string
-}
-
func testLSP(t *testing.T, datum *tests.Data) {
ctx := tests.Context(t)
@@ -110,6 +104,16 @@
tests.Run(t, r, datum)
}
+// runner implements tests.Tests by making LSP RPCs to a gopls server.
+type runner struct {
+ server *Server
+ data *tests.Data
+ diagnostics map[span.URI][]*source.Diagnostic
+ ctx context.Context
+ normalizers []tests.Normalizer
+ editRecv chan map[span.URI]string
+}
+
// testClient stubs any client functions that may be called by LSP functions.
type testClient struct {
protocol.Client
diff --git a/gopls/internal/lsp/source/source_test.go b/gopls/internal/lsp/source/source_test.go
index 97f602f..2959a59 100644
--- a/gopls/internal/lsp/source/source_test.go
+++ b/gopls/internal/lsp/source/source_test.go
@@ -33,10 +33,14 @@
os.Exit(m.Run())
}
+// TestSource runs the marker tests in files beneath testdata/ using
+// implementations of each of the marker operations (e.g. @hover) that
+// make direct calls to the server logic (such as source.Hover).
func TestSource(t *testing.T) {
tests.RunTests(t, "../testdata", true, testSource)
}
+// runner implements tests.Tests by making direct calls to the source package.
type runner struct {
session *cache.Session
view *cache.View
diff --git a/gopls/internal/lsp/tests/tests.go b/gopls/internal/lsp/tests/tests.go
index e9f1009..d9a55c1 100644
--- a/gopls/internal/lsp/tests/tests.go
+++ b/gopls/internal/lsp/tests/tests.go
@@ -143,12 +143,17 @@
mappers map[span.URI]*protocol.Mapper
}
-// TODO(adonovan): there are multiple implementations of this (undocumented)
-// interface, each of which must implement similar semantics. For example:
-// - *runner in ../cmd/test/check.go
-// - *runner in ../source/source_test.go
-// - *runner in ../lsp_test.go
-// Can we avoid this duplication?
+// The Tests interface abstracts a set of implementations of marker
+// test operators (such as @hover) appearing
+//
+// There are three implementations:
+// - *runner in ../cmd/test/check.go, which runs the command-line tool (e.g. "gopls hover")
+// - *runner in ../source/source_test.go, which makes direct calls (e.g. to source.Hover)
+// - *runner in ../lsp_test.go, which makes LSP requests (textDocument/hover) to a gopls server.
+//
+// Not all implementations implement all methods.
+//
+// TODO(adonovan): reduce duplication; see https://github.com/golang/go/issues/54845.
type Tests interface {
CallHierarchy(*testing.T, span.Span, *CallHierarchyResult)
CodeLens(*testing.T, span.URI, []protocol.CodeLens)
