Skip to content

Commit

Permalink
gopls/internal/lsp/tests: pass in a *testing.T to Data.Golden
Browse files Browse the repository at this point in the history
Data.Golden is called from subtests: use the *testing.T from the caller,
so that we can get a more meaningful failure.

For golang/go#54845

Change-Id: I136df0c6a7a11bcf364b78ecac42ba2b51a15bb0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/431844
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
  • Loading branch information
findleyr committed Sep 21, 2022
1 parent 14462ef commit 81a42f0
Show file tree
Hide file tree
Showing 13 changed files with 43 additions and 42 deletions.
2 changes: 1 addition & 1 deletion gopls/internal/lsp/cmd/test/definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) {
if mode&jsonGoDef != 0 && runtime.GOOS == "windows" {
got = strings.Replace(got, "file:///", "file://", -1)
}
expect := strings.TrimSpace(string(r.data.Golden(tag, uri.Filename(), func() ([]byte, error) {
expect := strings.TrimSpace(string(r.data.Golden(t, tag, uri.Filename(), func() ([]byte, error) {
return []byte(got), nil
})))
if expect != "" && !strings.HasPrefix(got, expect) {
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/cmd/test/folding_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func (r *runner) FoldingRanges(t *testing.T, spn span.Span) {
uri := spn.URI()
filename := uri.Filename()
got, _ := r.NormalizeGoplsCmd(t, "folding_ranges", filename)
expect := string(r.data.Golden(goldenTag, filename, func() ([]byte, error) {
expect := string(r.data.Golden(t, goldenTag, filename, func() ([]byte, error) {
return []byte(got), nil
}))

Expand Down
5 changes: 3 additions & 2 deletions gopls/internal/lsp/cmd/test/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ package cmdtest

import (
"bytes"
exec "golang.org/x/sys/execabs"
"io/ioutil"
"os"
"regexp"
"strings"
"testing"

exec "golang.org/x/sys/execabs"

"golang.org/x/tools/internal/span"
"golang.org/x/tools/internal/testenv"
)
Expand All @@ -21,7 +22,7 @@ func (r *runner) Format(t *testing.T, spn span.Span) {
tag := "gofmt"
uri := spn.URI()
filename := uri.Filename()
expect := string(r.data.Golden(tag, filename, func() ([]byte, error) {
expect := string(r.data.Golden(t, tag, filename, func() ([]byte, error) {
cmd := exec.Command("gofmt", filename)
contents, _ := cmd.Output() // ignore error, sometimes we have intentionally ungofmt-able files
contents = []byte(r.Normalize(fixFileHeader(string(contents))))
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/cmd/test/imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func (r *runner) Import(t *testing.T, spn span.Span) {
uri := spn.URI()
filename := uri.Filename()
got, _ := r.NormalizeGoplsCmd(t, "imports", filename)
want := string(r.data.Golden("goimports", filename, func() ([]byte, error) {
want := string(r.data.Golden(t, "goimports", filename, func() ([]byte, error) {
return []byte(got), nil
}))
if want != got {
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/cmd/test/rename.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) {
loc := fmt.Sprintf("%v", spn)
got, err := r.NormalizeGoplsCmd(t, "rename", loc, newText)
got += err
expect := string(r.data.Golden(goldenTag, filename, func() ([]byte, error) {
expect := string(r.data.Golden(t, goldenTag, filename, func() ([]byte, error) {
return []byte(got), nil
}))
if expect != got {
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/cmd/test/semanticdriver.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func (r *runner) SemanticTokens(t *testing.T, spn span.Span) {
if stderr != "" {
t.Fatalf("%s: %q", filename, stderr)
}
want := string(r.data.Golden("semantic", filename, func() ([]byte, error) {
want := string(r.data.Golden(t, "semantic", filename, func() ([]byte, error) {
return []byte(got), nil
}))
if want != got {
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/cmd/test/signature.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func (r *runner) SignatureHelp(t *testing.T, spn span.Span, want *protocol.Signa
return
}
goldenTag := want.Signatures[0].Label + "-signature"
expect := string(r.data.Golden(goldenTag, filename, func() ([]byte, error) {
expect := string(r.data.Golden(t, goldenTag, filename, func() ([]byte, error) {
return []byte(got), nil
}))
if tests.NormalizeAny(expect) != tests.NormalizeAny(got) {
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/cmd/test/suggested_fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, suggestedFixes []test
if stderr == "ExecuteCommand is not yet supported on the command line" {
return // don't skip to keep the summary counts correct
}
want := string(r.data.Golden("suggestedfix_"+tests.SpanName(spn), filename, func() ([]byte, error) {
want := string(r.data.Golden(t, "suggestedfix_"+tests.SpanName(spn), filename, func() ([]byte, error) {
return []byte(got), nil
}))
if want != got {
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/cmd/test/symbols.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
func (r *runner) Symbols(t *testing.T, uri span.URI, expectedSymbols []protocol.DocumentSymbol) {
filename := uri.Filename()
got, _ := r.NormalizeGoplsCmd(t, "symbols", filename)
expect := string(r.data.Golden("symbols", filename, func() ([]byte, error) {
expect := string(r.data.Golden(t, "symbols", filename, func() ([]byte, error) {
return []byte(got), nil
}))
if expect != got {
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/cmd/test/workspace_symbol.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (r *runner) runWorkspaceSymbols(t *testing.T, uri span.URI, matcher, query
sort.Strings(filtered)
got := r.Normalize(strings.Join(filtered, "\n") + "\n")

expect := string(r.data.Golden(fmt.Sprintf("workspace_symbol-%s-%s", strings.ToLower(string(matcher)), query), uri.Filename(), func() ([]byte, error) {
expect := string(r.data.Golden(t, fmt.Sprintf("workspace_symbol-%s-%s", strings.ToLower(string(matcher)), query), uri.Filename(), func() ([]byte, error) {
return []byte(got), nil
}))

Expand Down
26 changes: 13 additions & 13 deletions gopls/internal/lsp/lsp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ func (r *runner) foldingRanges(t *testing.T, prefix string, uri span.URI, ranges
continue
}
tag := fmt.Sprintf("%s-%d", prefix, i)
want := string(r.data.Golden(tag, uri.Filename(), func() ([]byte, error) {
want := string(r.data.Golden(t, tag, uri.Filename(), func() ([]byte, error) {
return []byte(got), nil
}))

Expand All @@ -314,7 +314,7 @@ func (r *runner) foldingRanges(t *testing.T, prefix string, uri span.URI, ranges
continue
}
tag := fmt.Sprintf("%s-%s-%d", prefix, kind, i)
want := string(r.data.Golden(tag, uri.Filename(), func() ([]byte, error) {
want := string(r.data.Golden(t, tag, uri.Filename(), func() ([]byte, error) {
return []byte(got), nil
}))

Expand Down Expand Up @@ -388,7 +388,7 @@ func foldRanges(m *protocol.ColumnMapper, contents string, ranges []protocol.Fol
func (r *runner) Format(t *testing.T, spn span.Span) {
uri := spn.URI()
filename := uri.Filename()
gofmted := string(r.data.Golden("gofmt", filename, func() ([]byte, error) {
gofmted := string(r.data.Golden(t, "gofmt", filename, func() ([]byte, error) {
cmd := exec.Command("gofmt", filename)
out, _ := cmd.Output() // ignore error, sometimes we have intentionally ungofmt-able files
return out, nil
Expand Down Expand Up @@ -475,7 +475,7 @@ func (r *runner) Import(t *testing.T, spn span.Span) {
}
got = res[uri]
}
want := string(r.data.Golden("goimports", filename, func() ([]byte, error) {
want := string(r.data.Golden(t, "goimports", filename, func() ([]byte, error) {
return []byte(got), nil
}))

Expand Down Expand Up @@ -574,7 +574,7 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []tests.S
}
}
for u, got := range res {
want := string(r.data.Golden("suggestedfix_"+tests.SpanName(spn), u.Filename(), func() ([]byte, error) {
want := string(r.data.Golden(t, "suggestedfix_"+tests.SpanName(spn), u.Filename(), func() ([]byte, error) {
return []byte(got), nil
}))
if want != got {
Expand Down Expand Up @@ -626,7 +626,7 @@ func (r *runner) FunctionExtraction(t *testing.T, start span.Span, end span.Span
}
res := <-r.editRecv
for u, got := range res {
want := string(r.data.Golden("functionextraction_"+tests.SpanName(spn), u.Filename(), func() ([]byte, error) {
want := string(r.data.Golden(t, "functionextraction_"+tests.SpanName(spn), u.Filename(), func() ([]byte, error) {
return []byte(got), nil
}))
if want != got {
Expand Down Expand Up @@ -678,7 +678,7 @@ func (r *runner) MethodExtraction(t *testing.T, start span.Span, end span.Span)
}
res := <-r.editRecv
for u, got := range res {
want := string(r.data.Golden("methodextraction_"+tests.SpanName(spn), u.Filename(), func() ([]byte, error) {
want := string(r.data.Golden(t, "methodextraction_"+tests.SpanName(spn), u.Filename(), func() ([]byte, error) {
return []byte(got), nil
}))
if want != got {
Expand Down Expand Up @@ -730,7 +730,7 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) {
if hover != nil {
didSomething = true
tag := fmt.Sprintf("%s-hoverdef", d.Name)
expectHover := string(r.data.Golden(tag, d.Src.URI().Filename(), func() ([]byte, error) {
expectHover := string(r.data.Golden(t, tag, d.Src.URI().Filename(), func() ([]byte, error) {
return []byte(hover.Contents.Value), nil
}))
got := tests.StripSubscripts(hover.Contents.Value)
Expand Down Expand Up @@ -982,7 +982,7 @@ func (r *runner) InlayHints(t *testing.T, spn span.Span) {
}
got := diff.ApplyEdits(string(m.Content), sedits)

withinlayHints := string(r.data.Golden("inlayHint", filename, func() ([]byte, error) {
withinlayHints := string(r.data.Golden(t, "inlayHint", filename, func() ([]byte, error) {
return []byte(got), nil
}))

Expand Down Expand Up @@ -1013,7 +1013,7 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) {
NewName: newText,
})
if err != nil {
renamed := string(r.data.Golden(tag, filename, func() ([]byte, error) {
renamed := string(r.data.Golden(t, tag, filename, func() ([]byte, error) {
return []byte(err.Error()), nil
}))
if err.Error() != renamed {
Expand Down Expand Up @@ -1043,7 +1043,7 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) {
val := res[uri]
got += val
}
want := string(r.data.Golden(tag, filename, func() ([]byte, error) {
want := string(r.data.Golden(t, tag, filename, func() ([]byte, error) {
return []byte(got), nil
}))
if want != got {
Expand Down Expand Up @@ -1191,7 +1191,7 @@ func (r *runner) callWorkspaceSymbols(t *testing.T, uri span.URI, query string,
t.Fatal(err)
}
got = filepath.ToSlash(tests.Normalize(got, r.normalizers))
want := string(r.data.Golden(fmt.Sprintf("workspace_symbol-%s-%s", strings.ToLower(string(matcher)), query), uri.Filename(), func() ([]byte, error) {
want := string(r.data.Golden(t, fmt.Sprintf("workspace_symbol-%s-%s", strings.ToLower(string(matcher)), query), uri.Filename(), func() ([]byte, error) {
return []byte(got), nil
}))
if diff := compare.Text(want, got); diff != "" {
Expand Down Expand Up @@ -1301,7 +1301,7 @@ func (r *runner) AddImport(t *testing.T, uri span.URI, expectedImport string) {
t.Fatal(err)
}
got := (<-r.editRecv)[uri]
want := r.data.Golden("addimport", uri.Filename(), func() ([]byte, error) {
want := r.data.Golden(t, "addimport", uri.Filename(), func() ([]byte, error) {
return []byte(got), nil
})
if want == nil {
Expand Down
22 changes: 11 additions & 11 deletions gopls/internal/lsp/source/source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ import (
"strings"
"testing"

"golang.org/x/tools/internal/bug"
"golang.org/x/tools/gopls/internal/lsp/cache"
"golang.org/x/tools/internal/diff"
"golang.org/x/tools/internal/fuzzy"
"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/lsp/tests/compare"
"golang.org/x/tools/internal/bug"
"golang.org/x/tools/internal/diff"
"golang.org/x/tools/internal/fuzzy"
"golang.org/x/tools/internal/span"
"golang.org/x/tools/internal/testenv"
)
Expand Down Expand Up @@ -366,7 +366,7 @@ func (r *runner) foldingRanges(t *testing.T, prefix string, uri span.URI, data s
continue
}
tag := fmt.Sprintf("%s-%d", prefix, i)
want := string(r.data.Golden(tag, uri.Filename(), func() ([]byte, error) {
want := string(r.data.Golden(t, tag, uri.Filename(), func() ([]byte, error) {
return []byte(got), nil
}))

Expand All @@ -393,7 +393,7 @@ func (r *runner) foldingRanges(t *testing.T, prefix string, uri span.URI, data s
continue
}
tag := fmt.Sprintf("%s-%s-%d", prefix, kind, i)
want := string(r.data.Golden(tag, uri.Filename(), func() ([]byte, error) {
want := string(r.data.Golden(t, tag, uri.Filename(), func() ([]byte, error) {
return []byte(got), nil
}))

Expand Down Expand Up @@ -463,7 +463,7 @@ func foldRanges(contents string, ranges []*source.FoldingRangeInfo) (string, err
}

func (r *runner) Format(t *testing.T, spn span.Span) {
gofmted := string(r.data.Golden("gofmt", spn.URI().Filename(), func() ([]byte, error) {
gofmted := string(r.data.Golden(t, "gofmt", spn.URI().Filename(), func() ([]byte, error) {
cmd := exec.Command("gofmt", spn.URI().Filename())
out, _ := cmd.Output() // ignore error, sometimes we have intentionally ungofmt-able files
return out, nil
Expand Down Expand Up @@ -523,7 +523,7 @@ func (r *runner) Import(t *testing.T, spn span.Span) {
t.Error(err)
}
got := diff.ApplyEdits(string(data), diffEdits)
want := string(r.data.Golden("goimports", spn.URI().Filename(), func() ([]byte, error) {
want := string(r.data.Golden(t, "goimports", spn.URI().Filename(), func() ([]byte, error) {
return []byte(got), nil
}))
if d := compare.Text(got, want); d != "" {
Expand Down Expand Up @@ -567,7 +567,7 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) {
if hover != "" {
didSomething = true
tag := fmt.Sprintf("%s-hoverdef", d.Name)
expectHover := string(r.data.Golden(tag, d.Src.URI().Filename(), func() ([]byte, error) {
expectHover := string(r.data.Golden(t, tag, d.Src.URI().Filename(), func() ([]byte, error) {
return []byte(hover), nil
}))
hover = tests.StripSubscripts(hover)
Expand Down Expand Up @@ -768,7 +768,7 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) {
}
changes, err := source.Rename(r.ctx, r.snapshot, fh, srcRng.Start, newText)
if err != nil {
renamed := string(r.data.Golden(tag, spn.URI().Filename(), func() ([]byte, error) {
renamed := string(r.data.Golden(t, tag, spn.URI().Filename(), func() ([]byte, error) {
return []byte(err.Error()), nil
}))
if err.Error() != renamed {
Expand Down Expand Up @@ -814,7 +814,7 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) {
got += val
}

renamed := string(r.data.Golden(tag, spn.URI().Filename(), func() ([]byte, error) {
renamed := string(r.data.Golden(t, tag, spn.URI().Filename(), func() ([]byte, error) {
return []byte(got), nil
}))

Expand Down Expand Up @@ -913,7 +913,7 @@ func (r *runner) callWorkspaceSymbols(t *testing.T, uri span.URI, query string,
t.Fatal(err)
}
got = filepath.ToSlash(tests.Normalize(got, r.normalizers))
want := string(r.data.Golden(fmt.Sprintf("workspace_symbol-%s-%s", strings.ToLower(string(matcher)), query), uri.Filename(), func() ([]byte, error) {
want := string(r.data.Golden(t, fmt.Sprintf("workspace_symbol-%s-%s", strings.ToLower(string(matcher)), query), uri.Filename(), func() ([]byte, error) {
return []byte(got), nil
}))
if d := compare.Text(want, got); d != "" {
Expand Down
14 changes: 7 additions & 7 deletions gopls/internal/lsp/tests/tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -1013,7 +1013,7 @@ func checkData(t *testing.T, data *Data) {
fmt.Fprintf(buf, "LinksCount = %v\n", linksCount)
fmt.Fprintf(buf, "ImplementationsCount = %v\n", len(data.Implementations))

want := string(data.Golden("summary", summaryFile, func() ([]byte, error) {
want := string(data.Golden(t, "summary", summaryFile, func() ([]byte, error) {
return buf.Bytes(), nil
}))
got := buf.String()
Expand All @@ -1039,19 +1039,19 @@ func (data *Data) Mapper(uri span.URI) (*protocol.ColumnMapper, error) {
return data.mappers[uri], nil
}

func (data *Data) Golden(tag string, target string, update func() ([]byte, error)) []byte {
data.t.Helper()
func (data *Data) Golden(t *testing.T, tag, target string, update func() ([]byte, error)) []byte {
t.Helper()
fragment, found := data.fragments[target]
if !found {
if filepath.IsAbs(target) {
data.t.Fatalf("invalid golden file fragment %v", target)
t.Fatalf("invalid golden file fragment %v", target)
}
fragment = target
}
golden := data.golden[fragment]
if golden == nil {
if !*UpdateGolden {
data.t.Fatalf("could not find golden file %v: %v", fragment, tag)
t.Fatalf("could not find golden file %v: %v", fragment, tag)
}
golden = &Golden{
Filename: filepath.Join(data.dir, fragment+goldenFileSuffix),
Expand All @@ -1077,14 +1077,14 @@ func (data *Data) Golden(tag string, target string, update func() ([]byte, error
}
contents, err := update()
if err != nil {
data.t.Fatalf("could not update golden file %v: %v", fragment, err)
t.Fatalf("could not update golden file %v: %v", fragment, err)
}
file.Data = append(contents, '\n') // add trailing \n for txtar
golden.Modified = true

}
if file == nil {
data.t.Fatalf("could not find golden contents %v: %v", fragment, tag)
t.Fatalf("could not find golden contents %v: %v", fragment, tag)
}
if len(file.Data) == 0 {
return file.Data
Expand Down

0 comments on commit 81a42f0

Please sign in to comment.