diff --git a/internal/lsp/cmd/cmd_test.go b/internal/lsp/cmd/cmd_test.go index af04e8dc5a..0e417c68a9 100644 --- a/internal/lsp/cmd/cmd_test.go +++ b/internal/lsp/cmd/cmd_test.go @@ -17,7 +17,6 @@ import ( cmdtest "golang.org/x/tools/internal/lsp/cmd/test" "golang.org/x/tools/internal/lsp/tests" "golang.org/x/tools/internal/testenv" - "golang.org/x/tools/internal/tool" ) func TestMain(m *testing.M) { @@ -53,9 +52,8 @@ func TestDefinitionHelpExample(t *testing.T) { fmt.Sprintf("%v:%v:%v", thisFile, cmd.ExampleLine, cmd.ExampleColumn), fmt.Sprintf("%v:#%v", thisFile, cmd.ExampleOffset)} { args := append(baseArgs, query) - got := cmdtest.CaptureStdOut(t, func() { - _ = tool.Run(tests.Context(t), cmd.New("gopls-test", "", nil, nil), args) - }) + r := cmdtest.NewRunner(nil, nil, tests.Context(t), nil) + got, _ := r.NormalizeGoplsCmd(t, args...) if !expect.MatchString(got) { t.Errorf("test with %v\nexpected:\n%s\ngot:\n%s", args, expect, got) } diff --git a/internal/lsp/cmd/test/check.go b/internal/lsp/cmd/test/check.go index 98d390c253..024d987818 100644 --- a/internal/lsp/cmd/test/check.go +++ b/internal/lsp/cmd/test/check.go @@ -10,10 +10,8 @@ import ( "strings" "testing" - "golang.org/x/tools/internal/lsp/cmd" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/span" - "golang.org/x/tools/internal/tool" ) func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnostic) { @@ -21,11 +19,7 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnosti return } fname := uri.Filename() - args := []string{"-remote=internal", "check", fname} - app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Exported.Config.Env, r.options) - out := CaptureStdOut(t, func() { - _ = tool.Run(r.ctx, app, args) - }) + out, _ := r.RunGoplsCmd(t, "check", fname) // parse got into a collection of reports got := map[string]struct{}{} for _, l := range strings.Split(out, "\n") { @@ -48,13 +42,14 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnosti } l = fmt.Sprintf("%s: %s", s, strings.TrimSpace(bits[1])) } - got[l] = struct{}{} + got[r.NormalizePrefix(l)] = struct{}{} } for _, diag := range want { expect := fmt.Sprintf("%v:%v:%v: %v", uri.Filename(), diag.Range.Start.Line+1, diag.Range.Start.Character+1, diag.Message) if diag.Range.Start.Character == 0 { expect = fmt.Sprintf("%v:%v: %v", uri.Filename(), diag.Range.Start.Line+1, diag.Message) } + expect = r.NormalizePrefix(expect) // Skip the badimport test for now, until we do a better job with diagnostic ranges. if strings.Contains(uri.Filename(), "badimport") { continue diff --git a/internal/lsp/cmd/test/cmdtest.go b/internal/lsp/cmd/test/cmdtest.go index 3daf9b620c..7c8e21bdf7 100644 --- a/internal/lsp/cmd/test/cmdtest.go +++ b/internal/lsp/cmd/test/cmdtest.go @@ -8,6 +8,7 @@ package cmdtest import ( "bytes" "context" + "fmt" "io/ioutil" "os" "path/filepath" @@ -16,25 +17,54 @@ import ( "testing" "golang.org/x/tools/go/packages/packagestest" + "golang.org/x/tools/internal/lsp/cmd" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/lsp/tests" "golang.org/x/tools/internal/span" + "golang.org/x/tools/internal/tool" ) type runner struct { - exporter packagestest.Exporter - data *tests.Data - ctx context.Context - options func(*source.Options) + exporter packagestest.Exporter + data *tests.Data + ctx context.Context + options func(*source.Options) + normalizers []normalizer } -func NewRunner(exporter packagestest.Exporter, data *tests.Data, ctx context.Context, options func(*source.Options)) tests.Tests { - return &runner{ - exporter: exporter, - data: data, - ctx: ctx, - options: options, +type normalizer struct { + path string + slashed string + escaped string + fragment string +} + +func NewRunner(exporter packagestest.Exporter, data *tests.Data, ctx context.Context, options func(*source.Options)) *runner { + r := &runner{ + exporter: exporter, + data: data, + ctx: ctx, + options: options, + normalizers: make([]normalizer, 0, len(data.Exported.Modules)), } + // build the path normalizing patterns + for _, m := range data.Exported.Modules { + for fragment := range m.Files { + n := normalizer{ + path: data.Exported.File(m.Name, fragment), + fragment: fragment, + } + if n.slashed = filepath.ToSlash(n.path); n.slashed == n.path { + n.slashed = "" + } + quoted := strconv.Quote(n.path) + if n.escaped = quoted[1 : len(quoted)-1]; n.escaped == n.path { + n.escaped = "" + } + r.normalizers = append(r.normalizers, n) + } + } + return r } func (r *runner) Completion(t *testing.T, src span.Span, test tests.Completion, items tests.CompletionItems) { @@ -77,56 +107,92 @@ func (r *runner) Implementation(t *testing.T, spn span.Span, imp tests.Implement //TODO: add implements tests when it works } -func CaptureStdOut(t testing.TB, f func()) string { - r, out, err := os.Pipe() +func (r *runner) RunGoplsCmd(t testing.TB, args ...string) (string, string) { + rStdout, wStdout, err := os.Pipe() if err != nil { t.Fatal(err) } - old := os.Stdout + oldStdout := os.Stdout + rStderr, wStderr, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + oldStderr := os.Stderr defer func() { - os.Stdout = old - out.Close() - r.Close() + os.Stdout = oldStdout + os.Stderr = oldStderr + wStdout.Close() + rStdout.Close() + wStderr.Close() + rStderr.Close() }() - os.Stdout = out - f() - out.Close() - data, err := ioutil.ReadAll(r) + os.Stdout = wStdout + os.Stderr = wStderr + err = tool.Run(tests.Context(t), + cmd.New("gopls-test", r.data.Config.Dir, r.data.Exported.Config.Env, r.options), + append([]string{"-remote=internal"}, args...)) + if err != nil { + fmt.Fprint(os.Stderr, err) + } + wStdout.Close() + wStderr.Close() + stdout, err := ioutil.ReadAll(rStdout) if err != nil { t.Fatal(err) } - return string(data) + stderr, err := ioutil.ReadAll(rStderr) + if err != nil { + t.Fatal(err) + } + return string(stdout), string(stderr) } -// normalizePaths replaces all paths present in s with just the fragment portion +func (r *runner) NormalizeGoplsCmd(t testing.TB, args ...string) (string, string) { + stdout, stderr := r.RunGoplsCmd(t, args...) + return r.Normalize(stdout), r.Normalize(stderr) +} + +// NormalizePrefix normalizes a single path at the front of the input string. +func (r *runner) NormalizePrefix(s string) string { + for _, n := range r.normalizers { + if t := strings.TrimPrefix(s, n.path); t != s { + return n.fragment + t + } + if t := strings.TrimPrefix(s, n.slashed); t != s { + return n.fragment + t + } + if t := strings.TrimPrefix(s, n.escaped); t != s { + return n.fragment + t + } + } + return s +} + +// Normalize replaces all paths present in s with just the fragment portion // this is used to make golden files not depend on the temporary paths of the files -func normalizePaths(data *tests.Data, s string) string { +func (r *runner) Normalize(s string) string { type entry struct { path string index int fragment string } - match := make([]entry, 0, len(data.Exported.Modules)) + match := make([]entry, 0, len(r.normalizers)) // collect the initial state of all the matchers - for _, m := range data.Exported.Modules { - for fragment := range m.Files { - filename := data.Exported.File(m.Name, fragment) - index := strings.Index(s, filename) + for _, n := range r.normalizers { + index := strings.Index(s, n.path) + if index >= 0 { + match = append(match, entry{n.path, index, n.fragment}) + } + if n.slashed != "" { + index := strings.Index(s, n.slashed) if index >= 0 { - match = append(match, entry{filename, index, fragment}) + match = append(match, entry{n.slashed, index, n.fragment}) } - if slash := filepath.ToSlash(filename); slash != filename { - index := strings.Index(s, slash) - if index >= 0 { - match = append(match, entry{slash, index, fragment}) - } - } - quoted := strconv.Quote(filename) - if escaped := quoted[1 : len(quoted)-1]; escaped != filename { - index := strings.Index(s, escaped) - if index >= 0 { - match = append(match, entry{escaped, index, fragment}) - } + } + if n.escaped != "" { + index := strings.Index(s, n.escaped) + if index >= 0 { + match = append(match, entry{n.escaped, index, n.fragment}) } } } diff --git a/internal/lsp/cmd/test/definition.go b/internal/lsp/cmd/test/definition.go index 40da50be61..9cd1415de5 100644 --- a/internal/lsp/cmd/test/definition.go +++ b/internal/lsp/cmd/test/definition.go @@ -10,10 +10,8 @@ import ( "strings" "testing" - "golang.org/x/tools/internal/lsp/cmd" "golang.org/x/tools/internal/lsp/tests" "golang.org/x/tools/internal/span" - "golang.org/x/tools/internal/tool" ) const ( @@ -40,7 +38,7 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) { } d.Src = span.New(d.Src.URI(), span.NewPoint(0, 0, d.Src.Start().Offset()), span.Point{}) for _, mode := range godefModes { - args := []string{"-remote=internal", "query"} + args := []string{"query"} tag := d.Name + "-definition" if mode&jsonGoDef != 0 { tag += "-json" @@ -49,11 +47,7 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) { args = append(args, "definition") uri := d.Src.URI() args = append(args, fmt.Sprint(d.Src)) - got := CaptureStdOut(t, func() { - app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Exported.Config.Env, r.options) - _ = tool.Run(r.ctx, app, args) - }) - got = normalizePaths(r.data, got) + got, _ := r.NormalizeGoplsCmd(t, args...) if mode&jsonGoDef != 0 && runtime.GOOS == "windows" { got = strings.Replace(got, "file:///", "file://", -1) } diff --git a/internal/lsp/cmd/test/folding_range.go b/internal/lsp/cmd/test/folding_range.go index 30f977ed3c..6edb58ee0c 100644 --- a/internal/lsp/cmd/test/folding_range.go +++ b/internal/lsp/cmd/test/folding_range.go @@ -1,27 +1,16 @@ package cmdtest import ( - "fmt" "testing" - "golang.org/x/tools/internal/lsp/cmd" "golang.org/x/tools/internal/span" - "golang.org/x/tools/internal/tool" ) func (r *runner) FoldingRanges(t *testing.T, spn span.Span) { goldenTag := "foldingRange-cmd" uri := spn.URI() filename := uri.Filename() - - app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Config.Env, r.options) - got := CaptureStdOut(t, func() { - err := tool.Run(r.ctx, app, append([]string{"-remote=internal", "folding_ranges"}, filename)) - if err != nil { - fmt.Println(err) - } - }) - + got, _ := r.NormalizeGoplsCmd(t, "folding_ranges", filename) expect := string(r.data.Golden(goldenTag, filename, func() ([]byte, error) { return []byte(got), nil })) diff --git a/internal/lsp/cmd/test/format.go b/internal/lsp/cmd/test/format.go index 355f0d9cf7..f5b4631604 100644 --- a/internal/lsp/cmd/test/format.go +++ b/internal/lsp/cmd/test/format.go @@ -13,10 +13,8 @@ import ( "strings" "testing" - "golang.org/x/tools/internal/lsp/cmd" "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/testenv" - "golang.org/x/tools/internal/tool" ) func (r *runner) Format(t *testing.T, spn span.Span) { @@ -26,25 +24,19 @@ func (r *runner) Format(t *testing.T, spn span.Span) { expect := string(r.data.Golden(tag, filename, func() ([]byte, error) { cmd := exec.Command("gofmt", filename) contents, _ := cmd.Output() // ignore error, sometimes we have intentionally ungofmt-able files - contents = []byte(normalizePaths(r.data, fixFileHeader(string(contents)))) + contents = []byte(r.Normalize(fixFileHeader(string(contents)))) return contents, nil })) if expect == "" { //TODO: our error handling differs, for now just skip unformattable files t.Skip("Unformattable file") } - app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Config.Env, r.options) - got := CaptureStdOut(t, func() { - _ = tool.Run(r.ctx, app, append([]string{"-remote=internal", "format"}, filename)) - }) - got = normalizePaths(r.data, got) + got, _ := r.NormalizeGoplsCmd(t, "format", filename) if expect != got { t.Errorf("format failed for %s expected:\n%s\ngot:\n%s", filename, expect, got) } // now check we can build a valid unified diff - unified := CaptureStdOut(t, func() { - _ = tool.Run(r.ctx, app, append([]string{"-remote=internal", "format", "-d"}, filename)) - }) + unified, _ := r.NormalizeGoplsCmd(t, "format", "-d", filename) checkUnified(t, filename, expect, unified) } diff --git a/internal/lsp/cmd/test/imports.go b/internal/lsp/cmd/test/imports.go index c51897a51a..c5b1a50715 100644 --- a/internal/lsp/cmd/test/imports.go +++ b/internal/lsp/cmd/test/imports.go @@ -8,19 +8,13 @@ import ( "os/exec" "testing" - "golang.org/x/tools/internal/lsp/cmd" "golang.org/x/tools/internal/span" - "golang.org/x/tools/internal/tool" ) func (r *runner) Import(t *testing.T, spn span.Span) { uri := spn.URI() filename := uri.Filename() - args := []string{"-remote=internal", "imports", filename} - app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Exported.Config.Env, r.options) - got := CaptureStdOut(t, func() { - _ = tool.Run(r.ctx, app, args) - }) + got, _ := r.NormalizeGoplsCmd(t, "imports", filename) want := string(r.data.Golden("goimports", filename, func() ([]byte, error) { cmd := exec.Command("goimports", filename) out, _ := cmd.Output() // ignore error, sometimes we have intentionally ungofmt-able files diff --git a/internal/lsp/cmd/test/links.go b/internal/lsp/cmd/test/links.go index c74ea6a2c6..88df768323 100644 --- a/internal/lsp/cmd/test/links.go +++ b/internal/lsp/cmd/test/links.go @@ -8,11 +8,9 @@ import ( "encoding/json" "testing" - "golang.org/x/tools/internal/lsp/cmd" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/tests" "golang.org/x/tools/internal/span" - "golang.org/x/tools/internal/tool" ) func (r *runner) Link(t *testing.T, uri span.URI, wantLinks []tests.Link) { @@ -20,11 +18,7 @@ func (r *runner) Link(t *testing.T, uri span.URI, wantLinks []tests.Link) { if err != nil { t.Fatal(err) } - args := []string{"-remote=internal", "links", "-json", uri.Filename()} - app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Exported.Config.Env, r.options) - out := CaptureStdOut(t, func() { - _ = tool.Run(r.ctx, app, args) - }) + out, _ := r.NormalizeGoplsCmd(t, "links", "-json", uri.Filename()) var got []protocol.DocumentLink err = json.Unmarshal([]byte(out), &got) if err != nil { diff --git a/internal/lsp/cmd/test/references.go b/internal/lsp/cmd/test/references.go index 9cb5a66982..3039f53483 100644 --- a/internal/lsp/cmd/test/references.go +++ b/internal/lsp/cmd/test/references.go @@ -9,9 +9,6 @@ import ( "sort" "testing" - "golang.org/x/tools/internal/lsp/cmd" - "golang.org/x/tools/internal/tool" - "golang.org/x/tools/internal/span" ) @@ -25,20 +22,12 @@ func (r *runner) References(t *testing.T, spn span.Span, itemList []span.Span) { for _, i := range itemStrings { expect += i + "\n" } + expect = r.Normalize(expect) uri := spn.URI() filename := uri.Filename() target := filename + fmt.Sprintf(":%v:%v", spn.Start().Line(), spn.Start().Column()) - - app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Config.Env, r.options) - got := CaptureStdOut(t, func() { - err := tool.Run(r.ctx, app, append([]string{"-remote=internal", "references"}, target)) - if err != nil { - fmt.Println(spn.Start().Line()) - fmt.Println(err) - } - }) - + got, _ := r.NormalizeGoplsCmd(t, "references", target) if expect != got { t.Errorf("references failed for %s expected:\n%s\ngot:\n%s", target, expect, got) } diff --git a/internal/lsp/cmd/test/rename.go b/internal/lsp/cmd/test/rename.go index 9b227b9bd9..0fe2d1e182 100644 --- a/internal/lsp/cmd/test/rename.go +++ b/internal/lsp/cmd/test/rename.go @@ -8,33 +8,22 @@ import ( "fmt" "testing" - "golang.org/x/tools/internal/lsp/cmd" "golang.org/x/tools/internal/span" - "golang.org/x/tools/internal/tool" ) func (r *runner) Rename(t *testing.T, spn span.Span, newText string) { filename := spn.URI().Filename() goldenTag := newText + "-rename" - app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Config.Env, r.options) loc := fmt.Sprintf("%v", spn) - var err error - got := CaptureStdOut(t, func() { - err = tool.Run(r.ctx, app, []string{"-remote=internal", "rename", loc, newText}) - }) - if err != nil { - got = err.Error() - } - got = normalizePaths(r.data, got) + got, err := r.NormalizeGoplsCmd(t, "rename", loc, newText) + got += err expect := string(r.data.Golden(goldenTag, filename, func() ([]byte, error) { return []byte(got), nil })) if expect != got { - t.Errorf("rename failed with %v %v expected:\n%s\ngot:\n%s", loc, newText, expect, got) + t.Errorf("rename failed with %v %v\nexpected:\n%s\ngot:\n%s", loc, newText, expect, got) } // now check we can build a valid unified diff - unified := CaptureStdOut(t, func() { - _ = tool.Run(r.ctx, app, []string{"-remote=internal", "rename", "-d", loc, newText}) - }) + unified, _ := r.NormalizeGoplsCmd(t, "rename", "-d", loc, newText) checkUnified(t, filename, expect, unified) } diff --git a/internal/lsp/cmd/test/signature.go b/internal/lsp/cmd/test/signature.go index 5ba55bd86d..8f55aeaa35 100644 --- a/internal/lsp/cmd/test/signature.go +++ b/internal/lsp/cmd/test/signature.go @@ -8,9 +8,7 @@ import ( "fmt" "testing" - "golang.org/x/tools/internal/lsp/cmd" "golang.org/x/tools/internal/lsp/source" - "golang.org/x/tools/internal/tool" "golang.org/x/tools/internal/span" ) @@ -23,12 +21,7 @@ func (r *runner) SignatureHelp(t *testing.T, spn span.Span, expectedSignature *s uri := spn.URI() filename := uri.Filename() target := filename + fmt.Sprintf(":%v:%v", spn.Start().Line(), spn.Start().Column()) - - app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Config.Env, r.options) - got := CaptureStdOut(t, func() { - tool.Run(r.ctx, app, append([]string{"-remote=internal", "signature"}, target)) - }) - + got, _ := r.NormalizeGoplsCmd(t, "signature", target) expect := string(r.data.Golden(goldenTag, filename, func() ([]byte, error) { return []byte(got), nil })) diff --git a/internal/lsp/cmd/test/suggested_fix.go b/internal/lsp/cmd/test/suggested_fix.go index d41c590129..f88131b92a 100644 --- a/internal/lsp/cmd/test/suggested_fix.go +++ b/internal/lsp/cmd/test/suggested_fix.go @@ -7,19 +7,13 @@ package cmdtest import ( "testing" - "golang.org/x/tools/internal/lsp/cmd" "golang.org/x/tools/internal/span" - "golang.org/x/tools/internal/tool" ) func (r *runner) SuggestedFix(t *testing.T, spn span.Span) { uri := spn.URI() filename := uri.Filename() - args := []string{"-remote=internal", "fix", "-a", filename} - app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Exported.Config.Env, r.options) - got := CaptureStdOut(t, func() { - _ = tool.Run(r.ctx, app, args) - }) + got, _ := r.NormalizeGoplsCmd(t, "fix", "-a", filename) want := string(r.data.Golden("suggestedfix", filename, func() ([]byte, error) { return []byte(got), nil })) diff --git a/internal/lsp/cmd/test/symbols.go b/internal/lsp/cmd/test/symbols.go index 05f00abc59..055be03082 100644 --- a/internal/lsp/cmd/test/symbols.go +++ b/internal/lsp/cmd/test/symbols.go @@ -7,23 +7,13 @@ package cmdtest import ( "testing" - "fmt" - - "golang.org/x/tools/internal/lsp/cmd" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/span" - "golang.org/x/tools/internal/tool" ) func (r *runner) Symbols(t *testing.T, uri span.URI, expectedSymbols []protocol.DocumentSymbol) { filename := uri.Filename() - app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Config.Env, r.options) - got := CaptureStdOut(t, func() { - err := tool.Run(r.ctx, app, append([]string{"-remote=internal", "symbols"}, filename)) - if err != nil { - fmt.Println(err) - } - }) + got, _ := r.NormalizeGoplsCmd(t, "symbols", filename) expect := string(r.data.Golden("symbols", filename, func() ([]byte, error) { return []byte(got), nil }))