golangci-lint: Disable use of `os.Setenv()` and `os.Unsetenv()` in testspks-go-v1.17-infrastructure

Disallow use of `os.Setenv()` and `os.Unsetenv()` in tests. Callers
should instead use `testhelper.ModifyEnvironment()`. Adjust existing
callers to do so. Note: the log tests cannot use the testhelper package
due to a cyclic import and thus use `t.Setenv()` directly.

3 files changed, 10 insertions, 7 deletions

diff --git a/.golangci.yml b/.golangci.yml
index 4ef4e7da7..db6ed2d7e 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -58,6 +58,12 @@ linters-settings:
       # they're tested as expected.
       - ^context.Background$
       - ^context.TODO$
+      # Tests should not set the bare environment functions, but instead use
+      # `testhelper.ModifyEnvironment()`. This function has sanity checks to
+      # verify we don't use `t.Parallel()` when setting envvars by using the
+      # `t.Setenv()` helper.
+      - ^os.Setenv$
+      - ^os.Unsetenv$
   stylecheck:
     # ST1000 checks for missing package comments. We don't use these for most
     # packages, so let's disable this check.
diff --git a/internal/git/command_factory_test.go b/internal/git/command_factory_test.go
index d709dd130..ede45c745 100644
--- a/internal/git/command_factory_test.go
+++ b/internal/git/command_factory_test.go
@@ -33,9 +33,8 @@ func TestGitCommandProxy(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	oldHTTPProxy := os.Getenv("http_proxy")
-	defer require.NoError(t, os.Setenv("http_proxy", oldHTTPProxy))
-	require.NoError(t, os.Setenv("http_proxy", ts.URL))
+	testhelper.ModifyEnvironment(t, "http_proxy", ts.URL)
+
 	ctx := testhelper.Context(t)
 
 	dir := testhelper.TempDir(t)
diff --git a/internal/log/log_test.go b/internal/log/log_test.go
index f6bfa3a0a..1d0a70122 100644
--- a/internal/log/log_test.go
+++ b/internal/log/log_test.go
@@ -491,10 +491,8 @@ func TestLogDeciderOption_logByRegexpMatch(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			require.NoError(t, os.Setenv("GITALY_LOG_REQUEST_METHOD_DENY_PATTERN", tc.skip))
-			defer func() { require.NoError(t, os.Unsetenv("GITALY_LOG_REQUEST_METHOD_DENY_PATTERN")) }()
-			require.NoError(t, os.Setenv("GITALY_LOG_REQUEST_METHOD_ALLOW_PATTERN", tc.only))
-			defer func() { require.NoError(t, os.Unsetenv("GITALY_LOG_REQUEST_METHOD_ALLOW_PATTERN")) }()
+			t.Setenv("GITALY_LOG_REQUEST_METHOD_DENY_PATTERN", tc.skip)
+			t.Setenv("GITALY_LOG_REQUEST_METHOD_ALLOW_PATTERN", tc.only)
 
 			logger, hook := test.NewNullLogger()
 			interceptor := grpcmwlogrus.UnaryServerInterceptor(logrus.NewEntry(logger), DeciderOption())