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