Skip to content

Commit ee04b4f

Browse files
committed
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 <[email protected]>
1 parent 62f5c9f commit ee04b4f

File tree

6 files changed

+52
-147
lines changed

6 files changed

+52
-147
lines changed

libnetwork/cni/run_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -790,8 +790,8 @@ var _ = Describe("run CNI", func() {
790790
},
791791
}
792792

793-
os.Setenv("CNI_ARGS", "IP="+ip)
794-
defer os.Unsetenv("CNI_ARGS")
793+
t := GinkgoT()
794+
t.Setenv("CNI_ARGS", "IP="+ip)
795795

796796
res, err := libpodNet.Setup(netNSContainer.Path(), setupOpts)
797797
Expect(err).ToNot(HaveOccurred())

pkg/auth/auth_test.go

+9-16
Original file line numberDiff line numberDiff line change
@@ -13,35 +13,28 @@ import (
1313
var _ = Describe("Config", func() {
1414
Describe("ValidateAuth", func() {
1515
It("validate GetDefaultAuthFile", func() {
16+
t := GinkgoT()
1617
// Given
17-
oldDockerConf, envDockerSet := os.LookupEnv("DOCKER_CONFIG")
18-
os.Setenv("DOCKER_CONFIG", "/tmp")
19-
oldConf, envSet := os.LookupEnv("REGISTRY_AUTH_FILE")
20-
os.Setenv("REGISTRY_AUTH_FILE", "/tmp/registry.file")
18+
t.Setenv("DOCKER_CONFIG", "/tmp")
19+
t.Setenv("REGISTRY_AUTH_FILE", "/tmp/registry.file")
2120
// When // When
2221
authFile := GetDefaultAuthFile()
2322
// Then
2423
gomega.Expect(authFile).To(gomega.BeEquivalentTo("/tmp/registry.file"))
25-
os.Unsetenv("REGISTRY_AUTH_FILE")
2624

27-
// Fall back to DOCKER_CONFIG
25+
// Given
26+
t.Setenv("REGISTRY_AUTH_FILE", "")
27+
// When
2828
authFile = GetDefaultAuthFile()
2929
// Then
3030
gomega.Expect(authFile).To(gomega.BeEquivalentTo("/tmp/config.json"))
31-
os.Unsetenv("DOCKER_CONFIG")
3231

33-
// Fall back to DOCKER_CONFIG
32+
// Given
33+
t.Setenv("DOCKER_CONFIG", "")
34+
// When
3435
authFile = GetDefaultAuthFile()
3536
// Then
3637
gomega.Expect(authFile).To(gomega.BeEquivalentTo(""))
37-
38-
// Undo that
39-
if envSet {
40-
os.Setenv("REGISTRY_AUTH_FILE", oldConf)
41-
}
42-
if envDockerSet {
43-
os.Setenv("DOCKER_CONFIG", oldDockerConf)
44-
}
4538
})
4639
})
4740

pkg/config/config_local_test.go

+4-10
Original file line numberDiff line numberDiff line change
@@ -391,25 +391,19 @@ var _ = Describe("Config Local", func() {
391391
"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
392392
}
393393
// Given we do
394-
oldContainersConf, envSet := os.LookupEnv(containersConfEnv)
395-
os.Setenv(containersConfEnv, "/dev/null")
394+
t := GinkgoT()
395+
t.Setenv(containersConfEnv, "/dev/null")
396396

397397
// When
398398
config, err := Default()
399399

400-
// Undo that
401-
if envSet {
402-
os.Setenv(containersConfEnv, oldContainersConf)
403-
} else {
404-
os.Unsetenv(containersConfEnv)
405-
}
406400
// Then
407401
gomega.Expect(err).ToNot(gomega.HaveOccurred())
408402
gomega.Expect(config.GetDefaultEnv()).To(gomega.BeEquivalentTo(envs))
409403
config.Containers.HTTPProxy = true
410404
gomega.Expect(config.GetDefaultEnv()).To(gomega.BeEquivalentTo(envs))
411-
os.Setenv("HTTP_PROXY", "localhost")
412-
os.Setenv("FOO", "BAR")
405+
t.Setenv("HTTP_PROXY", "localhost")
406+
t.Setenv("FOO", "BAR")
413407
newenvs := []string{"HTTP_PROXY=localhost"}
414408
envs = append(newenvs, envs...)
415409
gomega.Expect(config.GetDefaultEnv()).To(gomega.BeEquivalentTo(envs))

pkg/config/config_test.go

+20-77
Original file line numberDiff line numberDiff line change
@@ -180,30 +180,24 @@ var _ = Describe("Config", func() {
180180

181181
Describe("readStorageTmp", func() {
182182
It("test image_copy_tmp_dir='storage'", func() {
183+
t := GinkgoT()
183184
// Reload from new configuration file
184-
testFile := "testdata/temp.conf"
185+
testFile := t.TempDir() + "/temp.conf"
185186
content := `[engine]
186187
image_copy_tmp_dir="storage"`
187188
err := os.WriteFile(testFile, []byte(content), os.ModePerm)
188189
// Then
189190
gomega.Expect(err).ToNot(gomega.HaveOccurred())
190-
defer os.Remove(testFile)
191191

192192
config, _ := NewConfig(testFile)
193193
path, err := config.ImageCopyTmpDir()
194194
gomega.Expect(err).ToNot(gomega.HaveOccurred())
195195
gomega.Expect(path).To(gomega.ContainSubstring("containers/storage/tmp"))
196196
// Given we do
197-
oldTMPDIR, set := os.LookupEnv("TMPDIR")
198-
os.Setenv("TMPDIR", "/var/tmp/foobar")
197+
t.Setenv("TMPDIR", "/var/tmp/foobar")
199198
path, err = config.ImageCopyTmpDir()
200199
gomega.Expect(err).ToNot(gomega.HaveOccurred())
201200
gomega.Expect(path).To(gomega.BeEquivalentTo("/var/tmp/foobar"))
202-
if set {
203-
os.Setenv("TMPDIR", oldTMPDIR)
204-
} else {
205-
os.Unsetenv("TMPDIR")
206-
}
207201
})
208202
})
209203

@@ -360,27 +354,15 @@ image_copy_tmp_dir="storage"`
360354
"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
361355
}
362356
httpEnvs := append([]string{"HTTP_PROXY=1.2.3.4"}, envs...)
363-
oldProxy, proxyEnvSet := os.LookupEnv("HTTP_PROXY")
364-
os.Setenv("HTTP_PROXY", "1.2.3.4")
365-
oldFoo, fooEnvSet := os.LookupEnv("foo")
366-
os.Setenv("foo", "bar")
357+
t := GinkgoT()
358+
t.Setenv("HTTP_PROXY", "1.2.3.4")
359+
t.Setenv("foo", "bar")
367360

368361
defaultConfig, _ := defaultConfig()
369362
gomega.Expect(defaultConfig.GetDefaultEnvEx(false, false)).To(gomega.BeEquivalentTo(envs))
370363
gomega.Expect(defaultConfig.GetDefaultEnvEx(false, true)).To(gomega.BeEquivalentTo(httpEnvs))
371364
gomega.Expect(strings.Join(defaultConfig.GetDefaultEnvEx(true, true), ",")).To(gomega.ContainSubstring("HTTP_PROXY"))
372365
gomega.Expect(strings.Join(defaultConfig.GetDefaultEnvEx(true, true), ",")).To(gomega.ContainSubstring("foo"))
373-
// Undo that
374-
if proxyEnvSet {
375-
os.Setenv("HTTP_PROXY", oldProxy)
376-
} else {
377-
os.Unsetenv("HTTP_PROXY")
378-
}
379-
if fooEnvSet {
380-
os.Setenv("foo", oldFoo)
381-
} else {
382-
os.Unsetenv("foo")
383-
}
384366
})
385367

386368
It("should succeed with commented out configuration", func() {
@@ -461,16 +443,9 @@ image_copy_tmp_dir="storage"`
461443
}
462444

463445
// Given we do
464-
oldContainersConf, envSet := os.LookupEnv(containersConfEnv)
465-
os.Setenv(containersConfEnv, "/dev/null")
446+
GinkgoT().Setenv(containersConfEnv, "/dev/null")
466447
// When
467448
config, err := NewConfig("")
468-
// Undo that
469-
if envSet {
470-
os.Setenv(containersConfEnv, oldContainersConf)
471-
} else {
472-
os.Unsetenv(containersConfEnv)
473-
}
474449
// Then
475450
gomega.Expect(err).ToNot(gomega.HaveOccurred())
476451
gomega.Expect(config.Containers.ApparmorProfile).To(gomega.Equal(apparmor.Profile))
@@ -529,16 +504,9 @@ image_copy_tmp_dir="storage"`
529504

530505
It("contents of passed-in file should override others", func() {
531506
// Given we do
532-
oldContainersConf, envSet := os.LookupEnv(containersConfEnv)
533-
os.Setenv(containersConfEnv, "containers.conf")
507+
GinkgoT().Setenv(containersConfEnv, "containers.conf")
534508
// When
535509
config, err := NewConfig("testdata/containers_override.conf")
536-
// Undo that
537-
if envSet {
538-
os.Setenv(containersConfEnv, oldContainersConf)
539-
} else {
540-
os.Unsetenv(containersConfEnv)
541-
}
542510

543511
crunWasm := "crun-wasm"
544512
PlatformToOCIRuntimeMap := map[string]string{
@@ -698,24 +666,12 @@ image_copy_tmp_dir="storage"`
698666
})
699667

700668
Describe("Service Destinations", func() {
701-
ConfPath := struct {
702-
Value string
703-
IsSet bool
704-
}{}
705-
706669
BeforeEach(func() {
707-
ConfPath.Value, ConfPath.IsSet = os.LookupEnv(containersConfEnv)
708-
conf, _ := os.CreateTemp("", "containersconf")
709-
os.Setenv(containersConfEnv, conf.Name())
710-
})
711-
712-
AfterEach(func() {
713-
os.Remove(os.Getenv(containersConfEnv))
714-
if ConfPath.IsSet {
715-
os.Setenv(containersConfEnv, ConfPath.Value)
716-
} else {
717-
os.Unsetenv(containersConfEnv)
718-
}
670+
t := GinkgoT()
671+
name := t.TempDir() + "/containersconf"
672+
err := os.WriteFile(name, []byte{}, os.ModePerm)
673+
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
674+
t.Setenv(containersConfEnv, name)
719675
})
720676

721677
It("test addConfigs", func() {
@@ -786,36 +742,24 @@ image_copy_tmp_dir="storage"`
786742

787743
Describe("Reload", func() {
788744
It("test new config from reload", func() {
745+
t := GinkgoT()
789746
// Default configuration
790747
defaultTestFile := "testdata/containers_default.conf"
791-
oldEnv, set := os.LookupEnv(containersConfEnv)
792-
os.Setenv(containersConfEnv, defaultTestFile)
748+
t.Setenv(containersConfEnv, defaultTestFile)
793749
cfg, err := Default()
794750
gomega.Expect(err).ToNot(gomega.HaveOccurred())
795-
if set {
796-
os.Setenv(containersConfEnv, oldEnv)
797-
} else {
798-
os.Unsetenv(containersConfEnv)
799-
}
800751

801752
// Reload from new configuration file
802-
testFile := "testdata/temp.conf"
753+
testFile := t.TempDir() + "/temp.conf"
803754
content := `[containers]
804755
env=["foo=bar"]`
805756
err = os.WriteFile(testFile, []byte(content), os.ModePerm)
806-
defer os.Remove(testFile)
807757
gomega.Expect(err).ToNot(gomega.HaveOccurred())
808-
oldEnv, set = os.LookupEnv(containersConfEnv)
809-
os.Setenv(containersConfEnv, testFile)
758+
t.Setenv(containersConfEnv, testFile)
810759
_, err = Reload()
811760
gomega.Expect(err).ToNot(gomega.HaveOccurred())
812761
newCfg, err := Default()
813762
gomega.Expect(err).ToNot(gomega.HaveOccurred())
814-
if set {
815-
os.Setenv(containersConfEnv, oldEnv)
816-
} else {
817-
os.Unsetenv(containersConfEnv)
818-
}
819763

820764
expectOldEnv := []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"}
821765
expectNewEnv := []string{"foo=bar"}
@@ -837,15 +781,14 @@ env=["foo=bar"]`
837781
})
838782

839783
It("CONTAINERS_CONF_OVERRIDE", func() {
840-
os.Setenv("CONTAINERS_CONF_OVERRIDE", "testdata/containers_override.conf")
841-
defer os.Unsetenv("CONTAINERS_CONF_OVERRIDE")
784+
t := GinkgoT()
785+
t.Setenv("CONTAINERS_CONF_OVERRIDE", "testdata/containers_override.conf")
842786
config, err := NewConfig("")
843787
gomega.Expect(err).ToNot(gomega.HaveOccurred())
844788
gomega.Expect(config.Containers.ApparmorProfile).To(gomega.Equal("overridden-default"))
845789

846790
// Make sure that _OVERRIDE is loaded even when CONTAINERS_CONF is set.
847-
os.Setenv(containersConfEnv, "testdata/containers_default.conf")
848-
defer os.Unsetenv(containersConfEnv)
791+
t.Setenv(containersConfEnv, "testdata/containers_default.conf")
849792
config, err = NewConfig("")
850793
gomega.Expect(err).ToNot(gomega.HaveOccurred())
851794
gomega.Expect(config.Containers.ApparmorProfile).To(gomega.Equal("overridden-default"))

pkg/config/connections_test.go

+4-12
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,12 @@ var _ = Describe("Connections conf", func() {
1717
)
1818

1919
BeforeEach(func() {
20-
dir := GinkgoT().TempDir()
20+
t := GinkgoT()
21+
dir := t.TempDir()
2122
connectionsConfFile = filepath.Join(dir, "connections.json")
22-
err := os.Setenv("PODMAN_CONNECTIONS_CONF", connectionsConfFile)
23-
gomega.Expect(err).ToNot(gomega.HaveOccurred())
23+
t.Setenv("PODMAN_CONNECTIONS_CONF", connectionsConfFile)
2424
containersConfFile = filepath.Join(dir, "containers.conf")
25-
err = os.Setenv(containersConfEnv, containersConfFile)
26-
gomega.Expect(err).ToNot(gomega.HaveOccurred())
27-
})
28-
29-
AfterEach(func() {
30-
err := os.Unsetenv("PODMAN_CONNECTIONS_CONF")
31-
gomega.Expect(err).ToNot(gomega.HaveOccurred())
32-
err = os.Unsetenv(containersConfEnv)
33-
gomega.Expect(err).ToNot(gomega.HaveOccurred())
25+
t.Setenv(containersConfEnv, containersConfFile)
3426
})
3527

3628
It("read non existent file", func() {

pkg/config/modules_test.go

+13-30
Original file line numberDiff line numberDiff line change
@@ -15,28 +15,22 @@ const (
1515
testBaseUsr = "testdata/modules/usr/share"
1616
)
1717

18-
func testSetModulePaths() (func(), error) {
19-
oldXDG := os.Getenv("XDG_CONFIG_HOME")
20-
oldEtc := moduleBaseEtc
21-
oldUsr := moduleBaseUsr
18+
func testSetModulePaths() {
19+
t := GinkgoT()
2220

2321
wd, err := os.Getwd()
24-
if err != nil {
25-
return nil, err
26-
}
22+
gomega.Expect(err).ToNot(gomega.HaveOccurred())
2723

28-
if err := os.Setenv("XDG_CONFIG_HOME", filepath.Join(wd, testBaseHome)); err != nil {
29-
return nil, err
30-
}
24+
t.Setenv("XDG_CONFIG_HOME", filepath.Join(wd, testBaseHome))
3125

26+
oldEtc := moduleBaseEtc
27+
oldUsr := moduleBaseUsr
3228
moduleBaseEtc = filepath.Join(wd, testBaseEtc)
3329
moduleBaseUsr = filepath.Join(wd, testBaseUsr)
34-
35-
return func() {
36-
os.Setenv("XDG_CONFIG_HOME", oldXDG)
30+
DeferCleanup(func() {
3731
moduleBaseEtc = oldEtc
3832
moduleBaseUsr = oldUsr
39-
}, nil
33+
})
4034
}
4135

4236
var _ = Describe("Config Modules", func() {
@@ -55,9 +49,7 @@ var _ = Describe("Config Modules", func() {
5549
It("resolve modules", func() {
5650
// This test makes sure that the correct module is being
5751
// returned.
58-
cleanUp, err := testSetModulePaths()
59-
gomega.Expect(err).ToNot(gomega.HaveOccurred())
60-
defer cleanUp()
52+
testSetModulePaths()
6153

6254
dirs, err := ModuleDirectories()
6355
gomega.Expect(err).ToNot(gomega.HaveOccurred())
@@ -106,9 +98,7 @@ var _ = Describe("Config Modules", func() {
10698
})
10799

108100
It("new config with modules", func() {
109-
cleanUp, err := testSetModulePaths()
110-
gomega.Expect(err).ToNot(gomega.HaveOccurred())
111-
defer cleanUp()
101+
testSetModulePaths()
112102

113103
wd, err := os.Getwd()
114104
gomega.Expect(err).ToNot(gomega.HaveOccurred())
@@ -162,17 +152,10 @@ var _ = Describe("Config Modules", func() {
162152
})
163153

164154
It("new config with modules and env variables", func() {
165-
cleanUp, err := testSetModulePaths()
166-
gomega.Expect(err).ToNot(gomega.HaveOccurred())
167-
defer cleanUp()
155+
testSetModulePaths()
168156

169-
oldOverride := os.Getenv(containersConfOverrideEnv)
170-
defer func() {
171-
os.Setenv(containersConfOverrideEnv, oldOverride)
172-
}()
173-
174-
err = os.Setenv(containersConfOverrideEnv, "testdata/modules/override.conf")
175-
gomega.Expect(err).ToNot(gomega.HaveOccurred())
157+
t := GinkgoT()
158+
t.Setenv(containersConfOverrideEnv, "testdata/modules/override.conf")
176159

177160
// Also make sure that absolute paths are loaded as is.
178161
wd, err := os.Getwd()

0 commit comments

Comments
 (0)