STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix pr create tests

Browse source 
Due to the refactor of BranchConfig, the tests for `pr create` no longer
require a handful of stubbed git commands.

Additionally, I noticed some overlap between `pr create`'s existing logic
with the new finder code. I suspect that the finder's new ParsePRRefs
helper could be leveraged here as well, which would allow us to respect
non-centralized workflows pr create in the same way we are respecting them
in `gh pr view` and `gh pr status`
This commit is contained in:
Tyler McGoffin 2025-01-24 14:01:44 -08:00
parent d684834ad9
commit 4382bdf072
3 changed files with 10 additions and 88 deletions
Download patch file Download diff file Expand all files Collapse all files

5
pkg/cmd/pr/create/create.go
View file

@ -518,6 +518,7 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, u
return nil
}
// TODO: Replace with the finder's PRRefs struct
// trackingRef represents a ref for a remote tracking branch.
type trackingRef struct {
remoteName string
@ -685,7 +686,9 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) {
return nil, err
}
if isPushEnabled {
// determine whether the head branch is already pushed to a remote
// TODO: This doesn't respect the @{push} revision resolution or triagular workflows assembled with
// remote.pushDefault, or branch.<branchName>.pushremote config settings. The finder's ParsePRRefs
// may be able to replace this function entirely.
if trackingRef, found := tryDetermineTrackingRef(gitClient, remotes, headBranch, headBranchConfig); found {
isPushEnabled = false
if r, err := remotes.FindByName(trackingRef.remoteName); err == nil {

87
pkg/cmd/pr/create/create_test.go
View file

@ -353,8 +353,6 @@ func Test_createRun(t *testing.T) {
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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",
@ -384,10 +382,6 @@ func Test_createRun(t *testing.T) {
opts.HeadBranch = "feature"
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
},
expectedOut: "https://github.com/OWNER/REPO/pull/12\n",
},
{
@ -402,10 +396,6 @@ func Test_createRun(t *testing.T) {
opts.DryRun = true
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
},
expectedOutputs: []string{
"Would have created a Pull Request with:",
`title: my title`,
@ -437,10 +427,6 @@ func Test_createRun(t *testing.T) {
opts.DryRun = true
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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`),
@ -501,10 +487,6 @@ func Test_createRun(t *testing.T) {
opts.DryRun = true
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
},
expectedOutputs: []string{
`Would have created a Pull Request with:`,
`Title: my title`,
@ -542,10 +524,6 @@ func Test_createRun(t *testing.T) {
opts.DryRun = true
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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`),
@ -612,10 +590,6 @@ func Test_createRun(t *testing.T) {
opts.DryRun = true
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
},
expectedOut: heredoc.Doc(`
Would have created a Pull Request with:
Title: TITLE
@ -662,8 +636,6 @@ func Test_createRun(t *testing.T) {
}))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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, "")
},
@ -727,8 +699,6 @@ func Test_createRun(t *testing.T) {
}))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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, "")
},
@ -775,8 +745,6 @@ func Test_createRun(t *testing.T) {
}))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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, "")
},
@ -826,8 +794,6 @@ func Test_createRun(t *testing.T) {
}))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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, "")
@ -886,8 +852,6 @@ func Test_createRun(t *testing.T) {
}))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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
@ -925,8 +889,6 @@ func Test_createRun(t *testing.T) {
branch.feature.remote origin
branch.feature.merge refs/heads/my-feat2
`)) // determineTrackingBranch
cs.Register(`git config remote.pushDefault`, 1, "")
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
@ -967,8 +929,6 @@ func Test_createRun(t *testing.T) {
}))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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) {
@ -1009,10 +969,6 @@ func Test_createRun(t *testing.T) {
opts.Milestone = "big one.oh"
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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`),
@ -1100,10 +1056,6 @@ 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 config remote.pushDefault`, 1, "")
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",
},
{
@ -1120,8 +1072,6 @@ func Test_createRun(t *testing.T) {
httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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, "")
@ -1154,8 +1104,6 @@ func Test_createRun(t *testing.T) {
mockRetrieveProjects(t, reg)
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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, "")
@ -1203,8 +1151,6 @@ func Test_createRun(t *testing.T) {
}))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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, "")
},
@ -1264,8 +1210,6 @@ func Test_createRun(t *testing.T) {
}))
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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) {
@ -1314,8 +1258,6 @@ func Test_createRun(t *testing.T) {
{
name: "web long URL",
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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() {
@ -1342,10 +1284,6 @@ func Test_createRun(t *testing.T) {
}
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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`),
@ -1365,8 +1303,6 @@ func Test_createRun(t *testing.T) {
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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,
@ -1442,8 +1378,6 @@ func Test_createRun(t *testing.T) {
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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,
@ -1480,8 +1414,6 @@ func Test_createRun(t *testing.T) {
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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,
@ -1532,8 +1464,6 @@ func Test_createRun(t *testing.T) {
return func() {}
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config remote.pushDefault`, 1, "")
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",
@ -1593,8 +1523,6 @@ func Test_createRun(t *testing.T) {
deadbeef HEAD
deadb00f refs/remotes/upstream/feature/feat2
deadbeef refs/remotes/origin/task1`)) // determineTrackingBranch
cs.Register(`git config remote.pushDefault`, 1, "")
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",
@ -1705,9 +1633,6 @@ 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 config remote.pushDefault`, 1, "")
cs.Register(`git rev-parse --verify --quiet --abbrev-ref feature@\{push\}`, 1, "")
cs.Register(`git show-ref --verify -- HEAD`, 0, "abc HEAD")
},
headBranchConfig: git.BranchConfig{},
@ -1717,9 +1642,6 @@ 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 config remote.pushDefault`, 1, "")
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")
},
headBranchConfig: git.BranchConfig{},
@ -1739,9 +1661,6 @@ 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 config remote.pushDefault`, 1, "")
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
@ -1768,12 +1687,6 @@ func Test_tryDetermineTrackingRef(t *testing.T) {
{
name: "respect tracking config",
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, heredoc.Doc(`
branch.feature.remote origin
branch.feature.merge refs/heads/great-feat
`))
cs.Register(`git config remote.pushDefault`, 1, "")
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

6
pkg/cmd/pr/shared/finder.go
View file

@ -97,6 +97,12 @@ type FindOptions struct {
States []string
}
// TODO: Does this also need the BaseBranchName?
// PR's are represented by the following:
// baseRef -----PR-----> headRef
//
// A ref is described as "remoteName/branchName", so
// baseRepoName/baseBranchName -----PR-----> headRepoName/headBranchName
type PRRefs struct {
BranchName string
HeadRepo ghrepo.Interface