Skip to content

Commit

Permalink
internal/lsp/tests: simplify comparison of markdown at go1.18
Browse files Browse the repository at this point in the history
In several places throughout the x/tools codebase, the internal/lsp/diff
package is used to provide clearer test output when comparing large,
multi-line strings. In some places, this is implemented via the
tests.Diff helper function, but in others it is not (presumably due to
import cycles).

Factor out this pattern into a diff.Pretty function, and add commentary.
Also remove the *testing.T argument, as diffs should never fail; opt to
panic instead. Also add a test.

Using this function, simplify the logic to comparing our 1.18 markdown
output with 1.19 golden content, by normalizing differences between the
two.

This is necessary as markdown content will change as a result of moving
internal/lsp to gopls/internal/lsp.

For golang/go#54509

Change-Id: Ie1fea1091bbbeb49e00c4efa7e02707cafa415cc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426776
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
  • Loading branch information
findleyr committed Aug 31, 2022
1 parent 3eb8a8f commit 4ccc73c
Show file tree
Hide file tree
Showing 19 changed files with 238 additions and 235 deletions.
12 changes: 6 additions & 6 deletions gopls/internal/regtest/codelens/codelens_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import (
"golang.org/x/tools/gopls/internal/hooks"
"golang.org/x/tools/internal/lsp/bug"
. "golang.org/x/tools/internal/lsp/regtest"
"golang.org/x/tools/internal/lsp/tests/compare"

"golang.org/x/tools/internal/lsp/command"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/tests"
"golang.org/x/tools/internal/testenv"
)

Expand Down Expand Up @@ -187,10 +187,10 @@ require golang.org/x/hello v1.2.3
}
env.Await(env.DoneWithChangeWatchedFiles())
if got := env.Editor.BufferText("a/go.mod"); got != wantGoModA {
t.Fatalf("a/go.mod upgrade failed:\n%s", tests.Diff(t, wantGoModA, got))
t.Fatalf("a/go.mod upgrade failed:\n%s", compare.Text(wantGoModA, got))
}
if got := env.Editor.BufferText("b/go.mod"); got != wantGoModB {
t.Fatalf("b/go.mod changed unexpectedly:\n%s", tests.Diff(t, wantGoModB, got))
t.Fatalf("b/go.mod changed unexpectedly:\n%s", compare.Text(wantGoModB, got))
}
})
})
Expand Down Expand Up @@ -220,10 +220,10 @@ require golang.org/x/hello v1.2.3
env.ApplyQuickFixes("a/go.mod", d.Diagnostics)
env.Await(env.DoneWithChangeWatchedFiles())
if got := env.Editor.BufferText("a/go.mod"); got != wantGoModA {
t.Fatalf("a/go.mod upgrade failed:\n%s", tests.Diff(t, wantGoModA, got))
t.Fatalf("a/go.mod upgrade failed:\n%s", compare.Text(wantGoModA, got))
}
if got := env.Editor.BufferText("b/go.mod"); got != wantGoModB {
t.Fatalf("b/go.mod changed unexpectedly:\n%s", tests.Diff(t, wantGoModB, got))
t.Fatalf("b/go.mod changed unexpectedly:\n%s", compare.Text(wantGoModB, got))
}
})
})
Expand Down Expand Up @@ -285,7 +285,7 @@ go 1.14
require golang.org/x/hello v1.0.0
`
if got != wantGoMod {
t.Fatalf("go.mod tidy failed:\n%s", tests.Diff(t, wantGoMod, got))
t.Fatalf("go.mod tidy failed:\n%s", compare.Text(wantGoMod, got))
}
})
}
Expand Down
4 changes: 2 additions & 2 deletions gopls/internal/regtest/misc/definition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import (

"golang.org/x/tools/internal/lsp/protocol"
. "golang.org/x/tools/internal/lsp/regtest"
"golang.org/x/tools/internal/lsp/tests/compare"
"golang.org/x/tools/internal/testenv"

"golang.org/x/tools/internal/lsp/fake"
"golang.org/x/tools/internal/lsp/tests"
)

const internalDefinition = `
Expand Down Expand Up @@ -133,7 +133,7 @@ func main() {
}
want := "```go\nfunc (error).Error() string\n```"
if content.Value != want {
t.Fatalf("hover failed:\n%s", tests.Diff(t, want, content.Value))
t.Fatalf("hover failed:\n%s", compare.Text(want, content.Value))
}
})
}
Expand Down
4 changes: 2 additions & 2 deletions gopls/internal/regtest/misc/fix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import (
"testing"

. "golang.org/x/tools/internal/lsp/regtest"
"golang.org/x/tools/internal/lsp/tests/compare"

"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/tests"
)

// A basic test for fillstruct, now that it uses a command.
Expand Down Expand Up @@ -56,7 +56,7 @@ func Foo() {
}
`
if got := env.Editor.BufferText("main.go"); got != want {
t.Fatalf("TestFillStruct failed:\n%s", tests.Diff(t, want, got))
t.Fatalf("TestFillStruct failed:\n%s", compare.Text(want, got))
}
})
}
Expand Down
19 changes: 9 additions & 10 deletions gopls/internal/regtest/misc/formatting_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ import (
"testing"

. "golang.org/x/tools/internal/lsp/regtest"

"golang.org/x/tools/internal/lsp/tests"
"golang.org/x/tools/internal/lsp/tests/compare"
)

const unformattedProgram = `
Expand All @@ -37,7 +36,7 @@ func TestFormatting(t *testing.T) {
got := env.Editor.BufferText("main.go")
want := env.ReadWorkspaceFile("main.go.golden")
if got != want {
t.Errorf("unexpected formatting result:\n%s", tests.Diff(t, want, got))
t.Errorf("unexpected formatting result:\n%s", compare.Text(want, got))
}
})
}
Expand All @@ -59,7 +58,7 @@ func f() {}
got := env.Editor.BufferText("a.go")
want := env.ReadWorkspaceFile("a.go.formatted")
if got != want {
t.Errorf("unexpected formatting result:\n%s", tests.Diff(t, want, got))
t.Errorf("unexpected formatting result:\n%s", compare.Text(want, got))
}
})
}
Expand All @@ -83,7 +82,7 @@ func f() { fmt.Println() }
got := env.Editor.BufferText("a.go")
want := env.ReadWorkspaceFile("a.go.imported")
if got != want {
t.Errorf("unexpected formatting result:\n%s", tests.Diff(t, want, got))
t.Errorf("unexpected formatting result:\n%s", compare.Text(want, got))
}
})
}
Expand All @@ -104,7 +103,7 @@ func f() {}
got := env.Editor.BufferText("a.go")
want := env.ReadWorkspaceFile("a.go.imported")
if got != want {
t.Errorf("unexpected formatting result:\n%s", tests.Diff(t, want, got))
t.Errorf("unexpected formatting result:\n%s", compare.Text(want, got))
}
})
}
Expand Down Expand Up @@ -150,7 +149,7 @@ func TestOrganizeImports(t *testing.T) {
got := env.Editor.BufferText("main.go")
want := env.ReadWorkspaceFile("main.go.organized")
if got != want {
t.Errorf("unexpected formatting result:\n%s", tests.Diff(t, want, got))
t.Errorf("unexpected formatting result:\n%s", compare.Text(want, got))
}
})
}
Expand All @@ -162,7 +161,7 @@ func TestFormattingOnSave(t *testing.T) {
got := env.Editor.BufferText("main.go")
want := env.ReadWorkspaceFile("main.go.formatted")
if got != want {
t.Errorf("unexpected formatting result:\n%s", tests.Diff(t, want, got))
t.Errorf("unexpected formatting result:\n%s", compare.Text(want, got))
}
})
}
Expand Down Expand Up @@ -262,7 +261,7 @@ func main() {
got := env.Editor.BufferText("main.go")
got = strings.ReplaceAll(got, "\r\n", "\n") // convert everything to LF for simplicity
if tt.want != got {
t.Errorf("unexpected content after save:\n%s", tests.Diff(t, tt.want, got))
t.Errorf("unexpected content after save:\n%s", compare.Text(tt.want, got))
}
})
})
Expand Down Expand Up @@ -361,7 +360,7 @@ const Bar = 42
got := env.Editor.BufferText("foo.go")
want := env.ReadWorkspaceFile("foo.go.formatted")
if got != want {
t.Errorf("unexpected formatting result:\n%s", tests.Diff(t, want, got))
t.Errorf("unexpected formatting result:\n%s", compare.Text(want, got))
}
})
}
4 changes: 2 additions & 2 deletions gopls/internal/regtest/misc/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"golang.org/x/tools/internal/lsp/command"
"golang.org/x/tools/internal/lsp/protocol"
. "golang.org/x/tools/internal/lsp/regtest"
"golang.org/x/tools/internal/lsp/tests"
"golang.org/x/tools/internal/lsp/tests/compare"
)

func TestAddImport(t *testing.T) {
Expand Down Expand Up @@ -51,7 +51,7 @@ func main() {
}, nil)
got := env.Editor.BufferText("main.go")
if got != want {
t.Fatalf("gopls.add_import failed\n%s", tests.Diff(t, want, got))
t.Fatalf("gopls.add_import failed\n%s", compare.Text(want, got))
}
})
}
Expand Down
30 changes: 15 additions & 15 deletions gopls/internal/regtest/modfile/modfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import (
"golang.org/x/tools/gopls/internal/hooks"
"golang.org/x/tools/internal/lsp/bug"
. "golang.org/x/tools/internal/lsp/regtest"
"golang.org/x/tools/internal/lsp/tests/compare"

"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/tests"
"golang.org/x/tools/internal/testenv"
)

Expand Down Expand Up @@ -101,7 +101,7 @@ func main() {
),
)
if got := env.ReadWorkspaceFile("a/go.mod"); got != goModContent {
t.Fatalf("go.mod changed on disk:\n%s", tests.Diff(t, goModContent, got))
t.Fatalf("go.mod changed on disk:\n%s", compare.Text(goModContent, got))
}
// Save the buffer, which will format and organize imports.
// Confirm that the go.mod file still does not change.
Expand All @@ -110,7 +110,7 @@ func main() {
env.DiagnosticAtRegexp("a/main.go", "\"example.com/blah\""),
)
if got := env.ReadWorkspaceFile("a/go.mod"); got != goModContent {
t.Fatalf("go.mod changed on disk:\n%s", tests.Diff(t, goModContent, got))
t.Fatalf("go.mod changed on disk:\n%s", compare.Text(goModContent, got))
}
})
})
Expand Down Expand Up @@ -154,7 +154,7 @@ func main() {
),
)
if got := env.ReadWorkspaceFile("a/go.mod"); got != goModContent {
t.Fatalf("go.mod changed on disk:\n%s", tests.Diff(t, goModContent, got))
t.Fatalf("go.mod changed on disk:\n%s", compare.Text(goModContent, got))
}
})
})
Expand Down Expand Up @@ -206,7 +206,7 @@ require example.com v1.2.3
}
env.ApplyQuickFixes("a/main.go", []protocol.Diagnostic{goGetDiag})
if got := env.ReadWorkspaceFile("a/go.mod"); got != want {
t.Fatalf("unexpected go.mod content:\n%s", tests.Diff(t, want, got))
t.Fatalf("unexpected go.mod content:\n%s", compare.Text(want, got))
}
})
}
Expand Down Expand Up @@ -256,7 +256,7 @@ require random.org v1.2.3
}
env.ApplyQuickFixes("a/main.go", []protocol.Diagnostic{randomDiag})
if got := env.ReadWorkspaceFile("a/go.mod"); got != want {
t.Fatalf("unexpected go.mod content:\n%s", tests.Diff(t, want, got))
t.Fatalf("unexpected go.mod content:\n%s", compare.Text(want, got))
}
})
}
Expand Down Expand Up @@ -312,7 +312,7 @@ require random.org v1.2.3
}
env.ApplyQuickFixes("a/main.go", []protocol.Diagnostic{randomDiag})
if got := env.ReadWorkspaceFile("a/go.mod"); got != want {
t.Fatalf("unexpected go.mod content:\n%s", tests.Diff(t, want, got))
t.Fatalf("unexpected go.mod content:\n%s", compare.Text(want, got))
}
})
}
Expand Down Expand Up @@ -359,7 +359,7 @@ require example.com v1.2.3
)
env.ApplyQuickFixes("a/go.mod", d.Diagnostics)
if got := env.Editor.BufferText("a/go.mod"); got != want {
t.Fatalf("unexpected go.mod content:\n%s", tests.Diff(t, want, got))
t.Fatalf("unexpected go.mod content:\n%s", compare.Text(want, got))
}
})
}
Expand Down Expand Up @@ -404,7 +404,7 @@ go 1.14
)
env.ApplyQuickFixes("a/go.mod", d.Diagnostics)
if got := env.Editor.BufferText("a/go.mod"); got != want {
t.Fatalf("unexpected go.mod content:\n%s", tests.Diff(t, want, got))
t.Fatalf("unexpected go.mod content:\n%s", compare.Text(want, got))
}
})
}
Expand Down Expand Up @@ -476,7 +476,7 @@ require (
)
`
if got := env.ReadWorkspaceFile("a/go.mod"); got != want {
t.Fatalf("TestNewDepWithUnusedDep failed:\n%s", tests.Diff(t, want, got))
t.Fatalf("TestNewDepWithUnusedDep failed:\n%s", compare.Text(want, got))
}
})
}
Expand Down Expand Up @@ -585,7 +585,7 @@ require (
env.SaveBuffer("a/go.mod")
env.Await(EmptyDiagnostics("a/main.go"))
if got := env.Editor.BufferText("a/go.mod"); got != want {
t.Fatalf("suggested fixes failed:\n%s", tests.Diff(t, want, got))
t.Fatalf("suggested fixes failed:\n%s", compare.Text(want, got))
}
})
}
Expand Down Expand Up @@ -773,7 +773,7 @@ func main() {
)
got := env.ReadWorkspaceFile("go.mod")
if got != original {
t.Fatalf("go.mod file modified:\n%s", tests.Diff(t, original, got))
t.Fatalf("go.mod file modified:\n%s", compare.Text(original, got))
}
env.RunGoCommand("get", "example.com/blah@v1.2.3")
env.RunGoCommand("mod", "tidy")
Expand Down Expand Up @@ -1026,7 +1026,7 @@ go 1.12
`
env.ApplyQuickFixes("go.mod", d.Diagnostics)
if got := env.Editor.BufferText("go.mod"); got != want {
t.Fatalf("unexpected content in go.mod:\n%s", tests.Diff(t, want, got))
t.Fatalf("unexpected content in go.mod:\n%s", compare.Text(want, got))
}
})
})
Expand Down Expand Up @@ -1079,7 +1079,7 @@ require random.com v1.2.3
}
env.ApplyQuickFixes("go.mod", diagnostics)
if got := env.Editor.BufferText("go.mod"); got != want {
t.Fatalf("unexpected content in go.mod:\n%s", tests.Diff(t, want, got))
t.Fatalf("unexpected content in go.mod:\n%s", compare.Text(want, got))
}
})
})
Expand Down Expand Up @@ -1125,7 +1125,7 @@ func main() {
example.com v1.2.3/go.mod h1:Y2Rc5rVWjWur0h3pd9aEvK5Pof8YKDANh9gHA2Maujo=
`
if got := env.ReadWorkspaceFile("go.sum"); got != want {
t.Fatalf("unexpected go.sum contents:\n%s", tests.Diff(t, want, got))
t.Fatalf("unexpected go.sum contents:\n%s", compare.Text(want, got))
}
})
}
Expand Down
3 changes: 2 additions & 1 deletion internal/lsp/cmd/test/suggested_fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"testing"

"golang.org/x/tools/internal/lsp/tests"
"golang.org/x/tools/internal/lsp/tests/compare"
"golang.org/x/tools/internal/span"
)

Expand All @@ -32,6 +33,6 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, suggestedFixes []test
return []byte(got), nil
}))
if want != got {
t.Errorf("suggested fixes failed for %s:\n%s", filename, tests.Diff(t, want, got))
t.Errorf("suggested fixes failed for %s:\n%s", filename, compare.Text(want, got))
}
}
3 changes: 2 additions & 1 deletion internal/lsp/cmd/test/workspace_symbol.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/lsp/tests"
"golang.org/x/tools/internal/lsp/tests/compare"
"golang.org/x/tools/internal/span"
)

Expand Down Expand Up @@ -48,6 +49,6 @@ func (r *runner) runWorkspaceSymbols(t *testing.T, uri span.URI, matcher, query
}))

if expect != got {
t.Errorf("workspace_symbol failed for %s:\n%s", query, tests.Diff(t, expect, got))
t.Errorf("workspace_symbol failed for %s:\n%s", query, compare.Text(expect, got))
}
}
Loading

0 comments on commit 4ccc73c

Please sign in to comment.