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 <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: dmitshur

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
+}

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, &notInstalled) || 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)
 			}
 		})

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 = <suffix> (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
+}

src/cmd/go/testdata/script/README

@@ -377,6 +377,8 @@ The available conditions are:
 	GOOS/GOARCH supports -asan
 [buildmode:*]
 	go supports -buildmode=<suffix>
+[bzr]
+	the 'bzr' executable exists and provides the standard CLI
 [case-sensitive]
 	$WORK filesystem is case-sensitive
 [cc:*]

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

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 <[email protected]>'