Use SetPrepareCmd hook to spy on OpenInBrowser

We are now able to assert that the browse command was called with the correct URL

command/pr_test.go

@@ -2,6 +2,7 @@ package command
 
 import (
 	"os"
+	"os/exec"
 	"regexp"
 	"testing"
 
@@ -63,8 +64,12 @@ func TestPRView(t *testing.T) {
 	defer jsonFile.Close()
 	http.StubResponse(200, jsonFile)
 
-	teardown, callCount := mockOpenInBrowser()
-	defer teardown()
+	var seenCmd *exec.Cmd
+	restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable {
+		seenCmd = cmd
+		return &outputStub{}
+	})
+	defer restoreCmd()
 
 	output, err := test.RunCommand(RootCmd, "pr view")
 	if err != nil {
@@ -75,9 +80,25 @@ func TestPRView(t *testing.T) {
 		t.Errorf("command output expected got an empty string")
 	}
 
-	if *callCount != 1 {
-		t.Errorf("OpenInBrowser should be called 1 time but was called %d time(s)", *callCount)
+	if seenCmd == nil {
+		t.Fatal("expected a command to run")
 	}
+	url := seenCmd.Args[len(seenCmd.Args)-1]
+	if url != "https://github.com/OWNER/REPO/pull/10" {
+		t.Errorf("got: %q", url)
+	}
+}
+
+type outputStub struct {
+	contents []byte
+}
+
+func (s outputStub) Output() ([]byte, error) {
+	return s.contents, nil
+}
+
+func (s outputStub) Run() error {
+	return nil
 }
 
 func TestPRView_NoActiveBranch(t *testing.T) {
@@ -88,16 +109,20 @@ func TestPRView_NoActiveBranch(t *testing.T) {
 	defer jsonFile.Close()
 	http.StubResponse(200, jsonFile)
 
-	teardown, callCount := mockOpenInBrowser()
-	defer teardown()
+	var seenCmd *exec.Cmd
+	restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable {
+		seenCmd = cmd
+		return &outputStub{}
+	})
+	defer restoreCmd()
 
 	output, err := test.RunCommand(RootCmd, "pr view")
 	if err == nil || err.Error() != "the 'master' branch has no open pull requests" {
 		t.Errorf("error running command `pr view`: %v", err)
 	}
 
-	if *callCount > 0 {
-		t.Errorf("OpenInBrowser should NOT be called but was called %d time(s)", *callCount)
+	if seenCmd != nil {
+		t.Fatalf("unexpected command: %v", seenCmd.Args)
 	}
 
 	// Now run again but provide a PR number
@@ -110,22 +135,11 @@ func TestPRView_NoActiveBranch(t *testing.T) {
 		t.Errorf("command output expected got an empty string")
 	}
 
-	if *callCount != 1 {
-		t.Errorf("OpenInBrowser should be called once but was called %d time(s)", *callCount)
+	if seenCmd == nil {
+		t.Fatal("expected a command to run")
+	}
+	url := seenCmd.Args[len(seenCmd.Args)-1]
+	if url != "https://github.com/OWNER/REPO/pull/23" {
+		t.Errorf("got: %q", url)
 	}
 }
-
-func mockOpenInBrowser() (func(), *int) {
-	callCount := 0
-	originalOpenInBrowser := utils.OpenInBrowser
-	teardown := func() {
-		utils.OpenInBrowser = originalOpenInBrowser
-	}
-
-	utils.OpenInBrowser = func(_ string) error {
-		callCount++
-		return nil
-	}
-
-	return teardown, &callCount
-}

test/fixtures/prView.json

@@ -6,7 +6,7 @@
           "node": {
             "number": 10,
             "title": "Blueberries are a good fruit",
-            "url": "https://github.com/github/gh-cli/pull/10",
+            "url": "https://github.com/OWNER/REPO/pull/10",
             "headRefName": "[blueberries]"
           }
         }
@@ -19,7 +19,7 @@
         "node": {
           "number": 8,
           "title": "Strawberries are not actually berries",
-          "url": "https://github.com/github/gh-cli/pull/8",
+          "url": "https://github.com/OWNER/REPO/pull/8",
           "headRefName": "[strawberries]"
         }
       }
@@ -32,7 +32,7 @@
         "node": {
           "number": 9,
           "title": "Apples are tasty",
-          "url": "https://github.com/github/gh-cli/pull/9",
+          "url": "https://github.com/OWNER/REPO/pull/9",
           "headRefName": "[apples]"
         }
       },
@@ -40,7 +40,7 @@
         "node": {
           "number": 11,
           "title": "Figs are my favorite",
-          "url": "https://github.com/github/gh-cli/pull/1",
+          "url": "https://github.com/OWNER/REPO/pull/1",
           "headRefName": "[figs]"
         }
       }

utils/utils.go

@@ -27,7 +27,7 @@ func ConcatPaths(paths ...string) string {
 	return strings.Join(paths, "/")
 }
 
-var OpenInBrowser = func(url string) error {
+func OpenInBrowser(url string) error {
 	browser := os.Getenv("BROWSER")
 	if browser == "" {
 		browser = searchBrowserLauncher(runtime.GOOS)
@@ -45,7 +45,8 @@ var OpenInBrowser = func(url string) error {
 	}
 
 	endingArgs := append(browserArgs[1:], url)
-	return exec.Command(browserArgs[0], endingArgs...).Run()
+	browseCmd := exec.Command(browserArgs[0], endingArgs...)
+	return PrepareCmd(browseCmd).Run()
 }
 
 func searchBrowserLauncher(goos string) (browser string) {