Procházet zdrojové kódy

Merge pull request #40115 from thaJeztah/integration_cli_denoise

integration-cli: make some tests less noisy and easier debuggable
Brian Goff před 5 roky
rodič
revize
352d840819

+ 62 - 58
integration-cli/docker_api_containers_test.go

@@ -1918,19 +1918,21 @@ func (s *DockerSuite) TestContainersAPICreateMountsValidation(c *testing.T) {
 		}...)
 
 	}
-	cli, err := client.NewClientWithOpts(client.FromEnv)
+	apiClient, err := client.NewClientWithOpts(client.FromEnv)
 	assert.NilError(c, err)
-	defer cli.Close()
+	defer apiClient.Close()
 
 	// TODO add checks for statuscode returned by API
 	for i, x := range cases {
-		c.Logf("case %d", i)
-		_, err = cli.ContainerCreate(context.Background(), &x.config, &x.hostConfig, &networktypes.NetworkingConfig{}, "")
-		if len(x.msg) > 0 {
-			assert.ErrorContains(c, err, x.msg, "%v", cases[i].config)
-		} else {
-			assert.NilError(c, err)
-		}
+		x := x
+		c.Run(fmt.Sprintf("case %d", i), func(c *testing.T) {
+			_, err = apiClient.ContainerCreate(context.Background(), &x.config, &x.hostConfig, &networktypes.NetworkingConfig{}, "")
+			if len(x.msg) > 0 {
+				assert.ErrorContains(c, err, x.msg, "%v", cases[i].config)
+			} else {
+				assert.NilError(c, err)
+			}
+		})
 	}
 }
 
@@ -2097,63 +2099,65 @@ func (s *DockerSuite) TestContainersAPICreateMountsCreate(c *testing.T) {
 	ctx := context.Background()
 	apiclient := testEnv.APIClient()
 	for i, x := range cases {
-		c.Logf("case %d - config: %v", i, x.spec)
-		container, err := apiclient.ContainerCreate(
-			ctx,
-			&containertypes.Config{Image: testImg},
-			&containertypes.HostConfig{Mounts: []mounttypes.Mount{x.spec}},
-			&networktypes.NetworkingConfig{},
-			"")
-		assert.NilError(c, err)
+		x := x
+		c.Run(fmt.Sprintf("%d config: %v", i, x.spec), func(c *testing.T) {
+			container, err := apiclient.ContainerCreate(
+				ctx,
+				&containertypes.Config{Image: testImg},
+				&containertypes.HostConfig{Mounts: []mounttypes.Mount{x.spec}},
+				&networktypes.NetworkingConfig{},
+				"")
+			assert.NilError(c, err)
 
-		containerInspect, err := apiclient.ContainerInspect(ctx, container.ID)
-		assert.NilError(c, err)
-		mps := containerInspect.Mounts
-		assert.Assert(c, is.Len(mps, 1))
-		mountPoint := mps[0]
+			containerInspect, err := apiclient.ContainerInspect(ctx, container.ID)
+			assert.NilError(c, err)
+			mps := containerInspect.Mounts
+			assert.Assert(c, is.Len(mps, 1))
+			mountPoint := mps[0]
 
-		if x.expected.Source != "" {
-			assert.Check(c, is.Equal(x.expected.Source, mountPoint.Source))
-		}
-		if x.expected.Name != "" {
-			assert.Check(c, is.Equal(x.expected.Name, mountPoint.Name))
-		}
-		if x.expected.Driver != "" {
-			assert.Check(c, is.Equal(x.expected.Driver, mountPoint.Driver))
-		}
-		if x.expected.Propagation != "" {
-			assert.Check(c, is.Equal(x.expected.Propagation, mountPoint.Propagation))
-		}
-		assert.Check(c, is.Equal(x.expected.RW, mountPoint.RW))
-		assert.Check(c, is.Equal(x.expected.Type, mountPoint.Type))
-		assert.Check(c, is.Equal(x.expected.Mode, mountPoint.Mode))
-		assert.Check(c, is.Equal(x.expected.Destination, mountPoint.Destination))
+			if x.expected.Source != "" {
+				assert.Check(c, is.Equal(x.expected.Source, mountPoint.Source))
+			}
+			if x.expected.Name != "" {
+				assert.Check(c, is.Equal(x.expected.Name, mountPoint.Name))
+			}
+			if x.expected.Driver != "" {
+				assert.Check(c, is.Equal(x.expected.Driver, mountPoint.Driver))
+			}
+			if x.expected.Propagation != "" {
+				assert.Check(c, is.Equal(x.expected.Propagation, mountPoint.Propagation))
+			}
+			assert.Check(c, is.Equal(x.expected.RW, mountPoint.RW))
+			assert.Check(c, is.Equal(x.expected.Type, mountPoint.Type))
+			assert.Check(c, is.Equal(x.expected.Mode, mountPoint.Mode))
+			assert.Check(c, is.Equal(x.expected.Destination, mountPoint.Destination))
 
-		err = apiclient.ContainerStart(ctx, container.ID, types.ContainerStartOptions{})
-		assert.NilError(c, err)
-		poll.WaitOn(c, containerExit(apiclient, container.ID), poll.WithDelay(time.Second))
+			err = apiclient.ContainerStart(ctx, container.ID, types.ContainerStartOptions{})
+			assert.NilError(c, err)
+			poll.WaitOn(c, containerExit(apiclient, container.ID), poll.WithDelay(time.Second))
 
-		err = apiclient.ContainerRemove(ctx, container.ID, types.ContainerRemoveOptions{
-			RemoveVolumes: true,
-			Force:         true,
-		})
-		assert.NilError(c, err)
+			err = apiclient.ContainerRemove(ctx, container.ID, types.ContainerRemoveOptions{
+				RemoveVolumes: true,
+				Force:         true,
+			})
+			assert.NilError(c, err)
 
-		switch {
+			switch {
 
-		// Named volumes still exist after the container is removed
-		case x.spec.Type == "volume" && len(x.spec.Source) > 0:
-			_, err := apiclient.VolumeInspect(ctx, mountPoint.Name)
-			assert.NilError(c, err)
+			// Named volumes still exist after the container is removed
+			case x.spec.Type == "volume" && len(x.spec.Source) > 0:
+				_, err := apiclient.VolumeInspect(ctx, mountPoint.Name)
+				assert.NilError(c, err)
 
-		// Bind mounts are never removed with the container
-		case x.spec.Type == "bind":
+			// Bind mounts are never removed with the container
+			case x.spec.Type == "bind":
 
-		// anonymous volumes are removed
-		default:
-			_, err := apiclient.VolumeInspect(ctx, mountPoint.Name)
-			assert.Check(c, client.IsErrNotFound(err))
-		}
+			// anonymous volumes are removed
+			default:
+				_, err := apiclient.VolumeInspect(ctx, mountPoint.Name)
+				assert.Check(c, client.IsErrNotFound(err))
+			}
+		})
 	}
 }
 

+ 51 - 98
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"))
 }

+ 1 - 1
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")
 }
 

+ 53 - 116
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")
 }

+ 8 - 5
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])

+ 26 - 30
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,25 +190,18 @@ 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()
 
-	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.Logf("running `docker start -a %s`", containerID)
+	c.Helper()
 
 	args := []string{"start", "-a", containerID}
 
@@ -219,6 +214,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 +236,54 @@ func isCpCannotCopyReadOnly(err error) bool {
 	return strings.Contains(err.Error(), "marked read-only")
 }
 
-func fileContentEquals(c *testing.T, filename, contents string) (err error) {
-	c.Logf("checking that file %q contains %q\n", filename, contents)
+func fileContentEquals(c *testing.T, filename, contents string) error {
+	c.Helper()
 
 	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) {
-	c.Logf("checking that the symlink %q points to %q\n", symlink, expectedTarget)
+func symlinkTargetEquals(c *testing.T, symlink, expectedTarget string) error {
+	c.Helper()
 
 	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) {
-	c.Logf("checking that container %q start output contains %q\n", containerID, contents)
+func containerStartOutputEquals(c *testing.T, containerID, contents string) error {
+	c.Helper()
 
 	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 {

+ 1 - 2
integration-cli/docker_cli_service_logs_test.go

@@ -46,8 +46,7 @@ func (s *DockerSwarmSuite) TestServiceLogs(c *testing.T) {
 	for name, message := range services {
 		out, err := d.Cmd("service", "logs", name)
 		assert.NilError(c, err)
-		c.Logf("log for %q: %q", name, out)
-		assert.Assert(c, strings.Contains(out, message))
+		assert.Assert(c, strings.Contains(out, message), "log for %q: %q", name, out)
 	}
 }