Merge pull request moby#50524 from thaJeztah/cleanup_testutils

integration, integration-cli: remove various deprecated test-utilities, and some minor (linting) fixes

Author: thaJeztah

integration-cli/check_test.go

@@ -422,19 +422,18 @@ func (s *DockerSuite) OnTimeout(t *testing.T) {
 	if testEnv.IsRemoteDaemon() {
 		return
 	}
-	path := filepath.Join(os.Getenv("DEST"), "docker.pid")
-	b, err := os.ReadFile(path)
+	pidFile := filepath.Join(os.Getenv("DEST"), "docker.pid")
+	b, err := os.ReadFile(pidFile)
 	if err != nil {
-		t.Fatalf("Failed to get daemon PID from %s\n", path)
+		t.Fatalf("Failed to get daemon PID from %s\n", pidFile)
 	}
 
 	rawPid, err := strconv.ParseInt(string(b), 10, 32)
 	if err != nil {
-		t.Fatalf("Failed to parse pid from %s: %s\n", path, err)
+		t.Fatalf("Failed to parse pid from %s: %s\n", pidFile, err)
 	}
 
-	daemonPid := int(rawPid)
-	if daemonPid > 0 {
+	if daemonPid := int(rawPid); daemonPid > 0 {
 		testdaemon.SignalDaemonDump(daemonPid)
 	}
 }

integration-cli/daemon/daemon.go

@@ -8,6 +8,9 @@ import (
 	"time"
 
 	"github.com/docker/docker/testutil/daemon"
+	"github.com/moby/moby/api/types/container"
+	"github.com/moby/moby/client"
+	"github.com/moby/moby/client/pkg/stringid"
 	"github.com/pkg/errors"
 	"gotest.tools/v3/assert"
 	"gotest.tools/v3/icmd"
@@ -83,12 +86,17 @@ func (d *Daemon) inspectFieldWithError(name, field string) (string, error) {
 func (d *Daemon) CheckActiveContainerCount(ctx context.Context) func(t *testing.T) (interface{}, string) {
 	return func(t *testing.T) (interface{}, string) {
 		t.Helper()
-		out, err := d.Cmd("ps", "-q")
+		apiClient, err := client.NewClientWithOpts(client.FromEnv, client.WithHost(d.Sock()))
 		assert.NilError(t, err)
-		if strings.TrimSpace(out) == "" {
-			return 0, ""
+
+		ctrs, err := apiClient.ContainerList(ctx, container.ListOptions{})
+		_ = apiClient.Close()
+		assert.NilError(t, err)
+		var out strings.Builder
+		for _, ctr := range ctrs {
+			out.WriteString(stringid.TruncateID(ctr.ID) + "\n")
 		}
-		return len(strings.Split(strings.TrimSpace(out), "\n")), fmt.Sprintf("output: %q", out)
+		return len(ctrs), out.String()
 	}
 }
 

integration-cli/docker_api_containers_test.go

@@ -484,14 +484,13 @@ func (s *DockerAPISuite) TestContainerAPICommitWithLabelInConfig(c *testing.T) {
 	img, err := apiClient.ContainerCommit(testutil.GetContext(c), cName, options)
 	assert.NilError(c, err)
 
-	label1 := inspectFieldMap(c, img.ID, "Config.Labels", "key1")
-	assert.Equal(c, label1, "value1")
-
-	label2 := inspectFieldMap(c, img.ID, "Config.Labels", "key2")
-	assert.Equal(c, label2, "value2")
+	imgInspect, err := apiClient.ImageInspect(testutil.GetContext(c), img.ID)
+	assert.NilError(c, err)
+	assert.Check(c, is.Equal(imgInspect.Config.Labels["key1"], "value1"))
+	assert.Check(c, is.Equal(imgInspect.Config.Labels["key2"], "value2"))
 
-	cmd := inspectField(c, img.ID, "Config.Cmd")
-	assert.Equal(c, cmd, "[/bin/sh -c touch /test]", fmt.Sprintf("got wrong Cmd from commit: %q", cmd))
+	expected := []string{"/bin/sh", "-c", "touch /test"}
+	assert.Check(c, is.DeepEqual(imgInspect.Config.Cmd, expected))
 
 	// sanity check, make sure the image is what we think it is
 	cli.DockerCmd(c, "run", img.ID, "ls", "/test")
@@ -1015,32 +1014,36 @@ func (s *DockerAPISuite) TestContainerAPIDeleteRemoveLinks(c *testing.T) {
 func (s *DockerAPISuite) TestContainerAPIDeleteRemoveVolume(c *testing.T) {
 	testRequires(c, testEnv.IsLocalDaemon)
 
-	vol := "/testvolume"
+	testVol := "/testvolume"
 	if testEnv.DaemonInfo.OSType == "windows" {
-		vol = `c:\testvolume`
+		testVol = `c:\testvolume`
 	}
 
-	id := runSleepingContainer(c, "-v", vol)
+	id := runSleepingContainer(c, "-v", testVol)
 	cli.WaitRun(c, id)
 
-	source, err := inspectMountSourceField(id, vol)
+	apiClient, err := client.NewClientWithOpts(client.FromEnv)
 	assert.NilError(c, err)
-	_, err = os.Stat(source)
+	defer apiClient.Close()
+
+	ctrInspect, err := apiClient.ContainerInspect(testutil.GetContext(c), id)
+	assert.NilError(c, err)
+	assert.Assert(c, is.Len(ctrInspect.Mounts, 1), "expected to have 1 mount")
+	mnt := ctrInspect.Mounts[0]
+	assert.Equal(c, mnt.Destination, testVol)
+
+	_, err = os.Stat(mnt.Source)
 	assert.NilError(c, err)
 
 	removeOptions := container.RemoveOptions{
 		Force:         true,
 		RemoveVolumes: true,
 	}
 
-	apiClient, err := client.NewClientWithOpts(client.FromEnv)
-	assert.NilError(c, err)
-	defer apiClient.Close()
-
 	err = apiClient.ContainerRemove(testutil.GetContext(c), id, removeOptions)
 	assert.NilError(c, err)
 
-	_, err = os.Stat(source)
+	_, err = os.Stat(mnt.Source)
 	assert.Assert(c, os.IsNotExist(err), "expected to get ErrNotExist error, got %v", err)
 }
 

integration-cli/docker_cli_by_digest_test.go

@@ -159,7 +159,7 @@ func (s *DockerRegistrySuite) TestRemoveImageByDigest(t *testing.T) {
 	assert.NilError(t, err, "unexpected error deleting image")
 
 	// try to inspect again - it should error this time
-	_, err = inspectFieldWithError(imageReference, "Id")
+	_, err = inspectFilter(imageReference, ".Id")
 	// unexpected nil err trying to inspect what should be a non-existent image
 	assert.ErrorContains(t, err, "No such object")
 }
@@ -441,7 +441,7 @@ func (s *DockerRegistrySuite) TestDeleteImageByIDOnlyPulledByDigest(t *testing.T
 
 	cli.DockerCmd(t, "rmi", imageID)
 
-	_, err = inspectFieldWithError(imageID, "Id")
+	_, err = inspectFilter(imageID, ".Id")
 	assert.ErrorContains(t, err, "", "image should have been deleted")
 }
 
@@ -468,7 +468,7 @@ func (s *DockerRegistrySuite) TestDeleteImageWithDigestAndTag(t *testing.T) {
 	cli.DockerCmd(t, "rmi", repoTag)
 
 	// rmi should have deleted the tag, the digest reference, and the image itself
-	_, err = inspectFieldWithError(imageID, "Id")
+	_, err = inspectFilter(imageID, ".Id")
 	assert.ErrorContains(t, err, "", "image should have been deleted")
 }
 
@@ -493,16 +493,16 @@ func (s *DockerRegistrySuite) TestDeleteImageWithDigestAndMultiRepoTag(t *testin
 
 	// rmi should have deleted repoTag and image reference, but left repoTag2
 	inspectField(t, repoTag2, "Id")
-	_, err = inspectFieldWithError(imageReference, "Id")
+	_, err = inspectFilter(imageReference, ".Id")
 	assert.ErrorContains(t, err, "", "image digest reference should have been removed")
 
-	_, err = inspectFieldWithError(repoTag, "Id")
+	_, err = inspectFilter(repoTag, ".Id")
 	assert.ErrorContains(t, err, "", "image tag reference should have been removed")
 
 	cli.DockerCmd(t, "rmi", repoTag2)
 
 	// rmi should have deleted the tag, the digest reference, and the image itself
-	_, err = inspectFieldWithError(imageID, "Id")
+	_, err = inspectFilter(imageID, ".Id")
 	assert.ErrorContains(t, err, "", "image should have been deleted")
 }
 

integration-cli/docker_cli_create_test.go

@@ -152,10 +152,10 @@ func (s *DockerCLICreateSuite) TestCreateVolumesCreated(c *testing.T) {
 	const name = "test_create_volume"
 	cli.DockerCmd(c, "create", "--name", name, "-v", prefix+slash+"foo", "busybox")
 
-	dir, err := inspectMountSourceField(name, prefix+slash+"foo")
+	mnt, err := inspectMountPoint(name, prefix+slash+"foo")
 	assert.Assert(c, err == nil, "Error getting volume host path: %q", err)
 
-	if _, err := os.Stat(dir); err != nil && os.IsNotExist(err) {
+	if _, err := os.Stat(mnt.Source); err != nil && os.IsNotExist(err) {
 		c.Fatalf("Volume was not created")
 	}
 	if err != nil {

integration-cli/docker_cli_events_test.go

@@ -690,7 +690,11 @@ func (s *DockerCLIEventSuite) TestEventsSinceInTheFuture(c *testing.T) {
 
 	since := daemonTime(c)
 	until := since.Add(time.Duration(-24) * time.Hour)
-	out, _, err := dockerCmdWithError("events", "--filter", "image=busybox", "--since", parseEventTime(since), "--until", parseEventTime(until))
+	out, _, err := dockerCmdWithError("events",
+		"--filter", "image=busybox",
+		"--since", fmt.Sprintf("%d.%09d", since.Unix(), int64(since.Nanosecond())),
+		"--until", fmt.Sprintf("%d.%09d", until.Unix(), int64(until.Nanosecond())),
+	)
 
 	assert.ErrorContains(c, err, "")
 	assert.Assert(c, is.Contains(out, "cannot be after `until`"))

integration-cli/docker_cli_restart_test.go

@@ -76,7 +76,7 @@ func (s *DockerCLIRestartSuite) TestRestartWithVolumes(c *testing.T) {
 	out = strings.Trim(out, " \n\r")
 	assert.Equal(c, out, "1")
 
-	source, err := inspectMountSourceField(cID, prefix+slash+"test")
+	mnt, err := inspectMountPoint(cID, prefix+slash+"test")
 	assert.NilError(c, err)
 
 	cli.DockerCmd(c, "restart", cID)
@@ -86,9 +86,9 @@ func (s *DockerCLIRestartSuite) TestRestartWithVolumes(c *testing.T) {
 	out = strings.Trim(out, " \n\r")
 	assert.Equal(c, out, "1")
 
-	sourceAfterRestart, err := inspectMountSourceField(cID, prefix+slash+"test")
+	mountAfterRestart, err := inspectMountPoint(cID, prefix+slash+"test")
 	assert.NilError(c, err)
-	assert.Equal(c, source, sourceAfterRestart)
+	assert.Equal(c, mnt.Source, mountAfterRestart.Source)
 }
 
 func (s *DockerCLIRestartSuite) TestRestartDisconnectedContainer(c *testing.T) {

integration-cli/docker_cli_run_test.go

@@ -639,17 +639,17 @@ func (s *DockerCLIRunSuite) TestRunCreateVolumeWithSymlink(c *testing.T) {
 		c.Fatalf("[run] err: %v, exitcode: %d", err, exitCode)
 	}
 
-	volPath, err := inspectMountSourceField("test-createvolumewithsymlink", "/bar/foo")
+	mnt, err := inspectMountPoint("test-createvolumewithsymlink", "/bar/foo")
 	assert.NilError(c, err)
 
 	_, exitCode, err = dockerCmdWithError("rm", "-v", "test-createvolumewithsymlink")
 	if err != nil || exitCode != 0 {
 		c.Fatalf("[rm] err: %v, exitcode: %d", err, exitCode)
 	}
 
-	_, err = os.Stat(volPath)
+	_, err = os.Stat(mnt.Source)
 	if !os.IsNotExist(err) {
-		c.Fatalf("[open] (expecting 'file does not exist' error) err: %v, volPath: %s", err, volPath)
+		c.Fatalf("[open] (expecting 'file does not exist' error) err: %v, mnt.Source: %s", err, mnt.Source)
 	}
 }
 
@@ -2109,26 +2109,26 @@ func (s *DockerCLIRunSuite) TestRunVolumesCleanPaths(c *testing.T) {
 		VOLUME `+prefix+`/foo/`))
 	cli.DockerCmd(c, "run", "-v", prefix+"/foo", "-v", prefix+"/bar/", "--name", "dark_helmet", "run_volumes_clean_paths")
 
-	out, err := inspectMountSourceField("dark_helmet", prefix+slash+"foo"+slash)
+	mnt, err := inspectMountPoint("dark_helmet", prefix+slash+"foo"+slash)
 	if !errors.Is(err, errMountNotFound) {
-		c.Fatalf("Found unexpected volume entry for '%s/foo/' in volumes\n%q", prefix, out)
+		c.Fatalf("Found unexpected volume entry for '%s/foo/' in volumes\n%q", prefix, mnt.Source)
 	}
 
-	out, err = inspectMountSourceField("dark_helmet", prefix+slash+`foo`)
+	mnt, err = inspectMountPoint("dark_helmet", prefix+slash+`foo`)
 	assert.NilError(c, err)
-	if !strings.Contains(strings.ToLower(out), strings.ToLower(testEnv.PlatformDefaults.VolumesConfigPath)) {
-		c.Fatalf("Volume was not defined for %s/foo\n%q", prefix, out)
+	if !strings.Contains(strings.ToLower(mnt.Source), strings.ToLower(testEnv.PlatformDefaults.VolumesConfigPath)) {
+		c.Fatalf("Volume was not defined for %s/foo\n%q", prefix, mnt.Source)
 	}
 
-	out, err = inspectMountSourceField("dark_helmet", prefix+slash+"bar"+slash)
+	mnt, err = inspectMountPoint("dark_helmet", prefix+slash+"bar"+slash)
 	if !errors.Is(err, errMountNotFound) {
-		c.Fatalf("Found unexpected volume entry for '%s/bar/' in volumes\n%q", prefix, out)
+		c.Fatalf("Found unexpected volume entry for '%s/bar/' in volumes\n%q", prefix, mnt.Source)
 	}
 
-	out, err = inspectMountSourceField("dark_helmet", prefix+slash+"bar")
+	mnt, err = inspectMountPoint("dark_helmet", prefix+slash+"bar")
 	assert.NilError(c, err)
-	if !strings.Contains(strings.ToLower(out), strings.ToLower(testEnv.PlatformDefaults.VolumesConfigPath)) {
-		c.Fatalf("Volume was not defined for %s/bar\n%q", prefix, out)
+	if !strings.Contains(strings.ToLower(mnt.Source), strings.ToLower(testEnv.PlatformDefaults.VolumesConfigPath)) {
+		c.Fatalf("Volume was not defined for %s/bar\n%q", prefix, mnt.Source)
 	}
 }
 
@@ -2286,23 +2286,22 @@ func (s *DockerCLIRunSuite) TestRunMountShmMqueueFromHost(c *testing.T) {
 	testRequires(c, testEnv.IsLocalDaemon, DaemonIsLinux, NotUserNamespace)
 
 	cli.DockerCmd(c, "run", "-d", "--name", "shmfromhost", "-v", "/dev/shm:/dev/shm", "-v", "/dev/mqueue:/dev/mqueue", "busybox", "sh", "-c", "echo -n test > /dev/shm/test && touch /dev/mqueue/toto && top")
-	defer os.Remove("/dev/mqueue/toto")
-	defer os.Remove("/dev/shm/test")
-	volPath, err := inspectMountSourceField("shmfromhost", "/dev/shm")
+	c.Cleanup(func() {
+		err := os.Remove("/dev/shm/test")
+		assert.Check(c, err == nil || errors.Is(err, os.ErrNotExist))
+		err = os.Remove("/dev/mqueue/toto")
+		assert.Check(c, err == nil || errors.Is(err, os.ErrNotExist))
+	})
+	mnt, err := inspectMountPoint("shmfromhost", "/dev/shm")
 	assert.NilError(c, err)
-	if volPath != "/dev/shm" {
-		c.Fatalf("volumePath should have been /dev/shm, was %s", volPath)
-	}
+	assert.Equal(c, mnt.Source, "/dev/shm")
 
 	out := cli.DockerCmd(c, "run", "--name", "ipchost", "--ipc", "host", "busybox", "cat", "/dev/shm/test").Combined()
-	if out != "test" {
-		c.Fatalf("Output of /dev/shm/test expected test but found: %s", out)
-	}
+	assert.Equal(c, out, "test", "unexpected content for /dev/shm/test")
 
 	// Check that the mq was created
-	if _, err := os.Stat("/dev/mqueue/toto"); err != nil {
-		c.Fatalf("Failed to confirm '/dev/mqueue/toto' presence on host: %s", err.Error())
-	}
+	_, err = os.Stat("/dev/mqueue/toto")
+	assert.NilError(c, err, "failed to confirm '/dev/mqueue/toto' presence on host")
 }
 
 func (s *DockerCLIRunSuite) TestContainerNetworkMode(c *testing.T) {
@@ -2941,10 +2940,11 @@ func (s *DockerCLIRunSuite) TestRunNetworkFilesBindMount(c *testing.T) {
 	// Not applicable on Windows as uses Unix specific functionality
 	testRequires(c, testEnv.IsLocalDaemon, DaemonIsLinux)
 
-	expected := "test123"
+	tmpDir := c.TempDir()
+	filename := filepath.Join(tmpDir, "testfile")
 
-	filename := createTmpFile(c, expected)
-	defer os.Remove(filename)
+	const expected = "test123"
+	assert.NilError(c, os.WriteFile(filename, []byte(expected), 0o644))
 
 	// #nosec G302 -- for user namespaced test runs, the temp file must be accessible to unprivileged root
 	if err := os.Chmod(filename, 0o646); err != nil {
@@ -2965,8 +2965,9 @@ func (s *DockerCLIRunSuite) TestRunNetworkFilesBindMountRO(c *testing.T) {
 	// Not applicable on Windows as uses Unix specific functionality
 	testRequires(c, testEnv.IsLocalDaemon, DaemonIsLinux)
 
-	filename := createTmpFile(c, "test123")
-	defer os.Remove(filename)
+	tmpDir := c.TempDir()
+	filename := filepath.Join(tmpDir, "testfile")
+	assert.NilError(c, os.WriteFile(filename, []byte("test123"), 0o644))
 
 	// #nosec G302 -- for user namespaced test runs, the temp file must be accessible to unprivileged root
 	if err := os.Chmod(filename, 0o646); err != nil {
@@ -2987,8 +2988,9 @@ func (s *DockerCLIRunSuite) TestRunNetworkFilesBindMountROFilesystem(c *testing.
 	// Not applicable on Windows as uses Unix specific functionality
 	testRequires(c, testEnv.IsLocalDaemon, DaemonIsLinux, UserNamespaceROMount)
 
-	filename := createTmpFile(c, "test123")
-	defer os.Remove(filename)
+	tmpDir := c.TempDir()
+	filename := filepath.Join(tmpDir, "testfile")
+	assert.NilError(c, os.WriteFile(filename, []byte("test123"), 0o644))
 
 	// #nosec G302 -- for user namespaced test runs, the temp file must be accessible to unprivileged root
 	if err := os.Chmod(filename, 0o646); err != nil {
@@ -3077,7 +3079,7 @@ func (s *DockerCLIRunSuite) TestRunCreateContainerFailedCleanUp(c *testing.T) {
 	_, _, err := dockerCmdWithError("run", "--name", name, "--link", "nothing:nothing", "busybox")
 	assert.Assert(c, err != nil, "Expected docker run to fail!")
 
-	containerID, err := inspectFieldWithError(name, "Id")
+	containerID, err := inspectFilter(name, ".Id")
 	assert.Assert(c, err != nil, "Expected not to have this container: %s!", containerID)
 	assert.Equal(c, containerID, "", fmt.Sprintf("Expected not to have this container: %s!", containerID))
 }