Find PRs using @{push}

When using a push.default = current central workflow [1], we should use
@{push} instead to locate the remote branch.

In fact, @{push} covers most cases in push.default = upstream too. The
branch.<name>.merge is probably only needed when using RemoteURL and
different remote / local branch names.

[1] https://github.com/tpope/vim-fugitive/issues/1172#issuecomment-522301607
This commit is contained in:
Frederick Zhang 2024-06-14 14:43:38 +10:00
parent 40984d1eb6
commit 0179381efd
No known key found for this signature in database
GPG key ID: 980A192C361BE1AE
8 changed files with 96 additions and 13 deletions

View file

@ -412,6 +412,9 @@ func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (cfg Branc
cfg.MergeBase = parts[1]
}
}
if out, err = c.revParse(ctx, "--verify", "--quiet", "--abbrev-ref", branch+"@{push}"); err == nil {
cfg.Push = strings.TrimSuffix(string(out), "\n")
}
return
}

View file

@ -727,28 +727,43 @@ func TestClientCommitBody(t *testing.T) {
func TestClientReadBranchConfig(t *testing.T) {
tests := []struct {
name string
cmdExitStatus int
cmdStdout string
cmdStderr string
wantCmdArgs string
cmdExitStatus []int
cmdStdout []string
cmdStderr []string
wantCmdArgs []string
wantBranchConfig BranchConfig
}{
{
name: "read branch config",
cmdStdout: "branch.trunk.remote origin\nbranch.trunk.merge refs/heads/trunk\nbranch.trunk.gh-merge-base trunk",
wantCmdArgs: `path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|gh-merge-base)$`,
wantBranchConfig: BranchConfig{RemoteName: "origin", MergeRef: "refs/heads/trunk", MergeBase: "trunk"},
cmdExitStatus: []int{0, 0},
cmdStdout: []string{"branch.trunk.remote origin\nbranch.trunk.merge refs/heads/trunk\nbranch.trunk.gh-merge-base trunk", "origin/trunk"},
cmdStderr: []string{"", ""},
wantCmdArgs: []string{`path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|gh-merge-base)$`, `path/to/git rev-parse --verify --quiet --abbrev-ref trunk@{push}`},
wantBranchConfig: BranchConfig{RemoteName: "origin", MergeRef: "refs/heads/trunk", MergeBase: "trunk", Push: "origin/trunk"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cmd, cmdCtx := createCommandContext(t, tt.cmdExitStatus, tt.cmdStdout, tt.cmdStderr)
var cmds []*exec.Cmd
var cmdCtxs []commandCtx
for i := 0; i < len(tt.cmdExitStatus); i++ {
cmd, cmdCtx := createCommandContext(t, tt.cmdExitStatus[i], tt.cmdStdout[i], tt.cmdStderr[i])
cmds = append(cmds, cmd)
cmdCtxs = append(cmdCtxs, cmdCtx)
}
i := -1
client := Client{
GitPath: "path/to/git",
commandContext: cmdCtx,
GitPath: "path/to/git",
commandContext: func(ctx context.Context, name string, args ...string) *exec.Cmd {
i++
cmdCtxs[i](ctx, name, args...)
return cmds[i]
},
}
branchConfig := client.ReadBranchConfig(context.Background(), "trunk")
assert.Equal(t, tt.wantCmdArgs, strings.Join(cmd.Args[3:], " "))
for i := 0; i < len(tt.cmdExitStatus); i++ {
assert.Equal(t, tt.wantCmdArgs[i], strings.Join(cmds[i].Args[3:], " "))
}
assert.Equal(t, tt.wantBranchConfig, branchConfig)
})
}

View file

@ -66,4 +66,5 @@ type BranchConfig struct {
// MergeBase is the optional base branch to target in a new PR if `--base` is not specified.
MergeBase string
MergeRef string
Push string
}

View file

@ -354,6 +354,7 @@ func Test_createRun(t *testing.T) {
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "")
},
expectedBrowse: "https://github.com/OWNER/REPO/compare/master...feature?body=&expand=1",
@ -383,6 +384,9 @@ func Test_createRun(t *testing.T) {
opts.HeadBranch = "feature"
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
},
expectedOut: "https://github.com/OWNER/REPO/pull/12\n",
},
{
@ -397,6 +401,9 @@ func Test_createRun(t *testing.T) {
opts.DryRun = true
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
},
expectedOutputs: []string{
"Would have created a Pull Request with:",
`title: my title`,
@ -428,6 +435,9 @@ func Test_createRun(t *testing.T) {
opts.DryRun = true
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
},
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
reg.Register(
httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`),
@ -488,6 +498,9 @@ func Test_createRun(t *testing.T) {
opts.DryRun = true
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
},
expectedOutputs: []string{
`Would have created a Pull Request with:`,
`Title: my title`,
@ -525,6 +538,9 @@ func Test_createRun(t *testing.T) {
opts.DryRun = true
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
},
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
reg.Register(
httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`),
@ -591,6 +607,9 @@ func Test_createRun(t *testing.T) {
opts.DryRun = true
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
},
expectedOut: heredoc.Doc(`
Would have created a Pull Request with:
Title: TITLE
@ -637,6 +656,7 @@ func Test_createRun(t *testing.T) {
}))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "")
cs.Register(`git push --set-upstream origin HEAD:refs/heads/feature`, 0, "")
},
@ -700,6 +720,7 @@ func Test_createRun(t *testing.T) {
}))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "")
cs.Register(`git push --set-upstream origin HEAD:refs/heads/feature`, 0, "")
},
@ -746,6 +767,7 @@ func Test_createRun(t *testing.T) {
}))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "")
cs.Register(`git push --set-upstream origin HEAD:refs/heads/feature`, 0, "")
},
@ -795,6 +817,7 @@ func Test_createRun(t *testing.T) {
}))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "")
cs.Register("git remote rename origin upstream", 0, "")
cs.Register(`git remote add origin https://github.com/monalisa/REPO.git`, 0, "")
@ -853,6 +876,7 @@ func Test_createRun(t *testing.T) {
}))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register("git show-ref --verify", 0, heredoc.Doc(`
deadbeef HEAD
deadb00f refs/remotes/upstream/feature
@ -890,6 +914,7 @@ func Test_createRun(t *testing.T) {
branch.feature.remote origin
branch.feature.merge refs/heads/my-feat2
`)) // determineTrackingBranch
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 0, "origin/my-feat2")
cs.Register("git show-ref --verify", 0, heredoc.Doc(`
deadbeef HEAD
deadbeef refs/remotes/origin/my-feat2
@ -930,6 +955,7 @@ func Test_createRun(t *testing.T) {
}))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "d3476a1\u0000commit 0\u0000\u0000\n7a6ea13\u0000commit 1\u0000\u0000")
},
promptStubs: func(pm *prompter.PrompterMock) {
@ -970,6 +996,9 @@ func Test_createRun(t *testing.T) {
opts.Milestone = "big one.oh"
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
},
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
reg.Register(
httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`),
@ -1057,6 +1086,9 @@ func Test_createRun(t *testing.T) {
opts.Finder = shared.NewMockFinder("feature", &api.PullRequest{URL: "https://github.com/OWNER/REPO/pull/123"}, ghrepo.New("OWNER", "REPO"))
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
},
wantErr: "a pull request for branch \"feature\" into branch \"master\" already exists:\nhttps://github.com/OWNER/REPO/pull/123",
},
{
@ -1073,6 +1105,7 @@ func Test_createRun(t *testing.T) {
httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "")
cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "")
cs.Register(`git push --set-upstream origin HEAD:refs/heads/feature`, 0, "")
@ -1105,6 +1138,7 @@ func Test_createRun(t *testing.T) {
mockRetrieveProjects(t, reg)
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "")
cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "")
cs.Register(`git push --set-upstream origin HEAD:refs/heads/feature`, 0, "")
@ -1152,6 +1186,7 @@ func Test_createRun(t *testing.T) {
}))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register(`git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b%x00 --cherry origin/master...feature`, 0, "")
cs.Register(`git rev-parse --show-toplevel`, 0, "")
},
@ -1211,6 +1246,7 @@ func Test_createRun(t *testing.T) {
}))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "")
},
promptStubs: func(pm *prompter.PrompterMock) {
@ -1259,6 +1295,7 @@ func Test_createRun(t *testing.T) {
{
name: "web long URL",
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "")
},
setup: func(opts *CreateOptions, t *testing.T) func() {
@ -1285,6 +1322,9 @@ func Test_createRun(t *testing.T) {
}
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
},
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
reg.Register(
httpmock.GraphQL(`mutation PullRequestCreate\b`),
@ -1304,6 +1344,7 @@ func Test_createRun(t *testing.T) {
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register(
"git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b%x00 --cherry origin/master...feature",
0,
@ -1379,6 +1420,7 @@ func Test_createRun(t *testing.T) {
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register(
"git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b%x00 --cherry origin/master...feature",
0,
@ -1415,6 +1457,7 @@ func Test_createRun(t *testing.T) {
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register(
"git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b%x00 --cherry origin/master...feature",
0,
@ -1465,6 +1508,7 @@ func Test_createRun(t *testing.T) {
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register("git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b%x00 --cherry origin/master...feature", 0, "")
},
expectedOut: "https://github.com/OWNER/REPO/pull/12\n",
@ -1524,6 +1568,7 @@ func Test_createRun(t *testing.T) {
deadbeef HEAD
deadb00f refs/remotes/upstream/feature/feat2
deadbeef refs/remotes/origin/task1`)) // determineTrackingBranch
cs.Register(`git rev-parse --verify --quiet --abbrev-ref task1@\{push\}`, 1, "")
},
expectedOut: "https://github.com/OWNER/REPO/pull/12\n",
expectedErrOut: "\nCreating pull request for monalisa:task1 into feature/feat2 in OWNER/REPO\n\n",
@ -1634,6 +1679,7 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
name: "empty",
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "")
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register(`git show-ref --verify -- HEAD`, 0, "abc HEAD")
},
expectedTrackingRef: trackingRef{},
@ -1643,6 +1689,7 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
name: "no match",
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "")
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register("git show-ref --verify -- HEAD refs/remotes/upstream/feature refs/remotes/origin/feature", 0, "abc HEAD\nbca refs/remotes/upstream/feature")
},
remotes: context.Remotes{
@ -1662,6 +1709,7 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
name: "match",
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "")
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 0, "origin/feature")
cs.Register(`git show-ref --verify -- HEAD refs/remotes/upstream/feature refs/remotes/origin/feature$`, 0, heredoc.Doc(`
deadbeef HEAD
deadb00f refs/remotes/upstream/feature
@ -1691,6 +1739,7 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
branch.feature.remote origin
branch.feature.merge refs/heads/great-feat
`))
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 0, "origin/great-feat")
cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/great-feat refs/remotes/origin/feature$`, 0, heredoc.Doc(`
deadbeef HEAD
deadb00f refs/remotes/origin/feature

View file

@ -261,7 +261,9 @@ func (f *finder) parseCurrentBranch() (string, int, error) {
}
if gitRemoteRepo != nil {
if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
if branchConfig.Push != "" {
prHeadRef = strings.TrimPrefix(branchConfig.Push, branchConfig.RemoteName+"/")
} else if 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

@ -232,8 +232,17 @@ func TestFind(t *testing.T) {
return "blueberries", nil
},
branchConfig: func(branch string) (c git.BranchConfig) {
c.MergeRef = "refs/heads/blueberries"
c.RemoteName = "origin"
c.Push = "origin/blueberries"
return
},
remotesFn: func() (context.Remotes, error) {
return context.Remotes{{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.New("OWNER", "REPO"),
}}, nil
},
},
httpStub: func(r *httpmock.Registry) {
r.Register(
@ -318,6 +327,7 @@ func TestFind(t *testing.T) {
branchConfig: func(branch string) (c git.BranchConfig) {
c.MergeRef = "refs/heads/blue-upstream-berries"
c.RemoteName = "origin"
c.Push = "origin/blue-upstream-berries"
return
},
remotesFn: func() (context.Remotes, error) {

View file

@ -209,7 +209,9 @@ func prSelectorForCurrentBranch(gitClient *git.Client, baseRepo ghrepo.Interface
}
if branchOwner != "" {
if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
if branchConfig.Push != "" {
selector = strings.TrimPrefix(branchConfig.Push, branchConfig.RemoteName+"/")
} else if 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

@ -383,6 +383,7 @@ func Test_prSelectorForCurrentBranch(t *testing.T) {
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, "")
repo := ghrepo.NewWithHost("octocat", "playground", "github.com")
rem := context.Remotes{