From 267a0294a76286a5ddc1b86e3b863f5606ff88f9 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 21 Oct 2019 17:06:20 +0200 Subject: [PATCH] integration-cli: cleanup: mark helpers as helpers, use assertion Signed-off-by: Sebastiaan van Stijn --- .../docker_cli_cp_from_container_test.go | 149 ++++++--------- integration-cli/docker_cli_cp_test.go | 2 +- .../docker_cli_cp_to_container_test.go | 169 ++++++------------ .../docker_cli_cp_to_container_unix_test.go | 13 +- integration-cli/docker_cli_cp_utils_test.go | 53 +++--- 5 files changed, 140 insertions(+), 246 deletions(-) diff --git a/integration-cli/docker_cli_cp_from_container_test.go b/integration-cli/docker_cli_cp_from_container_test.go index 7cfe1a93fb..981888f974 100644 --- a/integration-cli/docker_cli_cp_from_container_test.go +++ b/integration-cli/docker_cli_cp_from_container_test.go @@ -35,38 +35,26 @@ func (s *DockerSuite) TestCpFromSymlinkDestination(c *testing.T) { srcPath := containerCpPath(containerID, "/file2") dstPath := cpPath(tmpDir, "symlinkToFile1") - assert.Assert(c, runDockerCp(c, srcPath, dstPath, nil) == nil) - - // The symlink should not have been modified. - assert.Assert(c, symlinkTargetEquals(c, dstPath, "file1") == nil) - - // The file should have the contents of "file2" now. - assert.Assert(c, fileContentEquals(c, cpPath(tmpDir, "file1"), "file2\n") == nil) + assert.NilError(c, runDockerCp(c, srcPath, dstPath)) + assert.NilError(c, symlinkTargetEquals(c, dstPath, "file1"), "The symlink should not have been modified") + assert.NilError(c, fileContentEquals(c, cpPath(tmpDir, "file1"), "file2\n"), `The file should have the contents of "file2" now`) // Next, copy a file from the container to a symlink to a directory. This // should copy the file into the symlink target directory. dstPath = cpPath(tmpDir, "symlinkToDir1") - assert.Assert(c, runDockerCp(c, srcPath, dstPath, nil) == nil) - - // The symlink should not have been modified. - assert.Assert(c, symlinkTargetEquals(c, dstPath, "dir1") == nil) - - // The file should have the contents of "file2" now. - assert.Assert(c, fileContentEquals(c, cpPath(tmpDir, "file2"), "file2\n") == nil) + assert.NilError(c, runDockerCp(c, srcPath, dstPath)) + assert.NilError(c, symlinkTargetEquals(c, dstPath, "dir1"), "The symlink should not have been modified") + assert.NilError(c, fileContentEquals(c, cpPath(tmpDir, "file2"), "file2\n"), `The file should have the contents of "file2" now`) // Next, copy a file from the container to a symlink to a file that does // not exist (a broken symlink). This should create the target file with // the contents of the source file. dstPath = cpPath(tmpDir, "brokenSymlinkToFileX") - assert.Assert(c, runDockerCp(c, srcPath, dstPath, nil) == nil) - - // The symlink should not have been modified. - assert.Assert(c, symlinkTargetEquals(c, dstPath, "fileX") == nil) - - // The file should have the contents of "file2" now. - assert.Assert(c, fileContentEquals(c, cpPath(tmpDir, "fileX"), "file2\n") == nil) + assert.NilError(c, runDockerCp(c, srcPath, dstPath)) + assert.NilError(c, symlinkTargetEquals(c, dstPath, "fileX"), "The symlink should not have been modified") + assert.NilError(c, fileContentEquals(c, cpPath(tmpDir, "fileX"), "file2\n"), `The file should have the contents of "file2" now`) // Next, copy a directory from the container to a symlink to a local // directory. This should copy the directory into the symlink target @@ -74,13 +62,9 @@ func (s *DockerSuite) TestCpFromSymlinkDestination(c *testing.T) { srcPath = containerCpPath(containerID, "/dir2") dstPath = cpPath(tmpDir, "symlinkToDir1") - assert.Assert(c, runDockerCp(c, srcPath, dstPath, nil) == nil) - - // The symlink should not have been modified. - assert.Assert(c, symlinkTargetEquals(c, dstPath, "dir1") == nil) - - // The directory should now contain a copy of "dir2". - assert.Assert(c, fileContentEquals(c, cpPath(tmpDir, "dir1/dir2/file2-1"), "file2-1\n") == nil) + assert.NilError(c, runDockerCp(c, srcPath, dstPath)) + assert.NilError(c, symlinkTargetEquals(c, dstPath, "dir1"), "The symlink should not have been modified") + assert.NilError(c, fileContentEquals(c, cpPath(tmpDir, "dir1/dir2/file2-1"), "file2-1\n"), `The directory should now contain a copy of "dir2"`) // Next, copy a directory from the container to a symlink to a local // directory that does not exist (a broken symlink). This should create @@ -88,13 +72,9 @@ func (s *DockerSuite) TestCpFromSymlinkDestination(c *testing.T) { // should not modify the symlink. dstPath = cpPath(tmpDir, "brokenSymlinkToDirX") - assert.Assert(c, runDockerCp(c, srcPath, dstPath, nil) == nil) - - // The symlink should not have been modified. - assert.Assert(c, symlinkTargetEquals(c, dstPath, "dirX") == nil) - - // The "dirX" directory should now be a copy of "dir2". - assert.Assert(c, fileContentEquals(c, cpPath(tmpDir, "dirX/file2-1"), "file2-1\n") == nil) + assert.NilError(c, runDockerCp(c, srcPath, dstPath)) + assert.NilError(c, symlinkTargetEquals(c, dstPath, "dirX"), "The symlink should not have been modified") + assert.NilError(c, fileContentEquals(c, cpPath(tmpDir, "dirX/file2-1"), "file2-1\n"), `The "dirX" directory should now be a copy of "dir2"`) } // Possibilities are reduced to the remaining 10 cases: @@ -128,9 +108,8 @@ func (s *DockerSuite) TestCpFromCaseA(c *testing.T) { srcPath := containerCpPath(containerID, "/root/file1") dstPath := cpPath(tmpDir, "itWorks.txt") - assert.Assert(c, runDockerCp(c, srcPath, dstPath, nil) == nil) - - assert.Assert(c, fileContentEquals(c, dstPath, "file1\n") == nil) + assert.NilError(c, runDockerCp(c, srcPath, dstPath)) + assert.NilError(c, fileContentEquals(c, dstPath, "file1\n")) } // B. SRC specifies a file and DST (with trailing path separator) doesn't @@ -146,9 +125,8 @@ func (s *DockerSuite) TestCpFromCaseB(c *testing.T) { srcPath := containerCpPath(containerID, "/file1") dstDir := cpPathTrailingSep(tmpDir, "testDir") - err := runDockerCp(c, srcPath, dstDir, nil) + err := runDockerCp(c, srcPath, dstDir) assert.ErrorContains(c, err, "") - assert.Assert(c, isCpDirNotExist(err), "expected DirNotExists error, but got %T: %s", err, err) } @@ -169,11 +147,9 @@ func (s *DockerSuite) TestCpFromCaseC(c *testing.T) { dstPath := cpPath(tmpDir, "file2") // Ensure the local file starts with different content. - assert.Assert(c, fileContentEquals(c, dstPath, "file2\n") == nil) - - assert.Assert(c, runDockerCp(c, srcPath, dstPath, nil) == nil) - - assert.Assert(c, fileContentEquals(c, dstPath, "file1\n") == nil) + assert.NilError(c, fileContentEquals(c, dstPath, "file2\n")) + assert.NilError(c, runDockerCp(c, srcPath, dstPath)) + assert.NilError(c, fileContentEquals(c, dstPath, "file1\n")) } // D. SRC specifies a file and DST exists as a directory. This should place @@ -196,23 +172,18 @@ func (s *DockerSuite) TestCpFromCaseD(c *testing.T) { _, err := os.Stat(dstPath) assert.Assert(c, os.IsNotExist(err), "did not expect dstPath %q to exist", dstPath) - assert.Assert(c, runDockerCp(c, srcPath, dstDir, nil) == nil) - - assert.Assert(c, fileContentEquals(c, dstPath, "file1\n") == nil) + assert.NilError(c, runDockerCp(c, srcPath, dstDir)) + assert.NilError(c, fileContentEquals(c, dstPath, "file1\n")) // Now try again but using a trailing path separator for dstDir. - // unable to remove dstDir - assert.Assert(c, os.RemoveAll(dstDir) == nil) - - // unable to make dstDir - assert.Assert(c, os.MkdirAll(dstDir, os.FileMode(0755)) == nil) + assert.NilError(c, os.RemoveAll(dstDir), "unable to remove dstDir") + assert.NilError(c, os.MkdirAll(dstDir, os.FileMode(0755)), "unable to make dstDir") dstDir = cpPathTrailingSep(tmpDir, "dir1") - assert.Assert(c, runDockerCp(c, srcPath, dstDir, nil) == nil) - - assert.Assert(c, fileContentEquals(c, dstPath, "file1\n") == nil) + assert.NilError(c, runDockerCp(c, srcPath, dstDir)) + assert.NilError(c, fileContentEquals(c, dstPath, "file1\n")) } // E. SRC specifies a directory and DST does not exist. This should create a @@ -230,20 +201,17 @@ func (s *DockerSuite) TestCpFromCaseE(c *testing.T) { dstDir := cpPath(tmpDir, "testDir") dstPath := filepath.Join(dstDir, "file1-1") - assert.Assert(c, runDockerCp(c, srcDir, dstDir, nil) == nil) - - assert.Assert(c, fileContentEquals(c, dstPath, "file1-1\n") == nil) + assert.NilError(c, runDockerCp(c, srcDir, dstDir)) + assert.NilError(c, fileContentEquals(c, dstPath, "file1-1\n")) // Now try again but using a trailing path separator for dstDir. - // unable to remove dstDir - assert.Assert(c, os.RemoveAll(dstDir) == nil) + assert.NilError(c, os.RemoveAll(dstDir), "unable to remove dstDir") dstDir = cpPathTrailingSep(tmpDir, "testDir") - assert.Assert(c, runDockerCp(c, srcDir, dstDir, nil) == nil) - - assert.Assert(c, fileContentEquals(c, dstPath, "file1-1\n") == nil) + assert.NilError(c, runDockerCp(c, srcDir, dstDir)) + assert.NilError(c, fileContentEquals(c, dstPath, "file1-1\n")) } // F. SRC specifies a directory and DST exists as a file. This should cause an @@ -262,9 +230,8 @@ func (s *DockerSuite) TestCpFromCaseF(c *testing.T) { srcDir := containerCpPath(containerID, "/root/dir1") dstFile := cpPath(tmpDir, "file1") - err := runDockerCp(c, srcDir, dstFile, nil) + err := runDockerCp(c, srcDir, dstFile) assert.ErrorContains(c, err, "") - assert.Assert(c, isCpCannotCopyDir(err), "expected ErrCannotCopyDir error, but got %T: %s", err, err) } @@ -287,23 +254,18 @@ func (s *DockerSuite) TestCpFromCaseG(c *testing.T) { resultDir := filepath.Join(dstDir, "dir1") dstPath := filepath.Join(resultDir, "file1-1") - assert.Assert(c, runDockerCp(c, srcDir, dstDir, nil) == nil) - - assert.Assert(c, fileContentEquals(c, dstPath, "file1-1\n") == nil) + assert.NilError(c, runDockerCp(c, srcDir, dstDir)) + assert.NilError(c, fileContentEquals(c, dstPath, "file1-1\n")) // Now try again but using a trailing path separator for dstDir. - // unable to remove dstDir - assert.Assert(c, os.RemoveAll(dstDir) == nil) - - // unable to make dstDir - assert.Assert(c, os.MkdirAll(dstDir, os.FileMode(0755)) == nil) + assert.NilError(c, os.RemoveAll(dstDir), "unable to remove dstDir") + assert.NilError(c, os.MkdirAll(dstDir, os.FileMode(0755)), "unable to make dstDir") dstDir = cpPathTrailingSep(tmpDir, "dir2") - assert.Assert(c, runDockerCp(c, srcDir, dstDir, nil) == nil) - - assert.Assert(c, fileContentEquals(c, dstPath, "file1-1\n") == nil) + assert.NilError(c, runDockerCp(c, srcDir, dstDir)) + assert.NilError(c, fileContentEquals(c, dstPath, "file1-1\n")) } // H. SRC specifies a directory's contents only and DST does not exist. This @@ -321,20 +283,17 @@ func (s *DockerSuite) TestCpFromCaseH(c *testing.T) { dstDir := cpPath(tmpDir, "testDir") dstPath := filepath.Join(dstDir, "file1-1") - assert.Assert(c, runDockerCp(c, srcDir, dstDir, nil) == nil) - - assert.Assert(c, fileContentEquals(c, dstPath, "file1-1\n") == nil) + assert.NilError(c, runDockerCp(c, srcDir, dstDir)) + assert.NilError(c, fileContentEquals(c, dstPath, "file1-1\n")) // Now try again but using a trailing path separator for dstDir. - // unable to remove resultDir - assert.Assert(c, os.RemoveAll(dstDir) == nil) + assert.NilError(c, os.RemoveAll(dstDir), "unable to remove resultDir") dstDir = cpPathTrailingSep(tmpDir, "testDir") - assert.Assert(c, runDockerCp(c, srcDir, dstDir, nil) == nil) - - assert.Assert(c, fileContentEquals(c, dstPath, "file1-1\n") == nil) + assert.NilError(c, runDockerCp(c, srcDir, dstDir)) + assert.NilError(c, fileContentEquals(c, dstPath, "file1-1\n")) } // I. SRC specifies a directory's contents only and DST exists as a file. This @@ -354,9 +313,8 @@ func (s *DockerSuite) TestCpFromCaseI(c *testing.T) { srcDir := containerCpPathTrailingSep(containerID, "/root/dir1") + "." dstFile := cpPath(tmpDir, "file1") - err := runDockerCp(c, srcDir, dstFile, nil) + err := runDockerCp(c, srcDir, dstFile) assert.ErrorContains(c, err, "") - assert.Assert(c, isCpCannotCopyDir(err), "expected ErrCannotCopyDir error, but got %T: %s", err, err) } @@ -379,21 +337,16 @@ func (s *DockerSuite) TestCpFromCaseJ(c *testing.T) { dstDir := cpPath(tmpDir, "dir2") dstPath := filepath.Join(dstDir, "file1-1") - assert.Assert(c, runDockerCp(c, srcDir, dstDir, nil) == nil) - - assert.Assert(c, fileContentEquals(c, dstPath, "file1-1\n") == nil) + assert.NilError(c, runDockerCp(c, srcDir, dstDir)) + assert.NilError(c, fileContentEquals(c, dstPath, "file1-1\n")) // Now try again but using a trailing path separator for dstDir. - // unable to remove dstDir - assert.Assert(c, os.RemoveAll(dstDir) == nil) - - // unable to make dstDir - assert.Assert(c, os.MkdirAll(dstDir, os.FileMode(0755)) == nil) + assert.NilError(c, os.RemoveAll(dstDir), "unable to remove dstDir") + assert.NilError(c, os.MkdirAll(dstDir, os.FileMode(0755)), "unable to make dstDir") dstDir = cpPathTrailingSep(tmpDir, "dir2") - assert.Assert(c, runDockerCp(c, srcDir, dstDir, nil) == nil) - - assert.Assert(c, fileContentEquals(c, dstPath, "file1-1\n") == nil) + assert.NilError(c, runDockerCp(c, srcDir, dstDir)) + assert.NilError(c, fileContentEquals(c, dstPath, "file1-1\n")) } diff --git a/integration-cli/docker_cli_cp_test.go b/integration-cli/docker_cli_cp_test.go index 791138ed1a..3a84bad200 100644 --- a/integration-cli/docker_cli_cp_test.go +++ b/integration-cli/docker_cli_cp_test.go @@ -28,7 +28,7 @@ const ( // Ensure that an all-local path case returns an error. func (s *DockerSuite) TestCpLocalOnly(c *testing.T) { - err := runDockerCp(c, "foo", "bar", nil) + err := runDockerCp(c, "foo", "bar") assert.ErrorContains(c, err, "must specify at least one container source") } diff --git a/integration-cli/docker_cli_cp_to_container_test.go b/integration-cli/docker_cli_cp_to_container_test.go index 4a5c747aae..ddb1a19d22 100644 --- a/integration-cli/docker_cli_cp_to_container_test.go +++ b/integration-cli/docker_cli_cp_to_container_test.go @@ -39,38 +39,26 @@ func (s *DockerSuite) TestCpToSymlinkDestination(c *testing.T) { srcPath := cpPath(testVol, "file2") dstPath := containerCpPath(containerID, "/vol2/symlinkToFile1") - assert.Assert(c, runDockerCp(c, srcPath, dstPath, nil) == nil) - - // The symlink should not have been modified. - assert.Assert(c, symlinkTargetEquals(c, cpPath(testVol, "symlinkToFile1"), "file1") == nil) - - // The file should have the contents of "file2" now. - assert.Assert(c, fileContentEquals(c, cpPath(testVol, "file1"), "file2\n") == nil) + assert.NilError(c, runDockerCp(c, srcPath, dstPath)) + assert.NilError(c, symlinkTargetEquals(c, cpPath(testVol, "symlinkToFile1"), "file1"), "The symlink should not have been modified") + assert.NilError(c, fileContentEquals(c, cpPath(testVol, "file1"), "file2\n"), `The file should have the contents of "file2" now`) // Next, copy a local file to a symlink to a directory in the container. // This should copy the file into the symlink target directory. dstPath = containerCpPath(containerID, "/vol2/symlinkToDir1") - assert.Assert(c, runDockerCp(c, srcPath, dstPath, nil) == nil) - - // The symlink should not have been modified. - assert.Assert(c, symlinkTargetEquals(c, cpPath(testVol, "symlinkToDir1"), "dir1") == nil) - - // The file should have the contents of "file2" now. - assert.Assert(c, fileContentEquals(c, cpPath(testVol, "file2"), "file2\n") == nil) + assert.NilError(c, runDockerCp(c, srcPath, dstPath)) + assert.NilError(c, symlinkTargetEquals(c, cpPath(testVol, "symlinkToDir1"), "dir1"), "The symlink should not have been modified") + assert.NilError(c, fileContentEquals(c, cpPath(testVol, "file2"), "file2\n"), `The file should have the contents of "file2"" now`) // Next, copy a file to a symlink to a file that does not exist (a broken // symlink) in the container. This should create the target file with the // contents of the source file. dstPath = containerCpPath(containerID, "/vol2/brokenSymlinkToFileX") - assert.Assert(c, runDockerCp(c, srcPath, dstPath, nil) == nil) - - // The symlink should not have been modified. - assert.Assert(c, symlinkTargetEquals(c, cpPath(testVol, "brokenSymlinkToFileX"), "fileX") == nil) - - // The file should have the contents of "file2" now. - assert.Assert(c, fileContentEquals(c, cpPath(testVol, "fileX"), "file2\n") == nil) + assert.NilError(c, runDockerCp(c, srcPath, dstPath)) + assert.NilError(c, symlinkTargetEquals(c, cpPath(testVol, "brokenSymlinkToFileX"), "fileX"), "The symlink should not have been modified") + assert.NilError(c, fileContentEquals(c, cpPath(testVol, "fileX"), "file2\n"), `The file should have the contents of "file2"" now`) // Next, copy a local directory to a symlink to a directory in the // container. This should copy the directory into the symlink target @@ -78,13 +66,9 @@ func (s *DockerSuite) TestCpToSymlinkDestination(c *testing.T) { srcPath = cpPath(testVol, "/dir2") dstPath = containerCpPath(containerID, "/vol2/symlinkToDir1") - assert.Assert(c, runDockerCp(c, srcPath, dstPath, nil) == nil) - - // The symlink should not have been modified. - assert.Assert(c, symlinkTargetEquals(c, cpPath(testVol, "symlinkToDir1"), "dir1") == nil) - - // The directory should now contain a copy of "dir2". - assert.Assert(c, fileContentEquals(c, cpPath(testVol, "dir1/dir2/file2-1"), "file2-1\n") == nil) + assert.NilError(c, runDockerCp(c, srcPath, dstPath)) + assert.NilError(c, symlinkTargetEquals(c, cpPath(testVol, "symlinkToDir1"), "dir1"), "The symlink should not have been modified") + assert.NilError(c, fileContentEquals(c, cpPath(testVol, "dir1/dir2/file2-1"), "file2-1\n"), `The directory should now contain a copy of "dir2"`) // Next, copy a local directory to a symlink to a local directory that does // not exist (a broken symlink) in the container. This should create the @@ -92,13 +76,9 @@ func (s *DockerSuite) TestCpToSymlinkDestination(c *testing.T) { // should not modify the symlink. dstPath = containerCpPath(containerID, "/vol2/brokenSymlinkToDirX") - assert.Assert(c, runDockerCp(c, srcPath, dstPath, nil) == nil) - - // The symlink should not have been modified. - assert.Assert(c, symlinkTargetEquals(c, cpPath(testVol, "brokenSymlinkToDirX"), "dirX") == nil) - - // The "dirX" directory should now be a copy of "dir2". - assert.Assert(c, fileContentEquals(c, cpPath(testVol, "dirX/file2-1"), "file2-1\n") == nil) + assert.NilError(c, runDockerCp(c, srcPath, dstPath)) + assert.NilError(c, symlinkTargetEquals(c, cpPath(testVol, "brokenSymlinkToDirX"), "dirX"), "The symlink should not have been modified") + assert.NilError(c, fileContentEquals(c, cpPath(testVol, "dirX/file2-1"), "file2-1\n"), `The "dirX" directory should now be a copy of "dir2"`) } // Possibilities are reduced to the remaining 10 cases: @@ -133,9 +113,8 @@ func (s *DockerSuite) TestCpToCaseA(c *testing.T) { srcPath := cpPath(tmpDir, "file1") dstPath := containerCpPath(containerID, "/root/itWorks.txt") - assert.Assert(c, runDockerCp(c, srcPath, dstPath, nil) == nil) - - assert.Assert(c, containerStartOutputEquals(c, containerID, "file1\n") == nil) + assert.NilError(c, runDockerCp(c, srcPath, dstPath)) + assert.NilError(c, containerStartOutputEquals(c, containerID, "file1\n")) } // B. SRC specifies a file and DST (with trailing path separator) doesn't @@ -154,9 +133,8 @@ func (s *DockerSuite) TestCpToCaseB(c *testing.T) { srcPath := cpPath(tmpDir, "file1") dstDir := containerCpPathTrailingSep(containerID, "testDir") - err := runDockerCp(c, srcPath, dstDir, nil) + err := runDockerCp(c, srcPath, dstDir) assert.ErrorContains(c, err, "") - assert.Assert(c, isCpDirNotExist(err), "expected DirNotExists error, but got %T: %s", err, err) } @@ -178,12 +156,9 @@ func (s *DockerSuite) TestCpToCaseC(c *testing.T) { dstPath := containerCpPath(containerID, "/root/file2") // Ensure the container's file starts with the original content. - assert.Assert(c, containerStartOutputEquals(c, containerID, "file2\n") == nil) - - assert.Assert(c, runDockerCp(c, srcPath, dstPath, nil) == nil) - - // Should now contain file1's contents. - assert.Assert(c, containerStartOutputEquals(c, containerID, "file1\n") == nil) + assert.NilError(c, containerStartOutputEquals(c, containerID, "file2\n")) + assert.NilError(c, runDockerCp(c, srcPath, dstPath)) + assert.NilError(c, containerStartOutputEquals(c, containerID, "file1\n"), "Should now contain file1's contents") } // D. SRC specifies a file and DST exists as a directory. This should place @@ -204,13 +179,9 @@ func (s *DockerSuite) TestCpToCaseD(c *testing.T) { srcPath := cpPath(tmpDir, "file1") dstDir := containerCpPath(containerID, "dir1") - // Ensure that dstPath doesn't exist. - assert.Assert(c, containerStartOutputEquals(c, containerID, "") == nil) - - assert.Assert(c, runDockerCp(c, srcPath, dstDir, nil) == nil) - - // Should now contain file1's contents. - assert.Assert(c, containerStartOutputEquals(c, containerID, "file1\n") == nil) + assert.NilError(c, containerStartOutputEquals(c, containerID, ""), "dstPath should not have existed") + assert.NilError(c, runDockerCp(c, srcPath, dstDir)) + assert.NilError(c, containerStartOutputEquals(c, containerID, "file1\n"), "Should now contain file1's contents") // Now try again but using a trailing path separator for dstDir. @@ -222,13 +193,9 @@ func (s *DockerSuite) TestCpToCaseD(c *testing.T) { dstDir = containerCpPathTrailingSep(containerID, "dir1") - // Ensure that dstPath doesn't exist. - assert.Assert(c, containerStartOutputEquals(c, containerID, "") == nil) - - assert.Assert(c, runDockerCp(c, srcPath, dstDir, nil) == nil) - - // Should now contain file1's contents. - assert.Assert(c, containerStartOutputEquals(c, containerID, "file1\n") == nil) + assert.NilError(c, containerStartOutputEquals(c, containerID, ""), "dstPath should not have existed") + assert.NilError(c, runDockerCp(c, srcPath, dstDir)) + assert.NilError(c, containerStartOutputEquals(c, containerID, "file1\n"), "Should now contain file1's contents") } // E. SRC specifies a directory and DST does not exist. This should create a @@ -248,10 +215,8 @@ func (s *DockerSuite) TestCpToCaseE(c *testing.T) { srcDir := cpPath(tmpDir, "dir1") dstDir := containerCpPath(containerID, "testDir") - assert.Assert(c, runDockerCp(c, srcDir, dstDir, nil) == nil) - - // Should now contain file1-1's contents. - assert.Assert(c, containerStartOutputEquals(c, containerID, "file1-1\n") == nil) + assert.NilError(c, runDockerCp(c, srcDir, dstDir)) + assert.NilError(c, containerStartOutputEquals(c, containerID, "file1-1\n"), "Should now contain file1-1's contents") // Now try again but using a trailing path separator for dstDir. @@ -262,10 +227,8 @@ func (s *DockerSuite) TestCpToCaseE(c *testing.T) { dstDir = containerCpPathTrailingSep(containerID, "testDir") - assert.Assert(c, runDockerCp(c, srcDir, dstDir, nil) == nil) - - // Should now contain file1-1's contents. - assert.Assert(c, containerStartOutputEquals(c, containerID, "file1-1\n") == nil) + assert.NilError(c, runDockerCp(c, srcDir, dstDir)) + assert.NilError(c, containerStartOutputEquals(c, containerID, "file1-1\n"), "Should now contain file1-1's contents") } // F. SRC specifies a directory and DST exists as a file. This should cause an @@ -284,9 +247,8 @@ func (s *DockerSuite) TestCpToCaseF(c *testing.T) { srcDir := cpPath(tmpDir, "dir1") dstFile := containerCpPath(containerID, "/root/file1") - err := runDockerCp(c, srcDir, dstFile, nil) + err := runDockerCp(c, srcDir, dstFile) assert.ErrorContains(c, err, "") - assert.Assert(c, isCpCannotCopyDir(err), "expected ErrCannotCopyDir error, but got %T: %s", err, err) } @@ -308,13 +270,9 @@ func (s *DockerSuite) TestCpToCaseG(c *testing.T) { srcDir := cpPath(tmpDir, "dir1") dstDir := containerCpPath(containerID, "/root/dir2") - // Ensure that dstPath doesn't exist. - assert.Assert(c, containerStartOutputEquals(c, containerID, "") == nil) - - assert.Assert(c, runDockerCp(c, srcDir, dstDir, nil) == nil) - - // Should now contain file1-1's contents. - assert.Assert(c, containerStartOutputEquals(c, containerID, "file1-1\n") == nil) + assert.NilError(c, containerStartOutputEquals(c, containerID, ""), "dstPath should not have existed") + assert.NilError(c, runDockerCp(c, srcDir, dstDir)) + assert.NilError(c, containerStartOutputEquals(c, containerID, "file1-1\n"), "Should now contain file1-1's contents") // Now try again but using a trailing path separator for dstDir. @@ -326,13 +284,9 @@ func (s *DockerSuite) TestCpToCaseG(c *testing.T) { dstDir = containerCpPathTrailingSep(containerID, "/dir2") - // Ensure that dstPath doesn't exist. - assert.Assert(c, containerStartOutputEquals(c, containerID, "") == nil) - - assert.Assert(c, runDockerCp(c, srcDir, dstDir, nil) == nil) - - // Should now contain file1-1's contents. - assert.Assert(c, containerStartOutputEquals(c, containerID, "file1-1\n") == nil) + assert.NilError(c, containerStartOutputEquals(c, containerID, ""), "dstPath should not have existed") + assert.NilError(c, runDockerCp(c, srcDir, dstDir)) + assert.NilError(c, containerStartOutputEquals(c, containerID, "file1-1\n"), "Should now contain file1-1's contents") } // H. SRC specifies a directory's contents only and DST does not exist. This @@ -352,10 +306,8 @@ func (s *DockerSuite) TestCpToCaseH(c *testing.T) { srcDir := cpPathTrailingSep(tmpDir, "dir1") + "." dstDir := containerCpPath(containerID, "testDir") - assert.Assert(c, runDockerCp(c, srcDir, dstDir, nil) == nil) - - // Should now contain file1-1's contents. - assert.Assert(c, containerStartOutputEquals(c, containerID, "file1-1\n") == nil) + assert.NilError(c, runDockerCp(c, srcDir, dstDir)) + assert.NilError(c, containerStartOutputEquals(c, containerID, "file1-1\n"), "Should now contain file1-1's contents") // Now try again but using a trailing path separator for dstDir. @@ -366,10 +318,8 @@ func (s *DockerSuite) TestCpToCaseH(c *testing.T) { dstDir = containerCpPathTrailingSep(containerID, "testDir") - assert.Assert(c, runDockerCp(c, srcDir, dstDir, nil) == nil) - - // Should now contain file1-1's contents. - assert.Assert(c, containerStartOutputEquals(c, containerID, "file1-1\n") == nil) + assert.NilError(c, runDockerCp(c, srcDir, dstDir)) + assert.NilError(c, containerStartOutputEquals(c, containerID, "file1-1\n"), "Should now contain file1-1's contents") } // I. SRC specifies a directory's contents only and DST exists as a file. This @@ -389,9 +339,8 @@ func (s *DockerSuite) TestCpToCaseI(c *testing.T) { srcDir := cpPathTrailingSep(tmpDir, "dir1") + "." dstFile := containerCpPath(containerID, "/root/file1") - err := runDockerCp(c, srcDir, dstFile, nil) + err := runDockerCp(c, srcDir, dstFile) assert.ErrorContains(c, err, "") - assert.Assert(c, isCpCannotCopyDir(err), "expected ErrCannotCopyDir error, but got %T: %s", err, err) } @@ -414,13 +363,9 @@ func (s *DockerSuite) TestCpToCaseJ(c *testing.T) { srcDir := cpPathTrailingSep(tmpDir, "dir1") + "." dstDir := containerCpPath(containerID, "/dir2") - // Ensure that dstPath doesn't exist. - assert.Assert(c, containerStartOutputEquals(c, containerID, "") == nil) - - assert.Assert(c, runDockerCp(c, srcDir, dstDir, nil) == nil) - - // Should now contain file1-1's contents. - assert.Assert(c, containerStartOutputEquals(c, containerID, "file1-1\n") == nil) + assert.NilError(c, containerStartOutputEquals(c, containerID, ""), "dstPath should not have existed") + assert.NilError(c, runDockerCp(c, srcDir, dstDir)) + assert.NilError(c, containerStartOutputEquals(c, containerID, "file1-1\n"), "Should've contained file1-1's contents") // Now try again but using a trailing path separator for dstDir. @@ -431,13 +376,9 @@ func (s *DockerSuite) TestCpToCaseJ(c *testing.T) { dstDir = containerCpPathTrailingSep(containerID, "/dir2") - // Ensure that dstPath doesn't exist. - assert.Assert(c, containerStartOutputEquals(c, containerID, "") == nil) - - assert.Assert(c, runDockerCp(c, srcDir, dstDir, nil) == nil) - - // Should now contain file1-1's contents. - assert.Assert(c, containerStartOutputEquals(c, containerID, "file1-1\n") == nil) + assert.NilError(c, containerStartOutputEquals(c, containerID, ""), "dstPath should not have existed") + assert.NilError(c, runDockerCp(c, srcDir, dstDir)) + assert.NilError(c, containerStartOutputEquals(c, containerID, "file1-1\n"), "Should've contained file1-1's contents") } // The `docker cp` command should also ensure that you cannot @@ -458,13 +399,11 @@ func (s *DockerSuite) TestCpToErrReadOnlyRootfs(c *testing.T) { srcPath := cpPath(tmpDir, "file1") dstPath := containerCpPath(containerID, "/root/shouldNotExist") - err := runDockerCp(c, srcPath, dstPath, nil) + err := runDockerCp(c, srcPath, dstPath) assert.ErrorContains(c, err, "") assert.Assert(c, isCpCannotCopyReadOnly(err), "expected ErrContainerRootfsReadonly error, but got %T: %s", err, err) - - // Ensure that dstPath doesn't exist. - assert.Assert(c, containerStartOutputEquals(c, containerID, "") == nil) + assert.NilError(c, containerStartOutputEquals(c, containerID, ""), "dstPath should not have existed") } // The `docker cp` command should also ensure that you @@ -485,11 +424,9 @@ func (s *DockerSuite) TestCpToErrReadOnlyVolume(c *testing.T) { srcPath := cpPath(tmpDir, "file1") dstPath := containerCpPath(containerID, "/vol_ro/shouldNotExist") - err := runDockerCp(c, srcPath, dstPath, nil) + err := runDockerCp(c, srcPath, dstPath) assert.ErrorContains(c, err, "") assert.Assert(c, isCpCannotCopyReadOnly(err), "expected ErrVolumeReadonly error, but got %T: %s", err, err) - - // Ensure that dstPath doesn't exist. - assert.Assert(c, containerStartOutputEquals(c, containerID, "") == nil) + assert.NilError(c, containerStartOutputEquals(c, containerID, ""), "dstPath should not have existed") } diff --git a/integration-cli/docker_cli_cp_to_container_unix_test.go b/integration-cli/docker_cli_cp_to_container_unix_test.go index 9c1b8229b2..89a7c49981 100644 --- a/integration-cli/docker_cli_cp_to_container_unix_test.go +++ b/integration-cli/docker_cli_cp_to_container_unix_test.go @@ -5,6 +5,7 @@ package main import ( "fmt" "os" + "os/exec" "path/filepath" "strconv" "strings" @@ -30,9 +31,12 @@ func (s *DockerSuite) TestCpToContainerWithPermissions(c *testing.T) { srcPath := cpPath(tmpDir, "permdirtest") dstPath := containerCpPath(containerName, "/") - assert.NilError(c, runDockerCp(c, srcPath, dstPath, []string{"-a"})) - out, err := startContainerGetOutput(c, containerName) + args := []string{"cp", "-a", srcPath, dstPath} + out, _, err := runCommandWithOutput(exec.Command(dockerBinary, args...)) + assert.NilError(c, err, "output: %v", out) + + out, err = startContainerGetOutput(c, containerName) assert.NilError(c, err, "output: %v", out) assert.Equal(c, strings.TrimSpace(out), "2 2 700\n65534 65534 400", "output: %v", out) } @@ -52,8 +56,7 @@ func (s *DockerSuite) TestCpCheckDestOwnership(c *testing.T) { srcPath := cpPath(tmpDir, "file1") dstPath := containerCpPath(containerID, "/tmpvol", "file1") - err := runDockerCp(c, srcPath, dstPath, nil) - assert.NilError(c, err) + assert.NilError(c, runDockerCp(c, srcPath, dstPath)) stat, err := system.Stat(filepath.Join(tmpVolDir, "file1")) assert.NilError(c, err) @@ -66,7 +69,7 @@ func (s *DockerSuite) TestCpCheckDestOwnership(c *testing.T) { func getRootUIDGID() (int, int, error) { uidgid := strings.Split(filepath.Base(testEnv.DaemonInfo.DockerRootDir), ".") if len(uidgid) == 1 { - //user namespace remapping is not turned on; return 0 + // user namespace remapping is not turned on; return 0 return 0, 0, nil } uid, err := strconv.Atoi(uidgid[0]) diff --git a/integration-cli/docker_cli_cp_utils_test.go b/integration-cli/docker_cli_cp_utils_test.go index a7c317b86f..5de9d09eb4 100644 --- a/integration-cli/docker_cli_cp_utils_test.go +++ b/integration-cli/docker_cli_cp_utils_test.go @@ -93,6 +93,7 @@ func defaultMkContentCommand() string { } func makeTestContentInDir(c *testing.T, dir string) { + c.Helper() for _, fd := range defaultFileData { path := filepath.Join(dir, filepath.FromSlash(fd.path)) switch fd.filetype { @@ -119,6 +120,7 @@ type testContainerOptions struct { } func makeTestContainer(c *testing.T, options testContainerOptions) (containerID string) { + c.Helper() if options.addContent { mkContentCmd := defaultMkContentCommand() if options.command == "" { @@ -188,24 +190,19 @@ func containerCpPathTrailingSep(containerID string, pathElements ...string) stri return fmt.Sprintf("%s/", containerCpPath(containerID, pathElements...)) } -func runDockerCp(c *testing.T, src, dst string, params []string) (err error) { - c.Logf("running `docker cp %s %s %s`", strings.Join(params, " "), src, dst) +func runDockerCp(c *testing.T, src, dst string) error { + c.Helper() + c.Logf("running `docker cp %s %s`", src, dst) - args := []string{"cp"} - - args = append(args, params...) - - args = append(args, src, dst) - - out, _, err := runCommandWithOutput(exec.Command(dockerBinary, args...)) - if err != nil { - err = fmt.Errorf("error executing `docker cp` command: %s: %s", err, out) + args := []string{"cp", src, dst} + if out, _, err := runCommandWithOutput(exec.Command(dockerBinary, args...)); err != nil { + return fmt.Errorf("error executing `docker cp` command: %s: %s", err, out) } - - return + return nil } func startContainerGetOutput(c *testing.T, containerID string) (out string, err error) { + c.Helper() c.Logf("running `docker start -a %s`", containerID) args := []string{"start", "-a", containerID} @@ -219,6 +216,7 @@ func startContainerGetOutput(c *testing.T, containerID string) (out string, err } func getTestDir(c *testing.T, label string) (tmpDir string) { + c.Helper() var err error tmpDir, err = ioutil.TempDir("", label) @@ -240,54 +238,57 @@ func isCpCannotCopyReadOnly(err error) bool { return strings.Contains(err.Error(), "marked read-only") } -func fileContentEquals(c *testing.T, filename, contents string) (err error) { +func fileContentEquals(c *testing.T, filename, contents string) error { + c.Helper() c.Logf("checking that file %q contains %q\n", filename, contents) fileBytes, err := ioutil.ReadFile(filename) if err != nil { - return + return err } expectedBytes, err := ioutil.ReadAll(strings.NewReader(contents)) if err != nil { - return + return err } if !bytes.Equal(fileBytes, expectedBytes) { - err = fmt.Errorf("file content not equal - expected %q, got %q", string(expectedBytes), string(fileBytes)) + return fmt.Errorf("file content not equal - expected %q, got %q", string(expectedBytes), string(fileBytes)) } - return + return nil } -func symlinkTargetEquals(c *testing.T, symlink, expectedTarget string) (err error) { +func symlinkTargetEquals(c *testing.T, symlink, expectedTarget string) error { + c.Helper() c.Logf("checking that the symlink %q points to %q\n", symlink, expectedTarget) actualTarget, err := os.Readlink(symlink) if err != nil { - return + return err } if actualTarget != expectedTarget { - err = fmt.Errorf("symlink target points to %q not %q", actualTarget, expectedTarget) + return fmt.Errorf("symlink target points to %q not %q", actualTarget, expectedTarget) } - return + return nil } -func containerStartOutputEquals(c *testing.T, containerID, contents string) (err error) { +func containerStartOutputEquals(c *testing.T, containerID, contents string) error { + c.Helper() c.Logf("checking that container %q start output contains %q\n", containerID, contents) out, err := startContainerGetOutput(c, containerID) if err != nil { - return + return err } if out != contents { - err = fmt.Errorf("output contents not equal - expected %q, got %q", contents, out) + return fmt.Errorf("output contents not equal - expected %q, got %q", contents, out) } - return + return nil } func defaultVolumes(tmpDir string) []string {