gopls/internal/test: fix failures when running tests with GOTOOLCHAIN

Gopls integration tests want to use the ambient Go toolchain, to test
integration with older Go commands. But GOTOOLCHAIN injects the
toolchain binary into PATH, so gopls must remove this injected path
element before it runs the go command.

Unfortunately, if GOTOOLCHAIN=go1.N.P explicitly, those tests will also
try to *download* the explicit toolchain and fail because we have set
GOPROXY to a file based proxy.

Fix this by first adding a check that the initial workspace load did not
fail, as well as other related error annotations such that the failure
message more accurately identifies the problem. Additionally, the
preceding CL improved the integration test framework to better surface
such errors. Then, actually fix the problem by setting GOTOOLCHAIN=local
in our integration test sandbox.

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

Author: findleyr

gopls/internal/golang/completion/completion.go

@@ -668,7 +668,7 @@ func Completion(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p
 
 	err = c.collectCompletions(ctx)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, fmt.Errorf("failed to collect completions: %v", err)
 	}
 
 	// Deep search collected candidates and their members for more candidates.
@@ -688,7 +688,7 @@ func Completion(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p
 	for _, callback := range c.completionCallbacks {
 		if deadline == nil || time.Now().Before(*deadline) {
 			if err := c.snapshot.RunProcessEnvFunc(ctx, callback); err != nil {
-				return nil, nil, err
+				return nil, nil, fmt.Errorf("failed to run goimports callback: %v", err)
 			}
 		}
 	}
@@ -989,7 +989,10 @@ func (c *completer) populateImportCompletions(searchImport *ast.ImportSpec) erro
 	}
 
 	c.completionCallbacks = append(c.completionCallbacks, func(ctx context.Context, opts *imports.Options) error {
-		return imports.GetImportPaths(ctx, searchImports, prefix, c.filename, c.pkg.Types().Name(), opts.Env)
+		if err := imports.GetImportPaths(ctx, searchImports, prefix, c.filename, c.pkg.Types().Name(), opts.Env); err != nil {
+			return fmt.Errorf("getting import paths: %v", err)
+		}
+		return nil
 	})
 	return nil
 }
@@ -1529,7 +1532,10 @@ func (c *completer) selector(ctx context.Context, sel *ast.SelectorExpr) error {
 
 	c.completionCallbacks = append(c.completionCallbacks, func(ctx context.Context, opts *imports.Options) error {
 		defer cancel()
-		return imports.GetPackageExports(ctx, add, id.Name, c.filename, c.pkg.Types().Name(), opts.Env)
+		if err := imports.GetPackageExports(ctx, add, id.Name, c.filename, c.pkg.Types().Name(), opts.Env); err != nil {
+			return fmt.Errorf("getting package exports: %v", err)
+		}
+		return nil
 	})
 	return nil
 }
@@ -1916,7 +1922,10 @@ func (c *completer) unimportedPackages(ctx context.Context, seen map[string]stru
 	}
 
 	c.completionCallbacks = append(c.completionCallbacks, func(ctx context.Context, opts *imports.Options) error {
-		return imports.GetAllCandidates(ctx, add, prefix, c.filename, c.pkg.Types().Name(), opts.Env)
+		if err := imports.GetAllCandidates(ctx, add, prefix, c.filename, c.pkg.Types().Name(), opts.Env); err != nil {
+			return fmt.Errorf("getting completion candidates: %v", err)
+		}
+		return nil
 	})
 
 	return nil

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

@@ -208,6 +208,7 @@ func (sb *Sandbox) GoEnv() map[string]string {
 		"GO111MODULE":      "",
 		"GOSUMDB":          "off",
 		"GOPACKAGESDRIVER": "off",
+		"GOTOOLCHAIN":      "local", // tests should not download toolchains
 	}
 	if testenv.Go1Point() >= 5 {
 		vars["GOMODCACHE"] = ""

gopls/internal/test/marker/marker_test.go

@@ -971,7 +971,10 @@ func newEnv(t *testing.T, cache *cache.Cache, files, proxyFiles map[string][]byt
 		sandbox.Close() // ignore error
 		t.Fatal(err)
 	}
-	if err := awaiter.Await(ctx, integration.InitialWorkspaceLoad); err != nil {
+	if err := awaiter.Await(ctx, integration.OnceMet(
+		integration.InitialWorkspaceLoad,
+		integration.NoShownMessage(""),
+	)); err != nil {
 		sandbox.Close() // ignore error
 		t.Fatal(err)
 	}

internal/imports/fix.go

@@ -1030,7 +1030,7 @@ func (e *ProcessEnv) GetResolver() (Resolver, error) {
 		//
 		// For gopls, we can optionally explicitly choose a resolver type, since we
 		// already know the view type.
-		if len(e.Env["GOMOD"]) == 0 && len(e.Env["GOWORK"]) == 0 {
+		if e.Env["GOMOD"] == "" && (e.Env["GOWORK"] == "" || e.Env["GOWORK"] == "off") {
 			e.resolver = newGopathResolver(e)
 			e.logf("created gopath resolver")
 		} else if r, err := newModuleResolver(e, e.ModCache); err != nil {