Skip to content

Commit

Permalink
Clean up local dangling reference to deleted remote branches.
Browse files Browse the repository at this point in the history
  • Loading branch information
asakatida committed Jan 6, 2024
1 parent 586eb8b commit bce57f6
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 2 deletions.
2 changes: 1 addition & 1 deletion pkg/cmd/pr/close/close.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func closeRun(opts *CloseOptions) error {
return nil
}
} else {
if err := api.BranchDeleteRemote(apiClient, baseRepo, pr.HeadRefName); err != nil {
if err := shared.DeleteRemoteBranch(ctx, apiClient, opts.GitClient, baseRepo, pr.HeadRefName); err != nil {
return fmt.Errorf("failed to delete remote branch %s: %w", cs.Cyan(pr.HeadRefName), err)
}
}
Expand Down
36 changes: 36 additions & 0 deletions pkg/cmd/pr/close/close_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ func TestPrClose_deleteBranch_sameRepo(t *testing.T) {
defer cmdTeardown(t)

cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "")
cs.Register(`git rev-parse --git-dir`, 128, "")
cs.Register(`git branch -D blueberries`, 0, "")

output, err := runCommand(http, true, `96 --delete-branch`)
Expand Down Expand Up @@ -231,6 +232,7 @@ func TestPrClose_deleteBranch_sameBranch(t *testing.T) {

cs.Register(`git checkout main`, 0, "")
cs.Register(`git rev-parse --verify refs/heads/trunk`, 0, "")
cs.Register(`git rev-parse --git-dir`, 128, "")
cs.Register(`git branch -D trunk`, 0, "")

output, err := runCommand(http, true, `96 --delete-branch`)
Expand Down Expand Up @@ -308,3 +310,37 @@ func TestPrClose_withComment(t *testing.T) {
assert.Equal(t, "", output.String())
assert.Equal(t, "✓ Closed pull request #96 (The title of the PR)\n", output.Stderr())
}

func TestPrClose_deleteBranch_inGitRepo(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)

baseRepo, pr := stubPR("OWNER/REPO:main", "OWNER/REPO:trunk")
pr.Title = "The title of the PR"
shared.RunCommandFinder("96", pr, baseRepo)

http.Register(
httpmock.GraphQL(`mutation PullRequestClose\b`),
httpmock.GraphQLMutation(`{"id": "THE-ID"}`,
func(inputs map[string]interface{}) {
assert.Equal(t, inputs["pullRequestId"], "THE-ID")
}),
)

cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)

cs.Register(`git rev-parse --verify refs/heads/trunk`, 0, "")
cs.Register(`git rev-parse --git-dir`, 0, ".git")
cs.Register(`git checkout main`, 0, "")
cs.Register(`git branch -D trunk`, 0, "")
cs.Register(`git push --delete origin trunk`, 0, "")

output, err := runCommand(http, true, `96 --delete-branch`)
assert.NoError(t, err)
assert.Equal(t, "", output.String())
assert.Equal(t, heredoc.Doc(`
✓ Closed pull request #96 (The title of the PR)
✓ Deleted branch trunk and switched to branch main
`), output.Stderr())
}
3 changes: 2 additions & 1 deletion pkg/cmd/pr/merge/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,9 @@ func (m *mergeContext) deleteRemoteBranch() error {
}

if !m.merged {
ctx := context.Background()
apiClient := api.NewClientFromHTTP(m.httpClient)
err := api.BranchDeleteRemote(apiClient, m.baseRepo, m.pr.HeadRefName)
err := shared.DeleteRemoteBranch(ctx, apiClient, m.opts.GitClient, m.baseRepo, m.pr.HeadRefName)
var httpErr api.HTTPError
// The ref might have already been deleted by GitHub
if err != nil && (!errors.As(err, &httpErr) || httpErr.StatusCode != 422) {
Expand Down
5 changes: 5 additions & 0 deletions pkg/cmd/pr/merge/merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,7 @@ func TestPrMerge_deleteBranch(t *testing.T) {
defer cmdTeardown(t)

cs.Register(`git rev-parse --verify refs/heads/main`, 0, "")
cs.Register(`git rev-parse --git-dir`, 128, "")
cs.Register(`git checkout main`, 0, "")
cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "")
cs.Register(`git branch -D blueberries`, 0, "")
Expand Down Expand Up @@ -692,6 +693,7 @@ func TestPrMerge_deleteBranch_nonDefault(t *testing.T) {
defer cmdTeardown(t)

cs.Register(`git rev-parse --verify refs/heads/fruit`, 0, "")
cs.Register(`git rev-parse --git-dir`, 128, "")
cs.Register(`git checkout fruit`, 0, "")
cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "")
cs.Register(`git branch -D blueberries`, 0, "")
Expand Down Expand Up @@ -791,6 +793,7 @@ func TestPrMerge_deleteBranch_checkoutNewBranch(t *testing.T) {
defer cmdTeardown(t)

cs.Register(`git rev-parse --verify refs/heads/fruit`, 1, "")
cs.Register(`git rev-parse --git-dir`, 128, "")
cs.Register(`git checkout -b fruit --track origin/fruit`, 0, "")
cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "")
cs.Register(`git branch -D blueberries`, 0, "")
Expand Down Expand Up @@ -841,6 +844,7 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) {
defer cmdTeardown(t)

cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "")
cs.Register(`git rev-parse --git-dir`, 128, "")
cs.Register(`git branch -D blueberries`, 0, "")

output, err := runCommand(http, nil, "main", true, `pr merge --merge --delete-branch blueberries`)
Expand Down Expand Up @@ -1321,6 +1325,7 @@ func TestPRMergeTTY_withDeleteBranch(t *testing.T) {
defer cmdTeardown(t)

cs.Register(`git rev-parse --verify refs/heads/main`, 0, "")
cs.Register(`git rev-parse --git-dir`, 128, "")
cs.Register(`git checkout main`, 0, "")
cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "")
cs.Register(`git branch -D blueberries`, 0, "")
Expand Down
25 changes: 25 additions & 0 deletions pkg/cmd/pr/shared/shared.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package shared

import (
"context"

"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/ghrepo"
)

func DeleteRemoteBranch(ctx context.Context, client *api.Client, gitClient *git.Client, repo ghrepo.Interface, branch string) error {
if isLocal, err := gitClient.IsLocalGitRepo(ctx); err != nil || !isLocal {
return api.BranchDeleteRemote(client, repo, branch)
}
args := []string{"push", "--delete", "origin", branch}
cmd, err := gitClient.Command(ctx, args...)
if err != nil {
return err
}
_, err = cmd.Output()
if err != nil {
return err
}
return nil
}

0 comments on commit bce57f6

Please sign in to comment.