Remove use of run.SetPrepareCmd in tests

internal/run/run.go

@@ -22,21 +22,6 @@ var PrepareCmd = func(cmd *exec.Cmd) Runnable {
 	return &cmdWithStderr{cmd}
 }
 
-// Deprecated: use Stub
-func SetPrepareCmd(fn func(*exec.Cmd) Runnable) func() {
-	origPrepare := PrepareCmd
-	PrepareCmd = func(cmd *exec.Cmd) Runnable {
-		// normalize git executable name for consistency in tests
-		if baseName := filepath.Base(cmd.Args[0]); baseName == "git" || baseName == "git.exe" {
-			cmd.Args[0] = "git"
-		}
-		return fn(cmd)
-	}
-	return func() {
-		PrepareCmd = origPrepare
-	}
-}
-
 // cmdWithStderr augments exec.Cmd by adding stderr to the error message
 type cmdWithStderr struct {
 	*exec.Cmd

internal/run/stub.go

@@ -3,6 +3,7 @@ package run
 import (
 	"fmt"
 	"os/exec"
+	"path/filepath"
 	"regexp"
 	"strings"
 )
@@ -12,9 +13,11 @@ type T interface {
 	Errorf(string, ...interface{})
 }
 
+// Stub installs a catch-all for all external commands invoked from gh. It returns a restore func that, when
+// invoked from tests, fails the current test if some stubs that were registered were never matched.
 func Stub() (*CommandStubber, func(T)) {
 	cs := &CommandStubber{}
-	teardown := SetPrepareCmd(func(cmd *exec.Cmd) Runnable {
+	teardown := setPrepareCmd(func(cmd *exec.Cmd) Runnable {
 		s := cs.find(cmd.Args)
 		if s == nil {
 			panic(fmt.Sprintf("no exec stub for `%s`", strings.Join(cmd.Args, " ")))
@@ -43,13 +46,33 @@ func Stub() (*CommandStubber, func(T)) {
 	}
 }
 
+func setPrepareCmd(fn func(*exec.Cmd) Runnable) func() {
+	origPrepare := PrepareCmd
+	PrepareCmd = func(cmd *exec.Cmd) Runnable {
+		// normalize git executable name for consistency in tests
+		if baseName := filepath.Base(cmd.Args[0]); baseName == "git" || baseName == "git.exe" {
+			cmd.Args[0] = "git"
+		}
+		return fn(cmd)
+	}
+	return func() {
+		PrepareCmd = origPrepare
+	}
+}
+
+// CommandStubber stubs out invocations to external commands.
 type CommandStubber struct {
 	stubs []*commandStub
 }
 
-func (cs *CommandStubber) Register(p string, exitStatus int, output string, callbacks ...CommandCallback) {
+// Register a stub for an external command. Pattern is a regular expression, output is the standard output
+// from a command. Pass callbacks to inspect raw arguments that the command was invoked with.
+func (cs *CommandStubber) Register(pattern string, exitStatus int, output string, callbacks ...CommandCallback) {
+	if len(pattern) < 1 {
+		panic("cannot use empty regexp pattern")
+	}
 	cs.stubs = append(cs.stubs, &commandStub{
-		pattern:    regexp.MustCompile(p),
+		pattern:    regexp.MustCompile(pattern),
 		exitStatus: exitStatus,
 		stdout:     output,
 		callbacks:  callbacks,
@@ -76,13 +99,15 @@ type commandStub struct {
 	callbacks  []CommandCallback
 }
 
+// Run satisfies Runnable
 func (s *commandStub) Run() error {
 	if s.exitStatus != 0 {
 		return fmt.Errorf("%s exited with status %d", s.pattern, s.exitStatus)
 	}
 	return nil
 }
 
+// Output satisfies Runnable
 func (s *commandStub) Output() ([]byte, error) {
 	if s.exitStatus != 0 {
 		return []byte(nil), fmt.Errorf("%s exited with status %d", s.pattern, s.exitStatus)

pkg/cmd/issue/create/create_test.go

@@ -7,7 +7,6 @@ import (
 	"io/ioutil"
 	"net/http"
 	"os"
-	"os/exec"
 	"strings"
 	"testing"
 
@@ -281,13 +280,14 @@ func TestIssueCreate_continueInBrowser(t *testing.T) {
 		},
 	})
 
-	var seenCmd *exec.Cmd
-	//nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub
-	restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
-		seenCmd = cmd
-		return &test.OutputStub{}
+	cs, cmdTeardown := run.Stub()
+	defer cmdTeardown(t)
+
+	cs.Register(`git rev-parse --show-toplevel`, 0, "")
+	cs.Register(`https://github\.com`, 0, "", func(args []string) {
+		url := strings.ReplaceAll(args[len(args)-1], "^", "")
+		assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=body&title=hello", url)
 	})
-	defer restoreCmd()
 
 	output, err := runCommand(http, true, `-b body`)
 	if err != nil {
@@ -296,17 +296,11 @@ func TestIssueCreate_continueInBrowser(t *testing.T) {
 
 	assert.Equal(t, "", output.String())
 	assert.Equal(t, heredoc.Doc(`
-		
+
 		Creating issue in OWNER/REPO
-		
+
 		Opening github.com/OWNER/REPO/issues/new in your browser.
 	`), output.Stderr())
-
-	if seenCmd == nil {
-		t.Fatal("expected a command to run")
-	}
-	url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "")
-	assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=body&title=hello", url)
 }
 
 func TestIssueCreate_metadata(t *testing.T) {
@@ -419,24 +413,20 @@ func TestIssueCreate_web(t *testing.T) {
 		`),
 	)
 
-	var seenCmd *exec.Cmd
-	//nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub
-	restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
-		seenCmd = cmd
-		return &test.OutputStub{}
+	cs, cmdTeardown := run.Stub()
+	defer cmdTeardown(t)
+
+	cs.Register(`git rev-parse --show-toplevel`, 0, "")
+	cs.Register(`https://github\.com`, 0, "", func(args []string) {
+		url := strings.ReplaceAll(args[len(args)-1], "^", "")
+		assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa", url)
 	})
-	defer restoreCmd()
 
 	output, err := runCommand(http, true, `--web -a @me`)
 	if err != nil {
 		t.Errorf("error running command `issue create`: %v", err)
 	}
 
-	if seenCmd == nil {
-		t.Fatal("expected a command to run")
-	}
-	url := seenCmd.Args[len(seenCmd.Args)-1]
-	assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa", url)
 	assert.Equal(t, "", output.String())
 	assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr())
 }
@@ -445,24 +435,20 @@ func TestIssueCreate_webTitleBody(t *testing.T) {
 	http := &httpmock.Registry{}
 	defer http.Verify(t)
 
-	var seenCmd *exec.Cmd
-	//nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub
-	restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
-		seenCmd = cmd
-		return &test.OutputStub{}
+	cs, cmdTeardown := run.Stub()
+	defer cmdTeardown(t)
+
+	cs.Register(`git rev-parse --show-toplevel`, 0, "")
+	cs.Register(`https://github\.com`, 0, "", func(args []string) {
+		url := strings.ReplaceAll(args[len(args)-1], "^", "")
+		assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=mybody&title=mytitle", url)
 	})
-	defer restoreCmd()
 
 	output, err := runCommand(http, true, `-w -t mytitle -b mybody`)
 	if err != nil {
 		t.Errorf("error running command `issue create`: %v", err)
 	}
 
-	if seenCmd == nil {
-		t.Fatal("expected a command to run")
-	}
-	url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "")
-	assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=mybody&title=mytitle", url)
 	assert.Equal(t, "", output.String())
 	assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr())
 }
@@ -480,24 +466,20 @@ func TestIssueCreate_webTitleBodyAtMeAssignee(t *testing.T) {
 		`),
 	)
 
-	var seenCmd *exec.Cmd
-	//nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub
-	restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
-		seenCmd = cmd
-		return &test.OutputStub{}
+	cs, cmdTeardown := run.Stub()
+	defer cmdTeardown(t)
+
+	cs.Register(`git rev-parse --show-toplevel`, 0, "")
+	cs.Register(`https://github\.com`, 0, "", func(args []string) {
+		url := strings.ReplaceAll(args[len(args)-1], "^", "")
+		assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa&body=mybody&title=mytitle", url)
 	})
-	defer restoreCmd()
 
 	output, err := runCommand(http, true, `-w -t mytitle -b mybody -a @me`)
 	if err != nil {
 		t.Errorf("error running command `issue create`: %v", err)
 	}
 
-	if seenCmd == nil {
-		t.Fatal("expected a command to run")
-	}
-	url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "")
-	assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa&body=mybody&title=mytitle", url)
 	assert.Equal(t, "", output.String())
 	assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr())
 }
@@ -580,24 +562,20 @@ func TestIssueCreate_webProject(t *testing.T) {
 			} } } }
 			`))
 
-	var seenCmd *exec.Cmd
-	//nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub
-	restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
-		seenCmd = cmd
-		return &test.OutputStub{}
+	cs, cmdTeardown := run.Stub()
+	defer cmdTeardown(t)
+
+	cs.Register(`git rev-parse --show-toplevel`, 0, "")
+	cs.Register(`https://github\.com`, 0, "", func(args []string) {
+		url := strings.ReplaceAll(args[len(args)-1], "^", "")
+		assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?projects=OWNER%2FREPO%2F1&title=Title", url)
 	})
-	defer restoreCmd()
 
 	output, err := runCommand(http, true, `-w -t Title -p Cleanup`)
 	if err != nil {
 		t.Errorf("error running command `issue create`: %v", err)
 	}
 
-	if seenCmd == nil {
-		t.Fatal("expected a command to run")
-	}
-	url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "")
-	assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?projects=OWNER%2FREPO%2F1&title=Title", url)
 	assert.Equal(t, "", output.String())
 	assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr())
 }

pkg/cmd/issue/list/list_test.go

@@ -5,8 +5,8 @@ import (
 	"encoding/json"
 	"io/ioutil"
 	"net/http"
-	"os/exec"
 	"regexp"
+	"strings"
 	"testing"
 
 	"github.com/MakeNowJust/heredoc"
@@ -235,29 +235,21 @@ func TestIssueList_web(t *testing.T) {
 	http := &httpmock.Registry{}
 	defer http.Verify(t)
 
-	var seenCmd *exec.Cmd
-	//nolint:staticcheck // SA1019 TODO: rewrite to use run.Stub
-	restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable {
-		seenCmd = cmd
-		return &test.OutputStub{}
+	cs, cmdTeardown := run.Stub()
+	defer cmdTeardown(t)
+
+	cs.Register(`https://github\.com`, 0, "", func(args []string) {
+		url := strings.ReplaceAll(args[len(args)-1], "^", "")
+		assert.Equal(t, "https://github.com/OWNER/REPO/issues?q=is%3Aissue+assignee%3Apeter+label%3Abug+label%3Adocs+author%3Ajohn+mentions%3Afrank+milestone%3Av1.1", url)
 	})
-	defer restoreCmd()
 
 	output, err := runCommand(http, true, "--web -a peter -A john -l bug -l docs -L 10 -s all --mention frank --milestone v1.1")
 	if err != nil {
 		t.Errorf("error running command `issue list` with `--web` flag: %v", err)
 	}
 
-	expectedURL := "https://github.com/OWNER/REPO/issues?q=is%3Aissue+assignee%3Apeter+label%3Abug+label%3Adocs+author%3Ajohn+mentions%3Afrank+milestone%3Av1.1"
-
 	assert.Equal(t, "", output.String())
 	assert.Equal(t, "Opening github.com/OWNER/REPO/issues in your browser.\n", output.Stderr())
-
-	if seenCmd == nil {
-		t.Fatal("expected a command to run")
-	}
-	url := seenCmd.Args[len(seenCmd.Args)-1]
-	assert.Equal(t, expectedURL, url)
 }
 
 func TestIssueList_milestoneNotFound(t *testing.T) {