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 <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
gopls-CI: kokoro <[email protected]>
Run-TryBot: Alan Donovan <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>

Author: adonovan

gopls/internal/lsp/cmd/test/cmdtest.go

@@ -28,34 +28,38 @@ import (
 	"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" {
 		t.Skip("testdata directory not present on android")
 	}
 	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,

gopls/internal/lsp/lsp_test.go

@@ -43,19 +43,13 @@ func TestMain(m *testing.M) {
 	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 @@ func testLSP(t *testing.T, datum *tests.Data) {
 	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

gopls/internal/lsp/source/source_test.go

@@ -33,10 +33,14 @@ func TestMain(m *testing.M) {
 	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

gopls/internal/lsp/tests/tests.go

@@ -143,12 +143,17 @@ type Data struct {
 	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)