From 78b3e717651b2f0170900ca0ac1080c331822c22 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Wed, 1 Aug 2018 21:38:32 -0400 Subject: [PATCH] go/packages: include test deps in golist fallback In Go 1.10 and earlier, the Deps field returned by go list does not include the dependencies of any test files. To get the dependencies of the test packages, we need to run go list another time (for a total of 3 calls to go list), to get the dependencies of the packages the tests import. Fixes golang/go#26753 Change-Id: Id3b424ac935e5ed459b4d4a55630715171bb9710 Reviewed-on: https://go-review.googlesource.com/127457 Reviewed-by: Ian Cottrell Reviewed-by: Michael Matloob Run-TryBot: Michael Matloob TryBot-Result: Gobot Gobot --- go/packages/golist/golist_fallback.go | 38 ++++++++++++++++++++- go/packages/packages_test.go | 49 ++++++++++++++++++++++----- 2 files changed, 78 insertions(+), 9 deletions(-) diff --git a/go/packages/golist/golist_fallback.go b/go/packages/golist/golist_fallback.go index dda2b7b7bbe..a582434fe4a 100644 --- a/go/packages/golist/golist_fallback.go +++ b/go/packages/golist/golist_fallback.go @@ -92,12 +92,18 @@ func golistPackagesFallback(ctx context.Context, cfg *raw.Config, words ...strin if isRoot { roots = append(roots, xtestID) } + for i, imp := range p.XTestImports { + if imp == p.ImportPath { + p.XTestImports[i] = testID + break + } + } result = append(result, &raw.Package{ ID: xtestID, Name: p.Name + "_test", GoFiles: absJoin(p.Dir, p.XTestGoFiles), PkgPath: pkgpath, - Imports: importMap(append(p.XTestImports, testID)), + Imports: importMap(p.XTestImports), }) } } @@ -137,6 +143,7 @@ func getDeps(ctx context.Context, cfg *raw.Config, words ...string) (originalSet depsSet := make(map[string]bool) originalSet = make(map[string]*jsonPackage) + var testImports []string // Extract deps from the JSON. for dec := json.NewDecoder(buf); dec.More(); { @@ -149,7 +156,36 @@ func getDeps(ctx context.Context, cfg *raw.Config, words ...string) (originalSet for _, dep := range p.Deps { depsSet[dep] = true } + if cfg.Tests { + // collect the additional imports of the test packages. + pkgTestImports := append(p.TestImports, p.XTestImports...) + for _, imp := range pkgTestImports { + if depsSet[imp] { + continue + } + depsSet[imp] = true + testImports = append(testImports, imp) + } + } } + // Get the deps of the packages imported by tests. + if len(testImports) > 0 { + buf, err = golist(ctx, cfg, golistargs_fallback(cfg, testImports)) + if err != nil { + return nil, nil, err + } + // Extract deps from the JSON. + for dec := json.NewDecoder(buf); dec.More(); { + p := new(jsonPackage) + if err := dec.Decode(p); err != nil { + return nil, nil, fmt.Errorf("JSON decoding failed: %v", err) + } + for _, dep := range p.Deps { + depsSet[dep] = true + } + } + } + for orig := range originalSet { delete(depsSet, orig) } diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go index 9ad8b740284..6c037f0879a 100644 --- a/go/packages/packages_test.go +++ b/go/packages/packages_test.go @@ -39,14 +39,14 @@ var usesOldGolist = false // gracefully. // - test more Flags. // -// TypeCheck & WholeProgram modes: +// LoadSyntax & LoadAllSyntax modes: // - Fset may be user-supplied or not. // - Packages.Info is correctly set. // - typechecker configuration is honored // - import cycles are gracefully handled in type checker. // - test typechecking of generated test main and cgo. -func TestMetadataImportGraph(t *testing.T) { +func TestLoadImportsGraph(t *testing.T) { if runtime.GOOS != "linux" { t.Skipf("TODO: skipping on non-Linux; fix this test to run everywhere. golang.org/issue/26387") } @@ -131,8 +131,32 @@ func TestMetadataImportGraph(t *testing.T) { subdir/d_test [subdir/d.test] -> subdir/d [subdir/d.test] `[1:] + // Legacy go list support does not create test main package. + wantOldGraph := ` + a + b +* c +* e + errors + math/bits +* subdir/d +* subdir/d [subdir/d.test] +* subdir/d_test [subdir/d.test] + unsafe + b -> a + b -> errors + c -> b + c -> unsafe + e -> b + e -> c + subdir/d [subdir/d.test] -> math/bits + subdir/d_test [subdir/d.test] -> subdir/d [subdir/d.test] +`[1:] + if graph != wantGraph && !usesOldGolist { t.Errorf("wrong import graph: got <<%s>>, want <<%s>>", graph, wantGraph) + } else if graph != wantOldGraph && usesOldGolist { + t.Errorf("wrong import graph: got <<%s>>, want <<%s>>", graph, wantOldGraph) } // Check node information: kind, name, srcs. @@ -190,11 +214,6 @@ func TestMetadataImportGraph(t *testing.T) { } } - if usesOldGolist { - // TODO(matloob): Wildcards are not yet supported. - return - } - // Wildcards // See StdlibTest for effective test of "std" wildcard. // TODO(adonovan): test "all" returns everything in the current module. @@ -220,8 +239,21 @@ func TestMetadataImportGraph(t *testing.T) { subdir/d.test -> testing/internal/testdeps (pruned) subdir/d_test [subdir/d.test] -> subdir/d [subdir/d.test] `[1:] - if graph != wantGraph { + + // Legacy go list support does not create test main package. + wantOldGraph = ` + math/bits +* subdir/d +* subdir/d [subdir/d.test] +* subdir/d_test [subdir/d.test] +* subdir/e + subdir/d [subdir/d.test] -> math/bits + subdir/d_test [subdir/d.test] -> subdir/d [subdir/d.test] +`[1:] + if graph != wantGraph && !usesOldGolist { t.Errorf("wrong import graph: got <<%s>>, want <<%s>>", graph, wantGraph) + } else if graph != wantOldGraph && usesOldGolist { + t.Errorf("wrong import graph: got <<%s>>, want <<%s>>", graph, wantOldGraph) } } } @@ -844,6 +876,7 @@ func errorMessages(errors []error) []string { func srcs(p *packages.Package) (basenames []string) { files := append(p.GoFiles, p.OtherFiles...) for i, src := range files { + // TODO(suzmue): make cache names predictable on all os. if strings.Contains(src, ".cache/go-build") { src = fmt.Sprintf("%d.go", i) // make cache names predictable } else {