diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index d49139011..a23dc7390 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -199,7 +199,6 @@ type mergeContext struct { autoMerge bool crossRepoPR bool deleteBranch bool - switchedToBranch string mergeQueueRequired bool } @@ -370,7 +369,7 @@ func (m *mergeContext) merge() error { // Delete local branch if requested and if allowed. func (m *mergeContext) deleteLocalBranch() error { - if m.crossRepoPR || m.autoMerge { + if m.autoMerge { return nil } @@ -395,6 +394,8 @@ func (m *mergeContext) deleteLocalBranch() error { return err } + switchedToBranch := "" + ctx := context.Background() // branch the command was run on is the same as the pull request branch @@ -424,14 +425,19 @@ func (m *mergeContext) deleteLocalBranch() error { _ = m.warnf(fmt.Sprintf("%s warning: not possible to fast-forward to: %q\n", m.cs.WarningIcon(), targetBranch)) } - m.switchedToBranch = targetBranch + switchedToBranch = targetBranch } if err := m.opts.GitClient.DeleteLocalBranch(ctx, m.pr.HeadRefName); err != nil { return fmt.Errorf("failed to delete local branch %s: %w", m.cs.Cyan(m.pr.HeadRefName), err) } - return nil + switchedStatement := "" + if switchedToBranch != "" { + switchedStatement = fmt.Sprintf(" and switched to branch %s", m.cs.Cyan(switchedToBranch)) + } + + return m.infof("%s Deleted local branch %s%s\n", m.cs.SuccessIconWithColor(m.cs.Red), m.cs.Cyan(m.pr.HeadRefName), switchedStatement) } // Delete the remote branch if requested and if allowed. @@ -451,11 +457,7 @@ func (m *mergeContext) deleteRemoteBranch() error { } } - branch := "" - if m.switchedToBranch != "" { - branch = fmt.Sprintf(" and switched to branch %s", m.cs.Cyan(m.switchedToBranch)) - } - return m.infof("%s Deleted branch %s%s\n", m.cs.SuccessIconWithColor(m.cs.Red), m.cs.Cyan(m.pr.HeadRefName), branch) + return m.infof("%s Deleted remote branch %s\n", m.cs.SuccessIconWithColor(m.cs.Red), m.cs.Cyan(m.pr.HeadRefName)) } // Add the Pull Request to a merge queue @@ -577,11 +579,16 @@ func mergeMethodSurvey(p shared.Prompt, baseRepo *api.Repository) (PullRequestMe } func deleteBranchSurvey(opts *MergeOptions, crossRepoPR, localBranchExists bool) (bool, error) { - if !crossRepoPR && !opts.IsDeleteBranchIndicated { + if !opts.IsDeleteBranchIndicated { var message string + if opts.CanDeleteLocalBranch && localBranchExists { - message = "Delete the branch locally and on GitHub?" - } else { + if crossRepoPR { + message = "Delete the branch locally?" + } else { + message = "Delete the branch locally and on GitHub?" + } + } else if !crossRepoPR { 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 d2c8a4dcb..5055bf8f4 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -654,7 +654,8 @@ func TestPrMerge_deleteBranch(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, heredoc.Doc(` ✓ Merged pull request #10 (Blueberries are a good fruit) - ✓ Deleted branch blueberries and switched to branch main + ✓ Deleted local branch blueberries and switched to branch main + ✓ Deleted remote branch blueberries `), output.Stderr()) } @@ -704,7 +705,56 @@ func TestPrMerge_deleteBranch_nonDefault(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, heredoc.Doc(` ✓ Merged pull request #10 (Blueberries are a good fruit) - ✓ Deleted branch blueberries and switched to branch fruit + ✓ Deleted local branch blueberries and switched to branch fruit + ✓ Deleted remote branch blueberries + `), output.Stderr()) +} + +func TestPrMerge_deleteBranch_onlyLocally(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + shared.RunCommandFinder( + "", + &api.PullRequest{ + ID: "PR_10", + Number: 10, + State: "OPEN", + Title: "Blueberries are a good fruit", + HeadRefName: "blueberries", + BaseRefName: "main", + MergeStateStatus: "CLEAN", + HeadRepositoryOwner: api.Owner{Login: "HEAD"}, // Not the same owner as the base repo + }, + baseRepo("OWNER", "REPO", "main"), + ) + + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "PR_10", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + assert.NotContains(t, input, "commitHeadline") + })) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git rev-parse --verify refs/heads/main`, 0, "") + cs.Register(`git checkout main`, 0, "") + cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") + cs.Register(`git branch -D blueberries`, 0, "") + cs.Register(`git pull --ff-only`, 0, "") + + output, err := runCommand(http, nil, "blueberries", true, `pr merge --merge --delete-branch`) + if err != nil { + t.Fatalf("Got unexpected error running `pr merge` %s", err) + } + + assert.Equal(t, "", output.String()) + assert.Equal(t, heredoc.Doc(` + ✓ Merged pull request #10 (Blueberries are a good fruit) + ✓ Deleted local branch blueberries and switched to branch main `), output.Stderr()) } @@ -754,7 +804,8 @@ func TestPrMerge_deleteBranch_checkoutNewBranch(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, heredoc.Doc(` ✓ Merged pull request #10 (Blueberries are a good fruit) - ✓ Deleted branch blueberries and switched to branch fruit + ✓ Deleted local branch blueberries and switched to branch fruit + ✓ Deleted remote branch blueberries `), output.Stderr()) } @@ -800,7 +851,8 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, heredoc.Doc(` ✓ Merged pull request #10 (Blueberries are a good fruit) - ✓ Deleted branch blueberries + ✓ Deleted local branch blueberries + ✓ Deleted remote branch blueberries `), output.Stderr()) } @@ -1044,7 +1096,10 @@ func TestPrMerge_alreadyMerged(t *testing.T) { output, err := runCommand(http, pm, "blueberries", true, "pr merge 4") assert.NoError(t, err) assert.Equal(t, "", output.String()) - assert.Equal(t, "✓ Deleted branch blueberries and switched to branch main\n", output.Stderr()) + assert.Equal(t, heredoc.Doc(` + ✓ Deleted local branch blueberries and switched to branch main + ✓ Deleted remote branch blueberries + `), output.Stderr()) } func TestPrMerge_alreadyMerged_withMergeStrategy(t *testing.T) { @@ -1115,7 +1170,7 @@ func TestPrMerge_alreadyMerged_withMergeStrategy_TTY(t *testing.T) { } assert.Equal(t, "", output.String()) - assert.Equal(t, "✓ Deleted branch \n", output.Stderr()) + assert.Equal(t, "✓ Deleted local branch \n✓ Deleted remote branch \n", output.Stderr()) } func TestPrMerge_alreadyMerged_withMergeStrategy_crossRepo(t *testing.T) { @@ -1139,7 +1194,17 @@ func TestPrMerge_alreadyMerged_withMergeStrategy_crossRepo(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") - output, err := runCommand(http, nil, "blueberries", true, "pr merge 4 --merge") + pm := &prompter.PrompterMock{ + ConfirmFunc: func(p string, d bool) (bool, error) { + if p == "Pull request #4 was already merged. Delete the branch locally?" { + return d, nil + } else { + return false, prompter.NoSuchPromptErr(p) + } + }, + } + + output, err := runCommand(http, pm, "blueberries", true, "pr merge 4 --merge") if err != nil { t.Fatalf("Got unexpected error running `pr merge` %s", err) } @@ -1282,7 +1347,8 @@ func TestPRMergeTTY_withDeleteBranch(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, heredoc.Doc(` ✓ Merged pull request #3 (It was the best of times) - ✓ Deleted branch blueberries and switched to branch main + ✓ Deleted local branch blueberries and switched to branch main + ✓ Deleted remote branch blueberries `), output.Stderr()) }