Merge pull request moby#45474 from thaJeztah/testing_cleanups

assorted test fixes and cleanups
runpod · Jan 23, 2024 · 22a5049 · 22a5049
2 parents d86d24d + 20bd690
commit 22a5049
Show file tree

Hide file tree

Showing 9 changed files with 93 additions and 99 deletions.
diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go
@@ -1971,22 +1971,22 @@ func (s *DockerAPISuite) TestContainersAPICreateMountsCreate(c *testing.T) {
 			assert.NilError(c, err)
 			defer os.RemoveAll(tmpDir3)
 
-			assert.Assert(c, mountWrapper(tmpDir3, tmpDir3, "none", "bind,shared") == nil)
-
-			cases = append(cases, []testCase{
-				{
-					spec:     mount.Mount{Type: "bind", Source: tmpDir3, Target: destPath},
-					expected: types.MountPoint{Type: "bind", RW: true, Destination: destPath, Source: tmpDir3},
-				},
-				{
-					spec:     mount.Mount{Type: "bind", Source: tmpDir3, Target: destPath, ReadOnly: true},
-					expected: types.MountPoint{Type: "bind", RW: false, Destination: destPath, Source: tmpDir3},
-				},
-				{
-					spec:     mount.Mount{Type: "bind", Source: tmpDir3, Target: destPath, ReadOnly: true, BindOptions: &mount.BindOptions{Propagation: "shared"}},
-					expected: types.MountPoint{Type: "bind", RW: false, Destination: destPath, Source: tmpDir3, Propagation: "shared"},
-				},
-			}...)
+			if assert.Check(c, mountWrapper(c, tmpDir3, tmpDir3, "none", "bind,shared")) {
+				cases = append(cases, []testCase{
+					{
+						spec:     mount.Mount{Type: "bind", Source: tmpDir3, Target: destPath},
+						expected: types.MountPoint{Type: "bind", RW: true, Destination: destPath, Source: tmpDir3},
+					},
+					{
+						spec:     mount.Mount{Type: "bind", Source: tmpDir3, Target: destPath, ReadOnly: true},
+						expected: types.MountPoint{Type: "bind", RW: false, Destination: destPath, Source: tmpDir3},
+					},
+					{
+						spec:     mount.Mount{Type: "bind", Source: tmpDir3, Target: destPath, ReadOnly: true, BindOptions: &mount.BindOptions{Propagation: "shared"}},
+						expected: types.MountPoint{Type: "bind", RW: false, Destination: destPath, Source: tmpDir3, Propagation: "shared"},
+					},
+				}...)
+			}
 		}
 	}
 

diff --git a/integration-cli/docker_api_containers_unix_test.go b/integration-cli/docker_api_containers_unix_test.go
@@ -2,8 +2,18 @@
 
 package main
 
-import "github.com/moby/sys/mount"
+import (
+	"testing"
 
-func mountWrapper(device, target, mType, options string) error {
-	return mount.Mount(device, target, mType, options)
+	"github.com/moby/sys/mount"
+)
+
+func mountWrapper(t *testing.T, device, target, mType, options string) error {
+	t.Helper()
+	err := mount.Mount(device, target, mType, options)
+	if err != nil {
+		return err
+	}
+	t.Cleanup(func() { _ = mount.Unmount(target) })
+	return nil
 }
diff --git a/integration-cli/docker_api_containers_windows_test.go b/integration-cli/docker_api_containers_windows_test.go
@@ -73,7 +73,7 @@ func (s *DockerAPISuite) TestContainersAPICreateMountsBindNamedPipe(c *testing.T
 	assert.Check(c, is.Equal(text, strings.TrimSpace(string(b))))
 }
 
-func mountWrapper(device, target, mType, options string) error {
+func mountWrapper(t *testing.T, device, target, mType, options string) error {
 	// This should never be called.
 	return errors.Errorf("there is no implementation of Mount on this platform")
 }
diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go
@@ -968,8 +968,10 @@ func (s *DockerCLIBuildSuite) TestBuildAddBadLinks(c *testing.T) {
 	if err != nil {
 		c.Fatal(err)
 	}
+	defer tarOut.Close()
 
 	tarWriter := tar.NewWriter(tarOut)
+	defer tarWriter.Close()
 
 	header := &tar.Header{
 		Name:     "symlink",
@@ -985,18 +987,10 @@ func (s *DockerCLIBuildSuite) TestBuildAddBadLinks(c *testing.T) {
 		c.Fatal(err)
 	}
 
-	tarWriter.Close()
-	tarOut.Close()
-
-	foo, err := os.Create(fooPath)
+	err = os.WriteFile(fooPath, []byte("test"), 0666)
 	if err != nil {
 		c.Fatal(err)
 	}
-	defer foo.Close()
-
-	if _, err := foo.WriteString("test"); err != nil {
-		c.Fatal(err)
-	}
 
 	buildImageSuccessfully(c, name, build.WithExternalBuildContext(ctx))
 	if _, err := os.Stat(nonExistingFile); err == nil || !os.IsNotExist(err) {
@@ -1028,15 +1022,10 @@ func (s *DockerCLIBuildSuite) TestBuildAddBadLinksVolume(c *testing.T) {
 	defer ctx.Close()
 	fooPath := filepath.Join(ctx.Dir, targetFile)
 
-	foo, err := os.Create(fooPath)
+	err = os.WriteFile(fooPath, []byte("test"), 0666)
 	if err != nil {
 		c.Fatal(err)
 	}
-	defer foo.Close()
-
-	if _, err := foo.WriteString("test"); err != nil {
-		c.Fatal(err)
-	}
 
 	buildImageSuccessfully(c, "test-link-absolute-volume", build.WithExternalBuildContext(ctx))
 	if _, err := os.Stat(nonExistingFile); err == nil || !os.IsNotExist(err) {

diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go
@@ -2115,14 +2115,10 @@ func (s *DockerDaemonSuite) TestDaemonMaxConcurrencyWithConfigFile(c *testing.T)
 	testRequires(c, testEnv.IsLocalDaemon, DaemonIsLinux)
 
 	// daemon config file
-	configFilePath := "test.json"
-	configFile, err := os.Create(configFilePath)
+	const configFilePath = "test-daemon.json"
+	err := os.WriteFile(configFilePath, []byte(`{ "max-concurrent-downloads" : 8 }`), 0666)
 	assert.NilError(c, err)
 	defer os.Remove(configFilePath)
-
-	daemonConfig := `{ "max-concurrent-downloads" : 8 }`
-	fmt.Fprintf(configFile, "%s", daemonConfig)
-	configFile.Close()
 	s.d.Start(c, fmt.Sprintf("--config-file=%s", configFilePath))
 
 	expectedMaxConcurrentUploads := `level=debug msg="Max Concurrent Uploads: 5"`
@@ -2131,12 +2127,8 @@ func (s *DockerDaemonSuite) TestDaemonMaxConcurrencyWithConfigFile(c *testing.T)
 	assert.NilError(c, err)
 	assert.Assert(c, strings.Contains(string(content), expectedMaxConcurrentUploads))
 	assert.Assert(c, strings.Contains(string(content), expectedMaxConcurrentDownloads))
-	configFile, err = os.Create(configFilePath)
+	err = os.WriteFile(configFilePath, []byte(`{ "max-concurrent-uploads" : 7, "max-concurrent-downloads" : 9 }`), 0666)
 	assert.NilError(c, err)
-	daemonConfig = `{ "max-concurrent-uploads" : 7, "max-concurrent-downloads" : 9 }`
-	fmt.Fprintf(configFile, "%s", daemonConfig)
-	configFile.Close()
-
 	assert.Assert(c, s.d.Signal(unix.SIGHUP) == nil)
 	// unix.Kill(s.d.cmd.Process.Pid, unix.SIGHUP)
 
@@ -2157,14 +2149,11 @@ func (s *DockerDaemonSuite) TestDaemonMaxConcurrencyWithConfigFileReload(c *test
 	testRequires(c, testEnv.IsLocalDaemon, DaemonIsLinux)
 
 	// daemon config file
-	configFilePath := "test.json"
-	configFile, err := os.Create(configFilePath)
+	const configFilePath = "test-daemon.json"
+	err := os.WriteFile(configFilePath, []byte(`{ "max-concurrent-uploads" : null }`), 0666)
 	assert.NilError(c, err)
 	defer os.Remove(configFilePath)
 
-	daemonConfig := `{ "max-concurrent-uploads" : null }`
-	fmt.Fprintf(configFile, "%s", daemonConfig)
-	configFile.Close()
 	s.d.Start(c, fmt.Sprintf("--config-file=%s", configFilePath))
 
 	expectedMaxConcurrentUploads := `level=debug msg="Max Concurrent Uploads: 5"`
@@ -2173,11 +2162,8 @@ func (s *DockerDaemonSuite) TestDaemonMaxConcurrencyWithConfigFileReload(c *test
 	assert.NilError(c, err)
 	assert.Assert(c, strings.Contains(string(content), expectedMaxConcurrentUploads))
 	assert.Assert(c, strings.Contains(string(content), expectedMaxConcurrentDownloads))
-	configFile, err = os.Create(configFilePath)
+	err = os.WriteFile(configFilePath, []byte(`{ "max-concurrent-uploads" : 1, "max-concurrent-downloads" : null }`), 0666)
 	assert.NilError(c, err)
-	daemonConfig = `{ "max-concurrent-uploads" : 1, "max-concurrent-downloads" : null }`
-	fmt.Fprintf(configFile, "%s", daemonConfig)
-	configFile.Close()
 
 	assert.Assert(c, s.d.Signal(unix.SIGHUP) == nil)
 	// unix.Kill(s.d.cmd.Process.Pid, unix.SIGHUP)
@@ -2190,11 +2176,8 @@ func (s *DockerDaemonSuite) TestDaemonMaxConcurrencyWithConfigFileReload(c *test
 	assert.NilError(c, err)
 	assert.Assert(c, strings.Contains(string(content), expectedMaxConcurrentUploads))
 	assert.Assert(c, strings.Contains(string(content), expectedMaxConcurrentDownloads))
-	configFile, err = os.Create(configFilePath)
+	err = os.WriteFile(configFilePath, []byte(`{ "labels":["foo=bar"] }`), 0666)
 	assert.NilError(c, err)
-	daemonConfig = `{ "labels":["foo=bar"] }`
-	fmt.Fprintf(configFile, "%s", daemonConfig)
-	configFile.Close()
 
 	assert.Assert(c, s.d.Signal(unix.SIGHUP) == nil)
 
@@ -2533,21 +2516,15 @@ func (s *DockerDaemonSuite) TestDaemonShutdownTimeoutWithConfigFile(c *testing.T
 	testRequires(c, testEnv.IsLocalDaemon)
 
 	// daemon config file
-	configFilePath := "test.json"
-	configFile, err := os.Create(configFilePath)
+	const configFilePath = "test-daemon.json"
+	err := os.WriteFile(configFilePath, []byte(`{ "shutdown-timeout" : 8 }`), 0666)
 	assert.NilError(c, err)
 	defer os.Remove(configFilePath)
 
-	daemonConfig := `{ "shutdown-timeout" : 8 }`
-	fmt.Fprintf(configFile, "%s", daemonConfig)
-	configFile.Close()
 	s.d.Start(c, fmt.Sprintf("--config-file=%s", configFilePath))
 
-	configFile, err = os.Create(configFilePath)
+	err = os.WriteFile(configFilePath, []byte(`{ "shutdown-timeout" : 5 }`), 0666)
 	assert.NilError(c, err)
-	daemonConfig = `{ "shutdown-timeout" : 5 }`
-	fmt.Fprintf(configFile, "%s", daemonConfig)
-	configFile.Close()
 
 	assert.Assert(c, s.d.Signal(unix.SIGHUP) == nil)
 

diff --git a/integration-cli/docker_cli_run_unix_test.go b/integration-cli/docker_cli_run_unix_test.go
@@ -70,6 +70,7 @@ func (s *DockerCLIRunSuite) TestRunWithVolumesIsRecursive(c *testing.T) {
 	tmpfsDir := filepath.Join(tmpDir, "tmpfs")
 	assert.Assert(c, os.MkdirAll(tmpfsDir, 0o777) == nil, "failed to mkdir at %s", tmpfsDir)
 	assert.Assert(c, mount.Mount("tmpfs", tmpfsDir, "tmpfs", "") == nil, "failed to create a tmpfs mount at %s", tmpfsDir)
+	defer mount.Unmount(tmpfsDir)
 
 	f, err := os.CreateTemp(tmpfsDir, "touch-me")
 	assert.NilError(c, err)

diff --git a/libnetwork/osl/sandbox_linux_test.go b/libnetwork/osl/sandbox_linux_test.go
@@ -44,9 +44,11 @@ func newKey(t *testing.T) (string, error) {
 	}
 
 	name = filepath.Join("/tmp", name)
-	if _, err := os.Create(name); err != nil {
+	f, err := os.Create(name)
+	if err != nil {
 		return "", err
 	}
+	_ = f.Close()
 
 	// Set the rpmCleanupPeriod to be low to make the test run quicker
 	gpmLock.Lock()

diff --git a/pkg/archive/archive_test.go b/pkg/archive/archive_test.go
@@ -291,8 +291,15 @@ func TestUntarPathWithInvalidDest(t *testing.T) {
 	// Create a src file
 	srcFile := filepath.Join(tempFolder, "src")
 	tarFile := filepath.Join(tempFolder, "src.tar")
-	os.Create(srcFile)
-	os.Create(invalidDestFolder) // being a file (not dir) should cause an error
+	f, err := os.Create(srcFile)
+	if assert.Check(t, err) {
+		_ = f.Close()
+	}
+
+	d, err := os.Create(invalidDestFolder) // being a file (not dir) should cause an error
+	if assert.Check(t, err) {
+		_ = d.Close()
+	}
 
 	// Translate back to Unix semantics as next exec.Command is run under sh
 	srcFileU := srcFile
@@ -331,7 +338,10 @@ func TestUntarPath(t *testing.T) {
 	defer os.RemoveAll(tmpFolder)
 	srcFile := filepath.Join(tmpFolder, "src")
 	tarFile := filepath.Join(tmpFolder, "src.tar")
-	os.Create(filepath.Join(tmpFolder, "src"))
+	f, err := os.Create(filepath.Join(tmpFolder, "src"))
+	if assert.Check(t, err) {
+		_ = f.Close()
+	}
 
 	destFolder := filepath.Join(tmpFolder, "dest")
 	err = os.MkdirAll(destFolder, 0o740)
@@ -370,7 +380,10 @@ func TestUntarPathWithDestinationFile(t *testing.T) {
 	defer os.RemoveAll(tmpFolder)
 	srcFile := filepath.Join(tmpFolder, "src")
 	tarFile := filepath.Join(tmpFolder, "src.tar")
-	os.Create(filepath.Join(tmpFolder, "src"))
+	f, err := os.Create(filepath.Join(tmpFolder, "src"))
+	if assert.Check(t, err) {
+		_ = f.Close()
+	}
 
 	// Translate back to Unix semantics as next exec.Command is run under sh
 	srcFileU := srcFile
@@ -385,9 +398,9 @@ func TestUntarPathWithDestinationFile(t *testing.T) {
 		t.Fatal(err)
 	}
 	destFile := filepath.Join(tmpFolder, "dest")
-	_, err = os.Create(destFile)
-	if err != nil {
-		t.Fatalf("Fail to create the destination file")
+	f, err = os.Create(destFile)
+	if assert.Check(t, err) {
+		_ = f.Close()
 	}
 	err = defaultUntarPath(tarFile, destFile)
 	if err == nil {
@@ -406,7 +419,10 @@ func TestUntarPathWithDestinationSrcFileAsFolder(t *testing.T) {
 	defer os.RemoveAll(tmpFolder)
 	srcFile := filepath.Join(tmpFolder, "src")
 	tarFile := filepath.Join(tmpFolder, "src.tar")
-	os.Create(srcFile)
+	f, err := os.Create(srcFile)
+	if assert.Check(t, err) {
+		_ = f.Close()
+	}
 
 	// Translate back to Unix semantics as next exec.Command is run under sh
 	srcFileU := srcFile
@@ -562,9 +578,9 @@ func TestCopyFileWithTarInexistentDestWillCreateIt(t *testing.T) {
 	defer os.RemoveAll(tempFolder)
 	srcFile := filepath.Join(tempFolder, "src")
 	inexistentDestFolder := filepath.Join(tempFolder, "doesnotexists")
-	_, err = os.Create(srcFile)
-	if err != nil {
-		t.Fatal(err)
+	f, err := os.Create(srcFile)
+	if assert.Check(t, err) {
+		_ = f.Close()
 	}
 	err = defaultCopyFileWithTar(srcFile, inexistentDestFolder)
 	if err != nil {
@@ -800,21 +816,24 @@ func TestTarWithOptionsChownOptsAlwaysOverridesIdPair(t *testing.T) {
 		{&TarOptions{ChownOpts: &idtools.Identity{UID: 1, GID: 1}, NoLchown: true}, 1, 1},
 		{&TarOptions{ChownOpts: &idtools.Identity{UID: 1000, GID: 1000}, NoLchown: true}, 1000, 1000},
 	}
-	for _, testCase := range cases {
-		reader, err := TarWithOptions(filePath, testCase.opts)
-		assert.NilError(t, err)
-		tr := tar.NewReader(reader)
-		defer reader.Close()
-		for {
-			hdr, err := tr.Next()
-			if err == io.EOF {
-				// end of tar archive
-				break
-			}
+	for _, tc := range cases {
+		tc := tc
+		t.Run("", func(t *testing.T) {
+			reader, err := TarWithOptions(filePath, tc.opts)
 			assert.NilError(t, err)
-			assert.Check(t, is.Equal(hdr.Uid, testCase.expectedUID), "Uid equals expected value")
-			assert.Check(t, is.Equal(hdr.Gid, testCase.expectedGID), "Gid equals expected value")
-		}
+			tr := tar.NewReader(reader)
+			defer reader.Close()
+			for {
+				hdr, err := tr.Next()
+				if err == io.EOF {
+					// end of tar archive
+					break
+				}
+				assert.NilError(t, err)
+				assert.Check(t, is.Equal(hdr.Uid, tc.expectedUID), "Uid equals expected value")
+				assert.Check(t, is.Equal(hdr.Gid, tc.expectedGID), "Gid equals expected value")
+			}
+		})
 	}
 }
 

diff --git a/pkg/archive/changes_posix_test.go b/pkg/archive/changes_posix_test.go
@@ -24,14 +24,10 @@ func TestHardLinkOrder(t *testing.T) {
 	defer os.RemoveAll(src)
 	for _, name := range names {
 		func() {
-			fh, err := os.Create(path.Join(src, name))
+			err := os.WriteFile(path.Join(src, name), msg, 0666)
 			if err != nil {
 				t.Fatal(err)
 			}
-			defer fh.Close()
-			if _, err = fh.Write(msg); err != nil {
-				t.Fatal(err)
-			}
 		}()
 	}
 	// Create dest, with changes that includes hardlinks