From 7cc74c5bb6d106b3a1766ca028d7f528e90e3715 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 31 Jul 2020 16:16:36 +0200 Subject: [PATCH] Isolate `pr merge` command --- command/pr.go | 213 -------------- command/pr_test.go | 465 ------------------------------ command/root.go | 5 + pkg/cmd/pr/merge/merge.go | 272 ++++++++++++++++++ pkg/cmd/pr/merge/merge_test.go | 505 +++++++++++++++++++++++++++++++++ 5 files changed, 782 insertions(+), 678 deletions(-) create mode 100644 pkg/cmd/pr/merge/merge.go create mode 100644 pkg/cmd/pr/merge/merge_test.go diff --git a/command/pr.go b/command/pr.go index 11e65cc85..e5ff19ebd 100644 --- a/command/pr.go +++ b/command/pr.go @@ -8,7 +8,6 @@ import ( "strconv" "strings" - "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" "github.com/cli/cli/context" @@ -16,7 +15,6 @@ import ( "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/cmd/pr/shared" "github.com/cli/cli/pkg/cmdutil" - "github.com/cli/cli/pkg/prompt" "github.com/cli/cli/pkg/text" "github.com/cli/cli/utils" "github.com/spf13/cobra" @@ -31,11 +29,6 @@ func init() { prCmd.AddCommand(prStatusCmd) prCmd.AddCommand(prCloseCmd) prCmd.AddCommand(prReopenCmd) - prCmd.AddCommand(prMergeCmd) - prMergeCmd.Flags().BoolP("delete-branch", "d", true, "Delete the local and remote branch after merge") - prMergeCmd.Flags().BoolP("merge", "m", false, "Merge the commits with the base branch") - prMergeCmd.Flags().BoolP("rebase", "r", false, "Rebase the commits onto the base branch") - prMergeCmd.Flags().BoolP("squash", "s", false, "Squash the commits into one commit and merge it into the base branch") prCmd.AddCommand(prReadyCmd) prCmd.AddCommand(prListCmd) @@ -93,18 +86,6 @@ var prReopenCmd = &cobra.Command{ Args: cobra.ExactArgs(1), RunE: prReopen, } -var prMergeCmd = &cobra.Command{ - Use: "merge [ | | ]", - Short: "Merge a pull request", - Long: heredoc.Doc(` - Merge a pull request on GitHub. - - By default, the head branch of the pull request will get deleted on both remote and local repositories. - To retain the branch, use '--delete-branch=false'. - `), - Args: cobra.MaximumNArgs(1), - RunE: prMerge, -} var prReadyCmd = &cobra.Command{ Use: "ready [ | | ]", Short: "Mark a pull request as ready for review", @@ -367,200 +348,6 @@ func prReopen(cmd *cobra.Command, args []string) error { return nil } -func prMerge(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - - pr, baseRepo, err := prFromArgs(ctx, apiClient, cmd, args) - if err != nil { - return err - } - - if pr.Mergeable == "CONFLICTING" { - err := fmt.Errorf("%s Pull request #%d (%s) has conflicts and isn't mergeable ", utils.Red("!"), pr.Number, pr.Title) - return err - } else if pr.Mergeable == "UNKNOWN" { - err := fmt.Errorf("%s Pull request #%d (%s) can't be merged right now; try again in a few seconds", utils.Red("!"), pr.Number, pr.Title) - return err - } else if pr.State == "MERGED" { - err := fmt.Errorf("%s Pull request #%d (%s) was already merged", utils.Red("!"), pr.Number, pr.Title) - return err - } - - var mergeMethod api.PullRequestMergeMethod - deleteBranch, err := cmd.Flags().GetBool("delete-branch") - if err != nil { - return err - } - - deleteLocalBranch := !cmd.Flags().Changed("repo") - crossRepoPR := pr.HeadRepositoryOwner.Login != baseRepo.RepoOwner() - - // Ensure only one merge method is specified - enabledFlagCount := 0 - isInteractive := false - if b, _ := cmd.Flags().GetBool("merge"); b { - enabledFlagCount++ - mergeMethod = api.PullRequestMergeMethodMerge - } - if b, _ := cmd.Flags().GetBool("rebase"); b { - enabledFlagCount++ - mergeMethod = api.PullRequestMergeMethodRebase - } - if b, _ := cmd.Flags().GetBool("squash"); b { - enabledFlagCount++ - mergeMethod = api.PullRequestMergeMethodSquash - } - - if enabledFlagCount == 0 { - if !connectedToTerminal(cmd) { - return errors.New("--merge, --rebase, or --squash required when not attached to a tty") - } - isInteractive = true - } else if enabledFlagCount > 1 { - return errors.New("expected exactly one of --merge, --rebase, or --squash to be true") - } - - if isInteractive { - mergeMethod, deleteBranch, err = prInteractiveMerge(deleteLocalBranch, crossRepoPR) - if err != nil { - return nil - } - } - - var action string - if mergeMethod == api.PullRequestMergeMethodRebase { - action = "Rebased and merged" - err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodRebase) - } else if mergeMethod == api.PullRequestMergeMethodSquash { - action = "Squashed and merged" - err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodSquash) - } else if mergeMethod == api.PullRequestMergeMethodMerge { - action = "Merged" - err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodMerge) - } else { - err = fmt.Errorf("unknown merge method (%d) used", mergeMethod) - return err - } - - if err != nil { - return fmt.Errorf("API call failed: %w", err) - } - - if connectedToTerminal(cmd) { - fmt.Fprintf(colorableErr(cmd), "%s %s pull request #%d (%s)\n", utils.Magenta("✔"), action, pr.Number, pr.Title) - } - - if deleteBranch { - branchSwitchString := "" - - if deleteLocalBranch && !crossRepoPR { - currentBranch, err := ctx.Branch() - if err != nil { - return err - } - - var branchToSwitchTo string - if currentBranch == pr.HeadRefName { - branchToSwitchTo, err = api.RepoDefaultBranch(apiClient, baseRepo) - if err != nil { - return err - } - err = git.CheckoutBranch(branchToSwitchTo) - if err != nil { - return err - } - } - - 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", utils.Cyan(pr.HeadRefName), err) - return err - } - } - - if branchToSwitchTo != "" { - branchSwitchString = fmt.Sprintf(" and switched to branch %s", utils.Cyan(branchToSwitchTo)) - } - } - - if !crossRepoPR { - err = api.BranchDeleteRemote(apiClient, baseRepo, 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) { - err = fmt.Errorf("failed to delete remote branch %s: %w", utils.Cyan(pr.HeadRefName), err) - return err - } - } - - if connectedToTerminal(cmd) { - fmt.Fprintf(colorableErr(cmd), "%s Deleted branch %s%s\n", utils.Red("✔"), utils.Cyan(pr.HeadRefName), branchSwitchString) - } - } - - return nil -} - -func prInteractiveMerge(deleteLocalBranch bool, crossRepoPR bool) (api.PullRequestMergeMethod, bool, error) { - mergeMethodQuestion := &survey.Question{ - Name: "mergeMethod", - Prompt: &survey.Select{ - Message: "What merge method would you like to use?", - Options: []string{"Create a merge commit", "Rebase and merge", "Squash and merge"}, - Default: "Create a merge commit", - }, - } - - qs := []*survey.Question{mergeMethodQuestion} - - if !crossRepoPR { - var message string - if deleteLocalBranch { - message = "Delete the branch locally and on GitHub?" - } else { - message = "Delete the branch on GitHub?" - } - - deleteBranchQuestion := &survey.Question{ - Name: "deleteBranch", - Prompt: &survey.Confirm{ - Message: message, - Default: true, - }, - } - qs = append(qs, deleteBranchQuestion) - } - - answers := struct { - MergeMethod int - DeleteBranch bool - }{} - - err := prompt.SurveyAsk(qs, &answers) - if err != nil { - return 0, false, fmt.Errorf("could not prompt: %w", err) - } - - var mergeMethod api.PullRequestMergeMethod - switch answers.MergeMethod { - case 0: - mergeMethod = api.PullRequestMergeMethodMerge - case 1: - mergeMethod = api.PullRequestMergeMethodRebase - case 2: - mergeMethod = api.PullRequestMergeMethodSquash - } - - deleteBranch := answers.DeleteBranch - return mergeMethod, deleteBranch, nil -} - func prStateWithDraft(pr *api.PullRequest) string { if pr.IsDraft && pr.State == "OPEN" { return "DRAFT" diff --git a/command/pr_test.go b/command/pr_test.go index e46a75e31..0b49911fa 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -10,7 +10,6 @@ import ( "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/httpmock" - "github.com/cli/cli/pkg/prompt" "github.com/cli/cli/test" "github.com/stretchr/testify/assert" ) @@ -614,470 +613,6 @@ func TestPRReopen_alreadyMerged(t *testing.T) { } } -func TestPrMerge(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { "pullRequest": { - "id": "THE-ID", - "number": 1, - "title": "The title of the PR", - "state": "OPEN", - "headRefName": "blueberries", - "headRepositoryOwner": {"login": "OWNER"} - } } } }`)) - http.Register( - httpmock.GraphQL(`mutation PullRequestMerge\b`), - httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { - assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) - assert.Equal(t, "MERGE", input["mergeMethod"].(string)) - })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge) - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") - - output, err := RunCommand("pr merge 1 --merge") - if err != nil { - t.Fatalf("error running command `pr merge`: %v", err) - } - - r := regexp.MustCompile(`Merged pull request #1 \(The title of the PR\)`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestPrMerge_nontty(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(false)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "pullRequest": { "number": 1, "title": "The title of the PR", "state": "OPEN", "id": "THE-ID"} - } } }`)) - http.Register( - httpmock.GraphQL(`mutation PullRequestMerge\b`), - httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { - assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) - assert.Equal(t, "MERGE", input["mergeMethod"].(string)) - })) - http.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), - httpmock.StringResponse(`{ - "data": { - "repository": { - "defaultBranchRef": {"name": "master"} - } - } - }`)) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge) - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") - - output, err := RunCommand("pr merge 1 --merge") - if err != nil { - t.Fatalf("error running command `pr merge`: %v", err) - } - - assert.Equal(t, "", output.String()) - assert.Equal(t, "", output.Stderr()) -} - -func TestPrMerge_nontty_insufficient_flags(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(false)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "pullRequest": { "number": 1, "title": "The title of the PR", "state": "OPEN", "id": "THE-ID"} - } } }`)) - - output, err := RunCommand("pr merge 1") - if err == nil { - t.Fatal("expected error") - } - - assert.Equal(t, "--merge, --rebase, or --squash required when not attached to a tty", err.Error()) - assert.Equal(t, "", output.String()) -} - -func TestPrMerge_withRepoFlag(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.GraphQLQuery(` - { "data": { "repository": { - "pullRequest": { "number": 1, "title": "The title of the PR", "state": "OPEN", "id": "THE-ID"} - } } }`, func(_ string, params map[string]interface{}) { - assert.Equal(t, "stinky", params["owner"].(string)) - assert.Equal(t, "boi", params["repo"].(string)) - })) - http.Register( - httpmock.GraphQL(`mutation PullRequestMerge\b`), - httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { - assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) - assert.Equal(t, "MERGE", input["mergeMethod"].(string)) - })) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - eq(t, len(cs.Calls), 0) - - output, err := RunCommand("pr merge 1 --merge -R stinky/boi") - if err != nil { - t.Fatalf("error running command `pr merge`: %v", err) - } - - r := regexp.MustCompile(`Merged pull request #1 \(The title of the PR\)`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestPrMerge_deleteBranch(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestForBranch\b`), - httpmock.FileResponse("../test/fixtures/prViewPreviewWithMetadataByBranch.json")) - 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)) - })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git checkout master - cs.Stub("") // git rev-parse --verify blueberries` - cs.Stub("") // git branch -d - cs.Stub("") // git push origin --delete blueberries - - output, err := RunCommand(`pr merge --merge --delete-branch`) - if err != nil { - t.Fatalf("Got unexpected error running `pr merge` %s", err) - } - - test.ExpectLines(t, output.Stderr(), `Merged pull request #10 \(Blueberries are a good fruit\)`, `Deleted branch.*blueberries`) -} - -func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { - initBlankContext("", "OWNER/REPO", "another-branch") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestForBranch\b`), - httpmock.FileResponse("../test/fixtures/prViewPreviewWithMetadataByBranch.json")) - 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)) - })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - // We don't expect the default branch to be checked out, just that blueberries is deleted - cs.Stub("") // git rev-parse --verify blueberries - cs.Stub("") // git branch -d blueberries - cs.Stub("") // git push origin --delete blueberries - - output, err := RunCommand(`pr merge --merge --delete-branch blueberries`) - if err != nil { - t.Fatalf("Got unexpected error running `pr merge` %s", err) - } - - test.ExpectLines(t, output.Stderr(), `Merged pull request #10 \(Blueberries are a good fruit\)`, `Deleted branch.*blueberries`) -} - -func TestPrMerge_noPrNumberGiven(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestForBranch\b`), - httpmock.FileResponse("../test/fixtures/prViewPreviewWithMetadataByBranch.json")) - 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)) - })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge) - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") // git branch -d - - output, err := RunCommand("pr merge --merge") - if err != nil { - t.Fatalf("error running command `pr merge`: %v", err) - } - - r := regexp.MustCompile(`Merged pull request #10 \(Blueberries are a good fruit\)`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestPrMerge_rebase(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { "pullRequest": { - "id": "THE-ID", - "number": 2, - "title": "The title of the PR", - "state": "OPEN", - "headRefName": "blueberries", - "headRepositoryOwner": {"login": "OWNER"} - } } } }`)) - http.Register( - httpmock.GraphQL(`mutation PullRequestMerge\b`), - httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { - assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) - assert.Equal(t, "REBASE", input["mergeMethod"].(string)) - })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") // git branch -d - - output, err := RunCommand("pr merge 2 --rebase") - if err != nil { - t.Fatalf("error running command `pr merge`: %v", err) - } - - r := regexp.MustCompile(`Rebased and merged pull request #2 \(The title of the PR\)`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestPrMerge_squash(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { "pullRequest": { - "id": "THE-ID", - "number": 3, - "title": "The title of the PR", - "state": "OPEN", - "headRefName": "blueberries", - "headRepositoryOwner": {"login": "OWNER"} - } } } }`)) - http.Register( - httpmock.GraphQL(`mutation PullRequestMerge\b`), - httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { - assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) - assert.Equal(t, "SQUASH", input["mergeMethod"].(string)) - })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") // git branch -d - - output, err := RunCommand("pr merge 3 --squash") - if err != nil { - t.Fatalf("error running command `pr merge`: %v", err) - } - - test.ExpectLines(t, output.Stderr(), "Squashed and merged pull request #3", `Deleted branch.*blueberries`) -} - -func TestPrMerge_alreadyMerged(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "pullRequest": { "number": 4, "title": "The title of the PR", "state": "MERGED"} - } } }`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") // git branch -d - - output, err := RunCommand("pr merge 4") - if err == nil { - t.Fatalf("expected an error running command `pr merge`: %v", err) - } - - r := regexp.MustCompile(`Pull request #4 \(The title of the PR\) was already merged`) - - if !r.MatchString(err.Error()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestPRMerge_interactive(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - defer stubTerminal(true)() - http := initFakeHTTP() - defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query PullRequestForBranch\b`), - httpmock.StringResponse(` - { "data": { "repository": { "pullRequests": { "nodes": [{ - "headRefName": "blueberries", - "headRepositoryOwner": {"login": "OWNER"}, - "id": "THE-ID", - "number": 3 - }] } } } }`)) - http.Register( - httpmock.GraphQL(`mutation PullRequestMerge\b`), - httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { - assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) - assert.Equal(t, "MERGE", input["mergeMethod"].(string)) - })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ - cs.Stub("") // git symbolic-ref --quiet --short HEAD - cs.Stub("") // git checkout master - cs.Stub("") // git push origin --delete blueberries - cs.Stub("") // git branch -d - - as, surveyTeardown := prompt.InitAskStubber() - defer surveyTeardown() - - as.Stub([]*prompt.QuestionStub{ - { - Name: "mergeMethod", - Value: 0, - }, - { - Name: "deleteBranch", - Value: true, - }, - }) - - output, err := RunCommand(`pr merge`) - if err != nil { - t.Fatalf("Got unexpected error running `pr merge` %s", err) - } - - test.ExpectLines(t, output.Stderr(), "Merged pull request #3", `Deleted branch.*blueberries`) -} - -func TestPrMerge_multipleMergeMethods(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - - _, err := RunCommand("pr merge 1 --merge --squash") - if err == nil { - t.Fatal("expected error running `pr merge` with multiple merge methods") - } -} - -func TestPrMerge_multipleMergeMethods_nontty(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(false)() - - _, err := RunCommand("pr merge 1 --merge --squash") - if err == nil { - t.Fatal("expected error running `pr merge` with multiple merge methods") - } -} - func TestPRReady(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() diff --git a/command/root.go b/command/root.go index 3e05de3e6..8752a3d65 100644 --- a/command/root.go +++ b/command/root.go @@ -24,6 +24,7 @@ import ( gistCreateCmd "github.com/cli/cli/pkg/cmd/gist/create" prCheckoutCmd "github.com/cli/cli/pkg/cmd/pr/checkout" prDiffCmd "github.com/cli/cli/pkg/cmd/pr/diff" + prMergeCmd "github.com/cli/cli/pkg/cmd/pr/merge" prReviewCmd "github.com/cli/cli/pkg/cmd/pr/review" prViewCmd "github.com/cli/cli/pkg/cmd/pr/view" repoCmd "github.com/cli/cli/pkg/cmd/repo" @@ -176,6 +177,7 @@ func init() { prCmd.AddCommand(prDiffCmd.NewCmdDiff(&repoResolvingCmdFactory, nil)) prCmd.AddCommand(prCheckoutCmd.NewCmdCheckout(&repoResolvingCmdFactory, nil)) prCmd.AddCommand(prViewCmd.NewCmdView(&repoResolvingCmdFactory, nil)) + prCmd.AddCommand(prMergeCmd.NewCmdMerge(&repoResolvingCmdFactory, nil)) RootCmd.AddCommand(creditsCmd.NewCmdCredits(cmdFactory, nil)) } @@ -279,6 +281,9 @@ func httpClient(token string) *http.Client { opts = append(opts, api.AddHeader("Authorization", fmt.Sprintf("token %s", token)), api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version)), + // antiope-preview: Checks + // FIXME: avoid setting this header for `api` command + api.AddHeader("Accept", "application/vnd.github.antiope-preview+json"), ) return api.NewHTTPClient(opts...) } diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go new file mode 100644 index 000000000..00aa25695 --- /dev/null +++ b/pkg/cmd/pr/merge/merge.go @@ -0,0 +1,272 @@ +package merge + +import ( + "errors" + "fmt" + "net/http" + + "github.com/AlecAivazis/survey/v2" + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/api" + "github.com/cli/cli/context" + "github.com/cli/cli/git" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/prompt" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type MergeOptions struct { + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + Remotes func() (context.Remotes, error) + Branch func() (string, error) + + SelectorArg string + DeleteBranch bool + DeleteLocalBranch bool + MergeMethod api.PullRequestMergeMethod + InteractiveMode bool +} + +func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Command { + opts := &MergeOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + BaseRepo: f.BaseRepo, + Remotes: f.Remotes, + Branch: f.Branch, + } + + var ( + flagMerge bool + flagSquash bool + flagRebase bool + ) + + cmd := &cobra.Command{ + Use: "merge [ | | ]", + Short: "Merge a pull request", + Long: heredoc.Doc(` + Merge a pull request on GitHub. + + By default, the head branch of the pull request will get deleted on both remote and local repositories. + To retain the branch, use '--delete-branch=false'. + `), + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) > 0 { + opts.SelectorArg = args[0] + } + + methodFlags := 0 + if flagMerge { + opts.MergeMethod = api.PullRequestMergeMethodMerge + methodFlags++ + } + if flagRebase { + opts.MergeMethod = api.PullRequestMergeMethodRebase + methodFlags++ + } + if flagSquash { + opts.MergeMethod = api.PullRequestMergeMethodSquash + methodFlags++ + } + if methodFlags == 0 { + if !opts.IO.IsStdoutTTY() || !opts.IO.IsStdinTTY() { + return &cmdutil.FlagError{Err: errors.New("--merge, --rebase, or --squash required when not attached to a terminal")} + } + opts.InteractiveMode = true + } else if methodFlags > 1 { + return &cmdutil.FlagError{Err: errors.New("only one of --merge, --rebase, or --squash can be enabled")} + } + + opts.DeleteLocalBranch = !cmd.Flags().Changed("repo") + + if runF != nil { + return runF(opts) + } + return mergeRun(opts) + }, + } + + cmd.Flags().BoolVarP(&opts.DeleteBranch, "delete-branch", "d", true, "Delete the local and remote branch after merge") + cmd.Flags().BoolVarP(&flagMerge, "merge", "m", false, "Merge the commits with the base branch") + cmd.Flags().BoolVarP(&flagRebase, "rebase", "r", false, "Rebase the commits onto the base branch") + cmd.Flags().BoolVarP(&flagSquash, "squash", "s", false, "Squash the commits into one commit and merge it into the base branch") + + return cmd +} + +func mergeRun(opts *MergeOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + pr, baseRepo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg) + if err != nil { + return err + } + + if pr.Mergeable == "CONFLICTING" { + err := fmt.Errorf("%s Pull request #%d (%s) has conflicts and isn't mergeable ", utils.Red("!"), pr.Number, pr.Title) + return err + } else if pr.Mergeable == "UNKNOWN" { + err := fmt.Errorf("%s Pull request #%d (%s) can't be merged right now; try again in a few seconds", utils.Red("!"), pr.Number, pr.Title) + return err + } else if pr.State == "MERGED" { + err := fmt.Errorf("%s Pull request #%d (%s) was already merged", utils.Red("!"), pr.Number, pr.Title) + return err + } + + mergeMethod := opts.MergeMethod + deleteBranch := opts.DeleteBranch + crossRepoPR := pr.HeadRepositoryOwner.Login != baseRepo.RepoOwner() + + if opts.InteractiveMode { + mergeMethod, deleteBranch, err = prInteractiveMerge(opts.DeleteLocalBranch, crossRepoPR) + if err != nil { + return nil + } + } + + var action string + if mergeMethod == api.PullRequestMergeMethodRebase { + action = "Rebased and merged" + err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodRebase) + } else if mergeMethod == api.PullRequestMergeMethodSquash { + action = "Squashed and merged" + err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodSquash) + } else if mergeMethod == api.PullRequestMergeMethodMerge { + action = "Merged" + err = api.PullRequestMerge(apiClient, baseRepo, pr, api.PullRequestMergeMethodMerge) + } else { + err = fmt.Errorf("unknown merge method (%d) used", mergeMethod) + return err + } + + if err != nil { + return fmt.Errorf("API call failed: %w", err) + } + + isTerminal := opts.IO.IsStdoutTTY() + + if isTerminal { + fmt.Fprintf(opts.IO.ErrOut, "%s %s pull request #%d (%s)\n", utils.Magenta("✔"), action, pr.Number, pr.Title) + } + + if deleteBranch { + branchSwitchString := "" + + if opts.DeleteLocalBranch && !crossRepoPR { + currentBranch, err := opts.Branch() + if err != nil { + return err + } + + var branchToSwitchTo string + if currentBranch == pr.HeadRefName { + branchToSwitchTo, err = api.RepoDefaultBranch(apiClient, baseRepo) + if err != nil { + return err + } + err = git.CheckoutBranch(branchToSwitchTo) + if err != nil { + return err + } + } + + 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", utils.Cyan(pr.HeadRefName), err) + return err + } + } + + if branchToSwitchTo != "" { + branchSwitchString = fmt.Sprintf(" and switched to branch %s", utils.Cyan(branchToSwitchTo)) + } + } + + if !crossRepoPR { + err = api.BranchDeleteRemote(apiClient, baseRepo, 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) { + err = fmt.Errorf("failed to delete remote branch %s: %w", utils.Cyan(pr.HeadRefName), err) + return err + } + } + + if isTerminal { + fmt.Fprintf(opts.IO.ErrOut, "%s Deleted branch %s%s\n", utils.Red("✔"), utils.Cyan(pr.HeadRefName), branchSwitchString) + } + } + + return nil +} + +func prInteractiveMerge(deleteLocalBranch bool, crossRepoPR bool) (api.PullRequestMergeMethod, bool, error) { + mergeMethodQuestion := &survey.Question{ + Name: "mergeMethod", + Prompt: &survey.Select{ + Message: "What merge method would you like to use?", + Options: []string{"Create a merge commit", "Rebase and merge", "Squash and merge"}, + Default: "Create a merge commit", + }, + } + + qs := []*survey.Question{mergeMethodQuestion} + + if !crossRepoPR { + var message string + if deleteLocalBranch { + message = "Delete the branch locally and on GitHub?" + } else { + message = "Delete the branch on GitHub?" + } + + deleteBranchQuestion := &survey.Question{ + Name: "deleteBranch", + Prompt: &survey.Confirm{ + Message: message, + Default: true, + }, + } + qs = append(qs, deleteBranchQuestion) + } + + answers := struct { + MergeMethod int + DeleteBranch bool + }{} + + err := prompt.SurveyAsk(qs, &answers) + if err != nil { + return 0, false, fmt.Errorf("could not prompt: %w", err) + } + + var mergeMethod api.PullRequestMergeMethod + switch answers.MergeMethod { + case 0: + mergeMethod = api.PullRequestMergeMethodMerge + case 1: + mergeMethod = api.PullRequestMergeMethodRebase + case 2: + mergeMethod = api.PullRequestMergeMethodSquash + } + + deleteBranch := answers.DeleteBranch + return mergeMethod, deleteBranch, nil +} diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go new file mode 100644 index 000000000..e3382ef52 --- /dev/null +++ b/pkg/cmd/pr/merge/merge_test.go @@ -0,0 +1,505 @@ +package merge + +import ( + "bytes" + "io/ioutil" + "net/http" + "regexp" + "strings" + "testing" + + "github.com/cli/cli/api" + "github.com/cli/cli/context" + "github.com/cli/cli/git" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/prompt" + "github.com/cli/cli/test" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func runCommand(rt http.RoundTripper, branch string, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return &api.Repository{ + Name: "REPO", + Owner: api.RepositoryOwner{Login: "OWNER"}, + DefaultBranchRef: api.BranchRef{Name: "master"}, + }, nil + }, + Remotes: func() (context.Remotes, error) { + return context.Remotes{ + { + Remote: &git.Remote{Name: "origin"}, + Repo: ghrepo.New("OWNER", "REPO"), + }, + }, nil + }, + Branch: func() (string, error) { + return branch, nil + }, + } + + cmd := NewCmdMerge(factory, nil) + cmd.PersistentFlags().StringP("repo", "R", "", "") + + cli = strings.TrimPrefix(cli, "pr merge") + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func initFakeHTTP() *httpmock.Registry { + return &httpmock.Registry{} +} + +func TestPrMerge(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequest": { + "id": "THE-ID", + "number": 1, + "title": "The title of the PR", + "state": "OPEN", + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"} + } } } }`)) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge) + cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ + cs.Stub("") // git symbolic-ref --quiet --short HEAD + cs.Stub("") // git checkout master + cs.Stub("") + + output, err := runCommand(http, "master", true, "pr merge 1 --merge") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + r := regexp.MustCompile(`Merged pull request #1 \(The title of the PR\)`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPrMerge_nontty(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequest": { + "id": "THE-ID", + "number": 1, + "title": "The title of the PR", + "state": "OPEN", + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"} + } } } }`)) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge) + cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ + cs.Stub("") // git symbolic-ref --quiet --short HEAD + cs.Stub("") // git checkout master + cs.Stub("") + + output, err := runCommand(http, "master", false, "pr merge 1 --merge") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + assert.Equal(t, "", output.String()) + assert.Equal(t, "", output.Stderr()) +} + +func TestPrMerge_nontty_insufficient_flags(t *testing.T) { + output, err := runCommand(nil, "master", false, "pr merge 1") + if err == nil { + t.Fatal("expected error") + } + + assert.Equal(t, "--merge, --rebase, or --squash required when not attached to a terminal", err.Error()) + assert.Equal(t, "", output.String()) +} + +func TestPrMerge_withRepoFlag(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequest": { + "id": "THE-ID", + "number": 1, + "title": "The title of the PR", + "state": "OPEN", + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"} + } } } }`)) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + output, err := runCommand(http, "master", true, "pr merge 1 --merge -R OWNER/REPO") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + assert.Equal(t, 0, len(cs.Calls)) + + r := regexp.MustCompile(`Merged pull request #1 \(The title of the PR\)`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPrMerge_deleteBranch(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + // FIXME: references fixture from another package + httpmock.FileResponse("../view/fixtures/prViewPreviewWithMetadataByBranch.json")) + 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)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ + cs.Stub("") // git checkout master + cs.Stub("") // git rev-parse --verify blueberries` + cs.Stub("") // git branch -d + cs.Stub("") // git push origin --delete blueberries + + output, err := runCommand(http, "blueberries", true, `pr merge --merge --delete-branch`) + if err != nil { + t.Fatalf("Got unexpected error running `pr merge` %s", err) + } + + test.ExpectLines(t, output.Stderr(), `Merged pull request #10 \(Blueberries are a good fruit\)`, `Deleted branch.*blueberries`) +} + +func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + // FIXME: references fixture from another package + httpmock.FileResponse("../view/fixtures/prViewPreviewWithMetadataByBranch.json")) + 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)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + // We don't expect the default branch to be checked out, just that blueberries is deleted + cs.Stub("") // git rev-parse --verify blueberries + cs.Stub("") // git branch -d blueberries + cs.Stub("") // git push origin --delete blueberries + + output, err := runCommand(http, "master", true, `pr merge --merge --delete-branch blueberries`) + if err != nil { + t.Fatalf("Got unexpected error running `pr merge` %s", err) + } + + test.ExpectLines(t, output.Stderr(), `Merged pull request #10 \(Blueberries are a good fruit\)`, `Deleted branch.*blueberries`) +} + +func TestPrMerge_noPrNumberGiven(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + // FIXME: references fixture from another package + httpmock.FileResponse("../view/fixtures/prViewPreviewWithMetadataByBranch.json")) + 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)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("branch.blueberries.remote origin\nbranch.blueberries.merge refs/heads/blueberries") // git config --get-regexp ^branch\.master\.(remote|merge) + cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ + cs.Stub("") // git symbolic-ref --quiet --short HEAD + cs.Stub("") // git checkout master + cs.Stub("") // git branch -d + + output, err := runCommand(http, "blueberries", true, "pr merge --merge") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + r := regexp.MustCompile(`Merged pull request #10 \(Blueberries are a good fruit\)`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPrMerge_rebase(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequest": { + "id": "THE-ID", + "number": 2, + "title": "The title of the PR", + "state": "OPEN", + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"} + } } } }`)) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) + assert.Equal(t, "REBASE", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ + cs.Stub("") // git symbolic-ref --quiet --short HEAD + cs.Stub("") // git checkout master + cs.Stub("") // git branch -d + + output, err := runCommand(http, "master", true, "pr merge 2 --rebase") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + r := regexp.MustCompile(`Rebased and merged pull request #2 \(The title of the PR\)`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPrMerge_squash(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequest": { + "id": "THE-ID", + "number": 3, + "title": "The title of the PR", + "state": "OPEN", + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"} + } } } }`)) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) + assert.Equal(t, "SQUASH", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ + cs.Stub("") // git symbolic-ref --quiet --short HEAD + cs.Stub("") // git checkout master + cs.Stub("") // git branch -d + + output, err := runCommand(http, "master", true, "pr merge 3 --squash") + if err != nil { + t.Fatalf("error running command `pr merge`: %v", err) + } + + test.ExpectLines(t, output.Stderr(), "Squashed and merged pull request #3", `Deleted branch.*blueberries`) +} + +func TestPrMerge_alreadyMerged(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "pullRequest": { "number": 4, "title": "The title of the PR", "state": "MERGED"} + } } }`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ + cs.Stub("") // git symbolic-ref --quiet --short HEAD + cs.Stub("") // git checkout master + cs.Stub("") // git branch -d + + output, err := runCommand(http, "master", true, "pr merge 4") + if err == nil { + t.Fatalf("expected an error running command `pr merge`: %v", err) + } + + r := regexp.MustCompile(`Pull request #4 \(The title of the PR\) was already merged`) + + if !r.MatchString(err.Error()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPRMerge_interactive(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequests": { "nodes": [{ + "headRefName": "blueberries", + "headRepositoryOwner": {"login": "OWNER"}, + "id": "THE-ID", + "number": 3 + }] } } } }`)) + http.Register( + httpmock.GraphQL(`mutation PullRequestMerge\b`), + httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) { + assert.Equal(t, "THE-ID", input["pullRequestId"].(string)) + assert.Equal(t, "MERGE", input["mergeMethod"].(string)) + })) + http.Register( + httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), + httpmock.StringResponse(`{}`)) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("") // git config --get-regexp ^branch\.blueberries\.(remote|merge)$ + cs.Stub("") // git symbolic-ref --quiet --short HEAD + cs.Stub("") // git checkout master + cs.Stub("") // git push origin --delete blueberries + cs.Stub("") // git branch -d + + as, surveyTeardown := prompt.InitAskStubber() + defer surveyTeardown() + + as.Stub([]*prompt.QuestionStub{ + { + Name: "mergeMethod", + Value: 0, + }, + { + Name: "deleteBranch", + Value: true, + }, + }) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Fatalf("Got unexpected error running `pr merge` %s", err) + } + + test.ExpectLines(t, output.Stderr(), "Merged pull request #3", `Deleted branch.*blueberries`) +} + +func TestPrMerge_multipleMergeMethods(t *testing.T) { + _, err := runCommand(nil, "master", true, "1 --merge --squash") + if err == nil { + t.Fatal("expected error running `pr merge` with multiple merge methods") + } +} + +func TestPrMerge_multipleMergeMethods_nontty(t *testing.T) { + _, err := runCommand(nil, "master", false, "1 --merge --squash") + if err == nil { + t.Fatal("expected error running `pr merge` with multiple merge methods") + } +}