From c1172337e5d723c744057b552a425c9bf2ef5651 Mon Sep 17 00:00:00 2001 From: Michael Matloob Date: Wed, 30 Nov 2022 13:11:15 -0500 Subject: [PATCH] [internal-branch.go1.20-vendor] internal/gcimporter: fix TestImportStdLib The test attempted to find all stdlib packages by scanning pkg/$GOOS_$GOARCH for .a files and then tried to import all of them. Now that .a files are no longer being placed there, the test is a noop. Fix this by using go list std (and filtering out testonly packages) and trying to import all of those to recreate what the test intended to do. This also removes a dependency on the pkg/$GOOS_$GOARCH directory which will stop being produced by dist in CL 453496. Updates golang/go#47257 Change-Id: Idfa0cbb21093776183ce193eb5363a9727bf77ef Reviewed-on: https://go-review.googlesource.com/c/tools/+/454118 Run-TryBot: Michael Matloob Reviewed-by: Michael Matloob TryBot-Result: Gopher Robot gopls-CI: kokoro Reviewed-by: Bryan Mills (cherry picked from commit 0379b73aefe056f088ad2d8727c4caff859503d4) Reviewed-on: https://go-review.googlesource.com/c/tools/+/462636 Run-TryBot: Bryan Mills Reviewed-by: Russ Cox --- internal/gcimporter/gcimporter_test.go | 61 +++++++++++--------------- internal/testenv/testenv.go | 45 +++++++++++++++++++ 2 files changed, 71 insertions(+), 35 deletions(-) diff --git a/internal/gcimporter/gcimporter_test.go b/internal/gcimporter/gcimporter_test.go index 56f39182786..d04668e6312 100644 --- a/internal/gcimporter/gcimporter_test.go +++ b/internal/gcimporter/gcimporter_test.go @@ -20,6 +20,7 @@ import ( "io/ioutil" "os" "os/exec" + "path" "path/filepath" "runtime" "strings" @@ -88,37 +89,6 @@ func testPath(t *testing.T, path, srcDir string) *types.Package { return pkg } -const maxTime = 30 * time.Second - -func testDir(t *testing.T, dir string, endTime time.Time) (nimports int) { - dirname := filepath.Join(runtime.GOROOT(), "pkg", runtime.GOOS+"_"+runtime.GOARCH, dir) - list, err := ioutil.ReadDir(dirname) - if err != nil { - t.Fatalf("testDir(%s): %s", dirname, err) - } - for _, f := range list { - if time.Now().After(endTime) { - t.Log("testing time used up") - return - } - switch { - case !f.IsDir(): - // try extensions - for _, ext := range pkgExts { - if strings.HasSuffix(f.Name(), ext) { - name := f.Name()[0 : len(f.Name())-len(ext)] // remove extension - if testPath(t, filepath.Join(dir, name), dir) != nil { - nimports++ - } - } - } - case f.IsDir(): - nimports += testDir(t, filepath.Join(dir, f.Name()), endTime) - } - } - return -} - func mktmpdir(t *testing.T) string { tmpdir, err := ioutil.TempDir("", "gcimporter_test") if err != nil { @@ -371,15 +341,36 @@ func TestVersionHandling(t *testing.T) { } func TestImportStdLib(t *testing.T) { + if testing.Short() { + t.Skip("the imports can be expensive, and this test is especially slow when the build cache is empty") + } // This package only handles gc export data. needsCompiler(t, "gc") testenv.NeedsGoBuild(t) // to find stdlib export data in the build cache - dt := maxTime - if testing.Short() && os.Getenv("GO_BUILDER_NAME") == "" { - dt = 10 * time.Millisecond + // Get list of packages in stdlib. Filter out test-only packages with {{if .GoFiles}} check. + var stderr bytes.Buffer + cmd := exec.Command("go", "list", "-f", "{{if .GoFiles}}{{.ImportPath}}{{end}}", "std") + cmd.Stderr = &stderr + out, err := cmd.Output() + if err != nil { + t.Fatalf("failed to run go list to determine stdlib packages: %v\nstderr:\n%v", err, stderr.String()) } - nimports := testDir(t, "", time.Now().Add(dt)) // installed packages + pkgs := strings.Fields(string(out)) + + var nimports int + for _, pkg := range pkgs { + t.Run(pkg, func(t *testing.T) { + if testPath(t, pkg, filepath.Join(testenv.GOROOT(t), "src", path.Dir(pkg))) != nil { + nimports++ + } + }) + } + const minPkgs = 225 // 'GOOS=plan9 go1.18 list std | wc -l' reports 228; most other platforms have more. + if len(pkgs) < minPkgs { + t.Fatalf("too few packages (%d) were imported", nimports) + } + t.Logf("tested %d imports", nimports) } diff --git a/internal/testenv/testenv.go b/internal/testenv/testenv.go index ff38498e568..8184db0ba40 100644 --- a/internal/testenv/testenv.go +++ b/internal/testenv/testenv.go @@ -345,3 +345,48 @@ func WriteImportcfg(t testing.TB, dstPath string, additionalPackageFiles map[str t.Fatalf("writing the importcfg failed: %s", err) } } + +var ( + gorootOnce sync.Once + gorootPath string + gorootErr error +) + +func findGOROOT() (string, error) { + gorootOnce.Do(func() { + gorootPath = runtime.GOROOT() + if gorootPath != "" { + // If runtime.GOROOT() is non-empty, assume that it is valid. (It might + // not be: for example, the user may have explicitly set GOROOT + // to the wrong directory.) + return + } + + cmd := exec.Command("go", "env", "GOROOT") + out, err := cmd.Output() + if err != nil { + gorootErr = fmt.Errorf("%v: %v", cmd, err) + } + gorootPath = strings.TrimSpace(string(out)) + }) + + return gorootPath, gorootErr +} + +// GOROOT reports the path to the directory containing the root of the Go +// project source tree. This is normally equivalent to runtime.GOROOT, but +// works even if the test binary was built with -trimpath. +// +// If GOROOT cannot be found, GOROOT skips t if t is non-nil, +// or panics otherwise. +func GOROOT(t testing.TB) string { + path, err := findGOROOT() + if err != nil { + if t == nil { + panic(err) + } + t.Helper() + t.Skip(err) + } + return path +}