gopls/internal/test: fix path to local go in integration tests

As described in golang/go#69630, our 1.21 and 1.22 builders were not
actually testing gopls' integration with the Go command, because go test
modifies PATH and GOROOT when performing a toolchain switch.

"Fix" this by searching PATH for a local (=non-toolchain) go command,
and then mutating PATH and unsetting GOROOT to use this go command. This
is very much a hack, as noted in the relevant commentary, but allows us
to have much needed test coverage on the builders. In golang/go#69321,
we hope to design a better solution to this problem.

Many tests are updated to make their go version requirements accurate.

Fixes golang/go#69630

Change-Id: I431107b97845e1e99799c2c22f33b04f85ce6dd9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/623175
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Robert Findley <[email protected]>

Author: findleyr

gopls/internal/licenses/licenses_test.go

@@ -10,16 +10,9 @@ import (
 	"os/exec"
 	"runtime"
 	"testing"
-
-	"golang.org/x/tools/internal/testenv"
 )
 
 func TestLicenses(t *testing.T) {
-	// License text differs for older Go versions because staticcheck or gofumpt
-	// isn't supported for those versions, and this fails for unknown, unrelated
-	// reasons on Kokoro legacy CI.
-	testenv.NeedsGo1Point(t, 21)
-
 	if runtime.GOOS != "linux" && runtime.GOOS != "darwin" {
 		t.Skip("generating licenses only works on Unixes")
 	}

gopls/internal/test/integration/codelens/codelens_test.go

@@ -252,7 +252,8 @@ func TestUpgradeCodelens_ModVendor(t *testing.T) {
 	// This test checks the regression of golang/go#66055. The upgrade codelens
 	// should work in a mod vendor context (the test above using a go.work file
 	// was not broken).
-	testenv.NeedsGo1Point(t, 22)
+	testenv.NeedsGoCommand1Point(t, 22)
+
 	const shouldUpdateDep = `
 -- go.mod --
 module mod.com/a

gopls/internal/test/integration/codelens/gcdetails_test.go

@@ -14,16 +14,12 @@ import (
 	. "golang.org/x/tools/gopls/internal/test/integration"
 	"golang.org/x/tools/gopls/internal/test/integration/fake"
 	"golang.org/x/tools/gopls/internal/util/bug"
-	"golang.org/x/tools/internal/testenv"
 )
 
 func TestGCDetails_Toggle(t *testing.T) {
 	if runtime.GOOS == "android" {
 		t.Skipf("the gc details code lens doesn't work on Android")
 	}
-	// The overlay portion of the test fails with go1.19.
-	// I'm not sure why and not inclined to investigate.
-	testenv.NeedsGo1Point(t, 20)
 
 	const mod = `
 -- go.mod --

gopls/internal/test/integration/diagnostics/diagnostics_test.go

@@ -335,7 +335,7 @@ func Hello() {
 				InitialWorkspaceLoad,
 				Diagnostics(env.AtRegexp("main.go", `"mod.com/bob"`)),
 			)
-			if err := env.Sandbox.RunGoCommand(env.Ctx, "", "mod", []string{"init", "mod.com"}, nil, true); err != nil {
+			if _, err := env.Sandbox.RunGoCommand(env.Ctx, "", "mod", []string{"init", "mod.com"}, nil, true); err != nil {
 				t.Fatal(err)
 			}
 			env.AfterChange(

gopls/internal/test/integration/fake/sandbox.go

@@ -234,10 +234,10 @@ func (sb *Sandbox) goCommandInvocation() gocommand.Invocation {
 	return inv
 }
 
-// RunGoCommand executes a go command in the sandbox. If checkForFileChanges is
-// true, the sandbox scans the working directory and emits file change events
-// for any file changes it finds.
-func (sb *Sandbox) RunGoCommand(ctx context.Context, dir, verb string, args, env []string, checkForFileChanges bool) error {
+// RunGoCommand executes a go command in the sandbox and returns its standard
+// output. If checkForFileChanges is true, the sandbox scans the working
+// directory and emits file change events for any file changes it finds.
+func (sb *Sandbox) RunGoCommand(ctx context.Context, dir, verb string, args, env []string, checkForFileChanges bool) ([]byte, error) {
 	inv := sb.goCommandInvocation()
 	inv.Verb = verb
 	inv.Args = args
@@ -247,7 +247,7 @@ func (sb *Sandbox) RunGoCommand(ctx context.Context, dir, verb string, args, env
 	}
 	stdout, stderr, _, err := sb.goCommandRunner.RunRaw(ctx, inv)
 	if err != nil {
-		return fmt.Errorf("go command failed (stdout: %s) (stderr: %s): %v", stdout.String(), stderr.String(), err)
+		return nil, fmt.Errorf("go command failed (stdout: %s) (stderr: %s): %v", stdout.String(), stderr.String(), err)
 	}
 	// Since running a go command may result in changes to workspace files,
 	// check if we need to send any "watched" file events.
@@ -256,10 +256,10 @@ func (sb *Sandbox) RunGoCommand(ctx context.Context, dir, verb string, args, env
 	//                 for benchmarks. Consider refactoring.
 	if sb.Workdir != nil && checkForFileChanges {
 		if err := sb.Workdir.CheckForFileChanges(ctx); err != nil {
-			return fmt.Errorf("checking for file changes: %w", err)
+			return nil, fmt.Errorf("checking for file changes: %w", err)
 		}
 	}
-	return nil
+	return stdout.Bytes(), nil
 }
 
 // GoVersion checks the version of the go command.
@@ -275,7 +275,7 @@ func (sb *Sandbox) Close() error {
 	if sb.gopath != "" {
 		// Important: run this command in RootDir so that it doesn't interact with
 		// any toolchain downloads that may occur
-		goCleanErr = sb.RunGoCommand(context.Background(), sb.RootDir(), "clean", []string{"-modcache"}, nil, false)
+		_, goCleanErr = sb.RunGoCommand(context.Background(), sb.RootDir(), "clean", []string{"-modcache"}, nil, false)
 	}
 	err := robustio.RemoveAll(sb.rootdir)
 	if err != nil || goCleanErr != nil {

gopls/internal/test/integration/misc/configuration_test.go

@@ -15,11 +15,6 @@ import (
 // Test that enabling and disabling produces the expected results of showing
 // and hiding staticcheck analysis results.
 func TestChangeConfiguration(t *testing.T) {
-	// Staticcheck only supports Go versions >= 1.20.
-	// Note: keep this in sync with TestStaticcheckWarning. Below this version we
-	// should get an error when setting staticcheck configuration.
-	testenv.NeedsGo1Point(t, 20)
-
 	const files = `
 -- go.mod --
 module mod.com
@@ -164,8 +159,6 @@ type B struct {
 //
 // Gopls should not get confused about buffer content when recreating the view.
 func TestMajorOptionsChange(t *testing.T) {
-	testenv.NeedsGo1Point(t, 20) // needs staticcheck
-
 	const files = `
 -- go.mod --
 module mod.com

gopls/internal/test/integration/misc/formatting_test.go

@@ -10,7 +10,6 @@ import (
 
 	"golang.org/x/tools/gopls/internal/test/compare"
 	. "golang.org/x/tools/gopls/internal/test/integration"
-	"golang.org/x/tools/internal/testenv"
 )
 
 const unformattedProgram = `
@@ -303,7 +302,6 @@ func main() {
 }
 
 func TestGofumptFormatting(t *testing.T) {
-	testenv.NeedsGo1Point(t, 20) // gofumpt requires go 1.20+
 	// Exercise some gofumpt formatting rules:
 	//  - No empty lines following an assignment operator
 	//  - Octal integer literals should use the 0o prefix on modules using Go
@@ -367,8 +365,6 @@ const Bar = 42
 }
 
 func TestGofumpt_Issue61692(t *testing.T) {
-	testenv.NeedsGo1Point(t, 21)
-
 	const input = `
 -- go.mod --
 module foo

gopls/internal/test/integration/misc/hover_test.go

@@ -14,7 +14,6 @@ import (
 	"golang.org/x/tools/gopls/internal/protocol"
 	. "golang.org/x/tools/gopls/internal/test/integration"
 	"golang.org/x/tools/gopls/internal/test/integration/fake"
-	"golang.org/x/tools/internal/testenv"
 )
 
 func TestHoverUnexported(t *testing.T) {
@@ -282,7 +281,6 @@ go 1.16
 }
 
 func TestHoverCompletionMarkdown(t *testing.T) {
-	testenv.NeedsGo1Point(t, 19)
 	const source = `
 -- go.mod --
 module mod.com
@@ -343,7 +341,6 @@ func Hello() string {
 // Test that the generated markdown contains links for Go references.
 // https://github.com/golang/go/issues/58352
 func TestHoverLinks(t *testing.T) {
-	testenv.NeedsGo1Point(t, 19)
 	const input = `
 -- go.mod --
 go 1.19
@@ -465,7 +462,6 @@ SKIPPED
 `
 
 func TestHoverEmbedDirective(t *testing.T) {
-	testenv.NeedsGo1Point(t, 19)
 	Run(t, embedHover, func(t *testing.T, env *Env) {
 		env.OpenFile("main.go")
 		from := env.RegexpSearch("main.go", `\*.txt`)
@@ -606,8 +602,6 @@ func main() {
 }
 
 func TestHoverBuiltinFile(t *testing.T) {
-	testenv.NeedsGo1Point(t, 21) // uses 'min'
-
 	// This test verifies that hovering in the builtin file provides the same
 	// hover content as hovering over a use of a builtin.
 

gopls/internal/test/integration/misc/imports_test.go

@@ -292,9 +292,9 @@ var _, _ = x.X, y.Y
 // inclined to undertake.
 func cleanModCache(t *testing.T, modcache string) {
 	cmd := exec.Command("go", "clean", "-modcache")
-	cmd.Env = append(os.Environ(), "GOMODCACHE="+modcache)
-	if err := cmd.Run(); err != nil {
-		t.Errorf("cleaning modcache: %v", err)
+	cmd.Env = append(os.Environ(), "GOMODCACHE="+modcache, "GOTOOLCHAIN=local")
+	if output, err := cmd.CombinedOutput(); err != nil {
+		t.Errorf("cleaning modcache: %v\noutput:\n%s", err, string(output))
 	}
 }
 

gopls/internal/test/integration/misc/staticcheck_test.go

@@ -7,14 +7,10 @@ package misc
 import (
 	"testing"
 
-	"golang.org/x/tools/internal/testenv"
-
 	. "golang.org/x/tools/gopls/internal/test/integration"
 )
 
 func TestStaticcheckGenerics(t *testing.T) {
-	testenv.NeedsGo1Point(t, 20) // staticcheck requires go1.20+
-
 	// CL 583778 causes buildir not to run on packages that use
 	// range-over-func, since it might otherwise crash. But nearly
 	// all packages will soon meet this description, so the
@@ -85,8 +81,6 @@ var FooErr error = errors.New("foo")
 // Test for golang/go#56270: an analysis with related info should not panic if
 // analysis.RelatedInformation.End is not set.
 func TestStaticcheckRelatedInfo(t *testing.T) {
-	testenv.NeedsGo1Point(t, 20) // staticcheck is only supported at Go 1.20+
-
 	// CL 583778 causes buildir not to run on packages that use
 	// range-over-func, since it might otherwise crash. But nearly
 	// all packages will soon meet this description, so the

gopls/internal/test/integration/misc/webserver_test.go

@@ -361,7 +361,7 @@ func f(buf bytes.Buffer, greeting string) {
 
 // TestAssembly is a basic test of the web-based assembly listing.
 func TestAssembly(t *testing.T) {
-	testenv.NeedsGo1Point(t, 22) // for up-to-date assembly listing
+	testenv.NeedsGoCommand1Point(t, 22) // for up-to-date assembly listing
 
 	const files = `
 -- go.mod --

gopls/internal/test/integration/regtest.go

@@ -9,7 +9,10 @@ import (
 	"flag"
 	"fmt"
 	"os"
+	"os/exec"
+	"path/filepath"
 	"runtime"
+	"strings"
 	"testing"
 	"time"
 
@@ -189,5 +192,56 @@ func Main(m *testing.M) (code int) {
 	}
 	runner.tempDir = dir
 
+	FilterToolchainPathAndGOROOT()
+
 	return m.Run()
 }
+
+// FilterToolchainPathAndGOROOT updates the PATH and GOROOT environment
+// variables for the current process to effectively revert the changes made by
+// the go command when performing a toolchain switch in the context of `go
+// test` (see golang/go#68005).
+//
+// It does this by looking through PATH for a go command that is NOT a
+// toolchain go command, and adjusting PATH to find that go command. Then it
+// unsets GOROOT in order to use the default GOROOT for that go command.
+//
+// TODO(rfindley): this is very much a hack, so that our 1.21 and 1.22 builders
+// actually exercise integration with older go commands. In golang/go#69321, we
+// hope to do better.
+func FilterToolchainPathAndGOROOT() {
+	if localGo, first := findLocalGo(); localGo != "" && !first {
+		dir := filepath.Dir(localGo)
+		path := os.Getenv("PATH")
+		os.Setenv("PATH", dir+string(os.PathListSeparator)+path)
+		os.Unsetenv("GOROOT") // Remove the GOROOT value that was added by toolchain switch.
+	}
+}
+
+// findLocalGo returns a path to a local (=non-toolchain) Go version, or the
+// empty string if none is found.
+//
+// The second result reports if path matches the result of exec.LookPath.
+func findLocalGo() (path string, first bool) {
+	paths := filepath.SplitList(os.Getenv("PATH"))
+	for _, path := range paths {
+		// Use a simple heuristic to filter out toolchain paths.
+		if strings.Contains(path, "[email protected]") && filepath.Base(path) == "bin" {
+			continue // toolchain path
+		}
+		fullPath := filepath.Join(path, "go")
+		fi, err := os.Stat(fullPath)
+		if err != nil {
+			continue
+		}
+		if fi.Mode()&0111 != 0 {
+			first := false
+			pathGo, err := exec.LookPath("go")
+			if err == nil {
+				first = fullPath == pathGo
+			}
+			return fullPath, first
+		}
+	}
+	return "", false
+}

gopls/internal/test/integration/runner.go

@@ -203,7 +203,7 @@ func (r *Runner) Run(t *testing.T, files string, test TestFunc, opts ...RunOptio
 
 			// Write the go.sum file for the requested directories, before starting the server.
 			for _, dir := range config.writeGoSum {
-				if err := sandbox.RunGoCommand(context.Background(), dir, "list", []string{"-mod=mod", "./..."}, []string{"GOWORK=off"}, true); err != nil {
+				if _, err := sandbox.RunGoCommand(context.Background(), dir, "list", []string{"-mod=mod", "./..."}, []string{"GOWORK=off"}, true); err != nil {
 					t.Fatal(err)
 				}
 			}

gopls/internal/test/integration/watch/watch_test.go

@@ -588,7 +588,7 @@ func main() {
 		env.AfterChange(
 			NoDiagnostics(ForFile("main.go")),
 		)
-		if err := env.Sandbox.RunGoCommand(env.Ctx, "", "mod", []string{"init", "mod.com"}, nil, true); err != nil {
+		if _, err := env.Sandbox.RunGoCommand(env.Ctx, "", "mod", []string{"init", "mod.com"}, nil, true); err != nil {
 			t.Fatal(err)
 		}
 

gopls/internal/test/integration/workspace/broken_test.go

@@ -10,7 +10,6 @@ import (
 
 	"golang.org/x/tools/gopls/internal/server"
 	. "golang.org/x/tools/gopls/internal/test/integration"
-	"golang.org/x/tools/internal/testenv"
 )
 
 // This file holds various tests for UX with respect to broken workspaces.
@@ -23,10 +22,6 @@ import (
 
 // Test for golang/go#53933
 func TestBrokenWorkspace_DuplicateModules(t *testing.T) {
-	// The go command error message was improved in Go 1.20 to mention multiple
-	// modules.
-	testenv.NeedsGo1Point(t, 20)
-
 	// This proxy module content is replaced by the workspace, but is still
 	// required for module resolution to function in the Go command.
 	const proxy = `

gopls/internal/test/integration/workspace/workspace_test.go

@@ -255,7 +255,7 @@ func TestAutomaticWorkspaceModule_Interdependent(t *testing.T) {
 }
 
 func TestWorkspaceVendoring(t *testing.T) {
-	testenv.NeedsGo1Point(t, 22)
+	testenv.NeedsGoCommand1Point(t, 22)
 	WithOptions(
 		ProxyFiles(workspaceModuleProxy),
 	).Run(t, multiModule, func(t *testing.T, env *Env) {

gopls/internal/test/integration/wrappers.go

@@ -314,18 +314,20 @@ func (e *Env) RunGenerate(dir string) {
 
 // RunGoCommand runs the given command in the sandbox's default working
 // directory.
-func (e *Env) RunGoCommand(verb string, args ...string) {
+func (e *Env) RunGoCommand(verb string, args ...string) []byte {
 	e.T.Helper()
-	if err := e.Sandbox.RunGoCommand(e.Ctx, "", verb, args, nil, true); err != nil {
+	out, err := e.Sandbox.RunGoCommand(e.Ctx, "", verb, args, nil, true)
+	if err != nil {
 		e.T.Fatal(err)
 	}
+	return out
 }
 
 // RunGoCommandInDir is like RunGoCommand, but executes in the given
 // relative directory of the sandbox.
 func (e *Env) RunGoCommandInDir(dir, verb string, args ...string) {
 	e.T.Helper()
-	if err := e.Sandbox.RunGoCommand(e.Ctx, dir, verb, args, nil, true); err != nil {
+	if _, err := e.Sandbox.RunGoCommand(e.Ctx, dir, verb, args, nil, true); err != nil {
 		e.T.Fatal(err)
 	}
 }
@@ -334,7 +336,7 @@ func (e *Env) RunGoCommandInDir(dir, verb string, args ...string) {
 // relative directory of the sandbox with the given additional environment variables.
 func (e *Env) RunGoCommandInDirWithEnv(dir string, env []string, verb string, args ...string) {
 	e.T.Helper()
-	if err := e.Sandbox.RunGoCommand(e.Ctx, dir, verb, args, env, true); err != nil {
+	if _, err := e.Sandbox.RunGoCommand(e.Ctx, dir, verb, args, env, true); err != nil {
 		e.T.Fatal(err)
 	}
 }
@@ -355,7 +357,7 @@ func (e *Env) GoVersion() int {
 func (e *Env) DumpGoSum(dir string) {
 	e.T.Helper()
 
-	if err := e.Sandbox.RunGoCommand(e.Ctx, dir, "list", []string{"-mod=mod", "./..."}, nil, true); err != nil {
+	if _, err := e.Sandbox.RunGoCommand(e.Ctx, dir, "list", []string{"-mod=mod", "./..."}, nil, true); err != nil {
 		e.T.Fatal(err)
 	}
 	sumFile := path.Join(dir, "go.sum")