X Tutup
Skip to content

Commit 51f7cbd

Browse files
mislavmislav
committed
💅 cleanup and tests for PR finder
1 parent bc3bb97 commit 51f7cbd

File tree

6 files changed

+317
-47
lines changed

6 files changed

+317
-47
lines changed

‎api/queries_pr.go‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,6 @@ type PullRequestCommitCommit struct {
118118
}
119119
}
120120

121-
func (pr *PullRequest) StubCommit(oid string) {
122-
pr.Commits.Nodes = append(pr.Commits.Nodes, PullRequestCommit{
123-
Commit: PullRequestCommitCommit{Oid: oid},
124-
})
125-
}
126-
127121
type PullRequestFile struct {
128122
Path string `json:"path"`
129123
Additions int `json:"additions"`

‎pkg/cmd/pr/close/close.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ func closeRun(opts *CloseOptions) error {
122122
}
123123

124124
if pr.IsCrossRepository {
125-
fmt.Fprintf(opts.IO.ErrOut, "%s Avoiding deleting the remote branch of a pull request from fork\n", cs.WarningIcon())
125+
fmt.Fprintf(opts.IO.ErrOut, "%s Skipped deleting the remote branch of a pull request from fork\n", cs.WarningIcon())
126126
if !opts.DeleteLocalBranch {
127127
return nil
128128
}

‎pkg/cmd/pr/close/close_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ func TestPrClose_deleteBranch_crossRepo(t *testing.T) {
191191
assert.Equal(t, "", output.String())
192192
assert.Equal(t, heredoc.Doc(`
193193
✓ Closed pull request #96 (The title of the PR)
194-
! Avoiding deleting the remote branch of a pull request from fork
194+
! Skipped deleting the remote branch of a pull request from fork
195195
✓ Deleted branch blueberries
196196
`), output.Stderr())
197197
}

‎pkg/cmd/pr/merge/merge_test.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,12 @@ func baseRepo(owner, repo, branch string) ghrepo.Interface {
203203
}, "github.com")
204204
}
205205

206+
func stubCommit(pr *api.PullRequest, oid string) {
207+
pr.Commits.Nodes = append(pr.Commits.Nodes, api.PullRequestCommit{
208+
Commit: api.PullRequestCommitCommit{Oid: oid},
209+
})
210+
}
211+
206212
func runCommand(rt http.RoundTripper, branch string, isTTY bool, cli string) (*test.CmdOut, error) {
207213
io, _, stdout, stderr := iostreams.Test()
208214
io.SetStdoutTTY(isTTY)
@@ -456,7 +462,7 @@ func Test_nonDivergingPullRequest(t *testing.T) {
456462
Title: "Blueberries are a good fruit",
457463
State: "OPEN",
458464
}
459-
pr.StubCommit("COMMITSHA1")
465+
stubCommit(pr, "COMMITSHA1")
460466

461467
shared.RunCommandFinder(
462468
"",
@@ -497,7 +503,7 @@ func Test_divergingPullRequestWarning(t *testing.T) {
497503
Title: "Blueberries are a good fruit",
498504
State: "OPEN",
499505
}
500-
pr.StubCommit("COMMITSHA1")
506+
stubCommit(pr, "COMMITSHA1")
501507

502508
shared.RunCommandFinder(
503509
"",

‎pkg/cmd/pr/shared/finder.go‎

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,12 @@ type progressIndicator interface {
2828
}
2929

3030
type finder struct {
31-
baseRepoFn func() (ghrepo.Interface, error)
32-
branchFn func() (string, error)
33-
remotesFn func() (context.Remotes, error)
34-
httpClient func() (*http.Client, error)
35-
progress progressIndicator
31+
baseRepoFn func() (ghrepo.Interface, error)
32+
branchFn func() (string, error)
33+
remotesFn func() (context.Remotes, error)
34+
httpClient func() (*http.Client, error)
35+
branchConfig func(string) git.BranchConfig
36+
progress progressIndicator
3637

3738
repo ghrepo.Interface
3839
prNumber int
@@ -47,11 +48,12 @@ func NewFinder(factory *cmdutil.Factory) PRFinder {
4748
}
4849

4950
return &finder{
50-
baseRepoFn: factory.BaseRepo,
51-
branchFn: factory.Branch,
52-
remotesFn: factory.Remotes,
53-
httpClient: factory.HttpClient,
54-
progress: factory.IOStreams,
51+
baseRepoFn: factory.BaseRepo,
52+
branchFn: factory.Branch,
53+
remotesFn: factory.Remotes,
54+
httpClient: factory.HttpClient,
55+
progress: factory.IOStreams,
56+
branchConfig: git.ReadBranchConfig,
5557
}
5658
}
5759

@@ -79,7 +81,10 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err
7981
return nil, nil, errors.New("Find error: no fields specified")
8082
}
8183

82-
_ = f.parseURL(opts.Selector)
84+
if repo, prNumber, err := f.parseURL(opts.Selector); err == nil {
85+
f.prNumber = prNumber
86+
f.repo = repo
87+
}
8388

8489
if f.repo == nil {
8590
repo, err := f.baseRepoFn()
@@ -90,8 +95,12 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err
9095
}
9196

9297
if opts.Selector == "" {
93-
if err := f.parseCurrentBranch(); err != nil {
98+
if branch, prNumber, err := f.parseCurrentBranch(); err != nil {
9499
return nil, nil, err
100+
} else if prNumber > 0 {
101+
f.prNumber = prNumber
102+
} else {
103+
f.branchName = branch
95104
}
96105
} else if f.prNumber == 0 {
97106
if prNumber, err := strconv.Atoi(strings.TrimPrefix(opts.Selector, "#")); err == nil {
@@ -129,44 +138,44 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err
129138

130139
var pullURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/pull/(\d+)`)
131140

132-
func (f *finder) parseURL(prURL string) error {
141+
func (f *finder) parseURL(prURL string) (ghrepo.Interface, int, error) {
133142
if prURL == "" {
134-
return fmt.Errorf("invalid URL: %q", prURL)
143+
return nil, 0, fmt.Errorf("invalid URL: %q", prURL)
135144
}
136145

137146
u, err := url.Parse(prURL)
138147
if err != nil {
139-
return err
148+
return nil, 0, err
140149
}
141150

142151
if u.Scheme != "https" && u.Scheme != "http" {
143-
return fmt.Errorf("invalid scheme: %s", u.Scheme)
152+
return nil, 0, fmt.Errorf("invalid scheme: %s", u.Scheme)
144153
}
145154

146155
m := pullURLRE.FindStringSubmatch(u.Path)
147156
if m == nil {
148-
return fmt.Errorf("not a pull request URL: %s", prURL)
157+
return nil, 0, fmt.Errorf("not a pull request URL: %s", prURL)
149158
}
150159

151-
f.repo = ghrepo.NewWithHost(m[1], m[2], u.Hostname())
152-
f.prNumber, _ = strconv.Atoi(m[3])
153-
return nil
160+
repo := ghrepo.NewWithHost(m[1], m[2], u.Hostname())
161+
prNumber, _ := strconv.Atoi(m[3])
162+
return repo, prNumber, nil
154163
}
155164

156165
var prHeadRE = regexp.MustCompile(`^refs/pull/(\d+)/head$`)
157166

158-
func (f *finder) parseCurrentBranch() error {
167+
func (f *finder) parseCurrentBranch() (string, int, error) {
159168
prHeadRef, err := f.branchFn()
160169
if err != nil {
161-
return err
170+
return "", 0, err
162171
}
163172

164-
branchConfig := git.ReadBranchConfig(prHeadRef)
173+
branchConfig := f.branchConfig(prHeadRef)
165174

166175
// the branch is configured to merge a special PR head ref
167176
if m := prHeadRE.FindStringSubmatch(branchConfig.MergeRef); m != nil {
168-
f.prNumber, _ = strconv.Atoi(m[1])
169-
return nil
177+
prNumber, _ := strconv.Atoi(m[1])
178+
return "", prNumber, nil
170179
}
171180

172181
var branchOwner string
@@ -193,8 +202,7 @@ func (f *finder) parseCurrentBranch() error {
193202
}
194203
}
195204

196-
f.branchName = prHeadRef
197-
return nil
205+
return prHeadRef, 0, nil
198206
}
199207

200208
func findByNumber(httpClient *http.Client, repo ghrepo.Interface, number int, fields []string) (*api.PullRequest, error) {

0 commit comments

Comments
 (0)
X Tutup