From 29de133ccfe657feca48639bf3364f79767fce96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 3 Dec 2019 21:19:09 +0100 Subject: [PATCH] Accept PR URL or branch argument in `pr checkout ` --- api/queries_pr.go | 1 + command/pr.go | 53 +++++++++--------- command/pr_checkout_test.go | 105 ++++++++++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 24 deletions(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index 1c7a6de76..efea246c6 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -285,6 +285,7 @@ func PullRequestByNumber(client *Client, ghRepo Repo, number int) (*PullRequest, query($owner: String!, $repo: String!, $pr_number: Int!) { repository(owner: $owner, name: $repo) { pullRequest(number: $pr_number) { + number headRefName headRepositoryOwner { login diff --git a/command/pr.go b/command/pr.go index d00bfd7bc..5277e38b2 100644 --- a/command/pr.go +++ b/command/pr.go @@ -226,24 +226,11 @@ func prView(cmd *cobra.Command, args []string) error { var openURL string if len(args) > 0 { - if prNumber, err := strconv.Atoi(args[0]); err == nil { - pr, err := api.PullRequestByNumber(apiClient, baseRepo, prNumber) - if err != nil { - return err - } - openURL = pr.URL - } else { - prRE := regexp.MustCompile(`^https://github\.com/[^/]+/[^/]+/pull/\d+`) - if m := prRE.FindStringSubmatch(args[0]); m != nil { - openURL = m[0] - } else { - pr, err := api.PullRequestForBranch(apiClient, baseRepo, args[0]) - if err != nil { - return err - } - openURL = pr.URL - } + pr, err := prFromArg(apiClient, baseRepo, args[0]) + if err != nil { + return err } + openURL = pr.URL } else { prNumber, branchWithOwner, err := prSelectorForCurrentBranch(ctx) if err != nil { @@ -265,6 +252,20 @@ func prView(cmd *cobra.Command, args []string) error { return utils.OpenInBrowser(openURL) } +var prURLRE = regexp.MustCompile(`^https://github\.com/([^/]+)/([^/]+)/pull/(\d+)`) + +func prFromArg(apiClient *api.Client, baseRepo context.GitHubRepository, arg string) (*api.PullRequest, error) { + if prNumber, err := strconv.Atoi(arg); err == nil { + return api.PullRequestByNumber(apiClient, baseRepo, prNumber) + } + + if m := prURLRE.FindStringSubmatch(arg); m != nil { + return &api.PullRequest{URL: m[0]}, nil + } + + return api.PullRequestForBranch(apiClient, baseRepo, arg) +} + func prSelectorForCurrentBranch(ctx context.Context) (prNumber int, prHeadRef string, err error) { baseRepo, err := ctx.BaseRepo() if err != nil { @@ -311,11 +312,6 @@ func prSelectorForCurrentBranch(ctx context.Context) (prNumber int, prHeadRef st } func prCheckout(cmd *cobra.Command, args []string) error { - prNumber, err := strconv.Atoi(args[0]) - if err != nil { - return err - } - ctx := contextForCommand(cmd) currentBranch, _ := ctx.Branch() remotes, err := ctx.Remotes() @@ -332,10 +328,19 @@ func prCheckout(cmd *cobra.Command, args []string) error { return err } - pr, err := api.PullRequestByNumber(apiClient, baseRemote, prNumber) + pr, err := prFromArg(apiClient, baseRemote, args[0]) if err != nil { return err } + if pr.Number == 0 { + // hydrate the pr object by fetching extra information from the API + m := prURLRE.FindStringSubmatch(pr.URL) + prNumber, _ := strconv.Atoi(m[3]) + pr, err = api.PullRequestByNumber(apiClient, baseRemote, prNumber) + if err != nil { + return err + } + } headRemote := baseRemote if pr.IsCrossRepository { @@ -369,7 +374,7 @@ func prCheckout(cmd *cobra.Command, args []string) error { newBranchName = fmt.Sprintf("%s/%s", pr.HeadRepositoryOwner.Login, newBranchName) } - ref := fmt.Sprintf("refs/pull/%d/head", prNumber) + ref := fmt.Sprintf("refs/pull/%d/head", pr.Number) if newBranchName == currentBranch { // PR head matches currently checked out branch cmdQueue = append(cmdQueue, []string{"git", "fetch", baseRemote.Name, ref}) diff --git a/command/pr_checkout_test.go b/command/pr_checkout_test.go index f4dd95dcc..b3b991c39 100644 --- a/command/pr_checkout_test.go +++ b/command/pr_checkout_test.go @@ -23,6 +23,7 @@ func TestPRCheckout_sameRepo(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { + "number": 123, "headRefName": "feature", "headRepositoryOwner": { "login": "hubot" @@ -61,6 +62,104 @@ func TestPRCheckout_sameRepo(t *testing.T) { eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.merge refs/heads/feature") } +func TestPRCheckout_urlArg(t *testing.T) { + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequest": { + "number": 123, + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": false, + "maintainerCanModify": false + } } } } + `)) + + ranCommands := [][]string{} + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + switch strings.Join(cmd.Args, " ") { + case "git show-ref --verify --quiet refs/heads/feature": + return &errorStub{"exit status: 1"} + default: + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + } + }) + defer restoreCmd() + + output, err := RunCommand(prCheckoutCmd, `pr checkout https://github.com/OWNER/REPO/pull/123/files`) + eq(t, err, nil) + eq(t, output, "") + + eq(t, len(ranCommands), 4) + eq(t, strings.Join(ranCommands[1], " "), "git checkout -b feature --no-track origin/feature") +} + +func TestPRCheckout_branchArg(t *testing.T) { + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes": [ + { "number": 123, + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "REPO", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": true, + "maintainerCanModify": false } + ] } } } } + `)) + + ranCommands := [][]string{} + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + switch strings.Join(cmd.Args, " ") { + case "git show-ref --verify --quiet refs/heads/feature": + return &errorStub{"exit status: 1"} + default: + ranCommands = append(ranCommands, cmd.Args) + return &outputStub{} + } + }) + defer restoreCmd() + + output, err := RunCommand(prCheckoutCmd, `pr checkout hubot:feature`) + eq(t, err, nil) + eq(t, output, "") + + eq(t, len(ranCommands), 5) + eq(t, strings.Join(ranCommands[1], " "), "git fetch origin refs/pull/123/head:feature") +} + func TestPRCheckout_existingBranch(t *testing.T) { ctx := context.NewBlank() ctx.SetBranch("master") @@ -74,6 +173,7 @@ func TestPRCheckout_existingBranch(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { + "number": 123, "headRefName": "feature", "headRepositoryOwner": { "login": "hubot" @@ -125,6 +225,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { + "number": 123, "headRefName": "feature", "headRepositoryOwner": { "login": "hubot" @@ -176,6 +277,7 @@ func TestPRCheckout_differentRepo(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { + "number": 123, "headRefName": "feature", "headRepositoryOwner": { "login": "hubot" @@ -227,6 +329,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { + "number": 123, "headRefName": "feature", "headRepositoryOwner": { "login": "hubot" @@ -276,6 +379,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { + "number": 123, "headRefName": "feature", "headRepositoryOwner": { "login": "hubot" @@ -325,6 +429,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { + "number": 123, "headRefName": "feature", "headRepositoryOwner": { "login": "hubot"