From 41a2863aae6005a81f8c86f16f001d941b4beac6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20Ilavsky=CC=81?= Date: Sun, 4 Nov 2018 16:55:35 +0100 Subject: [PATCH 1/6] changed cp command handling so it can copy multiple files like ie SCP does fixed broken tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Filip Ilavský --- cli/command/container/cp.go | 85 +++++++++++++++++++++++--------- cli/command/container/cp_test.go | 44 +++++++++-------- 2 files changed, 84 insertions(+), 45 deletions(-) diff --git a/cli/command/container/cp.go b/cli/command/container/cp.go index ffb9a211c2d1..0148cad0c48d 100644 --- a/cli/command/container/cp.go +++ b/cli/command/container/cp.go @@ -17,8 +17,8 @@ import ( ) type copyOptions struct { - source string - destination string + source containerWithPath + destination containerWithPath followLink bool copyUIDGID bool } @@ -39,13 +39,19 @@ type cpConfig struct { container string } +type containerWithPath struct { + container string + path string + isContainer bool +} + // NewCopyCommand creates a new `docker cp` command func NewCopyCommand(dockerCli command.Cli) *cobra.Command { var opts copyOptions cmd := &cobra.Command{ - Use: `cp [OPTIONS] CONTAINER:SRC_PATH DEST_PATH|- - docker cp [OPTIONS] SRC_PATH|- CONTAINER:DEST_PATH`, + Use: `cp [OPTIONS] CONTAINER:SRC_PATH ... DEST_PATH|- + docker cp [OPTIONS] SRC_PATH|- ... CONTAINER:DEST_PATH`, Short: "Copy files/folders between a container and the local filesystem", Long: strings.Join([]string{ "Copy files/folders between a container and the local filesystem\n", @@ -54,7 +60,7 @@ func NewCopyCommand(dockerCli command.Cli) *cobra.Command { "Use '-' as the destination to stream a tar archive of a\n", "container source to stdout.", }, ""), - Args: cli.ExactArgs(2), + Args: cli.RequiresMinArgs(2), RunE: func(cmd *cobra.Command, args []string) error { if args[0] == "" { return errors.New("source can not be empty") @@ -62,9 +68,7 @@ func NewCopyCommand(dockerCli command.Cli) *cobra.Command { if args[1] == "" { return errors.New("destination can not be empty") } - opts.source = args[0] - opts.destination = args[1] - return runCopy(dockerCli, opts) + return separateCopyCommands(dockerCli, opts, args) }, } @@ -74,33 +78,66 @@ func NewCopyCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runCopy(dockerCli command.Cli, opts copyOptions) error { - srcContainer, srcPath := splitCpArg(opts.source) - destContainer, destPath := splitCpArg(opts.destination) +// In order to support copying multiple files to one destination as ie. SCP supports, +// we need to separate each copy command, and as we know, that sources can be one and more, +// but destination can be only one, solution is separating each copy source, joining with +// destination and calling each combination in loop as it has been before without this functionality. +func separateCopyCommands(dockerCli command.Cli, opts copyOptions, args []string) error { + for i := 0; i < len(args)-1; i++ { + source := splitCpArg(args[i]) + destination := splitCpArg(args[len(args)-1]) + direction := getCpDirection(source, destination) + switch direction { + case fromContainer: + break + case toContainer: + break + case acrossContainers: + return errors.New("copying between containers is not supported") + default: + return errors.New("Invalid use of cp command\n See 'docker cp --help'.") + } + } - copyConfig := cpConfig{ - followLink: opts.followLink, - copyUIDGID: opts.copyUIDGID, - sourcePath: srcPath, - destPath: destPath, + for i := 0; i < len(args)-1; i++ { + opts.source = splitCpArg(args[i]) + opts.destination = splitCpArg(args[len(args)-1]) + direction := getCpDirection(opts.source, opts.destination) + copyError := runCopy(dockerCli, opts, direction) + if copyError != nil { + return copyError + } } + return nil +} +func getCpDirection(source containerWithPath, destination containerWithPath) copyDirection { var direction copyDirection - if srcContainer != "" { + if source.container != "" { direction |= fromContainer - copyConfig.container = srcContainer } - if destContainer != "" { + if destination.container != "" { direction |= toContainer - copyConfig.container = destContainer + } + return direction +} + +func runCopy(dockerCli command.Cli, opts copyOptions, direction copyDirection) error { + copyConfig := cpConfig{ + followLink: opts.followLink, + copyUIDGID: opts.copyUIDGID, + sourcePath: opts.source.path, + destPath: opts.destination.path, } ctx := context.Background() switch direction { case fromContainer: + copyConfig.container = opts.source.container return copyFromContainer(ctx, dockerCli, copyConfig) case toContainer: + copyConfig.container = opts.destination.container return copyToContainer(ctx, dockerCli, copyConfig) case acrossContainers: return errors.New("copying between containers is not supported") @@ -286,10 +323,10 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo // so we have to check for a `/` or `.` prefix. Also, in the case of a Windows // client, a `:` could be part of an absolute Windows path, in which case it // is immediately proceeded by a backslash. -func splitCpArg(arg string) (container, path string) { +func splitCpArg(arg string) containerWithPath { if system.IsAbs(arg) { // Explicit local absolute path, e.g., `C:\foo` or `/foo`. - return "", arg + return containerWithPath{"", arg, false} } parts := strings.SplitN(arg, ":", 2) @@ -297,8 +334,8 @@ func splitCpArg(arg string) (container, path string) { if len(parts) == 1 || strings.HasPrefix(parts[0], ".") { // Either there's no `:` in the arg // OR it's an explicit local relative path like `./file:name.txt`. - return "", arg + return containerWithPath{"", arg, false} } - return parts[0], parts[1] + return containerWithPath{parts[0], parts[1], true} } diff --git a/cli/command/container/cp_test.go b/cli/command/container/cp_test.go index 67cdaf15a9bf..52a6a38a2ba8 100644 --- a/cli/command/container/cp_test.go +++ b/cli/command/container/cp_test.go @@ -22,27 +22,29 @@ func TestRunCopyWithInvalidArguments(t *testing.T) { doc string options copyOptions expectedErr string + direction copyDirection }{ { doc: "copy between container", options: copyOptions{ - source: "first:/path", - destination: "second:/path", + source: splitCpArg("first:/path"), + destination: splitCpArg("second:/path"), }, expectedErr: "copying between containers is not supported", + direction: acrossContainers, }, { doc: "copy without a container", options: copyOptions{ - source: "./source", - destination: "./dest", + source: splitCpArg("./source"), + destination: splitCpArg("./dest"), }, expectedErr: "must specify at least one container source", }, } for _, testcase := range testcases { t.Run(testcase.doc, func(t *testing.T) { - err := runCopy(test.NewFakeCli(nil), testcase.options) + err := runCopy(test.NewFakeCli(nil), testcase.options, testcase.direction) assert.Error(t, err, testcase.expectedErr) }) } @@ -57,9 +59,9 @@ func TestRunCopyFromContainerToStdout(t *testing.T) { return ioutil.NopCloser(strings.NewReader(tarContent)), types.ContainerPathStat{}, nil }, } - options := copyOptions{source: "container:/path", destination: "-"} + options := copyOptions{source: splitCpArg("container:/path"), destination: splitCpArg("-")} cli := test.NewFakeCli(fakeClient) - err := runCopy(cli, options) + err := runCopy(cli, options, fromContainer) assert.NilError(t, err) assert.Check(t, is.Equal(tarContent, cli.OutBuffer().String())) assert.Check(t, is.Equal("", cli.ErrBuffer().String())) @@ -77,9 +79,9 @@ func TestRunCopyFromContainerToFilesystem(t *testing.T) { return readCloser, types.ContainerPathStat{}, err }, } - options := copyOptions{source: "container:/path", destination: destDir.Path()} + options := copyOptions{source: splitCpArg("container:/path"), destination: splitCpArg(destDir.Path())} cli := test.NewFakeCli(fakeClient) - err := runCopy(cli, options) + err := runCopy(cli, options, fromContainer) assert.NilError(t, err) assert.Check(t, is.Equal("", cli.OutBuffer().String())) assert.Check(t, is.Equal("", cli.ErrBuffer().String())) @@ -103,11 +105,11 @@ func TestRunCopyFromContainerToFilesystemMissingDestinationDirectory(t *testing. } options := copyOptions{ - source: "container:/path", - destination: destDir.Join("missing", "foo"), + source: splitCpArg("container:/path"), + destination: splitCpArg(destDir.Join("missing", "foo")), } cli := test.NewFakeCli(fakeClient) - err := runCopy(cli, options) + err := runCopy(cli, options, fromContainer) assert.ErrorContains(t, err, destDir.Join("missing")) } @@ -116,11 +118,11 @@ func TestRunCopyToContainerFromFileWithTrailingSlash(t *testing.T) { defer srcFile.Remove() options := copyOptions{ - source: srcFile.Path() + string(os.PathSeparator), - destination: "container:/path", + source: splitCpArg(srcFile.Path() + string(os.PathSeparator)), + destination: splitCpArg("container:/path"), } cli := test.NewFakeCli(&fakeClient{}) - err := runCopy(cli, options) + err := runCopy(cli, options, toContainer) expectedError := "not a directory" if runtime.GOOS == "windows" { @@ -131,11 +133,11 @@ func TestRunCopyToContainerFromFileWithTrailingSlash(t *testing.T) { func TestRunCopyToContainerSourceDoesNotExist(t *testing.T) { options := copyOptions{ - source: "/does/not/exist", - destination: "container:/path", + source: splitCpArg("/does/not/exist"), + destination: splitCpArg("container:/path"), } cli := test.NewFakeCli(&fakeClient{}) - err := runCopy(cli, options) + err := runCopy(cli, options, toContainer) expected := "no such file or directory" if runtime.GOOS == "windows" { expected = "cannot find the file specified" @@ -184,9 +186,9 @@ func TestSplitCpArg(t *testing.T) { t.Run(testcase.doc, func(t *testing.T) { skip.If(t, testcase.os != "" && testcase.os != runtime.GOOS) - container, path := splitCpArg(testcase.path) - assert.Check(t, is.Equal(testcase.expectedContainer, container)) - assert.Check(t, is.Equal(testcase.expectedPath, path)) + containerWithPath := splitCpArg(testcase.path) + assert.Check(t, is.Equal(testcase.expectedContainer, containerWithPath.container)) + assert.Check(t, is.Equal(testcase.expectedPath, containerWithPath.path)) }) } } From 7732eeb20f32801596c058a645dee40db3fb907f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20Ilavsky=CC=81?= Date: Sun, 4 Nov 2018 16:55:35 +0100 Subject: [PATCH 2/6] changed cp command handling so it can copy multiple files like ie SCP does fixed broken tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Filip Ilavský --- cli/command/container/cp.go | 85 +++++++++++++++++++++++--------- cli/command/container/cp_test.go | 44 +++++++++-------- 2 files changed, 84 insertions(+), 45 deletions(-) diff --git a/cli/command/container/cp.go b/cli/command/container/cp.go index ffb9a211c2d1..0148cad0c48d 100644 --- a/cli/command/container/cp.go +++ b/cli/command/container/cp.go @@ -17,8 +17,8 @@ import ( ) type copyOptions struct { - source string - destination string + source containerWithPath + destination containerWithPath followLink bool copyUIDGID bool } @@ -39,13 +39,19 @@ type cpConfig struct { container string } +type containerWithPath struct { + container string + path string + isContainer bool +} + // NewCopyCommand creates a new `docker cp` command func NewCopyCommand(dockerCli command.Cli) *cobra.Command { var opts copyOptions cmd := &cobra.Command{ - Use: `cp [OPTIONS] CONTAINER:SRC_PATH DEST_PATH|- - docker cp [OPTIONS] SRC_PATH|- CONTAINER:DEST_PATH`, + Use: `cp [OPTIONS] CONTAINER:SRC_PATH ... DEST_PATH|- + docker cp [OPTIONS] SRC_PATH|- ... CONTAINER:DEST_PATH`, Short: "Copy files/folders between a container and the local filesystem", Long: strings.Join([]string{ "Copy files/folders between a container and the local filesystem\n", @@ -54,7 +60,7 @@ func NewCopyCommand(dockerCli command.Cli) *cobra.Command { "Use '-' as the destination to stream a tar archive of a\n", "container source to stdout.", }, ""), - Args: cli.ExactArgs(2), + Args: cli.RequiresMinArgs(2), RunE: func(cmd *cobra.Command, args []string) error { if args[0] == "" { return errors.New("source can not be empty") @@ -62,9 +68,7 @@ func NewCopyCommand(dockerCli command.Cli) *cobra.Command { if args[1] == "" { return errors.New("destination can not be empty") } - opts.source = args[0] - opts.destination = args[1] - return runCopy(dockerCli, opts) + return separateCopyCommands(dockerCli, opts, args) }, } @@ -74,33 +78,66 @@ func NewCopyCommand(dockerCli command.Cli) *cobra.Command { return cmd } -func runCopy(dockerCli command.Cli, opts copyOptions) error { - srcContainer, srcPath := splitCpArg(opts.source) - destContainer, destPath := splitCpArg(opts.destination) +// In order to support copying multiple files to one destination as ie. SCP supports, +// we need to separate each copy command, and as we know, that sources can be one and more, +// but destination can be only one, solution is separating each copy source, joining with +// destination and calling each combination in loop as it has been before without this functionality. +func separateCopyCommands(dockerCli command.Cli, opts copyOptions, args []string) error { + for i := 0; i < len(args)-1; i++ { + source := splitCpArg(args[i]) + destination := splitCpArg(args[len(args)-1]) + direction := getCpDirection(source, destination) + switch direction { + case fromContainer: + break + case toContainer: + break + case acrossContainers: + return errors.New("copying between containers is not supported") + default: + return errors.New("Invalid use of cp command\n See 'docker cp --help'.") + } + } - copyConfig := cpConfig{ - followLink: opts.followLink, - copyUIDGID: opts.copyUIDGID, - sourcePath: srcPath, - destPath: destPath, + for i := 0; i < len(args)-1; i++ { + opts.source = splitCpArg(args[i]) + opts.destination = splitCpArg(args[len(args)-1]) + direction := getCpDirection(opts.source, opts.destination) + copyError := runCopy(dockerCli, opts, direction) + if copyError != nil { + return copyError + } } + return nil +} +func getCpDirection(source containerWithPath, destination containerWithPath) copyDirection { var direction copyDirection - if srcContainer != "" { + if source.container != "" { direction |= fromContainer - copyConfig.container = srcContainer } - if destContainer != "" { + if destination.container != "" { direction |= toContainer - copyConfig.container = destContainer + } + return direction +} + +func runCopy(dockerCli command.Cli, opts copyOptions, direction copyDirection) error { + copyConfig := cpConfig{ + followLink: opts.followLink, + copyUIDGID: opts.copyUIDGID, + sourcePath: opts.source.path, + destPath: opts.destination.path, } ctx := context.Background() switch direction { case fromContainer: + copyConfig.container = opts.source.container return copyFromContainer(ctx, dockerCli, copyConfig) case toContainer: + copyConfig.container = opts.destination.container return copyToContainer(ctx, dockerCli, copyConfig) case acrossContainers: return errors.New("copying between containers is not supported") @@ -286,10 +323,10 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo // so we have to check for a `/` or `.` prefix. Also, in the case of a Windows // client, a `:` could be part of an absolute Windows path, in which case it // is immediately proceeded by a backslash. -func splitCpArg(arg string) (container, path string) { +func splitCpArg(arg string) containerWithPath { if system.IsAbs(arg) { // Explicit local absolute path, e.g., `C:\foo` or `/foo`. - return "", arg + return containerWithPath{"", arg, false} } parts := strings.SplitN(arg, ":", 2) @@ -297,8 +334,8 @@ func splitCpArg(arg string) (container, path string) { if len(parts) == 1 || strings.HasPrefix(parts[0], ".") { // Either there's no `:` in the arg // OR it's an explicit local relative path like `./file:name.txt`. - return "", arg + return containerWithPath{"", arg, false} } - return parts[0], parts[1] + return containerWithPath{parts[0], parts[1], true} } diff --git a/cli/command/container/cp_test.go b/cli/command/container/cp_test.go index 67cdaf15a9bf..52a6a38a2ba8 100644 --- a/cli/command/container/cp_test.go +++ b/cli/command/container/cp_test.go @@ -22,27 +22,29 @@ func TestRunCopyWithInvalidArguments(t *testing.T) { doc string options copyOptions expectedErr string + direction copyDirection }{ { doc: "copy between container", options: copyOptions{ - source: "first:/path", - destination: "second:/path", + source: splitCpArg("first:/path"), + destination: splitCpArg("second:/path"), }, expectedErr: "copying between containers is not supported", + direction: acrossContainers, }, { doc: "copy without a container", options: copyOptions{ - source: "./source", - destination: "./dest", + source: splitCpArg("./source"), + destination: splitCpArg("./dest"), }, expectedErr: "must specify at least one container source", }, } for _, testcase := range testcases { t.Run(testcase.doc, func(t *testing.T) { - err := runCopy(test.NewFakeCli(nil), testcase.options) + err := runCopy(test.NewFakeCli(nil), testcase.options, testcase.direction) assert.Error(t, err, testcase.expectedErr) }) } @@ -57,9 +59,9 @@ func TestRunCopyFromContainerToStdout(t *testing.T) { return ioutil.NopCloser(strings.NewReader(tarContent)), types.ContainerPathStat{}, nil }, } - options := copyOptions{source: "container:/path", destination: "-"} + options := copyOptions{source: splitCpArg("container:/path"), destination: splitCpArg("-")} cli := test.NewFakeCli(fakeClient) - err := runCopy(cli, options) + err := runCopy(cli, options, fromContainer) assert.NilError(t, err) assert.Check(t, is.Equal(tarContent, cli.OutBuffer().String())) assert.Check(t, is.Equal("", cli.ErrBuffer().String())) @@ -77,9 +79,9 @@ func TestRunCopyFromContainerToFilesystem(t *testing.T) { return readCloser, types.ContainerPathStat{}, err }, } - options := copyOptions{source: "container:/path", destination: destDir.Path()} + options := copyOptions{source: splitCpArg("container:/path"), destination: splitCpArg(destDir.Path())} cli := test.NewFakeCli(fakeClient) - err := runCopy(cli, options) + err := runCopy(cli, options, fromContainer) assert.NilError(t, err) assert.Check(t, is.Equal("", cli.OutBuffer().String())) assert.Check(t, is.Equal("", cli.ErrBuffer().String())) @@ -103,11 +105,11 @@ func TestRunCopyFromContainerToFilesystemMissingDestinationDirectory(t *testing. } options := copyOptions{ - source: "container:/path", - destination: destDir.Join("missing", "foo"), + source: splitCpArg("container:/path"), + destination: splitCpArg(destDir.Join("missing", "foo")), } cli := test.NewFakeCli(fakeClient) - err := runCopy(cli, options) + err := runCopy(cli, options, fromContainer) assert.ErrorContains(t, err, destDir.Join("missing")) } @@ -116,11 +118,11 @@ func TestRunCopyToContainerFromFileWithTrailingSlash(t *testing.T) { defer srcFile.Remove() options := copyOptions{ - source: srcFile.Path() + string(os.PathSeparator), - destination: "container:/path", + source: splitCpArg(srcFile.Path() + string(os.PathSeparator)), + destination: splitCpArg("container:/path"), } cli := test.NewFakeCli(&fakeClient{}) - err := runCopy(cli, options) + err := runCopy(cli, options, toContainer) expectedError := "not a directory" if runtime.GOOS == "windows" { @@ -131,11 +133,11 @@ func TestRunCopyToContainerFromFileWithTrailingSlash(t *testing.T) { func TestRunCopyToContainerSourceDoesNotExist(t *testing.T) { options := copyOptions{ - source: "/does/not/exist", - destination: "container:/path", + source: splitCpArg("/does/not/exist"), + destination: splitCpArg("container:/path"), } cli := test.NewFakeCli(&fakeClient{}) - err := runCopy(cli, options) + err := runCopy(cli, options, toContainer) expected := "no such file or directory" if runtime.GOOS == "windows" { expected = "cannot find the file specified" @@ -184,9 +186,9 @@ func TestSplitCpArg(t *testing.T) { t.Run(testcase.doc, func(t *testing.T) { skip.If(t, testcase.os != "" && testcase.os != runtime.GOOS) - container, path := splitCpArg(testcase.path) - assert.Check(t, is.Equal(testcase.expectedContainer, container)) - assert.Check(t, is.Equal(testcase.expectedPath, path)) + containerWithPath := splitCpArg(testcase.path) + assert.Check(t, is.Equal(testcase.expectedContainer, containerWithPath.container)) + assert.Check(t, is.Equal(testcase.expectedPath, containerWithPath.path)) }) } } From ba19bd5fef987bd366420adf12511e8ff9273fcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20Ilavsky=CC=81?= Date: Mon, 5 Nov 2018 10:00:46 +0100 Subject: [PATCH 3/6] fix remove punctuation and capitalization --- cli/command/container/cp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/command/container/cp.go b/cli/command/container/cp.go index 0148cad0c48d..e9358b551bb8 100644 --- a/cli/command/container/cp.go +++ b/cli/command/container/cp.go @@ -95,7 +95,7 @@ func separateCopyCommands(dockerCli command.Cli, opts copyOptions, args []string case acrossContainers: return errors.New("copying between containers is not supported") default: - return errors.New("Invalid use of cp command\n See 'docker cp --help'.") + return errors.New("invalid use of cp command\n see 'docker cp --help'") } } From 4fd4900be761f06f54e0e4f499ddd21661546af1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20Ilavsky=CC=81?= Date: Mon, 5 Nov 2018 10:10:52 +0100 Subject: [PATCH 4/6] gofmt --- cli/command/container/cp_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/command/container/cp_test.go b/cli/command/container/cp_test.go index 52a6a38a2ba8..8f394504c332 100644 --- a/cli/command/container/cp_test.go +++ b/cli/command/container/cp_test.go @@ -22,7 +22,7 @@ func TestRunCopyWithInvalidArguments(t *testing.T) { doc string options copyOptions expectedErr string - direction copyDirection + direction copyDirection }{ { doc: "copy between container", @@ -31,7 +31,7 @@ func TestRunCopyWithInvalidArguments(t *testing.T) { destination: splitCpArg("second:/path"), }, expectedErr: "copying between containers is not supported", - direction: acrossContainers, + direction: acrossContainers, }, { doc: "copy without a container", From 20d9bc2722415b0397b3cb04daa5279ad2784e4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20Ilavsky=CC=81?= Date: Sat, 10 Nov 2018 14:32:12 +0100 Subject: [PATCH 5/6] fixed and implemented tests for new methods in cli cp --- cli/command/container/cp_test.go | 71 +++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/cli/command/container/cp_test.go b/cli/command/container/cp_test.go index 8f394504c332..6afc988da2ad 100644 --- a/cli/command/container/cp_test.go +++ b/cli/command/container/cp_test.go @@ -17,15 +17,84 @@ import ( "gotest.tools/skip" ) -func TestRunCopyWithInvalidArguments(t *testing.T) { +func TestSeparateCopyCommands(t *testing.T) { var testcases = []struct { doc string options copyOptions expectedErr string direction copyDirection + args []string }{ { doc: "copy between container", + expectedErr: "copying between containers is not supported", + direction: acrossContainers, + args: []string {"first:/path", "first:/path"}, + }, + { + doc: "copy without container", + expectedErr: "invalid use of cp command\n see 'docker cp --help'", + direction: 0, + args: []string {"/path", "/path"}, + }, + } + for _, testcase := range testcases { + t.Run(testcase.doc, func(t *testing.T) { + err := separateCopyCommands(test.NewFakeCli(nil), testcase.options, testcase.args) + assert.Error(t, err, testcase.expectedErr) + }) + } +} + +func TestGetCpDirection(t *testing.T) { + var testcases = []struct { + doc string + source containerWithPath + destination containerWithPath + expectedResult copyDirection + }{ + { + doc: "container to container", + source: splitCpArg("first:/path"), + destination: splitCpArg("second:/path"), + expectedResult: acrossContainers, + }, + { + doc: "source to container", + source: splitCpArg("/path"), + destination: splitCpArg("second:/path"), + expectedResult: acrossContainers, + }, + { + doc: "container to source", + source: splitCpArg("first:/path"), + destination: splitCpArg("/path"), + expectedResult: acrossContainers, + }, + { + doc: "source to source", + source: splitCpArg("/path"), + destination: splitCpArg("/path"), + expectedResult: acrossContainers, + }, + } + for _, testcase := range testcases { + t.Run(testcase.doc, func(t *testing.T) { + direction := getCpDirection(testcase.source, testcase.destination) + assert.Equal(t, direction, testcase.expectedResult) + }) + } +} + +func TestRunCopyWithInvalidArguments(t *testing.T) { + var testcases = []struct { + doc string + options copyOptions + expectedErr string + direction copyDirection + }{ + { + doc: "copy between containers", options: copyOptions{ source: splitCpArg("first:/path"), destination: splitCpArg("second:/path"), From 2081752667caecf0f0a97567967ae0980e0c4e89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20Ilavsky=CC=81?= Date: Sat, 10 Nov 2018 14:53:40 +0100 Subject: [PATCH 6/6] fixed tests and gofmt --- cli/command/container/cp_test.go | 38 ++++++++++++++++---------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/cli/command/container/cp_test.go b/cli/command/container/cp_test.go index 6afc988da2ad..e5148e1011ac 100644 --- a/cli/command/container/cp_test.go +++ b/cli/command/container/cp_test.go @@ -26,16 +26,16 @@ func TestSeparateCopyCommands(t *testing.T) { args []string }{ { - doc: "copy between container", + doc: "copy between container", expectedErr: "copying between containers is not supported", direction: acrossContainers, - args: []string {"first:/path", "first:/path"}, + args: []string{"first:/path", "first:/path"}, }, { - doc: "copy without container", + doc: "copy without container", expectedErr: "invalid use of cp command\n see 'docker cp --help'", direction: 0, - args: []string {"/path", "/path"}, + args: []string{"/path", "/path"}, }, } for _, testcase := range testcases { @@ -54,28 +54,28 @@ func TestGetCpDirection(t *testing.T) { expectedResult copyDirection }{ { - doc: "container to container", - source: splitCpArg("first:/path"), - destination: splitCpArg("second:/path"), + doc: "container to container", + source: splitCpArg("first:/path"), + destination: splitCpArg("second:/path"), expectedResult: acrossContainers, }, { - doc: "source to container", - source: splitCpArg("/path"), - destination: splitCpArg("second:/path"), - expectedResult: acrossContainers, + doc: "source to container", + source: splitCpArg("/path"), + destination: splitCpArg("second:/path"), + expectedResult: toContainer, }, { - doc: "container to source", - source: splitCpArg("first:/path"), - destination: splitCpArg("/path"), - expectedResult: acrossContainers, + doc: "container to source", + source: splitCpArg("first:/path"), + destination: splitCpArg("/path"), + expectedResult: fromContainer, }, { - doc: "source to source", - source: splitCpArg("/path"), - destination: splitCpArg("/path"), - expectedResult: acrossContainers, + doc: "source to source", + source: splitCpArg("/path"), + destination: splitCpArg("/path"), + expectedResult: 0, }, } for _, testcase := range testcases {