Skip to content

Commit ece536a

Browse files
mislavmislav
authored
Merge pull request cli#3700 from cli/pr-checkout-deleted
Fix `pr checkout` for PRs coming from deleted forks
2 parents 6b49b48 + ebf2bb9 commit ece536a

File tree

2 files changed

+102
-6
lines changed

2 files changed

+102
-6
lines changed

‎pkg/cmd/pr/checkout/checkout.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobr
7272
func checkoutRun(opts *CheckoutOptions) error {
7373
findOptions := shared.FindOptions{
7474
Selector: opts.SelectorArg,
75-
Fields: []string{"number", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository"},
75+
Fields: []string{"number", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository", "maintainerCanModify"},
7676
}
7777
pr, baseRepo, err := opts.Finder.Find(findOptions)
7878
if err != nil {
@@ -96,7 +96,9 @@ func checkoutRun(opts *CheckoutOptions) error {
9696
}
9797

9898
headRemote := baseRemote
99-
if pr.IsCrossRepository {
99+
if pr.HeadRepository == nil {
100+
headRemote = nil
101+
} else if pr.IsCrossRepository {
100102
headRemote, _ = remotes.FindByRepo(pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name)
101103
}
102104

@@ -207,7 +209,7 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB
207209

208210
remote := baseURLOrName
209211
mergeRef := ref
210-
if pr.MaintainerCanModify {
212+
if pr.MaintainerCanModify && pr.HeadRepository != nil {
211213
headRepo := ghrepo.NewWithHost(pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name, repoHost)
212214
remote = ghrepo.FormatRemoteURL(headRepo, protocol)
213215
mergeRef = fmt.Sprintf("refs/heads/%s", pr.HeadRefName)

‎pkg/cmd/pr/checkout/checkout_test.go‎

Lines changed: 97 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package checkout
22

33
import (
44
"bytes"
5+
"errors"
56
"io/ioutil"
67
"net/http"
78
"strings"
@@ -59,6 +60,99 @@ func stubPR(repo, prHead string) (ghrepo.Interface, *api.PullRequest) {
5960
}
6061
}
6162

63+
func Test_checkoutRun(t *testing.T) {
64+
tests := []struct {
65+
name string
66+
opts *CheckoutOptions
67+
httpStubs func(*httpmock.Registry)
68+
runStubs func(*run.CommandStubber)
69+
remotes map[string]string
70+
wantStdout string
71+
wantStderr string
72+
wantErr bool
73+
}{
74+
{
75+
name: "fork repo was deleted",
76+
opts: &CheckoutOptions{
77+
SelectorArg: "123",
78+
Finder: func() shared.PRFinder {
79+
baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature")
80+
pr.MaintainerCanModify = true
81+
pr.HeadRepository = nil
82+
finder := shared.NewMockFinder("123", pr, baseRepo)
83+
return finder
84+
}(),
85+
Config: func() (config.Config, error) {
86+
return config.NewBlankConfig(), nil
87+
},
88+
Branch: func() (string, error) {
89+
return "main", nil
90+
},
91+
},
92+
remotes: map[string]string{
93+
"origin": "OWNER/REPO",
94+
},
95+
runStubs: func(cs *run.CommandStubber) {
96+
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
97+
cs.Register(`git config branch\.feature\.merge`, 1, "")
98+
cs.Register(`git checkout feature`, 0, "")
99+
cs.Register(`git config branch\.feature\.remote origin`, 0, "")
100+
cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "")
101+
},
102+
},
103+
}
104+
for _, tt := range tests {
105+
t.Run(tt.name, func(t *testing.T) {
106+
opts := tt.opts
107+
108+
io, _, stdout, stderr := iostreams.Test()
109+
opts.IO = io
110+
111+
httpReg := &httpmock.Registry{}
112+
defer httpReg.Verify(t)
113+
if tt.httpStubs != nil {
114+
tt.httpStubs(httpReg)
115+
}
116+
opts.HttpClient = func() (*http.Client, error) {
117+
return &http.Client{Transport: httpReg}, nil
118+
}
119+
120+
cmdStubs, cmdTeardown := run.Stub()
121+
defer cmdTeardown(t)
122+
if tt.runStubs != nil {
123+
tt.runStubs(cmdStubs)
124+
}
125+
126+
opts.Remotes = func() (context.Remotes, error) {
127+
if len(tt.remotes) == 0 {
128+
return nil, errors.New("no remotes")
129+
}
130+
var remotes context.Remotes
131+
for name, repo := range tt.remotes {
132+
r, err := ghrepo.FromFullName(repo)
133+
if err != nil {
134+
return remotes, err
135+
}
136+
remotes = append(remotes, &context.Remote{
137+
Remote: &git.Remote{Name: name},
138+
Repo: r,
139+
})
140+
}
141+
return remotes, nil
142+
}
143+
144+
err := checkoutRun(opts)
145+
if (err != nil) != tt.wantErr {
146+
t.Errorf("want error: %v, got: %v", tt.wantErr, err)
147+
}
148+
assert.Equal(t, tt.wantStdout, stdout.String())
149+
assert.Equal(t, tt.wantStderr, stderr.String())
150+
})
151+
}
152+
}
153+
154+
/** LEGACY TESTS **/
155+
62156
func runCommand(rt http.RoundTripper, remotes context.Remotes, branch string, cli string) (*test.CmdOut, error) {
63157
io, _, stdout, stderr := iostreams.Test()
64158

@@ -111,7 +205,7 @@ func TestPRCheckout_sameRepo(t *testing.T) {
111205

112206
baseRepo, pr := stubPR("OWNER/REPO", "OWNER/REPO:feature")
113207
finder := shared.RunCommandFinder("123", pr, baseRepo)
114-
finder.ExpectFields([]string{"number", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository"})
208+
finder.ExpectFields([]string{"number", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository", "maintainerCanModify"})
115209

116210
cs, cmdTeardown := run.Stub()
117211
defer cmdTeardown(t)
@@ -166,7 +260,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
166260

167261
baseRepo, pr := stubPR("OWNER/REPO", "hubot/REPO:feature")
168262
finder := shared.RunCommandFinder("123", pr, baseRepo)
169-
finder.ExpectFields([]string{"number", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository"})
263+
finder.ExpectFields([]string{"number", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository", "maintainerCanModify"})
170264

171265
cs, cmdTeardown := run.Stub()
172266
defer cmdTeardown(t)
@@ -189,7 +283,7 @@ func TestPRCheckout_differentRepo(t *testing.T) {
189283

190284
baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature")
191285
finder := shared.RunCommandFinder("123", pr, baseRepo)
192-
finder.ExpectFields([]string{"number", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository"})
286+
finder.ExpectFields([]string{"number", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository", "maintainerCanModify"})
193287

194288
cs, cmdTeardown := run.Stub()
195289
defer cmdTeardown(t)

0 commit comments

Comments
 (0)