diff options
author | Paul Jolly <paul@myitcv.io> | 2020-04-17 22:07:45 +0100 |
---|---|---|
committer | Paul Jolly <paul@myitcv.org.uk> | 2020-04-21 04:27:24 +0000 |
commit | cfa8b22178d2faeacea202c63787cc6193a51a8c (patch) | |
tree | e21e00ba57cd16b263da81a8f4b45728dbdc960e | |
parent | b4d5569d26345c4dfedd54a002906aad0bc709c9 (diff) | |
download | golang-x-tools-cfa8b22178d2faeacea202c63787cc6193a51a8c.tar.gz |
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>
-rw-r--r-- | go/packages/packagestest/expect.go | 12 | ||||
-rw-r--r-- | go/packages/packagestest/expect_test.go | 13 | ||||
-rw-r--r-- | go/packages/packagestest/testdata/test_test.go | 3 | ||||
-rw-r--r-- | go/packages/packagestest/testdata/x_test.go | 3 | ||||
-rw-r--r-- | internal/lsp/testdata/lsp/summary.txt.golden | 6 |
5 files changed, 30 insertions, 7 deletions
diff --git a/go/packages/packagestest/expect.go b/go/packages/packagestest/expect.go index ddbec4448..e8c684738 100644 --- a/go/packages/packagestest/expect.go +++ b/go/packages/packagestest/expect.go @@ -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 { diff --git a/go/packages/packagestest/expect_test.go b/go/packages/packagestest/expect_test.go index 527c8d732..098bdbe41 100644 --- a/go/packages/packagestest/expect_test.go +++ b/go/packages/packagestest/expect_test.go @@ -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) } } diff --git a/go/packages/packagestest/testdata/test_test.go b/go/packages/packagestest/testdata/test_test.go new file mode 100644 index 000000000..18b20805f --- /dev/null +++ b/go/packages/packagestest/testdata/test_test.go @@ -0,0 +1,3 @@ +package fake1 + +type ATestType string //@check("ATestType","ATestType") diff --git a/go/packages/packagestest/testdata/x_test.go b/go/packages/packagestest/testdata/x_test.go new file mode 100644 index 000000000..c8c4fa253 --- /dev/null +++ b/go/packages/packagestest/testdata/x_test.go @@ -0,0 +1,3 @@ +package fake1_test + +type AnXTestType string //@check("AnXTestType","AnXTestType") diff --git a/internal/lsp/testdata/lsp/summary.txt.golden b/internal/lsp/testdata/lsp/summary.txt.golden index 853af9628..5b902ce28 100644 --- a/internal/lsp/testdata/lsp/summary.txt.golden +++ b/internal/lsp/testdata/lsp/summary.txt.golden @@ -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 |