From 0268d3dd0724eb8b65d4e71a1285076f6efd643d Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Mon, 25 Mar 2019 18:30:55 -0400 Subject: [PATCH] internal/lsp: use main testdata folder This upgrades the current gopls query definition tests to use the main testdata folder. This considerably increases the coverage and also sets us up to better test the other command line features as we add them. Change-Id: If722f3f6d0270104000f1451d20851daf0757874 Reviewed-on: https://go-review.googlesource.com/c/tools/+/169159 Run-TryBot: Ian Cottrell TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/cmd/cmd_test.go | 214 ++++++++++++++++++ internal/lsp/cmd/definition_test.go | 168 ++++++++------ .../testdata/a/a.go => testdata/godef/a/d.go} | 0 .../testdata/b/b.go => testdata/godef/b/e.go} | 10 +- 4 files changed, 315 insertions(+), 77 deletions(-) create mode 100644 internal/lsp/cmd/cmd_test.go rename internal/lsp/{cmd/testdata/a/a.go => testdata/godef/a/d.go} (100%) rename internal/lsp/{cmd/testdata/b/b.go => testdata/godef/b/e.go} (74%) diff --git a/internal/lsp/cmd/cmd_test.go b/internal/lsp/cmd/cmd_test.go new file mode 100644 index 00000000..2ffb0c21 --- /dev/null +++ b/internal/lsp/cmd/cmd_test.go @@ -0,0 +1,214 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package cmd_test + +import ( + "context" + "go/ast" + "go/parser" + "go/token" + "io/ioutil" + "os" + "strings" + "testing" + + "golang.org/x/tools/go/packages/packagestest" + "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/span" +) + +// We hardcode the expected number of test cases to ensure that all tests +// are being executed. If a test is added, this number must be changed. +const ( + expectedCompletionsCount = 64 + expectedDiagnosticsCount = 16 + expectedFormatCount = 4 +) + +func TestCommandLine(t *testing.T) { + packagestest.TestAll(t, testCommandLine) +} + +func testCommandLine(t *testing.T, exporter packagestest.Exporter) { + const dir = "../testdata" + + files := packagestest.MustCopyFileTree(dir) + for fragment, operation := range files { + if trimmed := strings.TrimSuffix(fragment, ".in"); trimmed != fragment { + delete(files, fragment) + files[trimmed] = operation + } + } + modules := []packagestest.Module{ + { + Name: "golang.org/x/tools/internal/lsp", + Files: files, + }, + } + exported := packagestest.Export(t, exporter, modules) + defer exported.Cleanup() + + // Merge the exported.Config with the view.Config. + cfg := *exported.Config + cfg.Fset = token.NewFileSet() + cfg.Context = context.Background() + cfg.ParseFile = func(fset *token.FileSet, filename string, src []byte) (*ast.File, error) { + return parser.ParseFile(fset, filename, src, parser.AllErrors|parser.ParseComments) + } + + // Do a first pass to collect special markers for completion. + if err := exported.Expect(map[string]interface{}{ + "item": func(name string, r packagestest.Range, _, _ string) { + exported.Mark(name, r) + }, + }); err != nil { + t.Fatal(err) + } + + expectedDiagnostics := make(diagnostics) + completionItems := make(completionItems) + expectedCompletions := make(completions) + expectedFormat := make(formats) + expectedDefinitions := make(definitions) + expectedTypeDefinitions := make(definitions) + + // Collect any data that needs to be used by subsequent tests. + if err := exported.Expect(map[string]interface{}{ + "diag": expectedDiagnostics.collect, + "item": completionItems.collect, + "complete": expectedCompletions.collect, + "format": expectedFormat.collect, + "godef": expectedDefinitions.godef, + "definition": expectedDefinitions.definition, + "typdef": expectedTypeDefinitions.typdef, + }); err != nil { + t.Fatal(err) + } + + t.Run("Completion", func(t *testing.T) { + t.Helper() + expectedCompletions.test(t, exported, completionItems) + }) + + t.Run("Diagnostics", func(t *testing.T) { + t.Helper() + expectedDiagnostics.test(t, exported) + }) + + t.Run("Format", func(t *testing.T) { + t.Helper() + expectedFormat.test(t, exported) + }) + + t.Run("Definitions", func(t *testing.T) { + t.Helper() + expectedDefinitions.testDefinitions(t, exported) + }) + + t.Run("TypeDefinitions", func(t *testing.T) { + t.Helper() + expectedTypeDefinitions.testTypeDefinitions(t, exported) + }) +} + +type diagnostics map[span.Span][]source.Diagnostic +type completionItems map[span.Range]*source.CompletionItem +type completions map[span.Span][]span.Span +type formats map[span.URI]span.Span + +func (l diagnostics) collect(spn span.Span, msgSource, msg string) { + l[spn] = append(l[spn], source.Diagnostic{ + Span: spn, + Message: msg, + Source: msgSource, + Severity: source.SeverityError, + }) +} + +func (l diagnostics) test(t *testing.T, e *packagestest.Exported) { + count := 0 + for _, want := range l { + if len(want) == 1 && want[0].Message == "" { + continue + } + count += len(want) + } + if count != expectedDiagnosticsCount { + t.Errorf("got %v diagnostics expected %v", count, expectedDiagnosticsCount) + } + //TODO: add command line diagnostics tests when it works +} + +func (l completionItems) collect(spn span.Range, label, detail, kind string) { + var k source.CompletionItemKind + switch kind { + case "struct": + k = source.StructCompletionItem + case "func": + k = source.FunctionCompletionItem + case "var": + k = source.VariableCompletionItem + case "type": + k = source.TypeCompletionItem + case "field": + k = source.FieldCompletionItem + case "interface": + k = source.InterfaceCompletionItem + case "const": + k = source.ConstantCompletionItem + case "method": + k = source.MethodCompletionItem + case "package": + k = source.PackageCompletionItem + } + l[spn] = &source.CompletionItem{ + Label: label, + Detail: detail, + Kind: k, + } +} + +func (l completions) collect(src span.Span, expected []span.Span) { + l[src] = expected +} + +func (l completions) test(t *testing.T, e *packagestest.Exported, items completionItems) { + if len(l) != expectedCompletionsCount { + t.Errorf("got %v completions expected %v", len(l), expectedCompletionsCount) + } + //TODO: add command line completions tests when it works +} + +func (l formats) collect(src span.Span) { + l[src.URI()] = src +} + +func (l formats) test(t *testing.T, e *packagestest.Exported) { + if len(l) != expectedFormatCount { + t.Errorf("got %v formats expected %v", len(l), expectedFormatCount) + } + //TODO: add command line formatting tests when it works +} + +func captureStdOut(t testing.TB, f func()) string { + r, out, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + old := os.Stdout + defer func() { + os.Stdout = old + out.Close() + r.Close() + }() + os.Stdout = out + f() + out.Close() + data, err := ioutil.ReadAll(r) + if err != nil { + t.Fatal(err) + } + return strings.TrimSpace(string(data)) +} diff --git a/internal/lsp/cmd/definition_test.go b/internal/lsp/cmd/definition_test.go index e9e9cb00..0c107376 100644 --- a/internal/lsp/cmd/definition_test.go +++ b/internal/lsp/cmd/definition_test.go @@ -8,7 +8,6 @@ import ( "context" "flag" "fmt" - "io/ioutil" "os" "os/exec" "path/filepath" @@ -24,6 +23,20 @@ import ( "golang.org/x/tools/internal/tool" ) +const ( + expectedDefinitionsCount = 25 + expectedTypeDefinitionsCount = 2 +) + +type definition struct { + src span.Span + flags string + def span.Span + match string +} + +type definitions map[span.Span]definition + var verifyGuru = flag.Bool("verify-guru", false, "Check that the guru compatability matches") func TestDefinitionHelpExample(t *testing.T) { @@ -51,77 +64,108 @@ func TestDefinitionHelpExample(t *testing.T) { } } -func TestDefinition(t *testing.T) { - exported := packagestest.Export(t, packagestest.GOPATH, []packagestest.Module{{ - Name: "golang.org/fake", - Files: packagestest.MustCopyFileTree("testdata"), - }}) - defer exported.Cleanup() - count := 0 - if err := exported.Expect(map[string]interface{}{ - "definition": func(src span.Span, flags string, def span.Span, match string) { - count++ - args := []string{"query"} - if flags != "" { - args = append(args, strings.Split(flags, " ")...) +func (l definitions) godef(src, def span.Span) { + l[src] = definition{ + src: src, + def: def, + } +} + +func (l definitions) typdef(src, def span.Span) { + l[src] = definition{ + src: src, + def: def, + } +} + +func (l definitions) definition(src span.Span, flags string, def span.Span, match string) { + l[src] = definition{ + src: src, + flags: flags, + def: def, + match: match, + } +} + +func (l definitions) testDefinitions(t *testing.T, e *packagestest.Exported) { + if len(l) != expectedDefinitionsCount { + t.Errorf("got %v definitions expected %v", len(l), expectedDefinitionsCount) + } + for _, d := range l { + args := []string{"query"} + if d.flags != "" { + args = append(args, strings.Split(d.flags, " ")...) + } + args = append(args, "definition") + src := span.New(d.src.URI(), span.NewPoint(0, 0, d.src.Start().Offset()), span.Point{}) + args = append(args, fmt.Sprint(src)) + app := &cmd.Application{} + app.Config = *e.Config + got := captureStdOut(t, func() { + tool.Main(context.Background(), app, args) + }) + if d.match == "" { + expect := fmt.Sprint(d.def) + if !strings.HasPrefix(got, expect) { + t.Errorf("definition %v\nexpected:\n%s\ngot:\n%s", args, expect, got) } - args = append(args, "definition") - args = append(args, fmt.Sprint(src)) - app := &cmd.Application{} - app.Config = *exported.Config - got := captureStdOut(t, func() { - tool.Main(context.Background(), app, args) - }) - expect := os.Expand(match, func(name string) string { + } else { + expect := os.Expand(d.match, func(name string) string { switch name { case "file": - fname, _ := def.URI().Filename() + fname, _ := d.def.URI().Filename() return fname case "efile": - fname, _ := def.URI().Filename() + fname, _ := d.def.URI().Filename() qfile := strconv.Quote(fname) return qfile[1 : len(qfile)-1] case "euri": - quri := strconv.Quote(string(def.URI())) + quri := strconv.Quote(string(d.def.URI())) return quri[1 : len(quri)-1] case "line": - return fmt.Sprint(def.Start().Line()) + return fmt.Sprint(d.def.Start().Line()) case "col": - return fmt.Sprint(def.Start().Column()) + return fmt.Sprint(d.def.Start().Column()) case "offset": - return fmt.Sprint(def.Start().Offset()) + return fmt.Sprint(d.def.Start().Offset()) case "eline": - return fmt.Sprint(def.End().Line()) + return fmt.Sprint(d.def.End().Line()) case "ecol": - return fmt.Sprint(def.End().Column()) + return fmt.Sprint(d.def.End().Column()) case "eoffset": - return fmt.Sprint(def.End().Offset()) + return fmt.Sprint(d.def.End().Offset()) default: return name } }) + if expect != got { + t.Errorf("definition %v\nexpected:\n%s\ngot:\n%s", args, expect, got) + } if *verifyGuru { + moduleMode := e.File(e.Modules[0].Name, "go.mod") != "" var guruArgs []string runGuru := false - for _, arg := range args { - switch { - case arg == "query": - // just ignore this one - case arg == "-json": - guruArgs = append(guruArgs, arg) - case arg == "-emulate=guru": - // if we don't see this one we should not run guru - runGuru = true - case strings.HasPrefix(arg, "-"): - // unknown flag, ignore it - break - default: - guruArgs = append(guruArgs, arg) + if !moduleMode { + for _, arg := range args { + switch { + case arg == "query": + // just ignore this one + case arg == "-json": + guruArgs = append(guruArgs, arg) + case arg == "-emulate=guru": + // if we don't see this one we should not run guru + runGuru = true + case strings.HasPrefix(arg, "-"): + // unknown flag, ignore it + break + default: + guruArgs = append(guruArgs, arg) + } } } if runGuru { cmd := exec.Command("guru", guruArgs...) - cmd.Env = exported.Config.Env + cmd.Env = e.Config.Env out, err := cmd.CombinedOutput() if err != nil { t.Errorf("Could not run guru %v: %v\n%s", guruArgs, err, out) @@ -133,35 +177,13 @@ func TestDefinition(t *testing.T) { } } } - if expect != got { - t.Errorf("definition %v\nexpected:\n%s\ngot:\n%s", args, expect, got) - } - }, - }); err != nil { - t.Fatal(err) - } - if count == 0 { - t.Fatalf("No tests were run") + } } } -func captureStdOut(t testing.TB, f func()) string { - r, out, err := os.Pipe() - if err != nil { - t.Fatal(err) +func (l definitions) testTypeDefinitions(t *testing.T, e *packagestest.Exported) { + if len(l) != expectedTypeDefinitionsCount { + t.Errorf("got %v definitions expected %v", len(l), expectedTypeDefinitionsCount) } - old := os.Stdout - defer func() { - os.Stdout = old - out.Close() - r.Close() - }() - os.Stdout = out - f() - out.Close() - data, err := ioutil.ReadAll(r) - if err != nil { - t.Fatal(err) - } - return strings.TrimSpace(string(data)) + //TODO: add command line type definition tests when it works } diff --git a/internal/lsp/cmd/testdata/a/a.go b/internal/lsp/testdata/godef/a/d.go similarity index 100% rename from internal/lsp/cmd/testdata/a/a.go rename to internal/lsp/testdata/godef/a/d.go diff --git a/internal/lsp/cmd/testdata/b/b.go b/internal/lsp/testdata/godef/b/e.go similarity index 74% rename from internal/lsp/cmd/testdata/b/b.go rename to internal/lsp/testdata/godef/b/e.go index 9f1f975f..7e0c4175 100644 --- a/internal/lsp/cmd/testdata/b/b.go +++ b/internal/lsp/testdata/godef/b/e.go @@ -1,7 +1,9 @@ package b import ( - "golang.org/fake/a" + "fmt" + + "golang.org/x/tools/internal/lsp/godef/a" ) func useThings() { @@ -13,14 +15,14 @@ func useThings() { /*@ definition(bStructType, "", Thing, "$file:$line:$col-$ecol: defined here as type a.Thing struct{Member string}") -definition(bStructType, "-emulate=guru", Thing, "$file:$line:$col: defined here as type golang.org/fake/a.Thing") +definition(bStructType, "-emulate=guru", Thing, "$file:$line:$col: defined here as type golang.org/x/tools/internal/lsp/godef/a.Thing") definition(bMember, "", Member, "$file:$line:$col-$ecol: defined here as field Member string") definition(bMember, "-emulate=guru", Member, "$file:$line:$col: defined here as field Member string") definition(bVar, "", Other, "$file:$line:$col-$ecol: defined here as var a.Other a.Thing") -definition(bVar, "-emulate=guru", Other, "$file:$line:$col: defined here as var golang.org/fake/a.Other") +definition(bVar, "-emulate=guru", Other, "$file:$line:$col: defined here as var golang.org/x/tools/internal/lsp/godef/a.Other") definition(bFunc, "", Things, "$file:$line:$col-$ecol: defined here as func a.Things(val []string) []a.Thing") -definition(bFunc, "-emulate=guru", Things, "$file:$line:$col: defined here as func golang.org/fake/a.Things(val []string) []golang.org/fake/a.Thing") +definition(bFunc, "-emulate=guru", Things, "$file:$line:$col: defined here as func golang.org/x/tools/internal/lsp/godef/a.Things(val []string) []golang.org/x/tools/internal/lsp/godef/a.Thing") */