Skip to content

Commit

Permalink
isolated clone command
Browse files Browse the repository at this point in the history
This commit hacks the existing repo clone tests into something usable by
the new isolated command. It went ok and was less effort than trying to
introduce the same kind of test format as repo view and gist create.
  • Loading branch information
vilmibm committed Jul 23, 2020
1 parent b9ce1a1 commit 1831d95
Show file tree
Hide file tree
Showing 9 changed files with 461 additions and 291 deletions.
120 changes: 14 additions & 106 deletions command/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import (
)

func init() {
repoCmd.AddCommand(repoCloneCmd)

repoCmd.AddCommand(repoCreateCmd)
repoCreateCmd.Flags().StringP("description", "d", "", "Description of repository")
repoCreateCmd.Flags().StringP("homepage", "h", "", "Repository home page URL")
Expand Down Expand Up @@ -57,19 +55,6 @@ A repository can be supplied as an argument in any of the following formats:
- by URL, e.g. "https://github.com/OWNER/REPO"`},
}

var repoCloneCmd = &cobra.Command{
Use: "clone <repository> [<directory>]",
Args: cobra.MinimumNArgs(1),
Short: "Clone a repository locally",
Long: `Clone a GitHub repository locally.
If the "OWNER/" portion of the "OWNER/REPO" repository argument is omitted, it
defaults to the name of the authenticating user.
To pass 'git clone' flags, separate them with '--'.`,
RunE: repoClone,
}

var repoCreateCmd = &cobra.Command{
Use: "create [<name>]",
Short: "Create a new repository",
Expand Down Expand Up @@ -120,95 +105,6 @@ var repoCreditsCmd = &cobra.Command{
Hidden: true,
}

func parseCloneArgs(extraArgs []string) (args []string, target string) {
args = extraArgs

if len(args) > 0 {
if !strings.HasPrefix(args[0], "-") {
target, args = args[0], args[1:]
}
}
return
}

func runClone(cloneURL string, args []string) (target string, err error) {
cloneArgs, target := parseCloneArgs(args)

cloneArgs = append(cloneArgs, cloneURL)

// If the args contain an explicit target, pass it to clone
// otherwise, parse the URL to determine where git cloned it to so we can return it
if target != "" {
cloneArgs = append(cloneArgs, target)
} else {
target = path.Base(strings.TrimSuffix(cloneURL, ".git"))
}

cloneArgs = append([]string{"clone"}, cloneArgs...)

cloneCmd := git.GitCommand(cloneArgs...)
cloneCmd.Stdin = os.Stdin
cloneCmd.Stdout = os.Stdout
cloneCmd.Stderr = os.Stderr

err = run.PrepareCmd(cloneCmd).Run()
return
}

func repoClone(cmd *cobra.Command, args []string) error {
ctx := contextForCommand(cmd)
apiClient, err := apiClientForContext(ctx)
if err != nil {
return err
}

cloneURL := args[0]
if !strings.Contains(cloneURL, ":") {
if !strings.Contains(cloneURL, "/") {
currentUser, err := api.CurrentLoginName(apiClient)
if err != nil {
return err
}
cloneURL = currentUser + "/" + cloneURL
}
repo, err := ghrepo.FromFullName(cloneURL)
if err != nil {
return err
}
cloneURL = formatRemoteURL(cmd, repo)
}

var repo ghrepo.Interface
var parentRepo ghrepo.Interface

// TODO: consider caching and reusing `git.ParseSSHConfig().Translator()`
// here to handle hostname aliases in SSH remotes
if u, err := git.ParseURL(cloneURL); err == nil {
repo, _ = ghrepo.FromURL(u)
}

if repo != nil {
parentRepo, err = api.RepoParent(apiClient, repo)
if err != nil {
return err
}
}

cloneDir, err := runClone(cloneURL, args[1:])
if err != nil {
return err
}

if parentRepo != nil {
err := addUpstreamRemote(cmd, parentRepo, cloneDir)
if err != nil {
return err
}
}

return nil
}

func addUpstreamRemote(cmd *cobra.Command, parentRepo ghrepo.Interface, cloneDir string) error {
upstreamURL := formatRemoteURL(cmd, parentRepo)

Expand Down Expand Up @@ -529,12 +425,24 @@ func repoFork(cmd *cobra.Command, args []string) error {
}
if cloneDesired {
forkedRepoCloneURL := formatRemoteURL(cmd, forkedRepo)
cloneDir, err := runClone(forkedRepoCloneURL, []string{})
cloneDir, err := git.RunClone(forkedRepoCloneURL, []string{})
if err != nil {
return fmt.Errorf("failed to clone fork: %w", err)
}

err = addUpstreamRemote(cmd, repoToFork, cloneDir)
// TODO This is overly wordy and I'd like to streamline this.
cfg, err := ctx.Config()
if err != nil {
return err
}
protocol, err := cfg.Get("", "git_protocol")
if err != nil {
return err
}

upstreamURL := ghrepo.FormatRemoteURL(repoToFork, protocol)

err = git.AddUpstreamRemote(upstreamURL, cloneDir)
if err != nil {
return err
}
Expand Down
185 changes: 0 additions & 185 deletions command/repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"encoding/json"
"io/ioutil"
"os/exec"
"reflect"
"regexp"
"strings"
"testing"
Expand Down Expand Up @@ -437,190 +436,6 @@ func TestRepoFork_in_parent_survey_no(t *testing.T) {
}
}

func TestParseExtraArgs(t *testing.T) {
type Wanted struct {
args []string
dir string
}
tests := []struct {
name string
args []string
want Wanted
}{
{
name: "args and target",
args: []string{"target_directory", "-o", "upstream", "--depth", "1"},
want: Wanted{
args: []string{"-o", "upstream", "--depth", "1"},
dir: "target_directory",
},
},
{
name: "only args",
args: []string{"-o", "upstream", "--depth", "1"},
want: Wanted{
args: []string{"-o", "upstream", "--depth", "1"},
dir: "",
},
},
{
name: "only target",
args: []string{"target_directory"},
want: Wanted{
args: []string{},
dir: "target_directory",
},
},
{
name: "no args",
args: []string{},
want: Wanted{
args: []string{},
dir: "",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
args, dir := parseCloneArgs(tt.args)
got := Wanted{
args: args,
dir: dir,
}

if !reflect.DeepEqual(got, tt.want) {
t.Errorf("got %#v want %#v", got, tt.want)
}
})
}

}

func TestRepoClone(t *testing.T) {
tests := []struct {
name string
args string
want string
}{
{
name: "shorthand",
args: "repo clone OWNER/REPO",
want: "git clone https://github.com/OWNER/REPO.git",
},
{
name: "shorthand with directory",
args: "repo clone OWNER/REPO target_directory",
want: "git clone https://github.com/OWNER/REPO.git target_directory",
},
{
name: "clone arguments",
args: "repo clone OWNER/REPO -- -o upstream --depth 1",
want: "git clone -o upstream --depth 1 https://github.com/OWNER/REPO.git",
},
{
name: "clone arguments with directory",
args: "repo clone OWNER/REPO target_directory -- -o upstream --depth 1",
want: "git clone -o upstream --depth 1 https://github.com/OWNER/REPO.git target_directory",
},
{
name: "HTTPS URL",
args: "repo clone https://github.com/OWNER/REPO",
want: "git clone https://github.com/OWNER/REPO",
},
{
name: "SSH URL",
args: "repo clone [email protected]:OWNER/REPO.git",
want: "git clone [email protected]:OWNER/REPO.git",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
http := initFakeHTTP()
http.Register(
httpmock.GraphQL(`query RepositoryFindParent\b`),
httpmock.StringResponse(`
{ "data": { "repository": {
"parent": null
} } }
`))

cs, restore := test.InitCmdStubber()
defer restore()

cs.Stub("") // git clone

output, err := RunCommand(tt.args)
if err != nil {
t.Fatalf("error running command `repo clone`: %v", err)
}

eq(t, output.String(), "")
eq(t, output.Stderr(), "")
eq(t, cs.Count, 1)
eq(t, strings.Join(cs.Calls[0].Args, " "), tt.want)
})
}
}

func TestRepoClone_hasParent(t *testing.T) {
http := initFakeHTTP()
http.Register(
httpmock.GraphQL(`query RepositoryFindParent\b`),
httpmock.StringResponse(`
{ "data": { "repository": {
"parent": {
"owner": {"login": "hubot"},
"name": "ORIG"
}
} } }
`))

cs, restore := test.InitCmdStubber()
defer restore()

cs.Stub("") // git clone
cs.Stub("") // git remote add

_, err := RunCommand("repo clone OWNER/REPO")
if err != nil {
t.Fatalf("error running command `repo clone`: %v", err)
}

eq(t, cs.Count, 2)
eq(t, strings.Join(cs.Calls[1].Args, " "), "git -C REPO remote add -f upstream https://github.com/hubot/ORIG.git")
}

func TestRepo_withoutUsername(t *testing.T) {
http := initFakeHTTP()
http.Register(
httpmock.GraphQL(`query UserCurrent\b`),
httpmock.StringResponse(`
{ "data": { "viewer": {
"login": "OWNER"
}}}`))
http.Register(
httpmock.GraphQL(`query RepositoryFindParent\b`),
httpmock.StringResponse(`
{ "data": { "repository": {
"parent": null
} } }`))

cs, restore := test.InitCmdStubber()
defer restore()

cs.Stub("") // git clone

output, err := RunCommand("repo clone REPO")
if err != nil {
t.Fatalf("error running command `repo clone`: %v", err)
}

eq(t, output.String(), "")
eq(t, output.Stderr(), "")
eq(t, cs.Count, 1)
eq(t, strings.Join(cs.Calls[0].Args, " "), "git clone https://github.com/OWNER/REPO.git")
}

func TestRepoCreate(t *testing.T) {
ctx := context.NewBlank()
ctx.SetBranch("master")
Expand Down
12 changes: 12 additions & 0 deletions command/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,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"
repoCloneCmd "github.com/cli/cli/pkg/cmd/repo/clone"
repoViewCmd "github.com/cli/cli/pkg/cmd/repo/view"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/iostreams"
Expand Down Expand Up @@ -95,6 +96,15 @@ func init() {
ctx := context.New()
return ctx.BaseRepo()
},
Config: func() (config.Config, error) {
cfg, err := config.ParseDefaultConfig()
if errors.Is(err, os.ErrNotExist) {
cfg = config.NewBlankConfig()
} else if err != nil {
return nil, err
}
return cfg, nil
},
}
RootCmd.AddCommand(apiCmd.NewCmdApi(cmdFactory, nil))

Expand Down Expand Up @@ -137,6 +147,7 @@ func init() {

RootCmd.AddCommand(repoCmd)
repoCmd.AddCommand(repoViewCmd.NewCmdView(&repoResolvingCmdFactory, nil))
repoCmd.AddCommand(repoCloneCmd.NewCmdClone(cmdFactory, nil))
}

// RootCmd is the entry point of command-line execution
Expand Down Expand Up @@ -355,6 +366,7 @@ func determineBaseRepo(apiClient *api.Client, cmd *cobra.Command, ctx context.Co
return baseRepo, nil
}

// TODO there is a parallel implementation for isolated commands
func formatRemoteURL(cmd *cobra.Command, repo ghrepo.Interface) string {
ctx := contextForCommand(cmd)

Expand Down
Loading

0 comments on commit 1831d95

Please sign in to comment.