From e832138124bbf0916f7d8613b0fe1205bb1f0750 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Tue, 7 Aug 2018 16:00:29 -0400 Subject: [PATCH] go/packages: make cache names predictable in tests Check for the go-build subdirectory to identify a filepath in the go build cache. The go command uses a subdirectory of the default os cache called "go-build" for the build cache. Additionally, this responds to the comments about function names and tests in the following CLs: https://golang.org/cl/125536 https://golang.org/cl/125302 Fixes https://github.com/golang/go/issues/26387 Change-Id: I4ec65822e1e178d3de215a773e42f09fcc5bdb0d Reviewed-on: https://go-review.googlesource.com/128360 Run-TryBot: Suzy Mueller TryBot-Result: Gobot Gobot Reviewed-by: Alan Donovan Reviewed-by: Michael Matloob --- go/packages/golist/golist_fallback.go | 8 +++--- go/packages/packages_test.go | 37 +++++++++++++-------------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/go/packages/golist/golist_fallback.go b/go/packages/golist/golist_fallback.go index d623cf93..0f8ac265 100644 --- a/go/packages/golist/golist_fallback.go +++ b/go/packages/golist/golist_fallback.go @@ -149,7 +149,7 @@ func golistPackagesFallback(ctx context.Context, cfg *raw.Config, words ...strin return roots, result, nil } - buf, err := golist(ctx, cfg, golistargs_fallback(cfg, deps)) + buf, err := golist(ctx, cfg, golistArgsFallback(cfg, deps)) if err != nil { return nil, nil, err } @@ -169,7 +169,7 @@ func golistPackagesFallback(ctx context.Context, cfg *raw.Config, words ...strin // getDeps runs an initial go list to determine all the dependency packages. func getDeps(ctx context.Context, cfg *raw.Config, words ...string) (originalSet map[string]*jsonPackage, deps []string, err error) { - buf, err := golist(ctx, cfg, golistargs_fallback(cfg, words)) + buf, err := golist(ctx, cfg, golistArgsFallback(cfg, words)) if err != nil { return nil, nil, err } @@ -203,7 +203,7 @@ func getDeps(ctx context.Context, cfg *raw.Config, words ...string) (originalSet } // Get the deps of the packages imported by tests. if len(testImports) > 0 { - buf, err = golist(ctx, cfg, golistargs_fallback(cfg, testImports)) + buf, err = golist(ctx, cfg, golistArgsFallback(cfg, testImports)) if err != nil { return nil, nil, err } @@ -230,7 +230,7 @@ func getDeps(ctx context.Context, cfg *raw.Config, words ...string) (originalSet return originalSet, deps, nil } -func golistargs_fallback(cfg *raw.Config, words []string) []string { +func golistArgsFallback(cfg *raw.Config, words []string) []string { fullargs := []string{"list", "-e", "-json"} fullargs = append(fullargs, cfg.Flags...) fullargs = append(fullargs, "--") diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go index 1ea1284b..59e59fdc 100644 --- a/go/packages/packages_test.go +++ b/go/packages/packages_test.go @@ -15,7 +15,6 @@ import ( "os" "path/filepath" "reflect" - "runtime" "sort" "strings" "sync" @@ -47,9 +46,6 @@ var usesOldGolist = false // - test typechecking of generated test main and cgo. 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") - } tmp, cleanup := makeTree(t, map[string]string{ "src/a/a.go": `package a; const A = 1`, "src/b/b.go": `package b; import ("a"; _ "errors"); var B = a.A`, @@ -390,12 +386,12 @@ package b`, pattern string tags []string wantSrcs string - wantImportSrcs map[string]string + wantImportSrcs string }{ - {`a`, []string{}, "a.go", map[string]string{"a/b": "a.go"}}, - {`a`, []string{`-tags=tag`}, "a.go b.go c.go", map[string]string{"a/b": "a.go b.go"}}, - {`a`, []string{`-tags=tag2`}, "a.go c.go", map[string]string{"a/b": "a.go"}}, - {`a`, []string{`-tags=tag tag2`}, "a.go b.go c.go d.go", map[string]string{"a/b": "a.go b.go"}}, + {`a`, []string{}, "a.go", "a.go"}, + {`a`, []string{`-tags=tag`}, "a.go b.go c.go", "a.go b.go"}, + {`a`, []string{`-tags=tag2`}, "a.go c.go", "a.go"}, + {`a`, []string{`-tags=tag tag2`}, "a.go b.go c.go d.go", "a.go b.go"}, } { cfg := &packages.Config{ Mode: packages.LoadFiles, @@ -406,19 +402,22 @@ package b`, initial, err := packages.Load(cfg, test.pattern) if err != nil { t.Error(err) + continue } if len(initial) != 1 { - t.Fatalf("test tags %v: pattern %s, expected 1 package, got %d packages.", test.tags, test.pattern, len(initial)) + t.Errorf("test tags %v: pattern %s, expected 1 package, got %d packages.", test.tags, test.pattern, len(initial)) + continue } pkg := initial[0] if srcs := strings.Join(srcs(pkg), " "); srcs != test.wantSrcs { t.Errorf("test tags %v: srcs of package %s = [%s], want [%s]", test.tags, test.pattern, srcs, test.wantSrcs) } for path, ipkg := range pkg.Imports { - if srcs := strings.Join(srcs(ipkg), " "); srcs != test.wantImportSrcs[path] { - t.Errorf("build tags %v: srcs of imported package %s = [%s], want [%s]", test.tags, path, srcs, test.wantImportSrcs[path]) + if srcs := strings.Join(srcs(ipkg), " "); srcs != test.wantImportSrcs { + t.Errorf("build tags %v: srcs of imported package %s = [%s], want [%s]", test.tags, path, srcs, test.wantImportSrcs) } } + } } @@ -433,7 +432,7 @@ func (ec *errCollector) add(err error) { ec.mu.Unlock() } -func TestTypeCheckOK(t *testing.T) { +func TestLoadSyntaxOK(t *testing.T) { tmp, cleanup := makeTree(t, map[string]string{ "src/a/a.go": `package a; import "b"; const A = "a" + b.B`, "src/b/b.go": `package b; import "c"; const B = "b" + c.C`, @@ -519,7 +518,7 @@ func TestTypeCheckOK(t *testing.T) { } } -func TestTypeCheckError(t *testing.T) { +func TestLoadSyntaxError(t *testing.T) { // A type error in a lower-level package (e) prevents go list // from producing export data for all packages that depend on it // [a-e]. Export data is only required for package d, so package @@ -610,7 +609,7 @@ func TestTypeCheckError(t *testing.T) { // This function tests use of the ParseFile hook to supply // alternative file contents to the parser and type-checker. -func TestWholeProgramOverlay(t *testing.T) { +func TestLoadAllSyntaxOverlay(t *testing.T) { type M = map[string]string tmp, cleanup := makeTree(t, M{ @@ -670,7 +669,7 @@ func TestWholeProgramOverlay(t *testing.T) { } } -func TestWholeProgramImportErrors(t *testing.T) { +func TestLoadAllSyntaxImportErrors(t *testing.T) { // TODO(matloob): Remove this once go list -e -compiled is fixed. See golang.org/issue/26755 t.Skip("go list -compiled -e fails with non-zero exit status for empty packages") @@ -819,7 +818,6 @@ func TestAbsoluteFilenames(t *testing.T) { continue } - // Don't check which files are included with the legacy loader (breaks with .s files). if got := strings.Join(srcs(pkgs[0]), " "); got != test.want { t.Errorf("in package %s, got %s, want %s", test.pattern, got, test.want) } @@ -909,8 +907,9 @@ 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") { + // The default location for cache data is a subdirectory named go-build + // in the standard user cache directory for the current operating system. + if strings.Contains(filepath.ToSlash(src), "/go-build/") { src = fmt.Sprintf("%d.go", i) // make cache names predictable } else { src = filepath.Base(src)