From b7521e16c653570f0231f58c74dc922ddae48ecd Mon Sep 17 00:00:00 2001 From: Frederick Zhang Date: Thu, 25 Jul 2024 17:11:02 +1000 Subject: [PATCH] fixup! Only find PRs using branch..merge if push.default = upstream --- pkg/cmd/pr/shared/finder.go | 2 +- pkg/cmd/pr/shared/finder_test.go | 45 ++++++++++++++++++++++++++++++++ pkg/cmd/pr/status/status.go | 2 +- pkg/cmd/pr/status/status_test.go | 34 +++++++++++++++++++++++- 4 files changed, 80 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/pr/shared/finder.go b/pkg/cmd/pr/shared/finder.go index c350e615082..2adc34dc3db 100644 --- a/pkg/cmd/pr/shared/finder.go +++ b/pkg/cmd/pr/shared/finder.go @@ -266,7 +266,7 @@ func (f *finder) parseCurrentBranch() (string, int, error) { if branchOwner != "" { if branchConfig.Push != "" { prHeadRef = strings.TrimPrefix(branchConfig.Push, branchConfig.PushRemoteName+"/") - } else if pushDefault, _ := f.pushDefault(); pushDefault == "upstream" && + } else if pushDefault, _ := f.pushDefault(); (pushDefault == "upstream" || pushDefault == "tracking") && strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") { prHeadRef = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/") } diff --git a/pkg/cmd/pr/shared/finder_test.go b/pkg/cmd/pr/shared/finder_test.go index dae75ae2916..4591a76358d 100644 --- a/pkg/cmd/pr/shared/finder_test.go +++ b/pkg/cmd/pr/shared/finder_test.go @@ -399,6 +399,51 @@ func TestFind(t *testing.T) { wantPR: 13, wantRepo: "https://github.com/OWNER/REPO", }, + { + name: "current branch with tracking (deprecated synonym of upstream) configuration", + args: args{ + selector: "", + fields: []string{"id", "number"}, + baseRepoFn: func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("OWNER/REPO") + }, + branchFn: func() (string, error) { + return "blueberries", nil + }, + branchConfig: func(branch string) (c git.BranchConfig) { + c.MergeRef = "refs/heads/blue-upstream-berries" + c.RemoteName = "origin" + c.PushRemoteName = "origin" + c.Push = "origin/blue-upstream-berries" + return + }, + pushDefault: func() (string, error) { return "tracking", nil }, + remotesFn: func() (context.Remotes, error) { + return context.Remotes{{ + Remote: &git.Remote{Name: "origin"}, + Repo: ghrepo.New("UPSTREAMOWNER", "REPO"), + }}, nil + }, + }, + httpStub: func(r *httpmock.Registry) { + r.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(`{"data":{"repository":{ + "pullRequests":{"nodes":[ + { + "number": 13, + "state": "OPEN", + "baseRefName": "main", + "headRefName": "blue-upstream-berries", + "isCrossRepository": true, + "headRepositoryOwner": {"login":"UPSTREAMOWNER"} + } + ]} + }}}`)) + }, + wantPR: 13, + wantRepo: "https://github.com/OWNER/REPO", + }, { name: "current branch made by pr checkout", args: args{ diff --git a/pkg/cmd/pr/status/status.go b/pkg/cmd/pr/status/status.go index 2ade18719ee..33fc096c8e8 100644 --- a/pkg/cmd/pr/status/status.go +++ b/pkg/cmd/pr/status/status.go @@ -211,7 +211,7 @@ func prSelectorForCurrentBranch(gitClient *git.Client, baseRepo ghrepo.Interface if branchOwner != "" { if branchConfig.Push != "" { selector = strings.TrimPrefix(branchConfig.Push, branchConfig.PushRemoteName+"/") - } else if pushDefault, _ := gitClient.Config(context.Background(), "push.default"); pushDefault == "upstream" && + } else if pushDefault, _ := gitClient.Config(context.Background(), "push.default"); (pushDefault == "upstream" || pushDefault == "tracking") && strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") { selector = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/") } diff --git a/pkg/cmd/pr/status/status_test.go b/pkg/cmd/pr/status/status_test.go index 7a4d73bdb1a..e60764c4f2c 100644 --- a/pkg/cmd/pr/status/status_test.go +++ b/pkg/cmd/pr/status/status_test.go @@ -375,7 +375,7 @@ Requesting a code review from you } } -func Test_prSelectorForCurrentBranch(t *testing.T) { +func Test_prSelectorForCurrentBranchPushDefaultUpstream(t *testing.T) { rs, cleanup := run.Stub() defer cleanup(t) @@ -406,3 +406,35 @@ func Test_prSelectorForCurrentBranch(t *testing.T) { t.Errorf("expected headRef to be \"Frederick888:main\", got %q", headRef) } } + +func Test_prSelectorForCurrentBranchPushDefaultTracking(t *testing.T) { + rs, cleanup := run.Stub() + defer cleanup(t) + + rs.Register(`git config --get-regexp \^branch\\.`, 0, heredoc.Doc(` + branch.Frederick888/main.remote git@github.com:Frederick888/playground.git + branch.Frederick888/main.merge refs/heads/main + `)) + rs.Register(`git config remote.pushDefault`, 1, "") + rs.Register(`git rev-parse --verify --quiet --abbrev-ref Frederick888/main@\{push\}`, 1, "") + rs.Register(`git config push\.default`, 0, "tracking") + + repo := ghrepo.NewWithHost("octocat", "playground", "github.com") + rem := context.Remotes{ + &context.Remote{ + Remote: &git.Remote{Name: "origin"}, + Repo: repo, + }, + } + gitClient := &git.Client{GitPath: "some/path/git"} + prNum, headRef, err := prSelectorForCurrentBranch(gitClient, repo, "Frederick888/main", rem) + if err != nil { + t.Fatalf("prSelectorForCurrentBranch error: %v", err) + } + if prNum != 0 { + t.Errorf("expected prNum to be 0, got %q", prNum) + } + if headRef != "Frederick888:main" { + t.Errorf("expected headRef to be \"Frederick888:main\", got %q", headRef) + } +}