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>
diff --git a/internal/lsp/cmd/test/format.go b/internal/lsp/cmd/test/format.go
index 7189fe1..0418b31 100644
--- a/internal/lsp/cmd/test/format.go
+++ b/internal/lsp/cmd/test/format.go
@@ -15,6 +15,7 @@
"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 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 7d8de1a..295cc45 100644
--- a/internal/testenv/testenv.go
+++ b/internal/testenv/testenv.go
@@ -8,6 +8,7 @@
import (
"fmt"
+ "io/ioutil"
"os"
"os/exec"
"runtime"
@@ -31,6 +32,28 @@
// 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 @@
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 @@
// 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 {
