dashboard, cmd/coordinator: add controls to skip subrepo dirs in GOPATH mode
This change adds controls to the dashboard package that makes it
possible to skip certain directories in subrepos from being tested
in GOPATH mode.
It uses this control mechanism to skip the gopls directory in x/tools
from being tested in GOPATH mode. That directory and everything inside
is developed with support for module mode only. All other packages
in x/tools continue to be supported both in module and GOPATH modes
for the current and previous releases, as per release policy¹.
¹ https://golang.org/doc/devel/release.html#policy
Fixes golang/go#34190
Change-Id: Icdb4d0e1419f69c84c9a3f9a33727a09a35599f4
Reviewed-on: https://go-review.googlesource.com/c/build/+/194397
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/cmd/coordinator/coordinator.go b/cmd/coordinator/coordinator.go
index 33c8966..d7bd72e 100644
--- a/cmd/coordinator/coordinator.go
+++ b/cmd/coordinator/coordinator.go
@@ -2372,6 +2372,12 @@
return nil, fmt.Errorf("failed to determine go env GOMOD value: %v", err)
}
+ // patterns defines import path patterns to provide to 'go test'.
+ // The starting default value is "golang.org/x/{repo}/...".
+ patterns := []string{
+ importPathOfRepo(st.SubName) + "/...",
+ }
+
// The next large step diverges into two code paths,
// one for module mode and another for GOPATH mode.
//
@@ -2419,7 +2425,7 @@
return nil, fmt.Errorf("exec go list on buildlet: %v", err)
}
if rErr != nil {
- fmt.Fprintf(st, "go list error:\n%s", &buf)
+ fmt.Fprintf(st, "go list error: %v\noutput:\n%s", rErr, &buf)
return rErr, nil
}
for _, p := range strings.Fields(buf.String()) {
@@ -2469,7 +2475,35 @@
}
}
- // TODO(dmitshur): Allow dashboard to filter out directories to test. See golang.org/issue/34190.
+ // The dashboard offers control over what packages to test in GOPATH mode.
+ // Compute a list of packages by calling 'go list'. See golang.org/issue/34190.
+ {
+ repoPath := importPathOfRepo(st.SubName)
+ var buf bytes.Buffer
+ sp := st.CreateSpan("listing_subrepo_packages", st.SubName)
+ rErr, err := st.bc.Exec(path.Join("go", "bin", "go"), buildlet.ExecOpts{
+ Output: &buf,
+ Dir: "gopath/src/" + repoPath,
+ ExtraEnv: append(st.conf.Env(), "GOROOT="+goroot, "GOPATH="+gopath),
+ Path: []string{"$WORKDIR/go/bin", "$PATH"},
+ Args: []string{"list", repoPath + "/..."},
+ })
+ sp.Done(firstNonNil(err, rErr))
+ if err != nil {
+ return nil, fmt.Errorf("exec go list on buildlet: %v", err)
+ }
+ if rErr != nil {
+ fmt.Fprintf(st, "go list error: %v\noutput:\n%s", rErr, &buf)
+ return rErr, nil
+ }
+ patterns = nil
+ for _, importPath := range strings.Fields(buf.String()) {
+ if !st.conf.ShouldTestPackageInGOPATHMode(importPath) {
+ continue
+ }
+ patterns = append(patterns, importPath)
+ }
+ }
}
// TODO(dmitshur): For some subrepos, test in both modes. See golang.org/issue/30233.
@@ -2491,7 +2525,7 @@
if st.conf.IsRace() {
args = append(args, "-race")
}
- args = append(args, importPathOfRepo(st.SubName)+"/...")
+ args = append(args, patterns...)
return st.bc.Exec(path.Join("go", "bin", "go"), buildlet.ExecOpts{
Debug: true, // make buildlet print extra debug in output for failures
@@ -2527,6 +2561,16 @@
return env.GoMod, err
}
+// firstNonNil returns the first non-nil error, or nil otherwise.
+func firstNonNil(errs ...error) error {
+ for _, e := range errs {
+ if e != nil {
+ return e
+ }
+ }
+ return nil
+}
+
// moduleProxy returns the GOPROXY environment value to use for module-enabled
// tests.
//
diff --git a/dashboard/builders.go b/dashboard/builders.go
index 1327d7d..5384bfd 100644
--- a/dashboard/builders.go
+++ b/dashboard/builders.go
@@ -733,7 +733,30 @@
case "oauth2", "build", "perf", "website":
env = append(env, "GO111MODULE=on")
}
- return
+ return env
+}
+
+// ShouldTestPackageInGOPATHMode is used to control whether the package
+// with the specified import path should be tested in GOPATH mode.
+//
+// When running tests for all golang.org/* repositories in GOPATH mode,
+// this method is called repeatedly with the full import path of each
+// package that is found and is being considered for testing in GOPATH
+// mode. It's not used and has no effect on import paths in the main
+// "go" repository. It has no effect on tests done in module mode.
+//
+// When considering making changes here, keep the release policy in mind:
+//
+// https://golang.org/doc/devel/release.html#policy
+//
+func (*BuildConfig) ShouldTestPackageInGOPATHMode(importPath string) bool {
+ if importPath == "golang.org/x/tools/gopls" ||
+ strings.HasPrefix(importPath, "golang.org/x/tools/gopls/") {
+ // Don't test golang.org/x/tools/gopls/... in GOPATH mode.
+ return false
+ }
+ // Test everything else in GOPATH mode as usual.
+ return true
}
func (c *BuildConfig) IsReverse() bool { return c.hostConf().IsReverse }
diff --git a/dashboard/builders_test.go b/dashboard/builders_test.go
index 6a029d9..c004433 100644
--- a/dashboard/builders_test.go
+++ b/dashboard/builders_test.go
@@ -595,3 +595,30 @@
}
}
}
+
+func TestShouldTestPackageInGOPATHMode(t *testing.T) {
+ // This function doesn't change behavior depending on the builder
+ // at this time, so just use a common one.
+ bc, ok := Builders["linux-amd64"]
+ if !ok {
+ t.Fatal("unknown builder")
+ }
+
+ tests := []struct {
+ importPath string
+ want bool
+ }{
+ {"golang.org/x/image/bmp", true},
+ {"golang.org/x/tools/go/ast/astutil", true},
+ {"golang.org/x/tools/go/packages", true},
+ {"golang.org/x/tools", true}, // Three isn't a package there, but if there was, it should be tested.
+ {"golang.org/x/tools/gopls", false},
+ {"golang.org/x/tools/gopls/internal/foobar", false},
+ }
+ for _, tt := range tests {
+ got := bc.ShouldTestPackageInGOPATHMode(tt.importPath)
+ if got != tt.want {
+ t.Errorf("ShouldTestPackageInGOPATHMode(%q) = %v; want %v", tt.importPath, got, tt.want)
+ }
+ }
+}
