Only find PRs w/ branch.<name>.merge if push.default = upstream/tracking

When push.default is not 'upstream' / 'tracking' (or 'nothing'), we can
expect local and remote branch names to be the same and solely rely on
@{push} or RemoteURL.

This fixes the wrong error message 'no pull requests found for branch
"<target branch>"' when the local branch is not pushed in the
push.default = simple / current and upstream = <target branch> setup.
This commit is contained in:
Frederick Zhang 2024-06-14 15:10:53 +10:00
parent 0179381efd
commit 7fc35fd47d
No known key found for this signature in database
GPG key ID: 980A192C361BE1AE
4 changed files with 92 additions and 5 deletions

View file

@ -38,6 +38,7 @@ type finder struct {
remotesFn func() (remotes.Remotes, error)
httpClient func() (*http.Client, error)
branchConfig func(string) git.BranchConfig
pushDefault func() (string, error)
progress progressIndicator
repo ghrepo.Interface
@ -57,7 +58,10 @@ func NewFinder(factory *cmdutil.Factory) PRFinder {
branchFn: factory.Branch,
remotesFn: factory.Remotes,
httpClient: factory.HttpClient,
progress: factory.IOStreams,
pushDefault: func() (string, error) {
return factory.GitClient.Config(context.Background(), "push.default")
},
progress: factory.IOStreams,
branchConfig: func(s string) git.BranchConfig {
return factory.GitClient.ReadBranchConfig(context.Background(), s)
},
@ -263,7 +267,8 @@ func (f *finder) parseCurrentBranch() (string, int, error) {
if gitRemoteRepo != nil {
if branchConfig.Push != "" {
prHeadRef = strings.TrimPrefix(branchConfig.Push, branchConfig.RemoteName+"/")
} else if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
} else if pushDefault, _ := f.pushDefault(); (pushDefault == "upstream" || pushDefault == "tracking") &&
strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
prHeadRef = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/")
}
// prepend `OWNER:` if this branch is pushed to a fork

View file

@ -18,6 +18,7 @@ func TestFind(t *testing.T) {
baseRepoFn func() (ghrepo.Interface, error)
branchFn func() (string, error)
branchConfig func(string) git.BranchConfig
pushDefault func() (string, error)
remotesFn func() (context.Remotes, error)
selector string
fields []string
@ -330,6 +331,7 @@ func TestFind(t *testing.T) {
c.Push = "origin/blue-upstream-berries"
return
},
pushDefault: func() (string, error) { return "upstream", nil },
remotesFn: func() (context.Remotes, error) {
return context.Remotes{{
Remote: &git.Remote{Name: "origin"},
@ -373,7 +375,52 @@ func TestFind(t *testing.T) {
c.RemoteURL = u
return
},
remotesFn: nil,
pushDefault: func() (string, error) { return "upstream", nil },
remotesFn: 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 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.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(
@ -407,6 +454,7 @@ func TestFind(t *testing.T) {
},
branchConfig: func(branch string) (c git.BranchConfig) {
c.RemoteName = "origin"
c.Push = "origin/blueberries"
return
},
remotesFn: func() (context.Remotes, error) {
@ -539,6 +587,7 @@ func TestFind(t *testing.T) {
baseRepoFn: tt.args.baseRepoFn,
branchFn: tt.args.branchFn,
branchConfig: tt.args.branchConfig,
pushDefault: tt.args.pushDefault,
remotesFn: tt.args.remotesFn,
}

View file

@ -211,7 +211,8 @@ func prSelectorForCurrentBranch(gitClient *git.Client, baseRepo ghrepo.Interface
if branchOwner != "" {
if branchConfig.Push != "" {
selector = strings.TrimPrefix(branchConfig.Push, branchConfig.RemoteName+"/")
} else if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
} 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/")
}
// prepend `OWNER:` if this branch is pushed to a fork

View file

@ -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)
@ -384,6 +384,38 @@ func Test_prSelectorForCurrentBranch(t *testing.T) {
branch.Frederick888/main.merge refs/heads/main
`))
rs.Register(`git rev-parse --verify --quiet --abbrev-ref Frederick888/main@\{push\}`, 1, "")
rs.Register(`git config push\.default`, 0, "upstream")
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)
}
}
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 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{