internal/lsp: fix patch tests on builders

Fixes golang/go#34620

Change-Id: Id8e2310c27d5697a8988b9e8dc85be979f9b8d40
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199737
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>

internal/lsp/cmd/test/format.go

@@ -15,6 +15,7 @@ import (
 
 	"golang.org/x/tools/internal/lsp/cmd"
 	"golang.org/x/tools/internal/span"
+	"golang.org/x/tools/internal/testenv"
 	"golang.org/x/tools/internal/tool"
 )
 
@@ -58,9 +59,7 @@ func fixFileHeader(s string) string {
 }
 
 func checkUnified(t *testing.T, filename string, expect string, patch string) {
-	if testing.Short() {
+	testenv.NeedsTool(t, "patch")
-		t.Skip("running patch is expensive")
-	}
 	if strings.Count(patch, "\n+++ ") > 1 {
 		// TODO(golang/go/#34580)
 		t.Skip("multi-file patch tests not supported yet")

internal/testenv/testenv.go

@@ -8,6 +8,7 @@ package testenv
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os"
 	"os/exec"
 	"runtime"
@@ -31,6 +32,28 @@ type helperer interface {
 // be development versions.
 var packageMainIsDevel = func() bool { return true }
 
+func hasTool(tool string) error {
+	_, err := exec.LookPath(tool)
+	if err != nil {
+		return err
+	}
+	switch tool {
+	case "patch":
+		// check that the patch tools supports the -o argument
+		temp, err := ioutil.TempFile("", "patch-test")
+		if err != nil {
+			return err
+		}
+		temp.Close()
+		defer os.Remove(temp.Name())
+		cmd := exec.Command(tool, "-o", temp.Name())
+		if err := cmd.Run(); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func allowMissingTool(tool string) bool {
 	if runtime.GOOS == "android" {
 		// Android builds generally run tests on a separate machine from the build,
@@ -38,10 +61,21 @@ func allowMissingTool(tool string) bool {
 		return true
 	}
 
-	if tool == "go" && os.Getenv("GO_BUILDER_NAME") == "illumos-amd64-joyent" {
+	switch tool {
+	case "go":
+		if os.Getenv("GO_BUILDER_NAME") == "illumos-amd64-joyent" {
 			// Work around a misconfigured builder (see https://golang.org/issue/33950).
 			return true
 		}
+	case "diff":
+		if os.Getenv("GO_BUILDER_NAME") != "" {
+			return true
+		}
+	case "patch":
+		if os.Getenv("GO_BUILDER_NAME") != "" {
+			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
@@ -52,14 +86,13 @@ func allowMissingTool(tool string) bool {
 
 // 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()
 	}
+	err := hasTool(tool)
+	if err == nil {
+		return
+	}
 	if allowMissingTool(tool) {
 		t.Skipf("skipping because %s tool not available: %v", tool, err)
 	} else {