Merge branch 'refactor/remove-extraenv' into 'master'

test: replace custom extraEnv with native t.setEnv

See merge request gitlab-org/gitlab-pages!810

7 files changed, 28 insertions, 31 deletions

diff --git a/test/acceptance/artifacts_test.go b/test/acceptance/artifacts_test.go
index 6283d896..c88f48bf 100644
--- a/test/acceptance/artifacts_test.go
+++ b/test/acceptance/artifacts_test.go
@@ -120,10 +120,11 @@ func TestArtifactProxyRequest(t *testing.T) {
 
 	args := []string{"-artifacts-server=" + artifactServerURL, "-artifacts-server-timeout=1"}
 
+	t.Setenv("SSL_CERT_FILE", certFile)
+
 	RunPagesProcess(t,
 		withListeners([]ListenSpec{httpListener}),
 		withArguments(args),
-		withEnv([]string{"SSL_CERT_FILE=" + certFile}),
 	)
 
 	for _, tt := range tests {
@@ -194,6 +195,8 @@ func TestPrivateArtifactProxyRequest(t *testing.T) {
 	cert, err := tls.LoadX509KeyPair(certFile, keyFile)
 	require.NoError(t, err)
 
+	t.Setenv("SSL_CERT_FILE", certFile)
+
 	RunPagesProcess(t,
 		withListeners([]ListenSpec{httpsListener}),
 		withArguments([]string{
@@ -202,7 +205,6 @@ func TestPrivateArtifactProxyRequest(t *testing.T) {
 		withPublicServer,
 		withExtraArgument("auth-redirect-uri", "https://projects.gitlab-example.com/auth"),
 		withExtraArgument("artifacts-server-timeout", "1"),
-		withEnv([]string{"SSL_CERT_FILE=" + certFile}),
 		withStubOptions(gitlabstub.WithCertificate(cert)),
 	)
 
diff --git a/test/acceptance/config_test.go b/test/acceptance/config_test.go
index 95be6e17..baa35f6e 100644
--- a/test/acceptance/config_test.go
+++ b/test/acceptance/config_test.go
@@ -11,12 +11,11 @@ import (
 )
 
 func TestEnvironmentVariablesConfig(t *testing.T) {
-	envVarValue := "LISTEN_HTTP=" + net.JoinHostPort(httpListener.Host, httpListener.Port)
+	t.Setenv("LISTEN_HTTP", net.JoinHostPort(httpListener.Host, httpListener.Port))
 
 	RunPagesProcess(t,
 		withoutWait,
 		withListeners([]ListenSpec{}), // explicitly disable listeners for this test
-		withEnv([]string{envVarValue}),
 	)
 	require.NoError(t, httpListener.WaitUntilRequestSucceeds(nil))
 
@@ -28,12 +27,11 @@ func TestEnvironmentVariablesConfig(t *testing.T) {
 }
 
 func TestMixedConfigSources(t *testing.T) {
-	envVarValue := "LISTEN_HTTP=" + net.JoinHostPort(httpListener.Host, httpListener.Port)
+	t.Setenv("LISTEN_HTTP", net.JoinHostPort(httpListener.Host, httpListener.Port))
 
 	RunPagesProcess(t,
 		withoutWait,
 		withListeners([]ListenSpec{httpsListener}),
-		withEnv([]string{envVarValue}),
 	)
 
 	for _, listener := range []ListenSpec{httpListener, httpsListener} {
@@ -48,12 +46,11 @@ func TestMixedConfigSources(t *testing.T) {
 
 func TestMultipleListenersFromEnvironmentVariables(t *testing.T) {
 	listenSpecs := []ListenSpec{{"http", "127.0.0.1", "37001"}, {"http", "127.0.0.1", "37002"}}
-	envVarValue := fmt.Sprintf("LISTEN_HTTP=%s,%s", net.JoinHostPort("127.0.0.1", "37001"), net.JoinHostPort("127.0.0.1", "37002"))
+	t.Setenv("LISTEN_HTTP", fmt.Sprintf("%s,%s", net.JoinHostPort("127.0.0.1", "37001"), net.JoinHostPort("127.0.0.1", "37002")))
 
 	RunPagesProcess(t,
 		withoutWait,
 		withListeners([]ListenSpec{}), // explicitly disable listeners for this test
-		withEnv([]string{envVarValue}),
 	)
 
 	for _, listener := range listenSpecs {
diff --git a/test/acceptance/helpers_test.go b/test/acceptance/helpers_test.go
index ba6443e0..f8638e36 100644
--- a/test/acceptance/helpers_test.go
+++ b/test/acceptance/helpers_test.go
@@ -269,7 +269,7 @@ func RunPagesProcess(t *testing.T, opts ...processOption) *LogCaptureBuffer {
 		processCfg.extraArgs = append(processCfg.extraArgs, "-gitlab-server", source.URL)
 	}
 
-	logBuf, cleanup := runPagesProcess(t, processCfg.wait, processCfg.pagesBinary, processCfg.listeners, "", processCfg.envs, processCfg.extraArgs...)
+	logBuf, cleanup := runPagesProcess(t, processCfg.wait, processCfg.pagesBinary, processCfg.listeners, "", processCfg.extraArgs...)
 
 	t.Cleanup(func() {
 		source.Close()
@@ -281,12 +281,13 @@ func RunPagesProcess(t *testing.T, opts ...processOption) *LogCaptureBuffer {
 }
 
 func RunPagesProcessWithSSLCertFile(t *testing.T, listeners []ListenSpec, sslCertFile string) {
+	t.Setenv("SSL_CERT_FILE", sslCertFile)
+
 	RunPagesProcess(t,
 		withListeners(listeners),
 		withArguments([]string{
 			"-config=" + defaultAuthConfig(t),
 		}),
-		withEnv([]string{"SSL_CERT_FILE=" + sslCertFile}),
 	)
 }
 
@@ -294,6 +295,8 @@ func RunPagesProcessWithSSLCertDir(t *testing.T, listeners []ListenSpec, sslCert
 	// Create temporary cert dir
 	sslCertDir := t.TempDir()
 
+	t.Setenv("SSL_CERT_DIR", sslCertDir)
+
 	// Copy sslCertFile into temp cert dir
 	err := copyFile(sslCertDir+"/"+path.Base(sslCertFile), sslCertFile)
 	require.NoError(t, err)
@@ -303,11 +306,10 @@ func RunPagesProcessWithSSLCertDir(t *testing.T, listeners []ListenSpec, sslCert
 		withArguments([]string{
 			"-config=" + defaultAuthConfig(t),
 		}),
-		withEnv([]string{"SSL_CERT_DIR=" + sslCertDir}),
 	)
 }
 
-func runPagesProcess(t *testing.T, wait bool, pagesBinary string, listeners []ListenSpec, promPort string, extraEnv []string, extraArgs ...string) (*LogCaptureBuffer, func()) {
+func runPagesProcess(t *testing.T, wait bool, pagesBinary string, listeners []ListenSpec, promPort string, extraArgs ...string) (*LogCaptureBuffer, func()) {
 	t.Helper()
 
 	_, err := os.Stat(pagesBinary)
@@ -318,7 +320,6 @@ func runPagesProcess(t *testing.T, wait bool, pagesBinary string, listeners []Li
 
 	args := getPagesArgs(t, listeners, promPort, extraArgs)
 	cmd := exec.Command(pagesBinary, args...)
-	cmd.Env = append(os.Environ(), extraEnv...)
 	cmd.Stdout = out
 	cmd.Stderr = out
 	require.NoError(t, cmd.Start())
diff --git a/test/acceptance/redirects_test.go b/test/acceptance/redirects_test.go
index 5846d2cd..a2bdde53 100644
--- a/test/acceptance/redirects_test.go
+++ b/test/acceptance/redirects_test.go
@@ -13,9 +13,10 @@ import (
 )
 
 func TestRedirectStatusPage(t *testing.T) {
+	t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true")
+
 	RunPagesProcess(t,
 		withListeners([]ListenSpec{httpListener}),
-		withEnv([]string{feature.RedirectsPlaceholders.EnvVariable + "=true"}),
 	)
 
 	rsp, err := GetPageFromListener(t, httpListener, "group.redirects.gitlab-example.com", "/project-redirects/_redirects")
@@ -30,9 +31,10 @@ func TestRedirectStatusPage(t *testing.T) {
 }
 
 func TestRedirect(t *testing.T) {
+	t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true")
+
 	RunPagesProcess(t,
 		withListeners([]ListenSpec{httpListener}),
-		withEnv([]string{feature.RedirectsPlaceholders.EnvVariable + "=true"}),
 	)
 
 	// Test that serving a file still works with redirects enabled
diff --git a/test/acceptance/rewrites_test.go b/test/acceptance/rewrites_test.go
index eefb1e82..cb11c470 100644
--- a/test/acceptance/rewrites_test.go
+++ b/test/acceptance/rewrites_test.go
@@ -12,9 +12,10 @@ import (
 )
 
 func TestRewrites(t *testing.T) {
+	t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true")
+
 	RunPagesProcess(t,
 		withListeners([]ListenSpec{httpListener}),
-		withEnv([]string{feature.RedirectsPlaceholders.EnvVariable + "=true"}),
 	)
 
 	tests := map[string]struct {
diff --git a/test/acceptance/serving_test.go b/test/acceptance/serving_test.go
index 410e5ca0..00289c58 100644
--- a/test/acceptance/serving_test.go
+++ b/test/acceptance/serving_test.go
@@ -486,11 +486,14 @@ func TestServerRepliesWithHeaders(t *testing.T) {
 	}
 
 	for name, test := range tests {
-		testFn := func(envArgs, headerArgs []string) func(*testing.T) {
+		testFn := func(headerEnv string, headerArgs []string) func(*testing.T) {
 			return func(t *testing.T) {
+				if headerEnv != "" {
+					t.Setenv("HEADER", headerEnv)
+				}
+
 				RunPagesProcess(t,
 					withListeners([]ListenSpec{httpListener}),
-					withEnv(envArgs),
 					withArguments(headerArgs),
 				)
 
@@ -509,7 +512,7 @@ func TestServerRepliesWithHeaders(t *testing.T) {
 
 		t.Run(name+"/from_single_flag", func(t *testing.T) {
 			args := []string{"-header", strings.Join(test.flags, ";;")}
-			testFn([]string{}, args)
+			testFn("", args)
 		})
 
 		t.Run(name+"/from_multiple_flags", func(t *testing.T) {
@@ -518,18 +521,17 @@ func TestServerRepliesWithHeaders(t *testing.T) {
 				args = append(args, "-header", arg)
 			}
 
-			testFn([]string{}, args)
+			testFn("", args)
 		})
 
 		t.Run(name+"/from_config_file", func(t *testing.T) {
 			file := newConfigFile(t, "-header="+strings.Join(test.flags, ";;"))
 
-			testFn([]string{}, []string{"-config", file})
+			testFn("", []string{"-config", file})
 		})
 
 		t.Run(name+"/from_env", func(t *testing.T) {
-			args := []string{"header", strings.Join(test.flags, ";;")}
-			testFn(args, []string{})
+			testFn(strings.Join(test.flags, ";;"), []string{})
 		})
 	}
 }
diff --git a/test/acceptance/stub_test.go b/test/acceptance/stub_test.go
index e65a86cc..3ab6def7 100644
--- a/test/acceptance/stub_test.go
+++ b/test/acceptance/stub_test.go
@@ -15,7 +15,6 @@ var defaultProcessConfig = processConfig{
 	wait:           true,
 	pagesBinary:    *pagesBinary,
 	listeners:      supportedListeners(),
-	envs:           []string{},
 	extraArgs:      []string{},
 	gitlabStubOpts: []gitlabstub.Option{},
 }
@@ -24,7 +23,6 @@ type processConfig struct {
 	wait           bool
 	pagesBinary    string
 	listeners      []ListenSpec
-	envs           []string
 	extraArgs      []string
 	gitlabStubOpts []gitlabstub.Option
 	publicServer   bool
@@ -42,12 +40,6 @@ func withListeners(listeners []ListenSpec) processOption {
 	}
 }
 
-func withEnv(envs []string) processOption {
-	return func(config *processConfig) {
-		config.envs = append(config.envs, envs...)
-	}
-}
-
 func withExtraArgument(key, value string) processOption {
 	return func(config *processConfig) {
 		config.extraArgs = append(config.extraArgs, fmt.Sprintf("-%s=%s", key, value))