diff --git a/internal/lsp/cmd/test/format.go b/internal/lsp/cmd/test/format.go index 7189fe1e9d..0418b3104e 100644 --- a/internal/lsp/cmd/test/format.go +++ b/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() { - t.Skip("running patch is expensive") - } + testenv.NeedsTool(t, "patch") if strings.Count(patch, "\n+++ ") > 1 { // TODO(golang/go/#34580) t.Skip("multi-file patch tests not supported yet") diff --git a/internal/testenv/testenv.go b/internal/testenv/testenv.go index 7d8de1a1e9..295cc45d3f 100644 --- a/internal/testenv/testenv.go +++ b/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,9 +61,20 @@ func allowMissingTool(tool string) bool { 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 + 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 @@ -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 {