1
0
mirror of https://github.com/golang/go synced 2024-11-18 11:14:39 -07:00

go/packages/packagestest: do not walk packages and their test variants

Currently, where a package has a test variant, we end up walking the
non-_test.go files twice: once when walking the package itself, the
second time when walking the test variant. In the gopls tests, this
means we end up double-counting the number of symbols (via the @symbol
annotation) in a package.

Extend the existing expect_test.go to demonstrate the problem is fixed
(a test which fails when the fix is not in place)

Change-Id: Ifd68b3d86e24f19d3f8efc3af71494b7d28b0acb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228761
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This commit is contained in:
Paul Jolly 2020-04-17 22:07:45 +01:00 committed by Paul Jolly
parent b4d5569d26
commit cfa8b22178
5 changed files with 30 additions and 7 deletions

View File

@ -16,6 +16,7 @@ import (
"golang.org/x/tools/go/expect"
"golang.org/x/tools/go/packages"
"golang.org/x/tools/internal/packagesinternal"
"golang.org/x/tools/internal/span"
)
@ -151,7 +152,18 @@ func (e *Exported) getNotes() error {
if err != nil {
return fmt.Errorf("unable to load packages for directories %s: %v", dirs, err)
}
// We need to avoid walking packages and their test variants, otherwise we
// double-count
pkgsWithTestVariants := make(map[string]bool)
for _, pkg := range pkgs {
if forTest := packagesinternal.GetForTest(pkg); forTest != "" {
pkgsWithTestVariants[forTest] = true
}
}
for _, pkg := range pkgs {
if pkgsWithTestVariants[pkg.ID] {
continue
}
for _, filename := range pkg.GoFiles {
content, err := e.FileContents(filename)
if err != nil {

View File

@ -19,10 +19,10 @@ func TestExpect(t *testing.T) {
Files: packagestest.MustCopyFileTree("testdata"),
}})
defer exported.Cleanup()
count := 0
checkCount := 0
if err := exported.Expect(map[string]interface{}{
"check": func(src, target token.Position) {
count++
checkCount++
},
"boolArg": func(n *expect.Note, yes, no bool) {
if !yes {
@ -61,7 +61,12 @@ func TestExpect(t *testing.T) {
}); err != nil {
t.Fatal(err)
}
if count == 0 {
t.Fatalf("No tests were run")
// We expect to have walked the @check annotations in all .go files,
// including _test.go files (XTest or otherwise). But to have walked the
// non-_test.go files only once. Hence wantCheck = 3 (testdata/test.go) + 1
// (testdata/test_test.go) + 1 (testdata/x_test.go)
wantCheck := 5
if wantCheck != checkCount {
t.Fatalf("Expected @check count of %v; got %v", wantCheck, checkCount)
}
}

View File

@ -0,0 +1,3 @@
package fake1
type ATestType string //@check("ATestType","ATestType")

View File

@ -0,0 +1,3 @@
package fake1_test
type AnXTestType string //@check("AnXTestType","AnXTestType")

View File

@ -1,8 +1,8 @@
-- summary --
CodeLensCount = 2
CompletionsCount = 238
CompletionSnippetCount = 75
UnimportedCompletionsCount = 11
CompletionsCount = 237
CompletionSnippetCount = 74
UnimportedCompletionsCount = 6
DeepCompletionsCount = 5
FuzzyCompletionsCount = 8
RankedCompletionsCount = 120