From a7cbea833276454597c583751629a3e11cfa9232 Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Tue, 4 Feb 2025 19:54:25 -0500 Subject: [PATCH] cmd/go: skip bzr tests if 'bzr help' has non-zero exit code It appears to be quite easy to end up with a broken 'bzr' installation. For example, if bzr was installed via a system-wide package manager and intends to work with a system-wide Python installation, it may break if another 'python3' binary is added to PATH. If something as simple as 'bzr help' fails to exit with zero code, consider it broken and skip tests that require a working bzr binary just like if the 'bzr' binary isn't present in PATH at all. This makes these tests more robust and capable of producing useful signal in more environments. Separately from this, we'll want to restore a working bzr installation on the linux-arm64 builders, but at least there's still one on linux-amd64 builders. For #71563. Fixes #71504. Change-Id: Ia147196f12b90a0731ebbfab63b5de308212ed65 Cq-Include-Trybots: luci.golang.try:gotip-linux-arm64-longtest,gotip-linux-amd64-longtest Reviewed-on: https://go-review.googlesource.com/c/go/+/646715 Auto-Submit: Dmitri Shuralyov Reviewed-by: Ian Lance Taylor Reviewed-by: Dmitri Shuralyov LUCI-TryBot-Result: Go LUCI --- src/cmd/go/internal/vcweb/script.go | 51 +++++++++++++++++++ .../go/internal/vcweb/vcstest/vcstest_test.go | 12 ++--- src/cmd/go/scriptconds_test.go | 12 +++++ src/cmd/go/testdata/script/README | 2 + .../testdata/script/version_buildvcs_bzr.txt | 2 +- src/cmd/go/testdata/vcstest/bzr/hello.txt | 1 + 6 files changed, 73 insertions(+), 7 deletions(-) diff --git a/src/cmd/go/internal/vcweb/script.go b/src/cmd/go/internal/vcweb/script.go index 1ba9c0aff4..3342ab200c 100644 --- a/src/cmd/go/internal/vcweb/script.go +++ b/src/cmd/go/internal/vcweb/script.go @@ -32,6 +32,17 @@ import ( func newScriptEngine() *script.Engine { conds := script.DefaultConds() + add := func(name string, cond script.Cond) { + if _, ok := conds[name]; ok { + panic(fmt.Sprintf("condition %q is already registered", name)) + } + conds[name] = cond + } + lazyBool := func(summary string, f func() bool) script.Cond { + return script.OnceCondition(summary, func() (bool, error) { return f(), nil }) + } + add("bzr", lazyBool("the 'bzr' executable exists and provides the standard CLI", hasWorkingBzr)) + interrupt := func(cmd *exec.Cmd) error { return cmd.Process.Signal(os.Interrupt) } gracePeriod := 30 * time.Second // arbitrary @@ -43,6 +54,7 @@ func newScriptEngine() *script.Engine { cmds["hg"] = script.Program("hg", interrupt, gracePeriod) cmds["handle"] = scriptHandle() cmds["modzip"] = scriptModzip() + cmds["skip"] = scriptSkip() cmds["svnadmin"] = script.Program("svnadmin", interrupt, gracePeriod) cmds["svn"] = script.Program("svn", interrupt, gracePeriod) cmds["unquote"] = scriptUnquote() @@ -321,6 +333,34 @@ func scriptModzip() script.Cmd { }) } +func scriptSkip() script.Cmd { + return script.Command( + script.CmdUsage{ + Summary: "skip the current test", + Args: "[msg]", + }, + func(_ *script.State, args ...string) (script.WaitFunc, error) { + if len(args) > 1 { + return nil, script.ErrUsage + } + if len(args) == 0 { + return nil, SkipError{""} + } + return nil, SkipError{args[0]} + }) +} + +type SkipError struct { + Msg string +} + +func (s SkipError) Error() string { + if s.Msg == "" { + return "skip" + } + return s.Msg +} + func scriptUnquote() script.Cmd { return script.Command( script.CmdUsage{ @@ -343,3 +383,14 @@ func scriptUnquote() script.Cmd { return wait, nil }) } + +func hasWorkingBzr() bool { + bzr, err := exec.LookPath("bzr") + if err != nil { + return false + } + // Check that 'bzr help' exits with code 0. + // See go.dev/issue/71504 for an example where 'bzr' exists in PATH but doesn't work. + err = exec.Command(bzr, "help").Run() + return err == nil +} diff --git a/src/cmd/go/internal/vcweb/vcstest/vcstest_test.go b/src/cmd/go/internal/vcweb/vcstest/vcstest_test.go index df707d529e..67234ac20d 100644 --- a/src/cmd/go/internal/vcweb/vcstest/vcstest_test.go +++ b/src/cmd/go/internal/vcweb/vcstest/vcstest_test.go @@ -158,13 +158,13 @@ func TestScripts(t *testing.T) { if notInstalled := (vcweb.ServerNotInstalledError{}); errors.As(err, ¬Installed) || errors.Is(err, exec.ErrNotFound) { t.Skip(err) } - - // For issue #71504 ignore an error about - // bzr not being able to find dependencies. - if strings.Contains(buf.String(), "brz: ERROR: Couldn't import breezy and dependencies.") { - t.Skip("skipping test due to bzr installation problem") + if skip := (vcweb.SkipError{}); errors.As(err, &skip) { + if skip.Msg == "" { + t.Skip("SKIP") + } else { + t.Skipf("SKIP: %v", skip.Msg) + } } - t.Error(err) } }) diff --git a/src/cmd/go/scriptconds_test.go b/src/cmd/go/scriptconds_test.go index 262214f6a9..af9691ad2a 100644 --- a/src/cmd/go/scriptconds_test.go +++ b/src/cmd/go/scriptconds_test.go @@ -37,6 +37,7 @@ func scriptConditions(t *testing.T) map[string]script.Cond { } add("abscc", script.Condition("default $CC path is absolute and exists", defaultCCIsAbsolute)) + add("bzr", lazyBool("the 'bzr' executable exists and provides the standard CLI", hasWorkingBzr)) add("case-sensitive", script.OnceCondition("$WORK filesystem is case-sensitive", isCaseSensitive)) add("cc", script.PrefixCondition("go env CC = (ignoring the go/env file)", ccIs)) add("git", lazyBool("the 'git' executable exists and provides the standard CLI", hasWorkingGit)) @@ -151,3 +152,14 @@ func hasWorkingGit() bool { _, err := exec.LookPath("git") return err == nil } + +func hasWorkingBzr() bool { + bzr, err := exec.LookPath("bzr") + if err != nil { + return false + } + // Check that 'bzr help' exits with code 0. + // See go.dev/issue/71504 for an example where 'bzr' exists in PATH but doesn't work. + err = exec.Command(bzr, "help").Run() + return err == nil +} diff --git a/src/cmd/go/testdata/script/README b/src/cmd/go/testdata/script/README index 8c95945ebe..7724bc10ec 100644 --- a/src/cmd/go/testdata/script/README +++ b/src/cmd/go/testdata/script/README @@ -377,6 +377,8 @@ The available conditions are: GOOS/GOARCH supports -asan [buildmode:*] go supports -buildmode= +[bzr] + the 'bzr' executable exists and provides the standard CLI [case-sensitive] $WORK filesystem is case-sensitive [cc:*] diff --git a/src/cmd/go/testdata/script/version_buildvcs_bzr.txt b/src/cmd/go/testdata/script/version_buildvcs_bzr.txt index fc80f45677..59796d1ffa 100644 --- a/src/cmd/go/testdata/script/version_buildvcs_bzr.txt +++ b/src/cmd/go/testdata/script/version_buildvcs_bzr.txt @@ -2,8 +2,8 @@ # controlled with -buildvcs. This test focuses on Bazaar specifics. # The Git test covers common functionality. -[!exec:bzr] skip [short] skip +[!bzr] skip 'requires a working bzr client' env GOBIN=$WORK/gopath/bin env oldpath=$PATH env HOME=$WORK diff --git a/src/cmd/go/testdata/vcstest/bzr/hello.txt b/src/cmd/go/testdata/vcstest/bzr/hello.txt index 59315852f7..68465ec553 100644 --- a/src/cmd/go/testdata/vcstest/bzr/hello.txt +++ b/src/cmd/go/testdata/vcstest/bzr/hello.txt @@ -1,3 +1,4 @@ +[!bzr] skip 'requires a working bzr client' handle bzr env BZR_EMAIL='Russ Cox ' -- 2.51.0