Use t.Setenv in tests

Using os.Setenv in tests is problematic, because the change is
process-wise and other tests running in parallel might be affected.
Also, a somewhat complicated cleanup is needed.

Both issues are solved by using t.Setenv.

This commit also uses t.TempDir, t.Cleanup, and t.Helper when it makes
sense.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

common/libnetwork/cni/run_test.go

@@ -790,8 +790,8 @@ var _ = Describe("run CNI", func() {
 					},
 				}
 
-				os.Setenv("CNI_ARGS", "IP="+ip)
+				t := GinkgoT()
-				defer os.Unsetenv("CNI_ARGS")
+				t.Setenv("CNI_ARGS", "IP="+ip)
 
 				res, err := libpodNet.Setup(netNSContainer.Path(), setupOpts)
 				Expect(err).ToNot(HaveOccurred())

common/pkg/auth/auth_test.go

@@ -13,35 +13,28 @@ import (
 var _ = Describe("Config", func() {
 	Describe("ValidateAuth", func() {
 		It("validate GetDefaultAuthFile", func() {
+			t := GinkgoT()
 			// Given
-			oldDockerConf, envDockerSet := os.LookupEnv("DOCKER_CONFIG")
+			t.Setenv("DOCKER_CONFIG", "/tmp")
-			os.Setenv("DOCKER_CONFIG", "/tmp")
+			t.Setenv("REGISTRY_AUTH_FILE", "/tmp/registry.file")
-			oldConf, envSet := os.LookupEnv("REGISTRY_AUTH_FILE")
-			os.Setenv("REGISTRY_AUTH_FILE", "/tmp/registry.file")
 			// When			// When
 			authFile := GetDefaultAuthFile()
 			// Then
 			gomega.Expect(authFile).To(gomega.BeEquivalentTo("/tmp/registry.file"))
-			os.Unsetenv("REGISTRY_AUTH_FILE")
 
-			// Fall back to DOCKER_CONFIG
+			// Given
+			t.Setenv("REGISTRY_AUTH_FILE", "")
+			// When
 			authFile = GetDefaultAuthFile()
 			// Then
 			gomega.Expect(authFile).To(gomega.BeEquivalentTo("/tmp/config.json"))
-			os.Unsetenv("DOCKER_CONFIG")
 
-			// Fall back to DOCKER_CONFIG
+			// Given
+			t.Setenv("DOCKER_CONFIG", "")
+			// When
 			authFile = GetDefaultAuthFile()
 			// Then
 			gomega.Expect(authFile).To(gomega.BeEquivalentTo(""))
-
-			// Undo that
-			if envSet {
-				os.Setenv("REGISTRY_AUTH_FILE", oldConf)
-			}
-			if envDockerSet {
-				os.Setenv("DOCKER_CONFIG", oldDockerConf)
-			}
 		})
 	})
 

common/pkg/config/config_local_test.go

@@ -391,25 +391,19 @@ var _ = Describe("Config Local", func() {
 			"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
 		}
 		// Given we do
-		oldContainersConf, envSet := os.LookupEnv(containersConfEnv)
+		t := GinkgoT()
-		os.Setenv(containersConfEnv, "/dev/null")
+		t.Setenv(containersConfEnv, "/dev/null")
 
 		// When
 		config, err := Default()
 
-		// Undo that
-		if envSet {
-			os.Setenv(containersConfEnv, oldContainersConf)
-		} else {
-			os.Unsetenv(containersConfEnv)
-		}
 		// Then
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
 		gomega.Expect(config.GetDefaultEnv()).To(gomega.BeEquivalentTo(envs))
 		config.Containers.HTTPProxy = true
 		gomega.Expect(config.GetDefaultEnv()).To(gomega.BeEquivalentTo(envs))
-		os.Setenv("HTTP_PROXY", "localhost")
+		t.Setenv("HTTP_PROXY", "localhost")
-		os.Setenv("FOO", "BAR")
+		t.Setenv("FOO", "BAR")
 		newenvs := []string{"HTTP_PROXY=localhost"}
 		envs = append(newenvs, envs...)
 		gomega.Expect(config.GetDefaultEnv()).To(gomega.BeEquivalentTo(envs))

common/pkg/config/config_test.go

@@ -180,30 +180,24 @@ var _ = Describe("Config", func() {
 
 	Describe("readStorageTmp", func() {
 		It("test image_copy_tmp_dir='storage'", func() {
+			t := GinkgoT()
 			// Reload from new configuration file
-			testFile := "testdata/temp.conf"
+			testFile := t.TempDir() + "/temp.conf"
 			content := `[engine]
 image_copy_tmp_dir="storage"`
 			err := os.WriteFile(testFile, []byte(content), os.ModePerm)
 			// Then
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
-			defer os.Remove(testFile)
 
 			config, _ := NewConfig(testFile)
 			path, err := config.ImageCopyTmpDir()
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
 			gomega.Expect(path).To(gomega.ContainSubstring("containers/storage/tmp"))
 			// Given we do
-			oldTMPDIR, set := os.LookupEnv("TMPDIR")
+			t.Setenv("TMPDIR", "/var/tmp/foobar")
-			os.Setenv("TMPDIR", "/var/tmp/foobar")
 			path, err = config.ImageCopyTmpDir()
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
 			gomega.Expect(path).To(gomega.BeEquivalentTo("/var/tmp/foobar"))
-			if set {
-				os.Setenv("TMPDIR", oldTMPDIR)
-			} else {
-				os.Unsetenv("TMPDIR")
-			}
 		})
 	})
 
@@ -360,27 +354,15 @@ image_copy_tmp_dir="storage"`
 				"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
 			}
 			httpEnvs := append([]string{"HTTP_PROXY=1.2.3.4"}, envs...)
-			oldProxy, proxyEnvSet := os.LookupEnv("HTTP_PROXY")
+			t := GinkgoT()
-			os.Setenv("HTTP_PROXY", "1.2.3.4")
+			t.Setenv("HTTP_PROXY", "1.2.3.4")
-			oldFoo, fooEnvSet := os.LookupEnv("foo")
+			t.Setenv("foo", "bar")
-			os.Setenv("foo", "bar")
 
 			defaultConfig, _ := defaultConfig()
 			gomega.Expect(defaultConfig.GetDefaultEnvEx(false, false)).To(gomega.BeEquivalentTo(envs))
 			gomega.Expect(defaultConfig.GetDefaultEnvEx(false, true)).To(gomega.BeEquivalentTo(httpEnvs))
 			gomega.Expect(strings.Join(defaultConfig.GetDefaultEnvEx(true, true), ",")).To(gomega.ContainSubstring("HTTP_PROXY"))
 			gomega.Expect(strings.Join(defaultConfig.GetDefaultEnvEx(true, true), ",")).To(gomega.ContainSubstring("foo"))
-			// Undo that
-			if proxyEnvSet {
-				os.Setenv("HTTP_PROXY", oldProxy)
-			} else {
-				os.Unsetenv("HTTP_PROXY")
-			}
-			if fooEnvSet {
-				os.Setenv("foo", oldFoo)
-			} else {
-				os.Unsetenv("foo")
-			}
 		})
 
 		It("should succeed with commented out configuration", func() {
@@ -461,16 +443,9 @@ image_copy_tmp_dir="storage"`
 			}
 
 			// Given we do
-			oldContainersConf, envSet := os.LookupEnv(containersConfEnv)
+			GinkgoT().Setenv(containersConfEnv, "/dev/null")
-			os.Setenv(containersConfEnv, "/dev/null")
 			// When
 			config, err := NewConfig("")
-			// Undo that
-			if envSet {
-				os.Setenv(containersConfEnv, oldContainersConf)
-			} else {
-				os.Unsetenv(containersConfEnv)
-			}
 			// Then
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
 			gomega.Expect(config.Containers.ApparmorProfile).To(gomega.Equal(apparmor.Profile))
@@ -529,16 +504,9 @@ image_copy_tmp_dir="storage"`
 
 		It("contents of passed-in file should override others", func() {
 			// Given we do
-			oldContainersConf, envSet := os.LookupEnv(containersConfEnv)
+			GinkgoT().Setenv(containersConfEnv, "containers.conf")
-			os.Setenv(containersConfEnv, "containers.conf")
 			// When
 			config, err := NewConfig("testdata/containers_override.conf")
-			// Undo that
-			if envSet {
-				os.Setenv(containersConfEnv, oldContainersConf)
-			} else {
-				os.Unsetenv(containersConfEnv)
-			}
 
 			crunWasm := "crun-wasm"
 			PlatformToOCIRuntimeMap := map[string]string{
@@ -698,24 +666,12 @@ image_copy_tmp_dir="storage"`
 	})
 
 	Describe("Service Destinations", func() {
-		ConfPath := struct {
-			Value string
-			IsSet bool
-		}{}
-
 		BeforeEach(func() {
-			ConfPath.Value, ConfPath.IsSet = os.LookupEnv(containersConfEnv)
+			t := GinkgoT()
-			conf, _ := os.CreateTemp("", "containersconf")
+			name := t.TempDir() + "/containersconf"
-			os.Setenv(containersConfEnv, conf.Name())
+			err := os.WriteFile(name, []byte{}, os.ModePerm)
-		})
+			gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
-
+			t.Setenv(containersConfEnv, name)
-		AfterEach(func() {
-			os.Remove(os.Getenv(containersConfEnv))
-			if ConfPath.IsSet {
-				os.Setenv(containersConfEnv, ConfPath.Value)
-			} else {
-				os.Unsetenv(containersConfEnv)
-			}
 		})
 
 		It("test addConfigs", func() {
@@ -786,36 +742,24 @@ image_copy_tmp_dir="storage"`
 
 	Describe("Reload", func() {
 		It("test new config from reload", func() {
+			t := GinkgoT()
 			// Default configuration
 			defaultTestFile := "testdata/containers_default.conf"
-			oldEnv, set := os.LookupEnv(containersConfEnv)
+			t.Setenv(containersConfEnv, defaultTestFile)
-			os.Setenv(containersConfEnv, defaultTestFile)
 			cfg, err := Default()
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
-			if set {
-				os.Setenv(containersConfEnv, oldEnv)
-			} else {
-				os.Unsetenv(containersConfEnv)
-			}
 
 			// Reload from new configuration file
-			testFile := "testdata/temp.conf"
+			testFile := t.TempDir() + "/temp.conf"
 			content := `[containers]
 env=["foo=bar"]`
 			err = os.WriteFile(testFile, []byte(content), os.ModePerm)
-			defer os.Remove(testFile)
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
-			oldEnv, set = os.LookupEnv(containersConfEnv)
+			t.Setenv(containersConfEnv, testFile)
-			os.Setenv(containersConfEnv, testFile)
 			_, err = Reload()
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
 			newCfg, err := Default()
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
-			if set {
-				os.Setenv(containersConfEnv, oldEnv)
-			} else {
-				os.Unsetenv(containersConfEnv)
-			}
 
 			expectOldEnv := []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"}
 			expectNewEnv := []string{"foo=bar"}
@@ -837,15 +781,14 @@ env=["foo=bar"]`
 	})
 
 	It("CONTAINERS_CONF_OVERRIDE", func() {
-		os.Setenv("CONTAINERS_CONF_OVERRIDE", "testdata/containers_override.conf")
+		t := GinkgoT()
-		defer os.Unsetenv("CONTAINERS_CONF_OVERRIDE")
+		t.Setenv("CONTAINERS_CONF_OVERRIDE", "testdata/containers_override.conf")
 		config, err := NewConfig("")
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
 		gomega.Expect(config.Containers.ApparmorProfile).To(gomega.Equal("overridden-default"))
 
 		// Make sure that _OVERRIDE is loaded even when CONTAINERS_CONF is set.
-		os.Setenv(containersConfEnv, "testdata/containers_default.conf")
+		t.Setenv(containersConfEnv, "testdata/containers_default.conf")
-		defer os.Unsetenv(containersConfEnv)
 		config, err = NewConfig("")
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
 		gomega.Expect(config.Containers.ApparmorProfile).To(gomega.Equal("overridden-default"))

common/pkg/config/connections_test.go

@@ -17,20 +17,12 @@ var _ = Describe("Connections conf", func() {
 	)
 
 	BeforeEach(func() {
-		dir := GinkgoT().TempDir()
+		t := GinkgoT()
+		dir := t.TempDir()
 		connectionsConfFile = filepath.Join(dir, "connections.json")
-		err := os.Setenv("PODMAN_CONNECTIONS_CONF", connectionsConfFile)
+		t.Setenv("PODMAN_CONNECTIONS_CONF", connectionsConfFile)
-		gomega.Expect(err).ToNot(gomega.HaveOccurred())
 		containersConfFile = filepath.Join(dir, "containers.conf")
-		err = os.Setenv(containersConfEnv, containersConfFile)
+		t.Setenv(containersConfEnv, containersConfFile)
-		gomega.Expect(err).ToNot(gomega.HaveOccurred())
-	})
-
-	AfterEach(func() {
-		err := os.Unsetenv("PODMAN_CONNECTIONS_CONF")
-		gomega.Expect(err).ToNot(gomega.HaveOccurred())
-		err = os.Unsetenv(containersConfEnv)
-		gomega.Expect(err).ToNot(gomega.HaveOccurred())
 	})
 
 	It("read non existent file", func() {

common/pkg/config/modules_test.go

@@ -15,28 +15,22 @@ const (
 	testBaseUsr  = "testdata/modules/usr/share"
 )
 
-func testSetModulePaths() (func(), error) {
+func testSetModulePaths() {
-	oldXDG := os.Getenv("XDG_CONFIG_HOME")
+	t := GinkgoT()
-	oldEtc := moduleBaseEtc
-	oldUsr := moduleBaseUsr
 
 	wd, err := os.Getwd()
-	if err != nil {
+	gomega.Expect(err).ToNot(gomega.HaveOccurred())
-		return nil, err
-	}
 
-	if err := os.Setenv("XDG_CONFIG_HOME", filepath.Join(wd, testBaseHome)); err != nil {
+	t.Setenv("XDG_CONFIG_HOME", filepath.Join(wd, testBaseHome))
-		return nil, err
-	}
 
+	oldEtc := moduleBaseEtc
+	oldUsr := moduleBaseUsr
 	moduleBaseEtc = filepath.Join(wd, testBaseEtc)
 	moduleBaseUsr = filepath.Join(wd, testBaseUsr)
-
+	DeferCleanup(func() {
-	return func() {
-		os.Setenv("XDG_CONFIG_HOME", oldXDG)
 		moduleBaseEtc = oldEtc
 		moduleBaseUsr = oldUsr
-	}, nil
+	})
 }
 
 var _ = Describe("Config Modules", func() {
@@ -55,9 +49,7 @@ var _ = Describe("Config Modules", func() {
 	It("resolve modules", func() {
 		// This test makes sure that the correct module is being
 		// returned.
-		cleanUp, err := testSetModulePaths()
+		testSetModulePaths()
-		gomega.Expect(err).ToNot(gomega.HaveOccurred())
-		defer cleanUp()
 
 		dirs, err := ModuleDirectories()
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
@@ -106,9 +98,7 @@ var _ = Describe("Config Modules", func() {
 	})
 
 	It("new config with modules", func() {
-		cleanUp, err := testSetModulePaths()
+		testSetModulePaths()
-		gomega.Expect(err).ToNot(gomega.HaveOccurred())
-		defer cleanUp()
 
 		wd, err := os.Getwd()
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
@@ -162,17 +152,10 @@ var _ = Describe("Config Modules", func() {
 	})
 
 	It("new config with modules and env variables", func() {
-		cleanUp, err := testSetModulePaths()
+		testSetModulePaths()
-		gomega.Expect(err).ToNot(gomega.HaveOccurred())
-		defer cleanUp()
 
-		oldOverride := os.Getenv(containersConfOverrideEnv)
+		t := GinkgoT()
-		defer func() {
+		t.Setenv(containersConfOverrideEnv, "testdata/modules/override.conf")
-			os.Setenv(containersConfOverrideEnv, oldOverride)
-		}()
-
-		err = os.Setenv(containersConfOverrideEnv, "testdata/modules/override.conf")
-		gomega.Expect(err).ToNot(gomega.HaveOccurred())
 
 		// Also make sure that absolute paths are loaded as is.
 		wd, err := os.Getwd()