From 2ad63fd0dfcf44a7c0c6fe02f451ef824117102d Mon Sep 17 00:00:00 2001 From: Thomas Gardner Date: Sat, 25 Jun 2016 22:37:40 +1000 Subject: [PATCH] commands: improve test coverage for FileArg parsing License: MIT Signed-off-by: Thomas Gardner --- commands/cli/parse.go | 8 ++-- commands/cli/parse_test.go | 93 +++++++++++++++++++++++++++++++------- 2 files changed, 81 insertions(+), 20 deletions(-) diff --git a/commands/cli/parse.go b/commands/cli/parse.go index d76709e24..c948c3471 100644 --- a/commands/cli/parse.go +++ b/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 } diff --git a/commands/cli/parse_test.go b/commands/cli/parse_test.go index 0688201ae..196c4b3d9 100644 --- a/commands/cli/parse_test.go +++ b/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()}) }