diff --git a/command/issue_test.go b/command/issue_test.go index 982cf69eb..9a9b83868 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -10,8 +10,8 @@ import ( "strings" "testing" + "github.com/cli/cli/internal/run" "github.com/cli/cli/test" - "github.com/cli/cli/utils" ) func TestIssueStatus(t *testing.T) { @@ -229,7 +229,7 @@ func TestIssueView_web(t *testing.T) { `)) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) @@ -263,7 +263,7 @@ func TestIssueView_web_numberArgWithHash(t *testing.T) { `)) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) @@ -372,7 +372,7 @@ func TestIssueView_web_notFound(t *testing.T) { `)) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) @@ -419,7 +419,7 @@ func TestIssueView_web_urlArg(t *testing.T) { `)) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) @@ -504,7 +504,7 @@ func TestIssueCreate_web(t *testing.T) { http.StubRepoResponse("OWNER", "REPO") var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) @@ -530,7 +530,7 @@ func TestIssueCreate_webTitleBody(t *testing.T) { http.StubRepoResponse("OWNER", "REPO") var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) diff --git a/command/pr.go b/command/pr.go index 13e84aa86..7c8799248 100644 --- a/command/pr.go +++ b/command/pr.go @@ -84,7 +84,7 @@ func prStatus(cmd *cobra.Command, args []string) error { repoOverride, _ := cmd.Flags().GetString("repo") currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx, baseRepo) if err != nil && repoOverride == "" && err.Error() != "git: not on any branch" { - return err + return fmt.Errorf("could not query for pull request for current branch: %w", err) } prPayload, err := api.PullRequests(apiClient, baseRepo, currentPRNumber, currentPRHeadRef, currentUser) diff --git a/command/pr_checkout.go b/command/pr_checkout.go index fc2a743ec..5db8fbd07 100644 --- a/command/pr_checkout.go +++ b/command/pr_checkout.go @@ -6,10 +6,11 @@ import ( "os" "os/exec" + "github.com/spf13/cobra" + "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" - "github.com/cli/cli/utils" - "github.com/spf13/cobra" + "github.com/cli/cli/internal/run" ) func prCheckout(cmd *cobra.Command, args []string) error { @@ -109,7 +110,7 @@ func prCheckout(cmd *cobra.Command, args []string) error { cmd := exec.Command(args[0], args[1:]...) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - if err := utils.PrepareCmd(cmd).Run(); err != nil { + if err := run.PrepareCmd(cmd).Run(); err != nil { return err } } diff --git a/command/pr_checkout_test.go b/command/pr_checkout_test.go index e155e2a82..a66163ed1 100644 --- a/command/pr_checkout_test.go +++ b/command/pr_checkout_test.go @@ -9,8 +9,8 @@ import ( "testing" "github.com/cli/cli/context" + "github.com/cli/cli/internal/run" "github.com/cli/cli/test" - "github.com/cli/cli/utils" ) func TestPRCheckout_sameRepo(t *testing.T) { @@ -44,7 +44,7 @@ func TestPRCheckout_sameRepo(t *testing.T) { `)) ranCommands := [][]string{} - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { switch strings.Join(cmd.Args, " ") { case "git show-ref --verify -- refs/heads/feature": return &errorStub{"exit status: 1"} @@ -96,7 +96,7 @@ func TestPRCheckout_urlArg(t *testing.T) { `)) ranCommands := [][]string{} - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { switch strings.Join(cmd.Args, " ") { case "git show-ref --verify -- refs/heads/feature": return &errorStub{"exit status: 1"} @@ -145,7 +145,7 @@ func TestPRCheckout_urlArg_differentBase(t *testing.T) { `)) ranCommands := [][]string{} - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { switch strings.Join(cmd.Args, " ") { case "git show-ref --verify -- refs/heads/feature": return &errorStub{"exit status: 1"} @@ -208,7 +208,7 @@ func TestPRCheckout_branchArg(t *testing.T) { `)) ranCommands := [][]string{} - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { switch strings.Join(cmd.Args, " ") { case "git show-ref --verify -- refs/heads/feature": return &errorStub{"exit status: 1"} @@ -258,7 +258,7 @@ func TestPRCheckout_existingBranch(t *testing.T) { `)) ranCommands := [][]string{} - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { switch strings.Join(cmd.Args, " ") { case "git show-ref --verify -- refs/heads/feature": return &test.OutputStub{} @@ -311,7 +311,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) { `)) ranCommands := [][]string{} - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { switch strings.Join(cmd.Args, " ") { case "git show-ref --verify -- refs/heads/feature": return &errorStub{"exit status: 1"} @@ -364,7 +364,7 @@ func TestPRCheckout_differentRepo(t *testing.T) { `)) ranCommands := [][]string{} - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { switch strings.Join(cmd.Args, " ") { case "git config branch.feature.merge": return &errorStub{"exit status 1"} @@ -417,10 +417,10 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { `)) ranCommands := [][]string{} - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { switch strings.Join(cmd.Args, " ") { case "git config branch.feature.merge": - return &test.OutputStub{[]byte("refs/heads/feature\n")} + return &test.OutputStub{[]byte("refs/heads/feature\n"), nil} default: ranCommands = append(ranCommands, cmd.Args) return &test.OutputStub{} @@ -468,10 +468,10 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) { `)) ranCommands := [][]string{} - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { switch strings.Join(cmd.Args, " ") { case "git config branch.feature.merge": - return &test.OutputStub{[]byte("refs/heads/feature\n")} + return &test.OutputStub{[]byte("refs/heads/feature\n"), nil} default: ranCommands = append(ranCommands, cmd.Args) return &test.OutputStub{} @@ -519,7 +519,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) { `)) ranCommands := [][]string{} - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { switch strings.Join(cmd.Args, " ") { case "git config branch.feature.merge": return &errorStub{"exit status 1"} diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 01b5516d3..72a439677 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -9,6 +9,7 @@ import ( "github.com/cli/cli/context" "github.com/cli/cli/git" + "github.com/cli/cli/test" ) func TestPRCreate(t *testing.T) { @@ -29,7 +30,7 @@ func TestPRCreate(t *testing.T) { } } } } `)) - cs, cmdTeardown := initCmdStubber() + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() cs.Stub("") // git config --get-regexp (determineTrackingBranch) @@ -80,7 +81,7 @@ func TestPRCreate_alreadyExists(t *testing.T) { ] } } } } `)) - cs, cmdTeardown := initCmdStubber() + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() cs.Stub("") // git config --get-regexp (determineTrackingBranch) @@ -114,7 +115,7 @@ func TestPRCreate_alreadyExistsDifferentBase(t *testing.T) { `)) http.StubResponse(200, bytes.NewBufferString("{}")) - cs, cmdTeardown := initCmdStubber() + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() cs.Stub("") // git config --get-regexp (determineTrackingBranch) @@ -138,7 +139,7 @@ func TestPRCreate_web(t *testing.T) { ] } } } } `)) - cs, cmdTeardown := initCmdStubber() + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() cs.Stub("") // git config --get-regexp (determineTrackingBranch) @@ -179,7 +180,7 @@ func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { } } } } `)) - cs, cmdTeardown := initCmdStubber() + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() cs.Stub("") // git config --get-regexp (determineTrackingBranch) @@ -251,7 +252,7 @@ func TestPRCreate_cross_repo_same_branch(t *testing.T) { } } } } `)) - cs, cmdTeardown := initCmdStubber() + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() cs.Stub("") // git config --get-regexp (determineTrackingBranch) @@ -306,7 +307,7 @@ func TestPRCreate_survey_defaults_multicommit(t *testing.T) { } } } } `)) - cs, cmdTeardown := initCmdStubber() + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() cs.Stub("") // git config --get-regexp (determineTrackingBranch) @@ -382,7 +383,7 @@ func TestPRCreate_survey_defaults_monocommit(t *testing.T) { } } } } `)) - cs, cmdTeardown := initCmdStubber() + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() cs.Stub("") // git config --get-regexp (determineTrackingBranch) @@ -459,7 +460,7 @@ func TestPRCreate_survey_autofill(t *testing.T) { } } } } `)) - cs, cmdTeardown := initCmdStubber() + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() cs.Stub("") // git config --get-regexp (determineTrackingBranch) @@ -504,7 +505,7 @@ func TestPRCreate_defaults_error_autofill(t *testing.T) { http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - cs, cmdTeardown := initCmdStubber() + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() cs.Stub("") // git config --get-regexp (determineTrackingBranch) @@ -522,7 +523,7 @@ func TestPRCreate_defaults_error_web(t *testing.T) { http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - cs, cmdTeardown := initCmdStubber() + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() cs.Stub("") // git config --get-regexp (determineTrackingBranch) @@ -549,7 +550,7 @@ func TestPRCreate_defaults_error_interactive(t *testing.T) { } } } } `)) - cs, cmdTeardown := initCmdStubber() + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() cs.Stub("") // git config --get-regexp (determineTrackingBranch) @@ -588,7 +589,7 @@ func TestPRCreate_defaults_error_interactive(t *testing.T) { } func Test_determineTrackingBranch_empty(t *testing.T) { - cs, cmdTeardown := initCmdStubber() + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() remotes := context.Remotes{} @@ -603,7 +604,7 @@ func Test_determineTrackingBranch_empty(t *testing.T) { } func Test_determineTrackingBranch_noMatch(t *testing.T) { - cs, cmdTeardown := initCmdStubber() + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() remotes := context.Remotes{ @@ -630,7 +631,7 @@ deadb00f refs/remotes/origin/feature`) // git show-ref --verify (ShowRefs) } func Test_determineTrackingBranch_hasMatch(t *testing.T) { - cs, cmdTeardown := initCmdStubber() + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() remotes := context.Remotes{ @@ -663,7 +664,7 @@ deadbeef refs/remotes/upstream/feature`) // git show-ref --verify (ShowRefs) } func Test_determineTrackingBranch_respectTrackingConfig(t *testing.T) { - cs, cmdTeardown := initCmdStubber() + cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() remotes := context.Remotes{ diff --git a/command/pr_test.go b/command/pr_test.go index 54e08dbfe..73d58bfb8 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -11,8 +11,8 @@ import ( "strings" "testing" + "github.com/cli/cli/internal/run" "github.com/cli/cli/test" - "github.com/cli/cli/utils" "github.com/google/shlex" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -108,11 +108,11 @@ func TestPRStatus_fork(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - defer utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { switch strings.Join(cmd.Args, " ") { case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`: return &test.OutputStub{[]byte(`branch.blueberries.remote origin -branch.blueberries.merge refs/heads/blueberries`)} +branch.blueberries.merge refs/heads/blueberries`), nil} default: panic("not implemented") } @@ -432,7 +432,7 @@ func TestPRView_previewCurrentBranch(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { return &test.OutputStub{} }) defer restoreCmd() @@ -460,7 +460,7 @@ func TestPRView_previewCurrentBranchWithEmptyBody(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { return &test.OutputStub{} }) defer restoreCmd() @@ -488,7 +488,7 @@ func TestPRView_web_currentBranch(t *testing.T) { http.StubResponse(200, jsonFile) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { switch strings.Join(cmd.Args, " ") { case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`: return &test.OutputStub{} @@ -526,7 +526,7 @@ func TestPRView_web_noResultsForBranch(t *testing.T) { http.StubResponse(200, jsonFile) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { switch strings.Join(cmd.Args, " ") { case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`: return &test.OutputStub{} @@ -559,7 +559,7 @@ func TestPRView_web_numberArg(t *testing.T) { `)) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) @@ -591,7 +591,7 @@ func TestPRView_web_numberArgWithHash(t *testing.T) { `)) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) @@ -622,7 +622,7 @@ func TestPRView_web_urlArg(t *testing.T) { `)) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) @@ -656,7 +656,7 @@ func TestPRView_web_branchArg(t *testing.T) { `)) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) @@ -691,7 +691,7 @@ func TestPRView_web_branchWithOwnerArg(t *testing.T) { `)) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) diff --git a/command/repo.go b/command/repo.go index e4899cf4c..959cfbaec 100644 --- a/command/repo.go +++ b/command/repo.go @@ -13,6 +13,7 @@ import ( "github.com/cli/cli/api" "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/internal/run" "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -100,7 +101,7 @@ func repoClone(cmd *cobra.Command, args []string) error { cloneCmd.Stdin = os.Stdin cloneCmd.Stdout = os.Stdout cloneCmd.Stderr = os.Stderr - return utils.PrepareCmd(cloneCmd).Run() + return run.PrepareCmd(cloneCmd).Run() } func repoCreate(cmd *cobra.Command, args []string) error { @@ -200,7 +201,7 @@ func repoCreate(cmd *cobra.Command, args []string) error { remoteAdd := git.GitCommand("remote", "add", "origin", remoteURL) remoteAdd.Stdout = os.Stdout remoteAdd.Stderr = os.Stderr - err = utils.PrepareCmd(remoteAdd).Run() + err = run.PrepareCmd(remoteAdd).Run() if err != nil { return err } @@ -220,14 +221,14 @@ func repoCreate(cmd *cobra.Command, args []string) error { gitInit := git.GitCommand("init", path) gitInit.Stdout = os.Stdout gitInit.Stderr = os.Stderr - err = utils.PrepareCmd(gitInit).Run() + err = run.PrepareCmd(gitInit).Run() if err != nil { return err } gitRemoteAdd := git.GitCommand("-C", path, "remote", "add", "origin", remoteURL) gitRemoteAdd.Stdout = os.Stdout gitRemoteAdd.Stderr = os.Stderr - err = utils.PrepareCmd(gitRemoteAdd).Run() + err = run.PrepareCmd(gitRemoteAdd).Run() if err != nil { return err } @@ -367,7 +368,7 @@ func repoFork(cmd *cobra.Command, args []string) error { cloneCmd.Stdin = os.Stdin cloneCmd.Stdout = os.Stdout cloneCmd.Stderr = os.Stderr - err = utils.PrepareCmd(cloneCmd).Run() + err = run.PrepareCmd(cloneCmd).Run() if err != nil { return fmt.Errorf("failed to clone fork: %w", err) } diff --git a/command/repo_test.go b/command/repo_test.go index 1d2e303a0..1b9aa2642 100644 --- a/command/repo_test.go +++ b/command/repo_test.go @@ -11,8 +11,8 @@ import ( "time" "github.com/cli/cli/context" + "github.com/cli/cli/internal/run" "github.com/cli/cli/test" - "github.com/cli/cli/utils" ) func TestRepoFork_already_forked(t *testing.T) { @@ -140,7 +140,7 @@ func TestRepoFork_in_parent_yes(t *testing.T) { defer http.StubWithFixture(200, "forkResult.json")() var seenCmds []*exec.Cmd - defer utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmds = append(seenCmds, cmd) return &test.OutputStub{} })() @@ -172,7 +172,7 @@ func TestRepoFork_outside_yes(t *testing.T) { defer http.StubWithFixture(200, "forkResult.json")() var seenCmd *exec.Cmd - defer utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} })() @@ -197,7 +197,7 @@ func TestRepoFork_outside_survey_yes(t *testing.T) { defer http.StubWithFixture(200, "forkResult.json")() var seenCmd *exec.Cmd - defer utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} })() @@ -229,7 +229,7 @@ func TestRepoFork_outside_survey_no(t *testing.T) { defer http.StubWithFixture(200, "forkResult.json")() cmdRun := false - defer utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { cmdRun = true return &test.OutputStub{} })() @@ -265,7 +265,7 @@ func TestRepoFork_in_parent_survey_yes(t *testing.T) { defer http.StubWithFixture(200, "forkResult.json")() var seenCmds []*exec.Cmd - defer utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmds = append(seenCmds, cmd) return &test.OutputStub{} })() @@ -306,7 +306,7 @@ func TestRepoFork_in_parent_survey_no(t *testing.T) { defer http.StubWithFixture(200, "forkResult.json")() cmdRun := false - defer utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { cmdRun = true return &test.OutputStub{} })() @@ -364,7 +364,7 @@ func TestRepoClone(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) @@ -404,7 +404,7 @@ func TestRepoCreate(t *testing.T) { `)) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) @@ -469,7 +469,7 @@ func TestRepoCreate_org(t *testing.T) { `)) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) @@ -534,7 +534,7 @@ func TestRepoCreate_orgWithTeam(t *testing.T) { `)) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) @@ -585,7 +585,7 @@ func TestRepoView_web(t *testing.T) { `)) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) @@ -618,7 +618,7 @@ func TestRepoView_web_ownerRepo(t *testing.T) { `)) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) @@ -650,7 +650,7 @@ func TestRepoView_web_fullURL(t *testing.T) { { } `)) var seenCmd *exec.Cmd - restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd return &test.OutputStub{} }) diff --git a/command/testing.go b/command/testing.go index 0221ce8cf..8ab97fc81 100644 --- a/command/testing.go +++ b/command/testing.go @@ -3,7 +3,6 @@ package command import ( "errors" "fmt" - "os/exec" "reflect" "github.com/AlecAivazis/survey/v2" @@ -11,43 +10,8 @@ import ( "github.com/cli/cli/api" "github.com/cli/cli/context" - "github.com/cli/cli/test" - "github.com/cli/cli/utils" ) -// TODO this is split between here and test/helpers.go. I did that because otherwise our test -// package would have to import utils (for utils.Runnable) which felt wrong. while utils_test -// currently doesn't import test helpers, i don't see why that ought to be precluded. -// I'm wondering if this is a case for having a global package just for storing Interfaces. -type CmdStubber struct { - Stubs []*test.OutputStub - Count int - Calls []*exec.Cmd -} - -func initCmdStubber() (*CmdStubber, func()) { - cs := CmdStubber{} - teardown := utils.SetPrepareCmd(createStubbedPrepareCmd(&cs)) - return &cs, teardown -} - -func (cs *CmdStubber) Stub(desiredOutput string) { - // TODO maybe have some kind of command mapping but going simple for now - cs.Stubs = append(cs.Stubs, &test.OutputStub{[]byte(desiredOutput)}) -} - -func createStubbedPrepareCmd(cs *CmdStubber) func(*exec.Cmd) utils.Runnable { - return func(cmd *exec.Cmd) utils.Runnable { - cs.Calls = append(cs.Calls, cmd) - call := cs.Count - cs.Count += 1 - if call >= len(cs.Stubs) { - panic(fmt.Sprintf("more execs than stubs. most recent call: %v", cmd)) - } - return cs.Stubs[call] - } -} - type askStubber struct { Asks [][]*survey.Question Count int diff --git a/context/context.go b/context/context.go index 27744a08a..eb1ace0eb 100644 --- a/context/context.go +++ b/context/context.go @@ -210,7 +210,7 @@ func (c *fsContext) Branch() (string, error) { currentBranch, err := git.CurrentBranch() if err != nil { - return "", err + return "", fmt.Errorf("could not determine current branch: %w", err) } c.branch = currentBranch diff --git a/git/git.go b/git/git.go index ce32742a4..360917eab 100644 --- a/git/git.go +++ b/git/git.go @@ -10,7 +10,7 @@ import ( "regexp" "strings" - "github.com/cli/cli/utils" + "github.com/cli/cli/internal/run" ) // Ref represents a git commit reference @@ -33,7 +33,7 @@ func (r TrackingRef) String() string { func ShowRefs(ref ...string) ([]Ref, error) { args := append([]string{"show-ref", "--verify", "--"}, ref...) showRef := exec.Command("git", args...) - output, err := utils.PrepareCmd(showRef).Output() + output, err := run.PrepareCmd(showRef).Output() var refs []Ref for _, line := range outputLines(output) { @@ -52,25 +52,35 @@ func ShowRefs(ref ...string) ([]Ref, error) { // CurrentBranch reads the checked-out branch for the git repository func CurrentBranch() (string, error) { - // we avoid using `git branch --show-current` for compatibility with git < 2.22 - branchCmd := exec.Command("git", "rev-parse", "--abbrev-ref", "HEAD") - output, err := utils.PrepareCmd(branchCmd).Output() - branchName := firstLine(output) - if err == nil && branchName == "HEAD" { - return "", errors.New("git: not on any branch") + refCmd := GitCommand("symbolic-ref", "--quiet", "--short", "HEAD") + + output, err := run.PrepareCmd(refCmd).Output() + if err == nil { + // Found the branch name + return firstLine(output), nil } - return branchName, err + + var cmdErr *run.CmdError + if errors.As(err, &cmdErr) { + if cmdErr.Stderr.Len() == 0 { + // Detached head + return "", errors.New("git: not on any branch") + } + } + + // Unknown error + return "", err } func listRemotes() ([]string, error) { remoteCmd := exec.Command("git", "remote", "-v") - output, err := utils.PrepareCmd(remoteCmd).Output() + output, err := run.PrepareCmd(remoteCmd).Output() return outputLines(output), err } func Config(name string) (string, error) { configCmd := exec.Command("git", "config", name) - output, err := utils.PrepareCmd(configCmd).Output() + output, err := run.PrepareCmd(configCmd).Output() if err != nil { return "", fmt.Errorf("unknown config key: %s", name) } @@ -85,7 +95,7 @@ var GitCommand = func(args ...string) *exec.Cmd { func UncommittedChangeCount() (int, error) { statusCmd := GitCommand("status", "--porcelain") - output, err := utils.PrepareCmd(statusCmd).Output() + output, err := run.PrepareCmd(statusCmd).Output() if err != nil { return 0, err } @@ -112,7 +122,7 @@ func Commits(baseRef, headRef string) ([]*Commit, error) { "-c", "log.ShowSignature=false", "log", "--pretty=format:%H,%s", "--cherry", fmt.Sprintf("%s...%s", baseRef, headRef)) - output, err := utils.PrepareCmd(logCmd).Output() + output, err := run.PrepareCmd(logCmd).Output() if err != nil { return []*Commit{}, err } @@ -140,7 +150,7 @@ func Commits(baseRef, headRef string) ([]*Commit, error) { func CommitBody(sha string) (string, error) { showCmd := GitCommand("-c", "log.ShowSignature=false", "show", "-s", "--pretty=format:%b", sha) - output, err := utils.PrepareCmd(showCmd).Output() + output, err := run.PrepareCmd(showCmd).Output() if err != nil { return "", err } @@ -152,7 +162,7 @@ func Push(remote string, ref string) error { pushCmd := GitCommand("push", "--set-upstream", remote, ref) pushCmd.Stdout = os.Stdout pushCmd.Stderr = os.Stderr - return utils.PrepareCmd(pushCmd).Run() + return run.PrepareCmd(pushCmd).Run() } type BranchConfig struct { @@ -165,7 +175,7 @@ type BranchConfig struct { func ReadBranchConfig(branch string) (cfg BranchConfig) { prefix := regexp.QuoteMeta(fmt.Sprintf("branch.%s.", branch)) configCmd := GitCommand("config", "--get-regexp", fmt.Sprintf("^%s(remote|merge)$", prefix)) - output, err := utils.PrepareCmd(configCmd).Output() + output, err := run.PrepareCmd(configCmd).Output() if err != nil { return } @@ -200,7 +210,7 @@ func isFilesystemPath(p string) bool { // ToplevelDir returns the top-level directory path of the current repository func ToplevelDir() (string, error) { showCmd := exec.Command("git", "rev-parse", "--show-toplevel") - output, err := utils.PrepareCmd(showCmd).Output() + output, err := run.PrepareCmd(showCmd).Output() return firstLine(output), err } diff --git a/git/git_test.go b/git/git_test.go index 9e3f9a333..b73104c95 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -4,8 +4,8 @@ import ( "os/exec" "testing" + "github.com/cli/cli/internal/run" "github.com/cli/cli/test" - "github.com/cli/cli/utils" ) func Test_UncommittedChangeCount(t *testing.T) { @@ -20,14 +20,14 @@ func Test_UncommittedChangeCount(t *testing.T) { c{Label: "untracked file", Expected: 2, Output: " M poem.txt\n?? new.txt"}, } - teardown := utils.SetPrepareCmd(func(*exec.Cmd) utils.Runnable { + teardown := run.SetPrepareCmd(func(*exec.Cmd) run.Runnable { return &test.OutputStub{} }) defer teardown() for _, v := range cases { - _ = utils.SetPrepareCmd(func(*exec.Cmd) utils.Runnable { - return &test.OutputStub{[]byte(v.Output)} + _ = run.SetPrepareCmd(func(*exec.Cmd) run.Runnable { + return &test.OutputStub{[]byte(v.Output), nil} }) ucc, _ := UncommittedChangeCount() @@ -36,3 +36,62 @@ func Test_UncommittedChangeCount(t *testing.T) { } } } + +func Test_CurrentBranch(t *testing.T) { + cs, teardown := test.InitCmdStubber() + defer teardown() + + expected := "branch-name" + + cs.Stub(expected) + + result, err := CurrentBranch() + if err != nil { + t.Errorf("got unexpected error: %w", err) + } + if len(cs.Calls) != 1 { + t.Errorf("expected 1 git call, saw %d", len(cs.Calls)) + } + if result != expected { + t.Errorf("unexpected branch name: %s instead of %s", result, expected) + } +} + +func Test_CurrentBranch_detached_head(t *testing.T) { + cs, teardown := test.InitCmdStubber() + defer teardown() + + cs.StubError("") + + _, err := CurrentBranch() + if err == nil { + t.Errorf("expected an error") + } + expectedError := "git: not on any branch" + if err.Error() != expectedError { + t.Errorf("got unexpected error: %s instead of %s", err.Error(), expectedError) + } + if len(cs.Calls) != 1 { + t.Errorf("expected 1 git call, saw %d", len(cs.Calls)) + } +} + +func Test_CurrentBranch_unexpected_error(t *testing.T) { + cs, teardown := test.InitCmdStubber() + defer teardown() + + cs.StubError("lol") + + expectedError := "lol\nstub: lol" + + _, err := CurrentBranch() + if err == nil { + t.Errorf("expected an error") + } + if err.Error() != expectedError { + t.Errorf("got unexpected error: %s instead of %s", err.Error(), expectedError) + } + if len(cs.Calls) != 1 { + t.Errorf("expected 1 git call, saw %d", len(cs.Calls)) + } +} diff --git a/git/remote.go b/git/remote.go index 9a8b3fccb..6c8608da9 100644 --- a/git/remote.go +++ b/git/remote.go @@ -6,7 +6,7 @@ import ( "regexp" "strings" - "github.com/cli/cli/utils" + "github.com/cli/cli/internal/run" ) var remoteRE = regexp.MustCompile(`(.+)\s+(.+)\s+\((push|fetch)\)`) @@ -74,7 +74,7 @@ func parseRemotes(gitRemotes []string) (remotes RemoteSet) { // AddRemote adds a new git remote and auto-fetches objects from it func AddRemote(name, u string) (*Remote, error) { addCmd := exec.Command("git", "remote", "add", "-f", name, u) - err := utils.PrepareCmd(addCmd).Run() + err := run.PrepareCmd(addCmd).Run() if err != nil { return nil, err } diff --git a/utils/prepare_cmd.go b/internal/run/run.go similarity index 99% rename from utils/prepare_cmd.go rename to internal/run/run.go index 6b354cb80..67de76fa2 100644 --- a/utils/prepare_cmd.go +++ b/internal/run/run.go @@ -1,4 +1,4 @@ -package utils +package run import ( "bytes" diff --git a/test/helpers.go b/test/helpers.go index c4c4f976b..556b27157 100644 --- a/test/helpers.go +++ b/test/helpers.go @@ -1,80 +1,71 @@ package test import ( + "bytes" + "errors" "fmt" - "io/ioutil" - "os" "os/exec" - "path/filepath" "regexp" "testing" + + "github.com/cli/cli/internal/run" ) // OutputStub implements a simple utils.Runnable type OutputStub struct { - Out []byte + Out []byte + Error error } func (s OutputStub) Output() ([]byte, error) { + if s.Error != nil { + return s.Out, s.Error + } return s.Out, nil } func (s OutputStub) Run() error { + if s.Error != nil { + return s.Error + } return nil } -type TempGitRepo struct { - Remote string - TearDown func() +type CmdStubber struct { + Stubs []*OutputStub + Count int + Calls []*exec.Cmd } -func UseTempGitRepo() *TempGitRepo { - pwd, _ := os.Getwd() - oldEnv := make(map[string]string) - overrideEnv := func(name, value string) { - oldEnv[name] = os.Getenv(name) - os.Setenv(name, value) - } +func InitCmdStubber() (*CmdStubber, func()) { + cs := CmdStubber{} + teardown := run.SetPrepareCmd(createStubbedPrepareCmd(&cs)) + return &cs, teardown +} - remotePath := filepath.Join(pwd, "..", "test", "fixtures", "test.git") - home, err := ioutil.TempDir("", "test-repo") - if err != nil { - panic(err) - } +func (cs *CmdStubber) Stub(desiredOutput string) { + // TODO maybe have some kind of command mapping but going simple for now + cs.Stubs = append(cs.Stubs, &OutputStub{[]byte(desiredOutput), nil}) +} - overrideEnv("HOME", home) - overrideEnv("XDG_CONFIG_HOME", "") - overrideEnv("XDG_CONFIG_DIRS", "") +func (cs *CmdStubber) StubError(errText string) { + // TODO support error types beyond CmdError + stderrBuff := bytes.NewBufferString(errText) + args := []string{"stub"} // TODO make more real? + err := errors.New(errText) + cs.Stubs = append(cs.Stubs, &OutputStub{[]byte{}, &run.CmdError{stderrBuff, args, err}}) +} - targetPath := filepath.Join(home, "test.git") - cmd := exec.Command("git", "clone", remotePath, targetPath) - if output, err := cmd.Output(); err != nil { - panic(fmt.Errorf("error running %s\n%s\n%s", cmd, err, output)) - } - - if err = os.Chdir(targetPath); err != nil { - panic(err) - } - - // Our libs expect the origin to be a github url - cmd = exec.Command("git", "remote", "set-url", "origin", "https://github.com/github/FAKE-GITHUB-REPO-NAME") - if output, err := cmd.Output(); err != nil { - panic(fmt.Errorf("error running %s\n%s\n%s", cmd, err, output)) - } - - tearDown := func() { - if err := os.Chdir(pwd); err != nil { - panic(err) - } - for name, value := range oldEnv { - os.Setenv(name, value) - } - if err = os.RemoveAll(home); err != nil { - panic(err) +func createStubbedPrepareCmd(cs *CmdStubber) func(*exec.Cmd) run.Runnable { + return func(cmd *exec.Cmd) run.Runnable { + cs.Calls = append(cs.Calls, cmd) + call := cs.Count + cs.Count += 1 + if call >= len(cs.Stubs) { + panic(fmt.Sprintf("more execs than stubs. most recent call: %v", cmd)) } + return cs.Stubs[call] } - - return &TempGitRepo{Remote: remotePath, TearDown: tearDown} } func ExpectLines(t *testing.T, output string, lines ...string) { diff --git a/utils/utils.go b/utils/utils.go index 77c643eba..4510db793 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -8,6 +8,7 @@ import ( "github.com/briandowns/spinner" "github.com/charmbracelet/glamour" + "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/browser" ) @@ -17,7 +18,7 @@ func OpenInBrowser(url string) error { if err != nil { return err } - return PrepareCmd(browseCmd).Run() + return run.PrepareCmd(browseCmd).Run() } func RenderMarkdown(text string) (string, error) {