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

Isolate pr checkout command

Browse source
This commit is contained in:
Mislav Marohnić 2020-07-29 22:50:34 +02:00
parent af68a749f0
commit 487dd06bf3
6 changed files with 195 additions and 168 deletions
Download patch file Download diff file Expand all files Collapse all files

9
api/queries_repo.go
View file

@ -27,9 +27,7 @@ type Repository struct {
IsPrivate bool
HasIssuesEnabled bool
ViewerPermission string
DefaultBranchRef struct {
Name string
}
DefaultBranchRef BranchRef
Parent *Repository
@ -42,6 +40,11 @@ type RepositoryOwner struct {
Login string
}
// BranchRef is the branch name in a GitHub repository
type BranchRef struct {
Name string
}
// RepoOwner is the login name of the owner
func (r Repository) RepoOwner() string {
return r.Owner.Login

1
command/pr.go
View file

@ -27,7 +27,6 @@ func init() {
prCmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format")
RootCmd.AddCommand(prCmd)
prCmd.AddCommand(prCheckoutCmd)
prCmd.AddCommand(prCreateCmd)
prCmd.AddCommand(prStatusCmd)
prCmd.AddCommand(prCloseCmd)

2
command/root.go
View file

@ -22,6 +22,7 @@ import (
"github.com/cli/cli/internal/run"
apiCmd "github.com/cli/cli/pkg/cmd/api"
gistCreateCmd "github.com/cli/cli/pkg/cmd/gist/create"
prCheckoutCmd "github.com/cli/cli/pkg/cmd/pr/checkout"
prDiffCmd "github.com/cli/cli/pkg/cmd/pr/diff"
prReviewCmd "github.com/cli/cli/pkg/cmd/pr/review"
repoCmd "github.com/cli/cli/pkg/cmd/repo"
@ -172,6 +173,7 @@ func init() {
prCmd.AddCommand(prReviewCmd.NewCmdReview(&repoResolvingCmdFactory, nil))
prCmd.AddCommand(prDiffCmd.NewCmdDiff(&repoResolvingCmdFactory, nil))
prCmd.AddCommand(prCheckoutCmd.NewCmdCheckout(&repoResolvingCmdFactory, nil))
RootCmd.AddCommand(creditsCmd.NewCmdCredits(cmdFactory, nil))
}

21
command/root_test.go
View file

@ -2,8 +2,6 @@ package command
import (
"testing"
"github.com/cli/cli/internal/ghrepo"
)
func TestChangelogURL(t *testing.T) {
@ -42,22 +40,3 @@ func TestChangelogURL(t *testing.T) {
t.Errorf("expected %s to create url %s but got %s", tag, url, result)
}
}
func TestRemoteURLFormatting_no_config(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
result := formatRemoteURL(prCheckoutCmd, ghrepo.New("OWNER", "REPO"))
eq(t, result, "https://github.com/OWNER/REPO.git")
}
func TestRemoteURLFormatting_ssh_config(t *testing.T) {
cfg := `---
hosts:
github.com:
user: OWNER
oauth_token: MUSTBEHIGHCUZIMATOKEN
git_protocol: ssh
`
initBlankContext(cfg, "OWNER/REPO", "master")
result := formatRemoteURL(prCheckoutCmd, ghrepo.New("OWNER", "REPO"))
eq(t, result, "git@github.com:OWNER/REPO.git")
}

94
command/pr_checkout.go → pkg/cmd/pr/checkout/checkout.go
View file

@ -1,41 +1,101 @@
package command
package checkout
import (
"errors"
"fmt"
"net/http"
"os"
"os/exec"
"strings"
"github.com/spf13/cobra"
"github.com/cli/cli/api"
"github.com/cli/cli/context"
"github.com/cli/cli/git"
"github.com/cli/cli/internal/config"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/internal/run"
"github.com/cli/cli/pkg/cmd/pr/shared"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/iostreams"
"github.com/spf13/cobra"
)
func prCheckout(cmd *cobra.Command, args []string) error {
ctx := contextForCommand(cmd)
currentBranch, _ := ctx.Branch()
remotes, err := ctx.Remotes()
type CheckoutOptions struct {
HttpClient func() (*http.Client, error)
Config func() (config.Config, error)
IO *iostreams.IOStreams
BaseRepo func() (ghrepo.Interface, error)
Remotes func() (context.Remotes, error)
Branch func() (string, error)
SelectorArg string
}
func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobra.Command {
opts := &CheckoutOptions{
IO: f.IOStreams,
HttpClient: f.HttpClient,
Config: f.Config,
BaseRepo: f.BaseRepo,
Remotes: f.Remotes,
Branch: f.Branch,
}
cmd := &cobra.Command{
Use: "checkout {<number> | <url> | <branch>}",
Short: "Check out a pull request in git",
Args: func(cmd *cobra.Command, args []string) error {
if len(args) == 0 {
return &cmdutil.FlagError{Err: errors.New("argument required")}
}
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
if len(args) > 0 {
opts.SelectorArg = args[0]
}
if runF != nil {
return runF(opts)
}
return checkoutRun(opts)
},
}
return cmd
}
func checkoutRun(opts *CheckoutOptions) error {
currentBranch, err := opts.Branch()
if err != nil {
return err
}
apiClient, err := apiClientForContext(ctx)
remotes, err := opts.Remotes()
if err != nil {
return err
}
pr, baseRepo, err := prFromArgs(ctx, apiClient, cmd, args)
httpClient, err := opts.HttpClient()
if err != nil {
return err
}
apiClient := api.NewClientFromHTTP(httpClient)
pr, baseRepo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg)
if err != nil {
return err
}
cfg, err := opts.Config()
if err != nil {
return err
}
protocol, _ := cfg.Get(baseRepo.RepoHost(), "git_protocol")
baseRemote, _ := remotes.FindByRepo(baseRepo.RepoOwner(), baseRepo.RepoName())
// baseRemoteSpec is a repository URL or a remote name to be used in git fetch
baseURLOrName := formatRemoteURL(cmd, baseRepo)
baseURLOrName := ghrepo.FormatRemoteURL(baseRepo, protocol)
if baseRemote != nil {
baseURLOrName = baseRemote.Name
}
@ -95,7 +155,7 @@ func prCheckout(cmd *cobra.Command, args []string) error {
mergeRef := ref
if pr.MaintainerCanModify {
headRepo := ghrepo.NewWithHost(pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name, baseRepo.RepoHost())
remote = formatRemoteURL(cmd, headRepo)
remote = ghrepo.FormatRemoteURL(headRepo, protocol)
mergeRef = fmt.Sprintf("refs/heads/%s", pr.HeadRefName)
}
if mc, err := git.Config(fmt.Sprintf("branch.%s.merge", newBranchName)); err != nil || mc == "" {
@ -115,15 +175,3 @@ func prCheckout(cmd *cobra.Command, args []string) error {
return nil
}
var prCheckoutCmd = &cobra.Command{
Use: "checkout {<number> | <url> | <branch>}",
Short: "Check out a pull request in Git",
Args: func(cmd *cobra.Command, args []string) error {
if len(args) < 1 {
return errors.New("requires a pull request number as an argument")
}
return nil
},
RunE: prCheckout,
}

236
command/pr_checkout_test.go → pkg/cmd/pr/checkout/checkout_test.go
View file

@ -1,31 +1,105 @@
package command
package checkout
import (
"bytes"
"encoding/json"
"errors"
"io/ioutil"
"net/http"
"os/exec"
"reflect"
"strings"
"testing"
"github.com/cli/cli/api"
"github.com/cli/cli/context"
"github.com/cli/cli/git"
"github.com/cli/cli/internal/config"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/internal/run"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/httpmock"
"github.com/cli/cli/pkg/iostreams"
"github.com/cli/cli/test"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
)
func TestPRCheckout_sameRepo(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
func eq(t *testing.T, got interface{}, expected interface{}) {
t.Helper()
if !reflect.DeepEqual(got, expected) {
t.Errorf("expected: %v, got: %v", expected, got)
}
http := initFakeHTTP()
}
type errorStub struct {
message string
}
func (s errorStub) Output() ([]byte, error) {
return nil, errors.New(s.message)
}
func (s errorStub) Run() error {
return errors.New(s.message)
}
func runCommand(rt http.RoundTripper, remotes context.Remotes, branch string, cli string) (*test.CmdOut, error) {
io, _, stdout, stderr := iostreams.Test()
factory := &cmdutil.Factory{
IOStreams: io,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: rt}, nil
},
Config: func() (config.Config, error) {
return config.NewBlankConfig(), nil
},
BaseRepo: func() (ghrepo.Interface, error) {
return &api.Repository{
Name: "REPO",
Owner: api.RepositoryOwner{Login: "OWNER"},
DefaultBranchRef: api.BranchRef{Name: "master"},
}, nil
},
Remotes: func() (context.Remotes, error) {
if remotes == nil {
return context.Remotes{
{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.New("OWNER", "REPO"),
},
}, nil
}
return remotes, nil
},
Branch: func() (string, error) {
return branch, nil
},
}
cmd := NewCmdCheckout(factory, nil)
argv, err := shlex.Split(cli)
if err != nil {
return nil, err
}
cmd.SetArgs(argv)
cmd.SetIn(&bytes.Buffer{})
cmd.SetOut(ioutil.Discard)
cmd.SetErr(ioutil.Discard)
_, err = cmd.ExecuteC()
return &test.CmdOut{
OutBuf: stdout,
ErrBuf: stderr,
}, err
}
func TestPRCheckout_sameRepo(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -54,7 +128,7 @@ func TestPRCheckout_sameRepo(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
output, err := runCommand(http, nil, "master", `123`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -66,15 +140,7 @@ func TestPRCheckout_sameRepo(t *testing.T) {
}
func TestPRCheckout_urlArg(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -103,7 +169,7 @@ func TestPRCheckout_urlArg(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout https://github.com/OWNER/REPO/pull/123/files`)
output, err := runCommand(http, nil, "master", `https://github.com/OWNER/REPO/pull/123/files`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -112,15 +178,7 @@ func TestPRCheckout_urlArg(t *testing.T) {
}
func TestPRCheckout_urlArg_differentBase(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -154,7 +212,7 @@ func TestPRCheckout_urlArg_differentBase(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout https://github.com/OTHER/POE/pull/123/files`)
output, err := runCommand(http, nil, "master", `https://github.com/OTHER/POE/pull/123/files`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -176,17 +234,8 @@ func TestPRCheckout_urlArg_differentBase(t *testing.T) {
}
func TestPRCheckout_branchArg(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequests": { "nodes": [
@ -215,7 +264,7 @@ func TestPRCheckout_branchArg(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout hubot:feature`)
output, err := runCommand(http, nil, "master", `hubot:feature`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -224,17 +273,8 @@ func TestPRCheckout_branchArg(t *testing.T) {
}
func TestPRCheckout_existingBranch(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -263,7 +303,7 @@ func TestPRCheckout_existingBranch(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
output, err := runCommand(http, nil, "master", `123`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -274,18 +314,19 @@ func TestPRCheckout_existingBranch(t *testing.T) {
}
func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
"robot-fork": "hubot/REPO",
})
initContext = func() context.Context {
return ctx
remotes := context.Remotes{
{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.New("OWNER", "REPO"),
},
{
Remote: &git.Remote{Name: "robot-fork"},
Repo: ghrepo.New("hubot", "REPO"),
},
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -314,7 +355,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
output, err := runCommand(http, remotes, "master", `123`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -326,17 +367,8 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
}
func TestPRCheckout_differentRepo(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -365,7 +397,7 @@ func TestPRCheckout_differentRepo(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
output, err := runCommand(http, nil, "master", `123`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -377,17 +409,8 @@ func TestPRCheckout_differentRepo(t *testing.T) {
}
func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -416,7 +439,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
output, err := runCommand(http, nil, "master", `123`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -426,17 +449,8 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
}
func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("feature")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -465,7 +479,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
output, err := runCommand(http, nil, "feature", `123`)
eq(t, err, nil)
eq(t, output.String(), "")
@ -475,17 +489,8 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
}
func TestPRCheckout_differentRepo_invalidBranchName(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("feature")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -508,7 +513,7 @@ func TestPRCheckout_differentRepo_invalidBranchName(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
output, err := runCommand(http, nil, "master", `123`)
if assert.Errorf(t, err, "expected command to fail") {
assert.Equal(t, `invalid branch name: "-foo"`, err.Error())
}
@ -516,17 +521,8 @@ func TestPRCheckout_differentRepo_invalidBranchName(t *testing.T) {
}
func TestPRCheckout_maintainerCanModify(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
ctx.SetRemotes(map[string]string{
"origin": "OWNER/REPO",
})
initContext = func() context.Context {
return ctx
}
http := initFakeHTTP()
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.StringResponse(`
{ "data": { "repository": { "pullRequest": {
@ -555,7 +551,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) {
})
defer restoreCmd()
output, err := RunCommand(`pr checkout 123`)
output, err := runCommand(http, nil, "master", `123`)
eq(t, err, nil)
eq(t, output.String(), "")