Skip to content

Commit

Permalink
gopls/internal/regtest: add a regtest-based version of the marker tests
Browse files Browse the repository at this point in the history
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 #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 #54845

Change-Id: I796f35c14370b9651316baa1f86c21c63cec25c7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/465255
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
  • Loading branch information
findleyr committed Feb 7, 2023
1 parent edddc5f commit 2b149ce
Show file tree
Hide file tree
Showing 17 changed files with 902 additions and 211 deletions.
13 changes: 13 additions & 0 deletions gopls/internal/lsp/fake/editor.go
Expand Up @@ -702,6 +702,19 @@ func (e *Editor) BufferText(name string) (string, bool) {
return buf.text(), true
}

// Mapper returns the protocol.Mapper for the given buffer name.
//
// If there is no open buffer with that name, it returns nil.
func (e *Editor) Mapper(name string) *protocol.Mapper {
e.mu.Lock()
defer e.mu.Unlock()
buf, ok := e.buffers[name]
if !ok {
return nil
}
return buf.mapper
}

// BufferVersion returns the current version of the buffer corresponding to
// name (or 0 if it is not being edited).
func (e *Editor) BufferVersion(name string) int {
Expand Down
43 changes: 5 additions & 38 deletions gopls/internal/lsp/lsp_test.go
Expand Up @@ -44,8 +44,8 @@ func TestMain(m *testing.M) {
}

// 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.
// implementations of each of the marker operations (e.g. @codelens) that
// make LSP RPCs (e.g. textDocument/codeLens) to a gopls server.
func TestLSP(t *testing.T) {
tests.RunTests(t, "testdata", true, testLSP)
}
Expand Down Expand Up @@ -719,12 +719,12 @@ func (r *runner) Definition(t *testing.T, _ span.Span, d tests.Definition) {
if hover != nil {
didSomething = true
tag := fmt.Sprintf("%s-hoverdef", d.Name)
expectHover := string(r.data.Golden(t, tag, d.Src.URI().Filename(), func() ([]byte, error) {
want := string(r.data.Golden(t, tag, d.Src.URI().Filename(), func() ([]byte, error) {
return []byte(hover.Contents.Value), nil
}))
got := hover.Contents.Value
if got != expectHover {
tests.CheckSameMarkdown(t, got, expectHover)
if diff := tests.DiffMarkdown(want, got); diff != "" {
t.Errorf("%s: markdown mismatch:\n%s", d.Src, diff)
}
}
if !d.OnlyHover {
Expand Down Expand Up @@ -818,39 +818,6 @@ func (r *runner) Highlight(t *testing.T, src span.Span, spans []span.Span) {
}
}

func (r *runner) Hover(t *testing.T, src span.Span, text string) {
m, err := r.data.Mapper(src.URI())
if err != nil {
t.Fatal(err)
}
loc, err := m.SpanLocation(src)
if err != nil {
t.Fatalf("failed for %v", err)
}
params := &protocol.HoverParams{
TextDocumentPositionParams: protocol.LocationTextDocumentPositionParams(loc),
}
hover, err := r.server.Hover(r.ctx, params)
if err != nil {
t.Fatal(err)
}
if text == "" {
if hover != nil {
t.Errorf("want nil, got %v\n", hover)
}
} else {
if hover == nil {
t.Fatalf("want hover result to include %s, but got nil", text)
}
if got := hover.Contents.Value; got != text {
t.Errorf("want %v, got %v\n", text, got)
}
if want, got := loc.Range, hover.Range; want != got {
t.Errorf("want range %v, got %v instead", want, got)
}
}
}

func (r *runner) References(t *testing.T, src span.Span, itemList []span.Span) {
// This test is substantially the same as (*runner).References in source/source_test.go.
// TODO(adonovan): Factor (and remove fluff). Where should the common code live?
Expand Down

0 comments on commit 2b149ce

Please sign in to comment.