refactor(test): use t.Setenv (#13036)

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>

cmd/argo/commands/client/conn_test.go

@@ -2,22 +2,19 @@ package client
 
 import (
 	"context"
-	"os"
 	"testing"
 
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestGetAuthString(t *testing.T) {
-	_ = os.Setenv("ARGO_TOKEN", "my-token")
-	defer func() { _ = os.Unsetenv("ARGO_TOKEN") }()
+	t.Setenv("ARGO_TOKEN", "my-token")
 	assert.Equal(t, "my-token", GetAuthString())
 }
 
 func TestNamespace(t *testing.T) {
-	_ = os.Setenv("ARGO_NAMESPACE", "my-ns")
-	defer func() { _ = os.Unsetenv("ARGO_NAMESPACE") }()
+	t.Setenv("ARGO_NAMESPACE", "my-ns")
 	assert.Equal(t, "my-ns", Namespace())
 }
 

server/auth/gatekeeper_test.go

@@ -2,7 +2,6 @@ package auth
 
 import (
 	"context"
-	"os"
 	"testing"
 
 	"github.com/go-jose/go-jose/v3/jwt"
@@ -26,8 +25,7 @@ import (
 
 func TestServer_GetWFClient(t *testing.T) {
 	// prevent using local KUBECONFIG - which will fail on CI
-	_ = os.Setenv("KUBECONFIG", "/dev/null")
-	defer func() { _ = os.Unsetenv("KUBECONFIG") }()
+	t.Setenv("KUBECONFIG", "/dev/null")
 	wfClient := fakewfclientset.NewSimpleClientset()
 	kubeClient := kubefake.NewSimpleClientset(
 		&corev1.ServiceAccount{
@@ -189,7 +187,7 @@ func TestServer_GetWFClient(t *testing.T) {
 		}
 	})
 	t.Run("SSO+RBAC, Namespace delegation ON, precedence=2, Delegated", func(t *testing.T) {
-		os.Setenv("SSO_DELEGATE_RBAC_TO_NAMESPACE", "true")
+		t.Setenv("SSO_DELEGATE_RBAC_TO_NAMESPACE", "true")
 		ssoIf := &ssomocks.Interface{}
 		ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Groups: []string{"my-group", "other-group"}}, nil)
 		ssoIf.On("IsRBACEnabled").Return(true)
@@ -208,7 +206,6 @@ func TestServer_GetWFClient(t *testing.T) {
 				assert.Equal(t, "user1-sa", hook.LastEntry().Data["serviceAccount"])
 			}
 		}
-		os.Unsetenv("SSO_DELEGATE_RBAC_TO_NAMESPACE")
 	})
 	t.Run("SSO+RBAC, Namespace delegation OFF, precedence=2, Not Delegated", func(t *testing.T) {
 		ssoIf := &ssomocks.Interface{}
@@ -231,7 +228,7 @@ func TestServer_GetWFClient(t *testing.T) {
 		}
 	})
 	t.Run("SSO+RBAC, Namespace delegation ON, precedence=0, Not delegated", func(t *testing.T) {
-		os.Setenv("SSO_DELEGATE_RBAC_TO_NAMESPACE", "true")
+		t.Setenv("SSO_DELEGATE_RBAC_TO_NAMESPACE", "true")
 		ssoIf := &ssomocks.Interface{}
 		ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Groups: []string{"my-group", "other-group"}}, nil)
 		ssoIf.On("IsRBACEnabled").Return(true)
@@ -250,10 +247,9 @@ func TestServer_GetWFClient(t *testing.T) {
 				assert.Equal(t, "my-sa", hook.LastEntry().Data["serviceAccount"])
 			}
 		}
-		os.Unsetenv("SSO_DELEGATE_RBAC_TO_NAMESPACE")
 	})
 	t.Run("SSO+RBAC, Namespace delegation ON, precedence=1, Not delegated", func(t *testing.T) {
-		os.Setenv("SSO_DELEGATE_RBAC_TO_NAMESPACE", "true")
+		t.Setenv("SSO_DELEGATE_RBAC_TO_NAMESPACE", "true")
 		ssoIf := &ssomocks.Interface{}
 		ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Groups: []string{"my-group", "other-group"}}, nil)
 		ssoIf.On("IsRBACEnabled").Return(true)
@@ -272,7 +268,6 @@ func TestServer_GetWFClient(t *testing.T) {
 				assert.Equal(t, "my-sa", hook.LastEntry().Data["serviceAccount"])
 			}
 		}
-		os.Unsetenv("SSO_DELEGATE_RBAC_TO_NAMESPACE")
 	})
 	t.Run("SSO+RBAC,precedence=0", func(t *testing.T) {
 		ssoIf := &ssomocks.Interface{}

server/cache/resource_cache_test.go

@@ -2,7 +2,6 @@ package cache
 
 import (
 	"context"
-	"os"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -24,8 +23,7 @@ func checkServiceAccountExists(saList []*v1.ServiceAccount, name string) bool {
 }
 
 func TestServer_K8sUtilsCache(t *testing.T) {
-	_ = os.Setenv("KUBECONFIG", "/dev/null")
-	defer func() { _ = os.Unsetenv("KUBECONFIG") }()
+	t.Setenv("KUBECONFIG", "/dev/null")
 	saLabels := make(map[string]string)
 	saLabels["hello"] = "world"
 

server/workflowarchive/archived_workflow_server_test.go

@@ -2,7 +2,6 @@ package workflowarchive
 
 import (
 	"context"
-	"os"
 	"testing"
 	"time"
 
@@ -241,13 +240,12 @@ func Test_archivedWorkflowServer(t *testing.T) {
 		assert.Len(t, resp.Items, 2)
 
 		assert.False(t, matchLabelKeyPattern("my-key"))
-		_ = os.Setenv(disableValueListRetrievalKeyPattern, "my-key")
+		t.Setenv(disableValueListRetrievalKeyPattern, "my-key")
 		assert.True(t, matchLabelKeyPattern("my-key"))
 		assert.False(t, matchLabelKeyPattern("wrong key"))
 		resp, err = w.ListArchivedWorkflowLabelValues(ctx, &workflowarchivepkg.ListArchivedWorkflowLabelValuesRequest{ListOptions: &metav1.ListOptions{LabelSelector: "my-key"}})
 		assert.NoError(t, err)
 		assert.Len(t, resp.Items, 0)
-		_ = os.Unsetenv(disableValueListRetrievalKeyPattern)
 	})
 	t.Run("RetryArchivedWorkflow", func(t *testing.T) {
 		_, err := w.RetryArchivedWorkflow(ctx, &workflowarchivepkg.RetryArchivedWorkflowRequest{Uid: "failed-uid"})

util/env/env_test.go

@@ -1,51 +1,46 @@
 package env
 
 import (
-	"os"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func TestLookupEnvDurationOr(t *testing.T) {
-	defer func() { _ = os.Unsetenv("FOO") }()
 	assert.Equal(t, time.Second, LookupEnvDurationOr("", time.Second), "default value")
-	_ = os.Setenv("FOO", "bar")
+	t.Setenv("FOO", "bar")
 	assert.Panics(t, func() { LookupEnvDurationOr("FOO", time.Second) }, "bad value")
-	_ = os.Setenv("FOO", "1h")
+	t.Setenv("FOO", "1h")
 	assert.Equal(t, time.Hour, LookupEnvDurationOr("FOO", time.Second), "env var value")
-	_ = os.Setenv("FOO", "")
+	t.Setenv("FOO", "")
 	assert.Equal(t, time.Second, LookupEnvDurationOr("FOO", time.Second), "empty var value; default value")
 }
 
 func TestLookupEnvIntOr(t *testing.T) {
-	defer func() { _ = os.Unsetenv("FOO") }()
 	assert.Equal(t, 1, LookupEnvIntOr("", 1), "default value")
-	_ = os.Setenv("FOO", "not-int")
+	t.Setenv("FOO", "not-int")
 	assert.Panics(t, func() { LookupEnvIntOr("FOO", 1) }, "bad value")
-	_ = os.Setenv("FOO", "2")
+	t.Setenv("FOO", "2")
 	assert.Equal(t, 2, LookupEnvIntOr("FOO", 1), "env var value")
-	_ = os.Setenv("FOO", "")
+	t.Setenv("FOO", "")
 	assert.Equal(t, 1, LookupEnvIntOr("FOO", 1), "empty var value; default value")
 }
 
 func TestLookupEnvFloatOr(t *testing.T) {
-	defer func() { _ = os.Unsetenv("FOO") }()
 	assert.Equal(t, 1., LookupEnvFloatOr("", 1.), "default value")
-	_ = os.Setenv("FOO", "not-float")
+	t.Setenv("FOO", "not-float")
 	assert.Panics(t, func() { LookupEnvFloatOr("FOO", 1.) }, "bad value")
-	_ = os.Setenv("FOO", "2.0")
+	t.Setenv("FOO", "2.0")
 	assert.Equal(t, 2., LookupEnvFloatOr("FOO", 1.), "env var value")
-	_ = os.Setenv("FOO", "")
+	t.Setenv("FOO", "")
 	assert.Equal(t, 1., LookupEnvFloatOr("FOO", 1.), "empty var value; default value")
 }
 
 func TestLookupEnvStringOr(t *testing.T) {
-	defer func() { _ = os.Unsetenv("FOO") }()
 	assert.Equal(t, "a", LookupEnvStringOr("", "a"), "default value")
-	_ = os.Setenv("FOO", "b")
+	t.Setenv("FOO", "b")
 	assert.Equal(t, "b", LookupEnvStringOr("FOO", "a"), "env var value")
-	_ = os.Setenv("FOO", "")
+	t.Setenv("FOO", "")
 	assert.Equal(t, "a", LookupEnvStringOr("FOO", "a"), "empty var value; default value")
 }

util/errors/errors_test.go

@@ -90,18 +90,16 @@ func TestIsTransientErr(t *testing.T) {
 		assert.True(t, IsTransientErr(connectionResetErr))
 	})
 	t.Run("TransientErrorPattern", func(t *testing.T) {
-		_ = os.Setenv(transientEnvVarKey, "this error is transient")
+		t.Setenv(transientEnvVarKey, "this error is transient")
 		assert.True(t, IsTransientErr(transientErr))
 		assert.True(t, IsTransientErr(&transientExitErr))
 
-		_ = os.Setenv(transientEnvVarKey, "this error is not transient")
+		t.Setenv(transientEnvVarKey, "this error is not transient")
 		assert.False(t, IsTransientErr(transientErr))
 		assert.False(t, IsTransientErr(&transientExitErr))
 
-		_ = os.Setenv(transientEnvVarKey, "")
+		t.Setenv(transientEnvVarKey, "")
 		assert.False(t, IsTransientErr(transientErr))
-
-		_ = os.Unsetenv(transientEnvVarKey)
 	})
 	t.Run("ExplicitTransientErr", func(t *testing.T) {
 		assert.True(t, IsTransientErr(NewErrTransient("")))

util/file/fileutil_test.go

@@ -16,7 +16,7 @@ import (
 // TestCompressContentString ensures compressing then decompressing a content string works as expected
 func TestCompressContentString(t *testing.T) {
 	for _, gzipImpl := range []string{file.GZIP, file.PGZIP} {
-		_ = os.Setenv(file.GZipImplEnvVarKey, gzipImpl)
+		t.Setenv(file.GZipImplEnvVarKey, gzipImpl)
 		content := "{\"pod-limits-rrdm8-591645159\":{\"id\":\"pod-limits-rrdm8-591645159\",\"name\":\"pod-limits-rrdm8[0]." +
 			"run-pod(0:0)\",\"displayName\":\"run-pod(0:0)\",\"type\":\"Pod\",\"templateName\":\"run-pod\",\"phase\":" +
 			"\"Succeeded\",\"boundaryID\":\"pod-limits-rrdm8\",\"startedAt\":\"2019-03-07T19:14:50Z\",\"finishedAt\":" +
@@ -28,13 +28,12 @@ func TestCompressContentString(t *testing.T) {
 
 		assert.Equal(t, content, resultString)
 	}
-	_ = os.Unsetenv(file.GZipImplEnvVarKey)
 }
 
 // TestGetGzipReader checks whether we can obtain the Gzip reader based on environment variable.
 func TestGetGzipReader(t *testing.T) {
 	for _, gzipImpl := range []string{file.GZIP, file.PGZIP} {
-		_ = os.Setenv(file.GZipImplEnvVarKey, gzipImpl)
+		t.Setenv(file.GZipImplEnvVarKey, gzipImpl)
 		rawContent := "this is the content"
 		content := file.CompressEncodeString(rawContent)
 		buf, err := base64.StdEncoding.DecodeString(content)
@@ -45,7 +44,6 @@ func TestGetGzipReader(t *testing.T) {
 		res, err := io.ReadAll(reader)
 		assert.NoError(t, err)
 		assert.Equal(t, rawContent, string(res))
-		_ = os.Unsetenv(file.GZipImplEnvVarKey)
 	}
 }
 

util/kubeconfig/kubeconfig_test.go

@@ -50,7 +50,7 @@ func Test_BasicAuthString(t *testing.T) {
 		assert.NoError(t, err)
 		err = file.Close()
 		assert.NoError(t, err)
-		os.Setenv("KUBECONFIG", file.Name())
+		t.Setenv("KUBECONFIG", file.Name())
 		config, err := GetRestConfig(authString)
 		if assert.NoError(t, err) {
 			assert.Equal(t, "admin", config.Username)

util/util_test.go

@@ -1,7 +1,6 @@
 package util
 
 import (
-	"os"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -74,11 +73,11 @@ func TestGetDeletePropagation(t *testing.T) {
 		assert.Equal(t, metav1.DeletePropagationBackground, *GetDeletePropagation())
 	})
 	t.Run("GetEnvPolicy", func(t *testing.T) {
-		os.Setenv("WF_DEL_PROPAGATION_POLICY", "Foreground")
+		t.Setenv("WF_DEL_PROPAGATION_POLICY", "Foreground")
 		assert.Equal(t, metav1.DeletePropagationForeground, *GetDeletePropagation())
 	})
 	t.Run("GetEnvPolicyWithEmpty", func(t *testing.T) {
-		os.Setenv("WF_DEL_PROPAGATION_POLICY", "")
+		t.Setenv("WF_DEL_PROPAGATION_POLICY", "")
 		assert.Equal(t, metav1.DeletePropagationBackground, *GetDeletePropagation())
 	})
 }

workflow/artifacts/s3/s3_test.go

@@ -215,7 +215,7 @@ func TestOpenStreamS3Artifact(t *testing.T) {
 		},
 	}
 
-	_ = os.Setenv(transientEnvVarKey, "this error is transient")
+	t.Setenv(transientEnvVarKey, "this error is transient")
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			stream, err := streamS3Artifact(tc.s3client, &wfv1.Artifact{
@@ -237,7 +237,6 @@ func TestOpenStreamS3Artifact(t *testing.T) {
 			}
 		})
 	}
-	_ = os.Unsetenv(transientEnvVarKey)
 }
 
 // Delete deletes an S3 artifact by artifact key
@@ -380,7 +379,7 @@ func TestLoadS3Artifact(t *testing.T) {
 		},
 	}
 
-	_ = os.Setenv(transientEnvVarKey, "this error is transient")
+	t.Setenv(transientEnvVarKey, "this error is transient")
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			success, err := loadS3Artifact(tc.s3client, &wfv1.Artifact{
@@ -401,7 +400,6 @@ func TestLoadS3Artifact(t *testing.T) {
 			}
 		})
 	}
-	_ = os.Unsetenv(transientEnvVarKey)
 }
 
 func TestSaveS3Artifact(t *testing.T) {
@@ -509,7 +507,7 @@ func TestSaveS3Artifact(t *testing.T) {
 	}
 
 	for name, tc := range tests {
-		_ = os.Setenv(transientEnvVarKey, "this error is transient")
+		t.Setenv(transientEnvVarKey, "this error is transient")
 		t.Run(name, func(t *testing.T) {
 			success, err := saveS3Artifact(
 				tc.s3client,
@@ -535,7 +533,6 @@ func TestSaveS3Artifact(t *testing.T) {
 				assert.Equal(t, tc.errMsg, "")
 			}
 		})
-		_ = os.Unsetenv(transientEnvVarKey)
 	}
 }
 
@@ -590,7 +587,7 @@ func TestListObjects(t *testing.T) {
 		},
 	}
 
-	_ = os.Setenv(transientEnvVarKey, "this error is transient")
+	t.Setenv(transientEnvVarKey, "this error is transient")
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			_, files, err := listObjects(tc.s3client,
@@ -617,5 +614,4 @@ func TestListObjects(t *testing.T) {
 			}
 		})
 	}
-	_ = os.Unsetenv(transientEnvVarKey)
 }