From 39bd9aafdcfe60cfdc3b78ef98f315f5f898e918 Mon Sep 17 00:00:00 2001 From: Parth <76231594+pxrth9@users.noreply.github.com> Date: Wed, 24 Nov 2021 10:50:25 -0500 Subject: [PATCH] pr merge: allow deleting remote branch if local branch does not exist (#4769) --- pkg/cmd/pr/merge/merge.go | 23 +++++++++++------------ pkg/cmd/pr/merge/merge_test.go | 34 ++++++++++++++++++++++++++-------- 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index 584a535b6..e356f17ec 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -216,6 +216,10 @@ func mergeRun(opts *MergeOptions) error { deleteBranch := opts.DeleteBranch crossRepoPR := pr.HeadRepositoryOwner.Login != baseRepo.RepoOwner() autoMerge := opts.AutoMergeEnable && !isImmediatelyMergeable(pr.MergeStateStatus) + localBranchExists := false + if opts.CanDeleteLocalBranch { + localBranchExists = git.HasLocalBranch(pr.HeadRefName) + } if !isPRAlreadyMerged { payload := mergePayload{ @@ -236,7 +240,7 @@ func mergeRun(opts *MergeOptions) error { if err != nil { return err } - deleteBranch, err = deleteBranchSurvey(opts, crossRepoPR) + deleteBranch, err = deleteBranchSurvey(opts, crossRepoPR, localBranchExists) if err != nil { return err } @@ -317,7 +321,7 @@ func mergeRun(opts *MergeOptions) error { branchSwitchString := "" - if opts.CanDeleteLocalBranch { + if opts.CanDeleteLocalBranch && localBranchExists { currentBranch, err := opts.Branch() if err != nil { return err @@ -335,20 +339,15 @@ func mergeRun(opts *MergeOptions) error { } } - localBranchExists := git.HasLocalBranch(pr.HeadRefName) - if localBranchExists { - err = git.DeleteLocalBranch(pr.HeadRefName) - if err != nil { - err = fmt.Errorf("failed to delete local branch %s: %w", cs.Cyan(pr.HeadRefName), err) - return err - } + if err := git.DeleteLocalBranch(pr.HeadRefName); err != nil { + err = fmt.Errorf("failed to delete local branch %s: %w", cs.Cyan(pr.HeadRefName), err) + return err } if branchToSwitchTo != "" { branchSwitchString = fmt.Sprintf(" and switched to branch %s", cs.Cyan(branchToSwitchTo)) } } - if !isPRAlreadyMerged { err = api.BranchDeleteRemote(apiClient, baseRepo, pr.HeadRefName) var httpErr api.HTTPError @@ -401,10 +400,10 @@ func mergeMethodSurvey(baseRepo *api.Repository) (PullRequestMergeMethod, error) return mergeOpts[result].method, err } -func deleteBranchSurvey(opts *MergeOptions, crossRepoPR bool) (bool, error) { +func deleteBranchSurvey(opts *MergeOptions, crossRepoPR, localBranchExists bool) (bool, error) { if !crossRepoPR && !opts.IsDeleteBranchIndicated { var message string - if opts.CanDeleteLocalBranch { + if opts.CanDeleteLocalBranch && localBranchExists { message = "Delete the branch locally and on GitHub?" } else { message = "Delete the branch on GitHub?" diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go index 6d1f50558..08c959546 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -274,9 +274,11 @@ func TestPrMerge(t *testing.T) { assert.NotContains(t, input, "commitHeadline") })) - _, cmdTeardown := run.Stub() + cs, cmdTeardown := run.Stub() defer cmdTeardown(t) + cs.Register(`git rev-parse --verify refs/heads/`, 0, "") + output, err := runCommand(http, "master", true, "pr merge 1 --merge") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) @@ -343,9 +345,11 @@ func TestPrMerge_nontty(t *testing.T) { assert.NotContains(t, input, "commitHeadline") })) - _, cmdTeardown := run.Stub() + cs, cmdTeardown := run.Stub() defer cmdTeardown(t) + cs.Register(`git rev-parse --verify refs/heads/`, 0, "") + output, err := runCommand(http, "master", false, "pr merge 1 --merge") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) @@ -515,6 +519,7 @@ func Test_nonDivergingPullRequest(t *testing.T) { defer cmdTeardown(t) cs.Register(`git .+ show .+ HEAD`, 0, "COMMITSHA1,title") + cs.Register(`git rev-parse --verify refs/heads/`, 0, "") output, err := runCommand(http, "blueberries", true, "pr merge --merge") if err != nil { @@ -554,6 +559,7 @@ func Test_divergingPullRequestWarning(t *testing.T) { defer cmdTeardown(t) cs.Register(`git .+ show .+ HEAD`, 0, "COMMITSHA2,title") + cs.Register(`git rev-parse --verify refs/heads/`, 0, "") output, err := runCommand(http, "blueberries", true, "pr merge --merge") if err != nil { @@ -590,9 +596,11 @@ func Test_pullRequestWithoutCommits(t *testing.T) { assert.NotContains(t, input, "commitHeadline") })) - _, cmdTeardown := run.Stub() + cs, cmdTeardown := run.Stub() defer cmdTeardown(t) + cs.Register(`git rev-parse --verify refs/heads/`, 0, "") + output, err := runCommand(http, "blueberries", true, "pr merge --merge") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) @@ -627,9 +635,11 @@ func TestPrMerge_rebase(t *testing.T) { assert.NotContains(t, input, "commitHeadline") })) - _, cmdTeardown := run.Stub() + cs, cmdTeardown := run.Stub() defer cmdTeardown(t) + cs.Register(`git rev-parse --verify refs/heads/`, 0, "") + output, err := runCommand(http, "master", true, "pr merge 2 --rebase") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) @@ -666,9 +676,11 @@ func TestPrMerge_squash(t *testing.T) { assert.NotContains(t, input, "commitHeadline") })) - _, cmdTeardown := run.Stub() + cs, cmdTeardown := run.Stub() defer cmdTeardown(t) + cs.Register(`git rev-parse --verify refs/heads/`, 0, "") + output, err := runCommand(http, "master", true, "pr merge 3 --squash") if err != nil { t.Fatalf("error running command `pr merge`: %v", err) @@ -730,9 +742,11 @@ func TestPrMerge_alreadyMerged_nonInteractive(t *testing.T) { baseRepo("OWNER", "REPO", "master"), ) - _, cmdTeardown := run.Stub() + cs, cmdTeardown := run.Stub() defer cmdTeardown(t) + cs.Register(`git rev-parse --verify refs/heads/`, 0, "") + output, err := runCommand(http, "blueberries", true, "pr merge 4 --merge") if err != nil { t.Fatalf("Got unexpected error running `pr merge` %s", err) @@ -774,9 +788,11 @@ func TestPRMerge_interactive(t *testing.T) { assert.NotContains(t, input, "commitHeadline") })) - _, cmdTeardown := run.Stub() + cs, cmdTeardown := run.Stub() defer cmdTeardown(t) + cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") + as, surveyTeardown := prompt.InitAskStubber() defer surveyTeardown() @@ -932,9 +948,11 @@ func TestPRMerge_interactiveCancelled(t *testing.T) { "squashMergeAllowed": true } } }`)) - _, cmdTeardown := run.Stub() + cs, cmdTeardown := run.Stub() defer cmdTeardown(t) + cs.Register(`git rev-parse --verify refs/heads/`, 0, "") + as, surveyTeardown := prompt.InitAskStubber() defer surveyTeardown()