From 201217d88ab075eb92482a61649865d822cde365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 2 Mar 2020 11:35:20 +0100 Subject: [PATCH] Fix detecting PR for current branch pushed to fork Now passing base repo as reference to `prSelectorForCurrentBranch()` so that it doesn't have to (inexactly) determine the base repo itself and risk it being different than the base repo determined during `pr status/view`. --- api/fake_http.go | 33 +++++++++++++++++++++++++++++++++ command/pr.go | 17 +++++++---------- command/pr_test.go | 30 ++++++++++++++++++++++++++++++ test/fixtures/prStatusFork.json | 31 +++++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 10 deletions(-) create mode 100644 test/fixtures/prStatusFork.json diff --git a/api/fake_http.go b/api/fake_http.go index 32b7d56eb..6a3605b0b 100644 --- a/api/fake_http.go +++ b/api/fake_http.go @@ -6,6 +6,7 @@ import ( "io" "io/ioutil" "net/http" + "strings" ) // FakeHTTP provides a mechanism by which to stub HTTP responses through @@ -56,3 +57,35 @@ func (f *FakeHTTP) StubRepoResponse(owner, repo string) { } f.responseStubs = append(f.responseStubs, resp) } + +func (f *FakeHTTP) StubForkedRepoResponse(forkFullName, parentFullName string) { + forkRepo := strings.SplitN(forkFullName, "/", 2) + parentRepo := strings.SplitN(parentFullName, "/", 2) + body := bytes.NewBufferString(fmt.Sprintf(` + { "data": { "repo_000": { + "id": "REPOID2", + "name": "%s", + "owner": {"login": "%s"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "ADMIN", + "parent": { + "id": "REPOID1", + "name": "%s", + "owner": {"login": "%s"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "READ" + } + } } } + `, forkRepo[1], forkRepo[0], parentRepo[1], parentRepo[0])) + resp := &http.Response{ + StatusCode: 200, + Body: ioutil.NopCloser(body), + } + f.responseStubs = append(f.responseStubs, resp) +} diff --git a/command/pr.go b/command/pr.go index a318154be..e6427aebb 100644 --- a/command/pr.go +++ b/command/pr.go @@ -71,10 +71,6 @@ func prStatus(cmd *cobra.Command, args []string) error { return err } - currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx) - if err != nil { - return err - } currentUser, err := ctx.AuthLogin() if err != nil { return err @@ -85,6 +81,11 @@ func prStatus(cmd *cobra.Command, args []string) error { return err } + currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx, baseRepo) + if err != nil { + return err + } + prPayload, err := api.PullRequests(apiClient, baseRepo, currentPRNumber, currentPRHeadRef, currentUser) if err != nil { return err @@ -275,7 +276,7 @@ func prView(cmd *cobra.Command, args []string) error { } openURL = pr.URL } else { - prNumber, branchWithOwner, err := prSelectorForCurrentBranch(ctx) + prNumber, branchWithOwner, err := prSelectorForCurrentBranch(ctx, baseRepo) if err != nil { return err } @@ -345,11 +346,7 @@ func prFromArg(apiClient *api.Client, baseRepo ghrepo.Interface, arg string) (*a return api.PullRequestForBranch(apiClient, baseRepo, arg) } -func prSelectorForCurrentBranch(ctx context.Context) (prNumber int, prHeadRef string, err error) { - baseRepo, err := ctx.BaseRepo() - if err != nil { - return - } +func prSelectorForCurrentBranch(ctx context.Context, baseRepo ghrepo.Interface) (prNumber int, prHeadRef string, err error) { prHeadRef, err = ctx.Branch() if err != nil { return diff --git a/command/pr_test.go b/command/pr_test.go index efa894802..51469342a 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -98,6 +98,36 @@ func TestPRStatus(t *testing.T) { } } +func TestPRStatus_fork(t *testing.T) { + initBlankContext("OWNER/REPO", "blueberries") + http := initFakeHTTP() + http.StubForkedRepoResponse("OWNER/REPO", "PARENT/REPO") + + jsonFile, _ := os.Open("../test/fixtures/prStatusFork.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + defer utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + switch strings.Join(cmd.Args, " ") { + case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`: + return &outputStub{[]byte(`branch.blueberries.remote origin +branch.blueberries.merge refs/heads/blueberries`)} + default: + panic("not implemented") + } + })() + + output, err := RunCommand(prStatusCmd, "pr status") + if err != nil { + t.Fatalf("error running command `pr status`: %v", err) + } + + branchRE := regexp.MustCompile(`#10.*\[OWNER:blueberries\]`) + if !branchRE.MatchString(output.String()) { + t.Errorf("did not match current branch:\n%v", output.String()) + } +} + func TestPRStatus_reviewsAndChecks(t *testing.T) { initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() diff --git a/test/fixtures/prStatusFork.json b/test/fixtures/prStatusFork.json new file mode 100644 index 000000000..df184aaea --- /dev/null +++ b/test/fixtures/prStatusFork.json @@ -0,0 +1,31 @@ +{ + "data": { + "repository": { + "pullRequests": { + "totalCount": 1, + "edges": [ + { + "node": { + "number": 10, + "title": "Blueberries are a good fruit", + "url": "https://github.com/PARENT/REPO/pull/10", + "headRefName": "blueberries", + "headRepositoryOwner": { + "login": "OWNER" + }, + "isCrossRepository": true + } + } + ] + } + }, + "viewerCreated": { + "totalCount": 0, + "edges": [] + }, + "reviewRequested": { + "totalCount": 0, + "edges": [] + } + } +}