mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

internal/lsp: fix a race in the command line tests 

except the race was a symptom of a larger problem, so the fix
actually invovles cleaning up the way we run command line tests
totally to have common shared infrastructure, and also to clean up
the way we handle errors and paths into the temporary directory

Fixes: golang/go#35436
Change-Id: I4c5602607bb70e082056132baa3d4b0f8df6b13b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208269
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This commit is contained in:
Ian Cottrell 2019-11-21 19:58:38 -05:00
parent f774e2e2e5
commit 0ae87fff1b
13 changed files with 129 additions and 152 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
internal/lsp/cmd/cmd_test.go
View File

@ -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)
}

11
internal/lsp/cmd/test/check.go
View File

@ -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

148
internal/lsp/cmd/test/cmdtest.go
View File

@ -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})
}
}
}

10
internal/lsp/cmd/test/definition.go
View File

@ -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)
}

13
internal/lsp/cmd/test/folding_range.go
View File

@ -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
}))

14
internal/lsp/cmd/test/format.go
View File

@ -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)
}

8
internal/lsp/cmd/test/imports.go
View File

@ -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

8
internal/lsp/cmd/test/links.go
View File

@ -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 {

15
internal/lsp/cmd/test/references.go
View File

@ -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)
}

19
internal/lsp/cmd/test/rename.go
View File

@ -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)
}

9
internal/lsp/cmd/test/signature.go
View File

@ -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
}))

8
internal/lsp/cmd/test/suggested_fix.go
View File

@ -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
}))

12
internal/lsp/cmd/test/symbols.go
View File

@ -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
}))