From fc969281cfa9b1f5a236b28b99532c7f314e66fd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mislav=20Marohni=C4=87?= <mislav@github.com>
Date: Thu, 29 Oct 2020 16:52:50 +0100
Subject: [PATCH] Better tests for clone argument parsing

---
 pkg/cmd/repo/clone/clone.go      |  3 +-
 pkg/cmd/repo/clone/clone_test.go | 93 +++++++++++++++++++++++++++-----
 2 files changed, 80 insertions(+), 16 deletions(-)

diff --git a/pkg/cmd/repo/clone/clone.go b/pkg/cmd/repo/clone/clone.go
index 3a444f44c..accd443fa 100644
--- a/pkg/cmd/repo/clone/clone.go
+++ b/pkg/cmd/repo/clone/clone.go
@@ -24,7 +24,6 @@ type CloneOptions struct {
 	IO         *iostreams.IOStreams
 
 	GitArgs    []string
-	Directory  string
 	Repository string
 }
 
@@ -41,7 +40,7 @@ func NewCmdClone(f *cmdutil.Factory, runF func(*CloneOptions) error) *cobra.Comm
 		Use: "clone <repository> [<directory>] [-- <gitflags>...]",
 		Args: func(cmd *cobra.Command, args []string) error {
 			if len(args) == 0 {
-				return &cmdutil.FlagError{Err: errors.New("cannot clone: no repository name given")}
+				return &cmdutil.FlagError{Err: errors.New("cannot clone: repository argument required")}
 			}
 			return nil
 		},
diff --git a/pkg/cmd/repo/clone/clone_test.go b/pkg/cmd/repo/clone/clone_test.go
index 8c81c629f..e7aa57b08 100644
--- a/pkg/cmd/repo/clone/clone_test.go
+++ b/pkg/cmd/repo/clone/clone_test.go
@@ -12,8 +12,87 @@ import (
 	"github.com/cli/cli/test"
 	"github.com/google/shlex"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
+func TestNewCmdClone(t *testing.T) {
+	testCases := []struct {
+		name     string
+		args     string
+		wantOpts CloneOptions
+		wantErr  string
+	}{
+		{
+			name:    "no arguments",
+			args:    "",
+			wantErr: "cannot clone: repository argument required",
+		},
+		{
+			name: "repo argument",
+			args: "OWNER/REPO",
+			wantOpts: CloneOptions{
+				Repository: "OWNER/REPO",
+				GitArgs:    []string{},
+			},
+		},
+		{
+			name: "directory argument",
+			args: "OWNER/REPO mydir",
+			wantOpts: CloneOptions{
+				Repository: "OWNER/REPO",
+				GitArgs:    []string{"mydir"},
+			},
+		},
+		{
+			name: "git clone arguments",
+			args: "OWNER/REPO -- --depth 1 --recurse-submodules",
+			wantOpts: CloneOptions{
+				Repository: "OWNER/REPO",
+				GitArgs:    []string{"--depth", "1", "--recurse-submodules"},
+			},
+		},
+		{
+			name:    "unknown argument",
+			args:    "OWNER/REPO --depth 1",
+			wantErr: "unknown flag: --depth\nSeparate git clone flags with '--'.",
+		},
+	}
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			io, stdin, stdout, stderr := iostreams.Test()
+			fac := &cmdutil.Factory{IOStreams: io}
+
+			var opts *CloneOptions
+			cmd := NewCmdClone(fac, func(co *CloneOptions) error {
+				opts = co
+				return nil
+			})
+
+			argv, err := shlex.Split(tt.args)
+			require.NoError(t, err)
+			cmd.SetArgs(argv)
+
+			cmd.SetIn(stdin)
+			cmd.SetOut(stdout)
+			cmd.SetErr(stderr)
+
+			_, err = cmd.ExecuteC()
+			if err != nil {
+				assert.Equal(t, tt.wantErr, err.Error())
+				return
+			} else if tt.wantErr != "" {
+				t.Errorf("expected error %q, got nil", tt.wantErr)
+			}
+
+			assert.Equal(t, "", stdout.String())
+			assert.Equal(t, "", stderr.String())
+
+			assert.Equal(t, tt.wantOpts.Repository, opts.Repository)
+			assert.Equal(t, tt.wantOpts.GitArgs, opts.GitArgs)
+		})
+	}
+}
+
 func runCloneCommand(httpClient *http.Client, cli string) (*test.CmdOut, error) {
 	io, stdin, stdout, stderr := iostreams.Test()
 	fac := &cmdutil.Factory{
@@ -203,17 +282,3 @@ func Test_RepoClone_withoutUsername(t *testing.T) {
 	assert.Equal(t, 1, cs.Count)
 	assert.Equal(t, "git clone https://github.com/OWNER/REPO.git", strings.Join(cs.Calls[0].Args, " "))
 }
-
-func Test_RepoClone_flagError(t *testing.T) {
-	_, err := runCloneCommand(nil, "--depth 1 OWNER/REPO")
-	if err == nil || err.Error() != "unknown flag: --depth\nSeparate git clone flags with '--'." {
-		t.Errorf("unexpected error %v", err)
-	}
-}
-
-func Test_RepoClone_noArgError(t *testing.T) {
-	_, err := runCloneCommand(nil, "")
-	if err == nil || err.Error() != "cannot clone: no repository name given" {
-		t.Errorf("unexpected error %v", err)
-	}
-}