From 2415ce1593405eb9606fccff9df50ee457d6856a Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 26 Apr 2023 10:38:30 -0400 Subject: [PATCH] gopls: skip tests that load gopls packages if x/tools replacement is missing The gopls go.mod file often contains a replace directive, but the target of the replacement (a parent directory) is not present when the test is run from the module cache or in an equivalent setting. To avoid having more configurations to test and maintain, we skip these tests if the replacement directory does not exist or does not contain a plausible x/tools go.mod file. Fixes golang/go#59841. Change-Id: Icf86af46899686c3aae410250e6d26ffd11b429a Reviewed-on: https://go-review.googlesource.com/c/tools/+/489216 Run-TryBot: Bryan Mills Auto-Submit: Bryan Mills Reviewed-by: Robert Findley TryBot-Result: Gopher Robot --- gopls/doc/generate_test.go | 4 +- gopls/internal/lsp/command/interface_test.go | 3 +- .../internal/lsp/safetoken/safetoken_test.go | 13 +++++- gopls/test/debug/debug_test.go | 14 ++++++- internal/testenv/testenv.go | 40 +++++++++++++++++++ 5 files changed, 68 insertions(+), 6 deletions(-) diff --git a/gopls/doc/generate_test.go b/gopls/doc/generate_test.go index 5dc97d2dd0c..99f366c19a2 100644 --- a/gopls/doc/generate_test.go +++ b/gopls/doc/generate_test.go @@ -11,10 +11,12 @@ import ( ) func TestGenerated(t *testing.T) { + testenv.NeedsGoPackages(t) // This test fails on 1.18 Kokoro for unknown reasons; in any case, it // suffices to run this test on any builder. testenv.NeedsGo1Point(t, 19) - testenv.NeedsGoBuild(t) // This is a lie. We actually need the source code. + + testenv.NeedsLocalXTools(t) ok, err := doMain(false) if err != nil { diff --git a/gopls/internal/lsp/command/interface_test.go b/gopls/internal/lsp/command/interface_test.go index e602293a19f..2eb6f9ac819 100644 --- a/gopls/internal/lsp/command/interface_test.go +++ b/gopls/internal/lsp/command/interface_test.go @@ -14,7 +14,8 @@ import ( ) func TestGenerated(t *testing.T) { - testenv.NeedsGoBuild(t) // This is a lie. We actually need the source code. + testenv.NeedsGoPackages(t) + testenv.NeedsLocalXTools(t) onDisk, err := ioutil.ReadFile("command_gen.go") if err != nil { diff --git a/gopls/internal/lsp/safetoken/safetoken_test.go b/gopls/internal/lsp/safetoken/safetoken_test.go index 7f796d8382b..8f0a30a9dd7 100644 --- a/gopls/internal/lsp/safetoken/safetoken_test.go +++ b/gopls/internal/lsp/safetoken/safetoken_test.go @@ -72,10 +72,19 @@ func TestWorkaroundIssue57490(t *testing.T) { // suggests alternatives. func TestGoplsSourceDoesNotCallTokenFileMethods(t *testing.T) { testenv.NeedsGoPackages(t) + testenv.NeedsGo1Point(t, 18) + testenv.NeedsLocalXTools(t) - pkgs, err := packages.Load(&packages.Config{ + cfg := &packages.Config{ Mode: packages.NeedName | packages.NeedModule | packages.NeedCompiledGoFiles | packages.NeedTypes | packages.NeedTypesInfo | packages.NeedSyntax | packages.NeedImports | packages.NeedDeps, - }, "go/token", "golang.org/x/tools/gopls/...") + } + cfg.Env = os.Environ() + cfg.Env = append(cfg.Env, + "GOPACKAGESDRIVER=off", + "GOFLAGS=-mod=mod", + ) + + pkgs, err := packages.Load(cfg, "go/token", "golang.org/x/tools/gopls/...") if err != nil { t.Fatal(err) } diff --git a/gopls/test/debug/debug_test.go b/gopls/test/debug/debug_test.go index be1f5097131..5ea07fbde38 100644 --- a/gopls/test/debug/debug_test.go +++ b/gopls/test/debug/debug_test.go @@ -13,6 +13,7 @@ package debug_test import ( "go/ast" "html/template" + "os" "runtime" "sort" "strings" @@ -44,11 +45,18 @@ var templates = map[string]struct { } func TestTemplates(t *testing.T) { - testenv.NeedsGoBuild(t) + testenv.NeedsGoPackages(t) + testenv.NeedsLocalXTools(t) cfg := &packages.Config{ - Mode: packages.NeedTypesInfo | packages.LoadAllSyntax, // figure out what's necessary PJW + Mode: packages.NeedTypes | packages.NeedSyntax | packages.NeedTypesInfo, } + cfg.Env = os.Environ() + cfg.Env = append(cfg.Env, + "GOPACKAGESDRIVER=off", + "GOFLAGS=-mod=mod", + ) + pkgs, err := packages.Load(cfg, "golang.org/x/tools/gopls/internal/lsp/debug") if err != nil { t.Fatal(err) @@ -107,7 +115,9 @@ func TestTemplates(t *testing.T) { // the FuncMap is an annoyance; should not be necessary if err := templatecheck.CheckHTML(v.tmpl, v.data); err != nil { t.Errorf("%s: %v", k, err) + continue } + t.Logf("%s ok", k) } } diff --git a/internal/testenv/testenv.go b/internal/testenv/testenv.go index fa08e82ebcc..9b01888adaa 100644 --- a/internal/testenv/testenv.go +++ b/internal/testenv/testenv.go @@ -12,6 +12,7 @@ import ( "go/build" "io/ioutil" "os" + "path/filepath" "runtime" "runtime/debug" "strings" @@ -19,6 +20,7 @@ import ( "testing" "time" + "golang.org/x/mod/modfile" "golang.org/x/tools/internal/goroot" exec "golang.org/x/sys/execabs" @@ -400,3 +402,41 @@ func GOROOT(t testing.TB) string { } return path } + +// NeedsLocalXTools skips t if the golang.org/x/tools module is replaced and +// its replacement directory does not exist (or does not contain the module). +func NeedsLocalXTools(t testing.TB) { + t.Helper() + + NeedsTool(t, "go") + + cmd := Command(t, "go", "list", "-f", "{{with .Replace}}{{.Dir}}{{end}}", "-m", "golang.org/x/tools") + out, err := cmd.Output() + if err != nil { + if ee, ok := err.(*exec.ExitError); ok && len(ee.Stderr) > 0 { + t.Skipf("skipping test: %v: %v\n%s", cmd, err, ee.Stderr) + } + t.Skipf("skipping test: %v: %v", cmd, err) + } + + dir := string(bytes.TrimSpace(out)) + if dir == "" { + // No replacement directory, and (since we didn't set -e) no error either. + // Maybe x/tools isn't replaced at all (as in a gopls release, or when + // using a go.work file that includes the x/tools module). + return + } + + // We found the directory where x/tools would exist if we're in a clone of the + // repo. Is it there? (If not, we're probably in the module cache instead.) + modFilePath := filepath.Join(dir, "go.mod") + b, err := os.ReadFile(modFilePath) + if err != nil { + t.Skipf("skipping test: x/tools replacement not found: %v", err) + } + modulePath := modfile.ModulePath(b) + + if want := "golang.org/x/tools"; modulePath != want { + t.Skipf("skipping test: %s module path is %q, not %q", modFilePath, modulePath, want) + } +}