all: skip tests if required tools are not found

Fixes golang/go#33950

Change-Id: Iefcb757e773bc052793611c099c25a457fd7e243
Reviewed-on: https://go-review.googlesource.com/c/tools/+/192400
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>

cmd/callgraph/main_test.go

@@ -17,6 +17,8 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+
+	"golang.org/x/tools/internal/testenv"
 )
 
 func init() {
@@ -32,6 +34,8 @@ func init() {
 }
 
 func TestCallgraph(t *testing.T) {
+	testenv.NeedsTool(t, "go")
+
 	gopath, err := filepath.Abs("testdata")
 	if err != nil {
 		t.Fatal(err)

cmd/cover/cover_test.go

@@ -16,6 +16,8 @@ import (
 	"os/exec"
 	"path/filepath"
 	"testing"
+
+	"golang.org/x/tools/internal/testenv"
 )
 
 const (
@@ -44,6 +46,8 @@ var debug = false // Keeps the rewritten files around if set.
 //	go run ./testdata/main.go ./testdata/test.go
 //
 func TestCover(t *testing.T) {
+	testenv.NeedsTool(t, "go")
+
 	// Read in the test file (testTest) and write it, with LINEs specified, to coverInput.
 	file, err := ioutil.ReadFile(testTest)
 	if err != nil {

cmd/fiximports/main_test.go

@@ -16,6 +16,8 @@ import (
 	"runtime"
 	"strings"
 	"testing"
+
+	"golang.org/x/tools/internal/testenv"
 )
 
 // TODO(adonovan):
@@ -52,6 +54,8 @@ func init() {
 }
 
 func TestFixImports(t *testing.T) {
+	testenv.NeedsTool(t, "go")
+
 	defer func() {
 		stderr = os.Stderr
 		*badDomains = "code.google.com"
@@ -239,6 +243,8 @@ import (
 
 // TestDryRun tests that the -n flag suppresses calls to writeFile.
 func TestDryRun(t *testing.T) {
+	testenv.NeedsTool(t, "go")
+
 	*dryrun = true
 	defer func() { *dryrun = false }() // restore
 	stderr = new(bytes.Buffer)

cmd/godoc/godoc_test.go

@@ -21,6 +21,8 @@ import (
 	"strings"
 	"testing"
 	"time"
+
+	"golang.org/x/tools/internal/testenv"
 )
 
 // buildGodoc builds the godoc executable.
@@ -29,12 +31,15 @@ import (
 // TODO(adonovan): opt: do this at most once, and do the cleanup
 // exactly once.  How though?  There's no atexit.
 func buildGodoc(t *testing.T) (bin string, cleanup func()) {
+	t.Helper()
+
 	if runtime.GOARCH == "arm" {
 		t.Skip("skipping test on arm platforms; too slow")
 	}
 	if runtime.GOOS == "android" {
 		t.Skipf("the dependencies are not available on android")
 	}
+	testenv.NeedsTool(t, "go")
 
 	tmp, err := ioutil.TempDir("", "godoc-regtest-")
 	if err != nil {

cmd/gorename/gorename_test.go

@@ -14,6 +14,8 @@ import (
 	"strconv"
 	"strings"
 	"testing"
+
+	"golang.org/x/tools/internal/testenv"
 )
 
 var haveCGO bool
@@ -32,6 +34,7 @@ func TestGeneratedFiles(t *testing.T) {
 	if !haveCGO {
 		t.Skipf("skipping test: no cgo")
 	}
+	testenv.NeedsTool(t, "go")
 
 	tmp, bin, cleanup := buildGorename(t)
 	defer cleanup()

cmd/guru/guru_test.go

@@ -48,6 +48,7 @@ import (
 	"testing"
 
 	guru "golang.org/x/tools/cmd/guru"
+	"golang.org/x/tools/internal/testenv"
 )
 
 func init() {
@@ -296,6 +297,7 @@ func TestGuru(t *testing.T) {
 			default:
 				cmd = exec.Command("/usr/bin/diff", "-u", golden, got)
 			}
+			testenv.NeedsTool(t, cmd.Path)
 			buf := new(bytes.Buffer)
 			cmd.Stdout = buf
 			cmd.Stderr = os.Stderr

cmd/stringer/endtoend_test.go

@@ -19,6 +19,8 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+
+	"golang.org/x/tools/internal/testenv"
 )
 
 // This file contains a test that compiles and runs each program in testdata
@@ -151,6 +153,8 @@ func TestConstValueChange(t *testing.T) {
 // buildStringer creates a temporary directory and installs stringer there.
 func buildStringer(t *testing.T) (dir string, stringer string) {
 	t.Helper()
+	testenv.NeedsTool(t, "go")
+
 	dir, err := ioutil.TempDir("", "stringer")
 	if err != nil {
 		t.Fatal(err)

cmd/stringer/golden_test.go

@@ -15,6 +15,8 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+
+	"golang.org/x/tools/internal/testenv"
 )
 
 // Golden represents a test case.
@@ -398,6 +400,8 @@ func (i Token) String() string {
 `
 
 func TestGolden(t *testing.T) {
+	testenv.NeedsTool(t, "go")
+
 	dir, err := ioutil.TempDir("", "stringer")
 	if err != nil {
 		t.Error(err)

go/analysis/analysistest/analysistest.go

@@ -18,6 +18,7 @@ import (
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/internal/checker"
 	"golang.org/x/tools/go/packages"
+	"golang.org/x/tools/internal/testenv"
 )
 
 // WriteFiles is a helper function that creates a temporary directory
@@ -98,6 +99,10 @@ type Testing interface {
 // attempted, even if unsuccessful. It is safe for a test to ignore all
 // the results, but a test may use it to perform additional checks.
 func Run(t Testing, dir string, a *analysis.Analyzer, patterns ...string) []*Result {
+	if t, ok := t.(testenv.Testing); ok {
+		testenv.NeedsGoPackages(t)
+	}
+
 	pkgs, err := loadPackages(dir, patterns...)
 	if err != nil {
 		t.Errorf("loading %s: %v", patterns, err)

go/analysis/analysistest/analysistest_test.go

@@ -10,6 +10,7 @@ import (
 
 	"golang.org/x/tools/go/analysis/analysistest"
 	"golang.org/x/tools/go/analysis/passes/findcall"
+	"golang.org/x/tools/internal/testenv"
 )
 
 func init() {
@@ -26,6 +27,8 @@ func init() {
 
 // TestTheTest tests the analysistest testing infrastructure.
 func TestTheTest(t *testing.T) {
+	testenv.NeedsTool(t, "go")
+
 	// We'll simulate a partly failing test of the findcall analysis,
 	// which (by default) reports calls to functions named 'println'.
 	findcall.Analyzer.Flags.Set("name", "println")

go/analysis/internal/checker/checker_test.go

@@ -12,11 +12,14 @@ import (
 	"golang.org/x/tools/go/analysis/internal/checker"
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
+	"golang.org/x/tools/internal/testenv"
 )
 
 var from, to string
 
 func TestApplyFixes(t *testing.T) {
+	testenv.NeedsGoPackages(t)
+
 	from = "bar"
 	to = "baz"
 

go/analysis/internal/facts/facts_test.go

@@ -16,6 +16,7 @@ import (
 	"golang.org/x/tools/go/analysis/analysistest"
 	"golang.org/x/tools/go/analysis/internal/facts"
 	"golang.org/x/tools/go/packages"
+	"golang.org/x/tools/internal/testenv"
 )
 
 type myFact struct {
@@ -88,7 +89,7 @@ func TestEncodeDecode(t *testing.T) {
 		}},
 	} {
 		// load package
-		pkg, err := load(dir, test.path)
+		pkg, err := load(t, dir, test.path)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -155,12 +156,13 @@ func find(p *types.Package, expr string) types.Object {
 	return nil
 }
 
-func load(dir string, path string) (*types.Package, error) {
+func load(t *testing.T, dir string, path string) (*types.Package, error) {
 	cfg := &packages.Config{
 		Mode: packages.LoadSyntax,
 		Dir:  dir,
 		Env:  append(os.Environ(), "GOPATH="+dir, "GO111MODULE=off", "GOPROXY=off"),
 	}
+	testenv.NeedsGoPackagesEnv(t, cfg.Env)
 	pkgs, err := packages.Load(cfg, path)
 	if err != nil {
 		return nil, err
@@ -191,7 +193,7 @@ func TestFactFilter(t *testing.T) {
 	}
 	defer cleanup()
 
-	pkg, err := load(dir, "a")
+	pkg, err := load(t, dir, "a")
 	if err != nil {
 		t.Fatal(err)
 	}

go/analysis/multichecker/multichecker_test.go

@@ -12,6 +12,7 @@ import (
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/multichecker"
 	"golang.org/x/tools/go/analysis/passes/findcall"
+	"golang.org/x/tools/internal/testenv"
 )
 
 func main() {
@@ -46,6 +47,8 @@ func TestExitCode(t *testing.T) {
 		panic("unreachable")
 	}
 
+	testenv.NeedsTool(t, "go")
+
 	for _, test := range []struct {
 		args []string
 		want int

go/internal/gcimporter/gcimporter_test.go

@@ -19,6 +19,8 @@ import (
 	"strings"
 	"testing"
 	"time"
+
+	"golang.org/x/tools/internal/testenv"
 )
 
 // ----------------------------------------------------------------------------
@@ -52,6 +54,7 @@ func HasGoBuild() bool {
 // and then run them with os.StartProcess or exec.Command.
 // If not, MustHaveGoBuild calls t.Skip with an explanation.
 func MustHaveGoBuild(t *testing.T) {
+	testenv.NeedsTool(t, "go")
 	if !HasGoBuild() {
 		t.Skipf("skipping test: 'go build' not available on %s/%s", runtime.GOOS, runtime.GOARCH)
 	}

go/loader/stdlib_test.go

@@ -24,6 +24,7 @@ import (
 
 	"golang.org/x/tools/go/buildutil"
 	"golang.org/x/tools/go/loader"
+	"golang.org/x/tools/internal/testenv"
 )
 
 func TestStdlib(t *testing.T) {
@@ -33,6 +34,7 @@ func TestStdlib(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping in short mode; uses tons of memory (https://golang.org/issue/14113)")
 	}
+	testenv.NeedsTool(t, "go")
 
 	runtime.GC()
 	t0 := time.Now()
@@ -134,6 +136,8 @@ func TestCgoOption(t *testing.T) {
 	if !build.Default.CgoEnabled {
 		return
 	}
+	testenv.NeedsTool(t, "go")
+
 	// Test that we can load cgo-using packages with
 	// CGO_ENABLED=[01], which causes go/build to select pure
 	// Go/native implementations, respectively, based on build

go/packages/packages_test.go

@@ -24,6 +24,7 @@ import (
 
 	"golang.org/x/tools/go/packages"
 	"golang.org/x/tools/go/packages/packagestest"
+	"golang.org/x/tools/internal/testenv"
 )
 
 func TestMain(m *testing.M) {
@@ -55,6 +56,8 @@ func TestMain(m *testing.M) {
 
 // The zero-value of Config has LoadFiles mode.
 func TestLoadZeroConfig(t *testing.T) {
+	testenv.NeedsGoPackages(t)
+
 	initial, err := packages.Load(nil, "hash")
 	if err != nil {
 		t.Fatal(err)
@@ -1020,6 +1023,8 @@ func testNewPackagesInOverlay(t *testing.T, exporter packagestest.Exporter) {
 }
 
 func TestAdHocOverlays(t *testing.T) {
+	testenv.NeedsTool(t, "go")
+
 	// This test doesn't use packagestest because we are testing ad-hoc packages,
 	// which are outside of $GOPATH and outside of a module.
 	tmp, err := ioutil.TempDir("", "a")
@@ -1063,6 +1068,8 @@ const A = 1
 // TestOverlayModFileChanges tests the behavior resulting from having files from
 // multiple modules in overlays.
 func TestOverlayModFileChanges(t *testing.T) {
+	testenv.NeedsTool(t, "go")
+
 	// Create two unrelated modules in a temporary directory.
 	tmp, err := ioutil.TempDir("", "tmp")
 	if err != nil {

go/packages/packagescgo_test.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 
 	"golang.org/x/tools/go/packages"
+	"golang.org/x/tools/internal/testenv"
 )
 
 func TestLoadImportsC(t *testing.T) {
@@ -28,6 +29,7 @@ func TestLoadImportsC(t *testing.T) {
 		// See https://golang.org/issue/27100.
 		t.Skip(`skipping on plan9; for some reason "net [syscall.test]" is not loaded`)
 	}
+	testenv.NeedsGoPackages(t)
 
 	cfg := &packages.Config{
 		Mode:  packages.LoadImports,

go/packages/packagestest/export.go

@@ -25,6 +25,7 @@ import (
 	"golang.org/x/tools/go/expect"
 	"golang.org/x/tools/go/packages"
 	"golang.org/x/tools/internal/span"
+	"golang.org/x/tools/internal/testenv"
 )
 
 var (
@@ -127,6 +128,10 @@ func BenchmarkAll(b *testing.B, f func(*testing.B, Exporter)) {
 // debugging tests.
 func Export(t testing.TB, exporter Exporter, modules []Module) *Exported {
 	t.Helper()
+	if exporter == Modules {
+		testenv.NeedsTool(t, "go")
+	}
+
 	dirname := strings.Replace(t.Name(), "/", "_", -1)
 	dirname = strings.Replace(dirname, "#", "_", -1) // duplicate subtests get a #NNN suffix.
 	temp, err := ioutil.TempDir("", dirname)
@@ -185,6 +190,7 @@ func Export(t testing.TB, exporter Exporter, modules []Module) *Exported {
 	if err := exporter.Finalize(exported); err != nil {
 		t.Fatal(err)
 	}
+	testenv.NeedsGoPackagesEnv(t, exported.Config.Env)
 	return exported
 }
 

go/packages/stdlib_test.go

@@ -14,6 +14,7 @@ import (
 	"time"
 
 	"golang.org/x/tools/go/packages"
+	"golang.org/x/tools/internal/testenv"
 )
 
 // This test loads the metadata for the standard library,
@@ -23,6 +24,8 @@ func TestStdlibMetadata(t *testing.T) {
 	// 	t.Skipf("incomplete std lib on %s", runtime.GOOS)
 	// }
 
+	testenv.NeedsGoPackages(t)
+
 	runtime.GC()
 	t0 := time.Now()
 	var memstats runtime.MemStats
@@ -62,6 +65,8 @@ func TestCgoOption(t *testing.T) {
 		t.Skip("skipping in short mode; uses tons of memory (https://golang.org/issue/14113)")
 	}
 
+	testenv.NeedsGoPackages(t)
+
 	// TODO(adonovan): see if we can get away without these old
 	// go/loader hacks now that we use the go list command.
 	//

go/ssa/ssautil/load_test.go

@@ -17,6 +17,7 @@ import (
 
 	"golang.org/x/tools/go/packages"
 	"golang.org/x/tools/go/ssa/ssautil"
+	"golang.org/x/tools/internal/testenv"
 )
 
 const hello = `package main
@@ -53,6 +54,8 @@ func TestBuildPackage(t *testing.T) {
 }
 
 func TestPackages(t *testing.T) {
+	testenv.NeedsGoPackages(t)
+
 	cfg := &packages.Config{Mode: packages.LoadSyntax}
 	initial, err := packages.Load(cfg, "bytes")
 	if err != nil {
@@ -106,6 +109,8 @@ func TestBuildPackage_MissingImport(t *testing.T) {
 }
 
 func TestIssue28106(t *testing.T) {
+	testenv.NeedsGoPackages(t)
+
 	// In go1.10, go/packages loads all packages from source, not
 	// export data, but does not type check function bodies of
 	// imported packages. This test ensures that we do not attempt

go/ssa/stdlib_test.go

@@ -25,6 +25,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"
 )
 
 // Skip the set of packages that transitively depend on
@@ -50,6 +51,8 @@ func TestStdlib(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping in short mode; too slow (https://golang.org/issue/14113)")
 	}
+	testenv.NeedsTool(t, "go")
+
 	// Load, parse and type-check the program.
 	t0 := time.Now()
 	alloc0 := bytesAllocated()

internal/apidiff/apidiff_test.go

@@ -13,6 +13,7 @@ import (
 	"testing"
 
 	"golang.org/x/tools/go/packages"
+	"golang.org/x/tools/internal/testenv"
 )
 
 func TestChanges(t *testing.T) {
@@ -26,11 +27,11 @@ func TestChanges(t *testing.T) {
 	sort.Strings(wanti)
 	sort.Strings(wantc)
 
-	oldpkg, err := load("apidiff/old", dir)
+	oldpkg, err := load(t, "apidiff/old", dir)
 	if err != nil {
 		t.Fatal(err)
 	}
-	newpkg, err := load("apidiff/new", dir)
+	newpkg, err := load(t, "apidiff/new", dir)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -115,7 +116,9 @@ func splitIntoPackages(t *testing.T, dir string) (incompatibles, compatibles []s
 	return
 }
 
-func load(importPath, goPath string) (*packages.Package, error) {
+func load(t *testing.T, importPath, goPath string) (*packages.Package, error) {
+	testenv.NeedsGoPackages(t)
+
 	cfg := &packages.Config{
 		Mode: packages.LoadTypes,
 	}
@@ -134,7 +137,7 @@ func load(importPath, goPath string) (*packages.Package, error) {
 }
 
 func TestExportedFields(t *testing.T) {
-	pkg, err := load("golang.org/x/tools/internal/apidiff/testdata/exported_fields", "")
+	pkg, err := load(t, "golang.org/x/tools/internal/apidiff/testdata/exported_fields", "")
 	if err != nil {
 		t.Fatal(err)
 	}

internal/imports/mod_test.go

@@ -15,6 +15,7 @@ import (
 	"testing"
 
 	"golang.org/x/tools/internal/module"
+	"golang.org/x/tools/internal/testenv"
 	"golang.org/x/tools/internal/txtar"
 )
 
@@ -578,6 +579,8 @@ type modTest struct {
 // in testdata/mod, along the lines of TestScript in cmd/go.
 func setup(t *testing.T, main, wd string) *modTest {
 	t.Helper()
+	testenv.NeedsTool(t, "go")
+
 	proxyOnce.Do(func() {
 		var err error
 		proxyDir, err = ioutil.TempDir("", "proxy-")

internal/testenv/testenv.go

@@ -0,0 +1,108 @@
+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package testenv contains helper functions for skipping tests
+// based on which tools are present in the environment.
+package testenv
+
+import (
+	"os"
+	"os/exec"
+	"runtime"
+	"strings"
+)
+
+// Testing is an abstraction of a *testing.T.
+type Testing interface {
+	Skipf(format string, args ...interface{})
+	Fatalf(format string, args ...interface{})
+}
+
+type helperer interface {
+	Helper()
+}
+
+// packageMainIsDevel reports whether the module containing package main
+// is a development version (if module information is available).
+//
+// Builds in GOPATH mode and builds that lack module information are assumed to
+// be development versions.
+var packageMainIsDevel = func() bool { return true }
+
+func allowMissingTool(tool string) bool {
+	if runtime.GOOS == "android" {
+		// Android builds generally run tests on a separate machine from the build,
+		// so don't expect any external tools to be available.
+		return true
+	}
+
+	if tool == "go" && os.Getenv("GO_BUILDER_NAME") == "illumos-amd64-joyent" {
+		// Work around a misconfigured builder (see https://golang.org/issue/33950).
+		return true
+	}
+
+	// If a developer is actively working on this test, we expect them to have all
+	// of its dependencies installed. However, if it's just a dependency of some
+	// other module (for example, being run via 'go test all'), we should be more
+	// tolerant of unusual environments.
+	return !packageMainIsDevel()
+}
+
+// NeedsTool skips t if the named tool is not present in the path.
+func NeedsTool(t Testing, tool string) {
+	_, err := exec.LookPath(tool)
+	if err == nil {
+		return
+	}
+
+	if t, ok := t.(helperer); ok {
+		t.Helper()
+	}
+	if allowMissingTool(tool) {
+		t.Skipf("skipping because %s tool not available: %v", tool, err)
+	} else {
+		t.Fatalf("%s tool not available: %v", tool, err)
+	}
+}
+
+// NeedsGoPackages skips t if the go/packages driver (or 'go' tool) implied by
+// the current process environment is not present in the path.
+func NeedsGoPackages(t Testing) {
+	if t, ok := t.(helperer); ok {
+		t.Helper()
+	}
+
+	tool := os.Getenv("GOPACKAGESDRIVER")
+	if tool == "" {
+		if _, err := exec.LookPath("gopackagesdriver"); err == nil {
+			tool = "gopackagesdriver"
+		} else {
+			tool = "go"
+		}
+	}
+
+	NeedsTool(t, tool)
+}
+
+// NeedsGoPackagesEnv skips t if the go/packages driver (or 'go' tool) implied
+// by env is not present in the path.
+func NeedsGoPackagesEnv(t Testing, env []string) {
+	if t, ok := t.(helperer); ok {
+		t.Helper()
+	}
+
+	for _, v := range env {
+		if strings.HasPrefix(v, "GOPACKAGESDRIVER=") {
+			tool := strings.TrimPrefix(v, "GOPACKAGESDRIVER=")
+			if tool == "off" {
+				NeedsTool(t, "go")
+			} else {
+				NeedsTool(t, tool)
+			}
+			return
+		}
+	}
+
+	NeedsGoPackages(t)
+}

internal/testenv/testenv_112.go

@@ -0,0 +1,27 @@
+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build go1.12
+
+package testenv
+
+import "runtime/debug"
+
+func packageMainIsDevelModule() bool {
+	info, ok := debug.ReadBuildInfo()
+	if !ok {
+		// Most test binaries currently lack build info, but this should become more
+		// permissive once https://golang.org/issue/33976 is fixed.
+		return true
+	}
+
+	// Note: info.Main.Version describes the version of the module containing
+	// package main, not the version of “the main module”.
+	// See https://golang.org/issue/33975.
+	return info.Main.Version == "(devel)"
+}
+
+func init() {
+	packageMainIsDevel = packageMainIsDevelModule
+}

refactor/eg/eg_test.go

@@ -23,6 +23,7 @@ import (
 	"testing"
 
 	"golang.org/x/tools/go/loader"
+	"golang.org/x/tools/internal/testenv"
 	"golang.org/x/tools/refactor/eg"
 )
 
@@ -38,6 +39,8 @@ var (
 )
 
 func Test(t *testing.T) {
+	testenv.NeedsTool(t, "go")
+
 	switch runtime.GOOS {
 	case "windows":
 		t.Skipf("skipping test on %q (no /usr/bin/diff)", runtime.GOOS)

refactor/rename/rename_test.go

@@ -10,6 +10,7 @@ import (
 	"go/build"
 	"go/token"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"regexp"
 	"runtime"
@@ -17,6 +18,7 @@ import (
 	"testing"
 
 	"golang.org/x/tools/go/buildutil"
+	"golang.org/x/tools/internal/testenv"
 )
 
 // TODO(adonovan): test reported source positions, somehow.
@@ -1278,11 +1280,17 @@ func main() {
 
 func TestDiff(t *testing.T) {
 	switch runtime.GOOS {
-	case "windows", "android":
+	case "windows":
+		if os.Getenv("GO_BUILDER_NAME") != "" {
+			if _, err := exec.LookPath(DiffCmd); err != nil {
 				t.Skipf("diff tool non-existent for %s on builders", runtime.GOOS)
+			}
+		}
 	case "plan9":
 		t.Skipf("plan9 diff tool doesn't support -u flag")
 	}
+	testenv.NeedsTool(t, DiffCmd)
+	testenv.NeedsTool(t, "go") // to locate golang.org/x/tools/refactor/rename
 
 	defer func() {
 		Diff = false