Skip to content

Commit 2b149ce

Browse files
committed
gopls/internal/regtest: add a regtest-based version of the marker tests
Add a new implementation of the gopls marker tests that shares the same testing environment as the regression tests. Along the way, revisit the semantics of the marker framework, to address some problems we've identified over the years. Specifically: - Split tests into self-contained txtar encoded files. Each file determines an isolated set of markers, and is executed in a separate session. - Change the mechanisms for golden content, so that it is joined by identifiers, and passed to the test method as an argument. This makes it more apparent where golden content is used, and makes the identity of golden content stable under manipulations of the source (as opposed to some arbitrary munging of the note position) - Allow custom argument conversion that may be convenient for LSP-based test functions, by avoiding the packagestest framework and instead building directly on top of the x/tools/go/expect package. As an initial proof of concept, this allowed using a protocol.Location as a test method argument. - Add significant documentation and examples. - Deprecate the @hover marker in the old marker tests (gopls/internal/lsp). I believe that this lays the foundation to address the remaining concerns enumerated in golang/go#54845, as this new design solves the isolation problem, the problem of golden file naming, and the lack of clarity around the definition and construction of test annotations. For golang/go#54845 Change-Id: I796f35c14370b9651316baa1f86c21c63cec25c7 Reviewed-on: https://go-review.googlesource.com/c/tools/+/465255 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Alan Donovan <[email protected]> Run-TryBot: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]>
1 parent edddc5f commit 2b149ce

File tree

17 files changed

+902
-211
lines changed

17 files changed

+902
-211
lines changed

gopls/internal/lsp/fake/editor.go

+13
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,19 @@ func (e *Editor) BufferText(name string) (string, bool) {
702702
return buf.text(), true
703703
}
704704

705+
// Mapper returns the protocol.Mapper for the given buffer name.
706+
//
707+
// If there is no open buffer with that name, it returns nil.
708+
func (e *Editor) Mapper(name string) *protocol.Mapper {
709+
e.mu.Lock()
710+
defer e.mu.Unlock()
711+
buf, ok := e.buffers[name]
712+
if !ok {
713+
return nil
714+
}
715+
return buf.mapper
716+
}
717+
705718
// BufferVersion returns the current version of the buffer corresponding to
706719
// name (or 0 if it is not being edited).
707720
func (e *Editor) BufferVersion(name string) int {

gopls/internal/lsp/lsp_test.go

+5-38
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ func TestMain(m *testing.M) {
4444
}
4545

4646
// TestLSP runs the marker tests in files beneath testdata/ using
47-
// implementations of each of the marker operations (e.g. @hover) that
48-
// make LSP RPCs (e.g. textDocument/hover) to a gopls server.
47+
// implementations of each of the marker operations (e.g. @codelens) that
48+
// make LSP RPCs (e.g. textDocument/codeLens) to a gopls server.
4949
func TestLSP(t *testing.T) {
5050
tests.RunTests(t, "testdata", true, testLSP)
5151
}
@@ -719,12 +719,12 @@ func (r *runner) Definition(t *testing.T, _ span.Span, d tests.Definition) {
719719
if hover != nil {
720720
didSomething = true
721721
tag := fmt.Sprintf("%s-hoverdef", d.Name)
722-
expectHover := string(r.data.Golden(t, tag, d.Src.URI().Filename(), func() ([]byte, error) {
722+
want := string(r.data.Golden(t, tag, d.Src.URI().Filename(), func() ([]byte, error) {
723723
return []byte(hover.Contents.Value), nil
724724
}))
725725
got := hover.Contents.Value
726-
if got != expectHover {
727-
tests.CheckSameMarkdown(t, got, expectHover)
726+
if diff := tests.DiffMarkdown(want, got); diff != "" {
727+
t.Errorf("%s: markdown mismatch:\n%s", d.Src, diff)
728728
}
729729
}
730730
if !d.OnlyHover {
@@ -818,39 +818,6 @@ func (r *runner) Highlight(t *testing.T, src span.Span, spans []span.Span) {
818818
}
819819
}
820820

821-
func (r *runner) Hover(t *testing.T, src span.Span, text string) {
822-
m, err := r.data.Mapper(src.URI())
823-
if err != nil {
824-
t.Fatal(err)
825-
}
826-
loc, err := m.SpanLocation(src)
827-
if err != nil {
828-
t.Fatalf("failed for %v", err)
829-
}
830-
params := &protocol.HoverParams{
831-
TextDocumentPositionParams: protocol.LocationTextDocumentPositionParams(loc),
832-
}
833-
hover, err := r.server.Hover(r.ctx, params)
834-
if err != nil {
835-
t.Fatal(err)
836-
}
837-
if text == "" {
838-
if hover != nil {
839-
t.Errorf("want nil, got %v\n", hover)
840-
}
841-
} else {
842-
if hover == nil {
843-
t.Fatalf("want hover result to include %s, but got nil", text)
844-
}
845-
if got := hover.Contents.Value; got != text {
846-
t.Errorf("want %v, got %v\n", text, got)
847-
}
848-
if want, got := loc.Range, hover.Range; want != got {
849-
t.Errorf("want range %v, got %v instead", want, got)
850-
}
851-
}
852-
}
853-
854821
func (r *runner) References(t *testing.T, src span.Span, itemList []span.Span) {
855822
// This test is substantially the same as (*runner).References in source/source_test.go.
856823
// TODO(adonovan): Factor (and remove fluff). Where should the common code live?

0 commit comments

Comments
 (0)