From dea100b118f1bbb1367d7a1a201c670c5f61fbae Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Thu, 17 Nov 2022 11:11:44 -0500 Subject: [PATCH] internal/testenv: skip tests that need export data for std if 'go tool compile' does not exist Now that .a files are no longer being preinstalled for standard-library packages (#47257), tests that require the export data from those files must have access to 'go list -export' in order to generate and load export data from the Go build cache. `go list -export` does not work without a Go compiler, so assume that tests that need the 'go' command also need the compiler. This may cause the tests currently failing on the android-.*-emu builders to instead be skipped, since the test harness does not copy the compiler to the execution environment. For golang/go#47257. Change-Id: Ie82ab09ac8165a01fc1d3a083b1e86226efc469d Reviewed-on: https://go-review.googlesource.com/c/tools/+/451597 TryBot-Result: Gopher Robot Auto-Submit: Bryan Mills Run-TryBot: Bryan Mills Reviewed-by: Michael Matloob gopls-CI: kokoro --- go/gcexportdata/example_test.go | 4 ++-- go/packages/overlay_test.go | 1 + go/ssa/builder_test.go | 5 +++++ go/ssa/example_test.go | 3 +++ go/ssa/ssautil/load_test.go | 2 ++ gopls/internal/lsp/regtest/runner.go | 1 + gopls/internal/lsp/safetoken/safetoken_test.go | 3 +++ gopls/internal/vulncheck/command_test.go | 4 ++++ internal/gcimporter/gcimporter_test.go | 7 +++++++ internal/gcimporter/iexport_go118_test.go | 3 +++ internal/testenv/testenv.go | 17 +++++++++++++++++ 11 files changed, 48 insertions(+), 2 deletions(-) diff --git a/go/gcexportdata/example_test.go b/go/gcexportdata/example_test.go index cdd68f49c5..7371d31d43 100644 --- a/go/gcexportdata/example_test.go +++ b/go/gcexportdata/example_test.go @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build go1.7 && gc -// +build go1.7,gc +//go:build go1.7 && gc && !android && !ios && !js +// +build go1.7,gc,!android,!ios,!js package gcexportdata_test diff --git a/go/packages/overlay_test.go b/go/packages/overlay_test.go index f2164c274e..93e306a4b9 100644 --- a/go/packages/overlay_test.go +++ b/go/packages/overlay_test.go @@ -1046,6 +1046,7 @@ func Hi() { // This does not use go/packagestest because it needs to write a replace // directive with an absolute path in one of the module's go.mod files. func TestOverlaysInReplace(t *testing.T) { + testenv.NeedsGoPackages(t) t.Parallel() // Create module b.com in a temporary directory. Do not add any Go files diff --git a/go/ssa/builder_test.go b/go/ssa/builder_test.go index 1975e26e45..6fc844187d 100644 --- a/go/ssa/builder_test.go +++ b/go/ssa/builder_test.go @@ -24,6 +24,7 @@ import ( "golang.org/x/tools/go/loader" "golang.org/x/tools/go/ssa" "golang.org/x/tools/go/ssa/ssautil" + "golang.org/x/tools/internal/testenv" "golang.org/x/tools/internal/typeparams" ) @@ -32,6 +33,8 @@ func isEmpty(f *ssa.Function) bool { return f.Blocks == nil } // Tests that programs partially loaded from gc object files contain // functions with no code for the external portions, but are otherwise ok. func TestBuildPackage(t *testing.T) { + testenv.NeedsGoBuild(t) // for importer.Default() + input := ` package main @@ -164,6 +167,8 @@ func main() { // TestRuntimeTypes tests that (*Program).RuntimeTypes() includes all necessary types. func TestRuntimeTypes(t *testing.T) { + testenv.NeedsGoBuild(t) // for importer.Default() + tests := []struct { input string want []string diff --git a/go/ssa/example_test.go b/go/ssa/example_test.go index 492a02f766..9a5fd43692 100644 --- a/go/ssa/example_test.go +++ b/go/ssa/example_test.go @@ -2,6 +2,9 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +//go:build !android && !ios && !js +// +build !android,!ios,!js + package ssa_test import ( diff --git a/go/ssa/ssautil/load_test.go b/go/ssa/ssautil/load_test.go index 707a1d0b69..efa2ba40a8 100644 --- a/go/ssa/ssautil/load_test.go +++ b/go/ssa/ssautil/load_test.go @@ -33,6 +33,8 @@ func main() { ` func TestBuildPackage(t *testing.T) { + testenv.NeedsGoBuild(t) // for importer.Default() + // There is a more substantial test of BuildPackage and the // SSA program it builds in ../ssa/builder_test.go. diff --git a/gopls/internal/lsp/regtest/runner.go b/gopls/internal/lsp/regtest/runner.go index c83318fbee..02be6fa40e 100644 --- a/gopls/internal/lsp/regtest/runner.go +++ b/gopls/internal/lsp/regtest/runner.go @@ -141,6 +141,7 @@ func (r *Runner) Run(t *testing.T, files string, test TestFunc, opts ...RunOptio // refactoring. t.Helper() checkBuilder(t) + testenv.NeedsGoPackages(t) tests := []struct { name string diff --git a/gopls/internal/lsp/safetoken/safetoken_test.go b/gopls/internal/lsp/safetoken/safetoken_test.go index 1486d68f32..86ece1f8e6 100644 --- a/gopls/internal/lsp/safetoken/safetoken_test.go +++ b/gopls/internal/lsp/safetoken/safetoken_test.go @@ -10,6 +10,7 @@ import ( "testing" "golang.org/x/tools/go/packages" + "golang.org/x/tools/internal/testenv" ) // This test reports any unexpected uses of (*go/token.File).Offset within @@ -17,6 +18,8 @@ import ( // to panicking. All calls to (*go/token.File).Offset should be replaced with // calls to safetoken.Offset. func TestTokenOffset(t *testing.T) { + testenv.NeedsGoPackages(t) + fset := token.NewFileSet() pkgs, err := packages.Load(&packages.Config{ Fset: fset, diff --git a/gopls/internal/vulncheck/command_test.go b/gopls/internal/vulncheck/command_test.go index a9ab2c0a73..36fd76d785 100644 --- a/gopls/internal/vulncheck/command_test.go +++ b/gopls/internal/vulncheck/command_test.go @@ -25,6 +25,7 @@ import ( "golang.org/x/tools/gopls/internal/lsp/source" "golang.org/x/tools/gopls/internal/lsp/tests" "golang.org/x/tools/gopls/internal/vulncheck/vulntest" + "golang.org/x/tools/internal/testenv" ) func TestCmd_Run(t *testing.T) { @@ -276,6 +277,9 @@ modules: ` func runTest(t *testing.T, workspaceData, proxyData string, test func(context.Context, source.Snapshot)) { + testenv.NeedsGoBuild(t) + testenv.NeedsGoPackages(t) + ws, err := fake.NewSandbox(&fake.SandboxConfig{ Files: fake.UnpackTxt(workspaceData), ProxyFiles: fake.UnpackTxt(proxyData), diff --git a/internal/gcimporter/gcimporter_test.go b/internal/gcimporter/gcimporter_test.go index e4029c0d5e..56f3918278 100644 --- a/internal/gcimporter/gcimporter_test.go +++ b/internal/gcimporter/gcimporter_test.go @@ -373,6 +373,7 @@ func TestVersionHandling(t *testing.T) { func TestImportStdLib(t *testing.T) { // 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") == "" { @@ -422,6 +423,7 @@ func TestImportedTypes(t *testing.T) { testenv.NeedsGo1Point(t, 11) // This package only handles gc export data. needsCompiler(t, "gc") + testenv.NeedsGoBuild(t) // to find stdlib export data in the build cache for _, test := range importedObjectTests { obj := importObject(t, test.name) @@ -447,6 +449,8 @@ func TestImportedTypes(t *testing.T) { func TestImportedConsts(t *testing.T) { testenv.NeedsGo1Point(t, 11) + testenv.NeedsGoBuild(t) // to find stdlib export data in the build cache + tests := []struct { name string want constant.Kind @@ -530,6 +534,7 @@ func verifyInterfaceMethodRecvs(t *testing.T, named *types.Named, level int) { func TestIssue5815(t *testing.T) { // This package only handles gc export data. needsCompiler(t, "gc") + testenv.NeedsGoBuild(t) // to find stdlib export data in the build cache pkg := importPkg(t, "strings", ".") @@ -555,6 +560,7 @@ func TestIssue5815(t *testing.T) { func TestCorrectMethodPackage(t *testing.T) { // This package only handles gc export data. needsCompiler(t, "gc") + testenv.NeedsGoBuild(t) // to find stdlib export data in the build cache imports := make(map[string]*types.Package) _, err := Import(imports, "net/http", ".", nil) @@ -609,6 +615,7 @@ func TestIssue13566(t *testing.T) { func TestIssue13898(t *testing.T) { // This package only handles gc export data. needsCompiler(t, "gc") + testenv.NeedsGoBuild(t) // to find stdlib export data in the build cache // import go/internal/gcimporter which imports go/types partially imports := make(map[string]*types.Package) diff --git a/internal/gcimporter/iexport_go118_test.go b/internal/gcimporter/iexport_go118_test.go index 27ba8cec5a..c60a9b5ee8 100644 --- a/internal/gcimporter/iexport_go118_test.go +++ b/internal/gcimporter/iexport_go118_test.go @@ -22,6 +22,7 @@ import ( "testing" "golang.org/x/tools/internal/gcimporter" + "golang.org/x/tools/internal/testenv" ) // TODO(rfindley): migrate this to testdata, as has been done in the standard library. @@ -96,6 +97,8 @@ func testExportSrc(t *testing.T, src []byte) { } func TestImportTypeparamTests(t *testing.T) { + testenv.NeedsGoBuild(t) // to find stdlib export data in the build cache + // Check go files in test/typeparam. rootDir := filepath.Join(runtime.GOROOT(), "test", "typeparam") list, err := os.ReadDir(rootDir) diff --git a/internal/testenv/testenv.go b/internal/testenv/testenv.go index f606cb7154..22df96a858 100644 --- a/internal/testenv/testenv.go +++ b/internal/testenv/testenv.go @@ -100,6 +100,23 @@ func hasTool(tool string) error { GOROOT := strings.TrimSpace(string(out)) if GOROOT != runtime.GOROOT() { checkGoGoroot.err = fmt.Errorf("'go env GOROOT' does not match runtime.GOROOT:\n\tgo env: %s\n\tGOROOT: %s", GOROOT, runtime.GOROOT()) + return + } + + // Also ensure that that GOROOT includes a compiler: 'go' commands + // don't in general work without it, and some builders + // (such as android-amd64-emu) seem to lack it in the test environment. + cmd := exec.Command(tool, "tool", "-n", "compile") + stderr := new(bytes.Buffer) + stderr.Write([]byte("\n")) + cmd.Stderr = stderr + out, err = cmd.Output() + if err != nil { + checkGoGoroot.err = fmt.Errorf("%v: %v%s", cmd, err, stderr) + return + } + if _, err := os.Stat(string(bytes.TrimSpace(out))); err != nil { + checkGoGoroot.err = err } }) if checkGoGoroot.err != nil {