commands: improve test coverage for FileArg parsing

License: MIT
Signed-off-by: Thomas Gardner <tmg@fastmail.com>

commands/cli/parse.go

@@ -11,8 +11,8 @@ import (
 
 	cmds "github.com/ipfs/go-ipfs/commands"
 	files "github.com/ipfs/go-ipfs/commands/files"
-	u "gx/ipfs/QmZNVWh8LLjAavuQ2JXuFmuYH3C11xo988vSgp7UQrTRj1/go-ipfs-util"
 	logging "gx/ipfs/QmYtB7Qge8cJpXc4irsEp8zRqfnZMBeB7aTrMEkPk67DRv/go-log"
+	u "gx/ipfs/QmZNVWh8LLjAavuQ2JXuFmuYH3C11xo988vSgp7UQrTRj1/go-ipfs-util"
 )
 
 var log = logging.Logger("commands/cli")
@@ -305,7 +305,8 @@ func parseArgs(inputs []string, stdin *os.File, argDefs []cmds.Argument, recursi
 				var err error
 				if fpath == "-" {
 					if err = printReadInfo(stdin, msgStdinInfo); err == nil {
-						file = files.NewReaderFile("", "", stdin, nil)
+						fpath = stdin.Name()
+						file = files.NewReaderFile("", fpath, stdin, nil)
 					}
 				} else {
 					file, err = appendFile(fpath, argDef, recursive, hidden)
@@ -321,7 +322,8 @@ func parseArgs(inputs []string, stdin *os.File, argDefs []cmds.Argument, recursi
 					if err := printReadInfo(stdin, msgStdinInfo); err != nil {
 						return nil, nil, err
 					}
-					fileArgs[""] = files.NewReaderFile("", "", stdin, nil)
+					fpath := stdin.Name()
+					fileArgs[fpath] = files.NewReaderFile("", fpath, stdin, nil)
 				} else {
 					break
 				}

commands/cli/parse_test.go

@@ -179,7 +179,12 @@ func TestArgumentParsing(t *testing.T) {
 			},
 			"FileArg": {
 				Arguments: []commands.Argument{
-					commands.FileArg("a", false, false, "some arg"),
+					commands.FileArg("a", true, false, "some arg"),
+				},
+			},
+			"FileArg+Variadic": {
+				Arguments: []commands.Argument{
+					commands.FileArg("a", true, true, "some arg"),
 				},
 			},
 			"FileArg+Stdin": {
@@ -187,10 +192,34 @@ func TestArgumentParsing(t *testing.T) {
 					commands.FileArg("a", true, true, "some arg").EnableStdin(),
 				},
 			},
+			"StringArg+FileArg": {
+				Arguments: []commands.Argument{
+					commands.StringArg("a", true, false, "some arg"),
+					commands.FileArg("a", true, false, "some arg"),
+				},
+			},
+			"StringArg+FileArg+Stdin": {
+				Arguments: []commands.Argument{
+					commands.StringArg("a", true, false, "some arg"),
+					commands.FileArg("a", true, true, "some arg").EnableStdin(),
+				},
+			},
+			"StringArg+FileArg+Variadic": {
+				Arguments: []commands.Argument{
+					commands.StringArg("a", true, false, "some arg"),
+					commands.FileArg("a", true, true, "some arg"),
+				},
+			},
+			"StringArg+FileArg+Variadic+Stdin": {
+				Arguments: []commands.Argument{
+					commands.StringArg("a", true, false, "some arg"),
+					commands.FileArg("a", true, true, "some arg"),
+				},
+			},
 		},
 	}
 
-	test := func(cmd words, f *os.File, res words) {
+	test := func(cmd words, f *os.File, exp words) {
 		if f != nil {
 			if _, err := f.Seek(0, os.SEEK_SET); err != nil {
 				t.Fatal(err)
@@ -200,8 +229,18 @@ func TestArgumentParsing(t *testing.T) {
 		if err != nil {
 			t.Errorf("Command '%v' should have passed parsing: %v", cmd, err)
 		}
-		if !sameWords(req.Arguments(), res) {
-			t.Errorf("Arguments parsed from '%v' are '%v' instead of '%v'", cmd, req.Arguments(), res)
+
+		parsedWords := make([]string, len(req.Arguments()))
+		copy(parsedWords, req.Arguments())
+
+		if files := req.Files(); files != nil {
+			for file, err := files.NextFile(); err != io.EOF; file, err = files.NextFile() {
+				parsedWords = append(parsedWords, file.FullPath())
+			}
+		}
+
+		if !sameWords(parsedWords, exp) {
+			t.Errorf("Arguments parsed from '%v' are '%v' instead of '%v'", cmd, parsedWords, exp)
 		}
 	}
 
@@ -241,23 +280,43 @@ func TestArgumentParsing(t *testing.T) {
 	testFail([]string{"reversedoptional"}, nil, "didn't provide any args, 1 required")
 	testFail([]string{"reversedoptional", "value1", "value2", "value3"}, nil, "provided too many args, only takes 1")
 
-	// Use a temp file to simulate stdin
-	fileToSimulateStdin := func(t *testing.T, content string) *os.File {
-		fstdin, err := ioutil.TempFile("", "")
+	// Since FileArgs are presently stored ordered by Path, the enum string
+	// is used to construct a predictably ordered sequence of filenames.
+	tmpFile := func(t *testing.T, enum string) *os.File {
+		f, err := ioutil.TempFile("", enum)
 		if err != nil {
 			t.Fatal(err)
 		}
 
-		if _, err := io.WriteString(fstdin, content); err != nil {
-			t.Fatal(err)
-		}
-		return fstdin
+		return f
 	}
-	fstdin := fileToSimulateStdin(t, "stdin1")
-	defer os.Remove(fstdin.Name())
+	file1 := tmpFile(t, "1")
+	file2 := tmpFile(t, "2")
+	file3 := tmpFile(t, "3")
+	defer os.Remove(file3.Name())
+	defer os.Remove(file2.Name())
+	defer os.Remove(file1.Name())
 
-	test([]string{"noarg"}, fstdin, []string{})
-	test([]string{"FileArg", fstdin.Name()}, nil, []string{})
-	test([]string{"FileArg+Stdin"}, fstdin, []string{})
-	test([]string{"FileArg+Stdin", "-"}, fstdin, []string{})
+	test([]string{"noarg"}, file1, []string{})
+	test([]string{"FileArg", file1.Name()}, nil, []string{file1.Name()})
+	test([]string{"FileArg+Variadic", file1.Name(), file2.Name()}, nil,
+		[]string{file1.Name(), file2.Name()})
+	test([]string{"FileArg+Stdin"}, file1, []string{file1.Name()})
+	test([]string{"FileArg+Stdin", "-"}, file1, []string{file1.Name()})
+	test([]string{"FileArg+Stdin", file1.Name(), "-"}, file2,
+		[]string{file1.Name(), file2.Name()})
+	test([]string{"StringArg+FileArg",
+		"foo", file1.Name()}, nil, []string{"foo", file1.Name()})
+	test([]string{"StringArg+FileArg+Variadic",
+		"foo", file1.Name(), file2.Name()}, nil,
+		[]string{"foo", file1.Name(), file2.Name()})
+	test([]string{"StringArg+FileArg+Stdin",
+		"foo", file1.Name(), "-"}, file2,
+		[]string{"foo", file1.Name(), file2.Name()})
+	test([]string{"StringArg+FileArg+Variadic+Stdin",
+		"foo", file1.Name(), file2.Name()}, file3,
+		[]string{"foo", file1.Name(), file2.Name()})
+	test([]string{"StringArg+FileArg+Variadic+Stdin",
+		"foo", file1.Name(), file2.Name(), "-"}, file3,
+		[]string{"foo", file1.Name(), file2.Name(), file3.Name()})
 }