Merge branch '571-use-functional-options' into 'master'

Use functional options in gitlab stub

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

4 files changed, 88 insertions, 40 deletions

diff --git a/test/acceptance/helpers_test.go b/test/acceptance/helpers_test.go
index 97387786..ec712b6d 100644
--- a/test/acceptance/helpers_test.go
+++ b/test/acceptance/helpers_test.go
@@ -233,29 +233,37 @@ func RunPagesProcessWithOutput(t *testing.T, pagesBinary string, listeners []Lis
 	return runPagesProcess(t, true, pagesBinary, listeners, promPort, nil, extraArgs...)
 }
 
-func RunPagesProcessWithStubGitLabServer(t *testing.T, wait bool, pagesBinary string, listeners []ListenSpec, envs []string, extraArgs ...string) (*LogCaptureBuffer, func()) {
+func RunPagesProcessWithStubGitLabServer(t *testing.T, opts ...processOption) *LogCaptureBuffer {
 	chdir := false
 	chdirCleanup := testhelpers.ChdirInPath(t, "../../shared/pages", &chdir)
 
 	wd, err := os.Getwd()
 	require.NoError(t, err)
 
-	opts := &stubOpts{
-		pagesRoot: wd,
+	processCfg := defaultProcessConfig
+
+	for _, opt := range opts {
+		opt(&processCfg)
+	}
+
+	if processCfg.gitlabStubOpts.pagesRoot == "" {
+		processCfg.gitlabStubOpts.pagesRoot = wd
 	}
 
-	source := NewGitlabDomainsSourceStub(t, opts)
+	source := NewGitlabDomainsSourceStub(t, processCfg.gitlabStubOpts)
 
 	gitLabAPISecretKey := CreateGitLabAPISecretKeyFixtureFile(t)
-	pagesArgs := append([]string{"-pages-root", wd, "-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey, "-domain-config-source", "gitlab"}, extraArgs...)
+	processCfg.extraArgs = append(processCfg.extraArgs, "-pages-root", wd, "-gitlab-server", source.URL, "-api-secret-key", gitLabAPISecretKey, "-domain-config-source", "gitlab")
 
-	logBuf, cleanup := runPagesProcess(t, wait, pagesBinary, listeners, "", envs, pagesArgs...)
+	logBuf, cleanup := runPagesProcess(t, processCfg.wait, processCfg.pagesBinary, listeners, "", processCfg.envs, processCfg.extraArgs...)
 
-	return logBuf, func() {
+	t.Cleanup(func() {
 		source.Close()
 		chdirCleanup()
 		cleanup()
-	}
+	})
+
+	return logBuf
 }
 
 func RunPagesProcessWithAuth(t *testing.T, pagesBinary string, listeners []ListenSpec, promPort string) func() {
diff --git a/test/acceptance/metrics_test.go b/test/acceptance/metrics_test.go
index b1166bc7..6ba147aa 100644
--- a/test/acceptance/metrics_test.go
+++ b/test/acceptance/metrics_test.go
@@ -14,8 +14,10 @@ func TestPrometheusMetricsCanBeScraped(t *testing.T) {
 	_, cleanup := newZipFileServerURL(t, "../../shared/pages/group/zip.gitlab.io/public.zip")
 	defer cleanup()
 
-	_, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, supportedListeners(), []string{}, "-max-conns=10", "-metrics-address=:42345")
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t,
+		withExtraArgument("max-conns", "10"),
+		withExtraArgument("metrics-address", ":42345"),
+	)
 
 	// need to call an actual resource to populate certain metrics e.g. gitlab_pages_domains_source_api_requests_total
 	res, err := GetPageFromListener(t, httpListener, "zip.gitlab.io",
diff --git a/test/acceptance/serving_test.go b/test/acceptance/serving_test.go
index ba0069b5..92727d28 100644
--- a/test/acceptance/serving_test.go
+++ b/test/acceptance/serving_test.go
@@ -16,8 +16,7 @@ import (
 func TestUnknownHostReturnsNotFound(t *testing.T) {
 	skipUnlessEnabled(t)
 
-	_, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, supportedListeners(), []string{})
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t)
 
 	for _, spec := range supportedListeners() {
 		rsp, err := GetPageFromListener(t, spec, "invalid.invalid", "")
@@ -31,8 +30,7 @@ func TestUnknownHostReturnsNotFound(t *testing.T) {
 func TestUnknownProjectReturnsNotFound(t *testing.T) {
 	skipUnlessEnabled(t)
 
-	_, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, []ListenSpec{httpListener}, []string{})
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t)
 
 	rsp, err := GetPageFromListener(t, httpListener, "group.gitlab-example.com", "/nonexistent/")
 	require.NoError(t, err)
@@ -43,8 +41,7 @@ func TestUnknownProjectReturnsNotFound(t *testing.T) {
 func TestGroupDomainReturns200(t *testing.T) {
 	skipUnlessEnabled(t)
 
-	_, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, supportedListeners(), []string{})
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t)
 
 	rsp, err := GetPageFromListener(t, httpListener, "group.gitlab-example.com", "/")
 	require.NoError(t, err)
@@ -60,8 +57,7 @@ func TestGroupDomainReturns200(t *testing.T) {
 func TestKnownHostReturns200(t *testing.T) {
 	skipUnlessEnabled(t)
 
-	_, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, supportedListeners(), []string{})
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t)
 
 	tests := []struct {
 		name    string
@@ -230,8 +226,7 @@ func TestCustom404(t *testing.T) {
 func TestCORSWhenDisabled(t *testing.T) {
 	skipUnlessEnabled(t)
 
-	_, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, supportedListeners(), []string{}, "-disable-cross-origin-requests")
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t, withExtraArgument("disable-cross-origin-requests", "true"))
 
 	for _, spec := range supportedListeners() {
 		for _, method := range []string{"GET", "OPTIONS"} {
@@ -247,8 +242,7 @@ func TestCORSWhenDisabled(t *testing.T) {
 func TestCORSAllowsGET(t *testing.T) {
 	skipUnlessEnabled(t)
 
-	_, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, supportedListeners(), []string{})
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t)
 
 	for _, spec := range supportedListeners() {
 		for _, method := range []string{"GET", "OPTIONS"} {
@@ -264,8 +258,7 @@ func TestCORSAllowsGET(t *testing.T) {
 func TestCORSForbidsPOST(t *testing.T) {
 	skipUnlessEnabled(t)
 
-	_, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, supportedListeners(), []string{})
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t)
 
 	for _, spec := range supportedListeners() {
 		rsp := doCrossOriginRequest(t, spec, "OPTIONS", "POST", spec.URL("project/"))
@@ -278,8 +271,11 @@ func TestCORSForbidsPOST(t *testing.T) {
 
 func TestCustomHeaders(t *testing.T) {
 	skipUnlessEnabled(t)
-	_, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, supportedListeners(), []string{}, "-header", "X-Test1:Testing1", "-header", "X-Test2:Testing2")
-	defer teardown()
+
+	RunPagesProcessWithStubGitLabServer(t,
+		withExtraArgument("header", "X-Test1:Testing1"),
+		withExtraArgument("header", "X-Test2:Testing2"),
+	)
 
 	for _, spec := range supportedListeners() {
 		rsp, err := GetPageFromListener(t, spec, "group.gitlab-example.com:", "project/")
@@ -293,8 +289,7 @@ func TestCustomHeaders(t *testing.T) {
 func TestKnownHostWithPortReturns200(t *testing.T) {
 	skipUnlessEnabled(t)
 
-	_, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, supportedListeners(), []string{})
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t)
 
 	for _, spec := range supportedListeners() {
 		rsp, err := GetPageFromListener(t, spec, "group.gitlab-example.com:"+spec.Port, "project/")
@@ -308,8 +303,7 @@ func TestKnownHostWithPortReturns200(t *testing.T) {
 func TestHttpToHttpsRedirectDisabled(t *testing.T) {
 	skipUnlessEnabled(t)
 
-	_, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, supportedListeners(), []string{})
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t)
 
 	rsp, err := GetRedirectPage(t, httpListener, "group.gitlab-example.com", "project/")
 	require.NoError(t, err)
@@ -325,8 +319,7 @@ func TestHttpToHttpsRedirectDisabled(t *testing.T) {
 func TestHttpToHttpsRedirectEnabled(t *testing.T) {
 	skipUnlessEnabled(t)
 
-	_, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, supportedListeners(), []string{}, "-redirect-http=true")
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t, withExtraArgument("redirect-http", "true"))
 
 	rsp, err := GetRedirectPage(t, httpListener, "group.gitlab-example.com", "project/")
 	require.NoError(t, err)
@@ -587,8 +580,7 @@ func TestGitLabSourceBecomesUnauthorized(t *testing.T) {
 func TestKnownHostInReverseProxySetupReturns200(t *testing.T) {
 	skipUnlessEnabled(t)
 
-	_, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, []ListenSpec{proxyListener}, []string{})
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t)
 
 	rsp, err := GetProxiedPageFromListener(t, proxyListener, "localhost", "group.gitlab-example.com", "project/")
 
@@ -694,8 +686,7 @@ func doCrossOriginRequest(t *testing.T, spec ListenSpec, method, reqMethod, url
 func TestQueryStringPersistedInSlashRewrite(t *testing.T) {
 	skipUnlessEnabled(t)
 
-	_, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, supportedListeners(), []string{})
-	defer teardown()
+	RunPagesProcessWithStubGitLabServer(t)
 
 	rsp, err := GetRedirectPage(t, httpsListener, "group.gitlab-example.com", "project?q=test")
 	require.NoError(t, err)
@@ -731,8 +722,11 @@ func TestServerRepliesWithHeaders(t *testing.T) {
 	for name, test := range tests {
 		testFn := func(envArgs, headerArgs []string) func(*testing.T) {
 			return func(t *testing.T) {
-				_, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, []ListenSpec{httpListener}, envArgs, headerArgs...)
-				defer teardown()
+				RunPagesProcessWithStubGitLabServer(t,
+					withListeners([]ListenSpec{httpListener}),
+					withEnv(envArgs),
+					withArguments(headerArgs),
+				)
 
 				rsp, err := GetPageFromListener(t, httpListener, "group.gitlab-example.com", "/")
 				require.NoError(t, err)
@@ -777,8 +771,9 @@ func TestServerRepliesWithHeaders(t *testing.T) {
 func TestDiskDisabledFailsToServeFileAndLocalContent(t *testing.T) {
 	skipUnlessEnabled(t)
 
-	logBuf, teardown := RunPagesProcessWithStubGitLabServer(t, true, *pagesBinary, []ListenSpec{httpListener}, nil, "-enable-disk=false")
-	defer teardown()
+	logBuf := RunPagesProcessWithStubGitLabServer(t,
+		withExtraArgument("enable-disk", "false"),
+	)
 
 	for host, suffix := range map[string]string{
 		// API serves "source": { "type": "local" }
diff --git a/test/acceptance/stub_test.go b/test/acceptance/stub_test.go
index 8f52ec37..2ba7da41 100644
--- a/test/acceptance/stub_test.go
+++ b/test/acceptance/stub_test.go
@@ -12,6 +12,49 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/internal/fixture"
 )
 
+var defaultProcessConfig = processConfig{
+	wait:           true,
+	pagesBinary:    *pagesBinary,
+	listeners:      supportedListeners(),
+	envs:           []string{},
+	extraArgs:      []string{},
+	gitlabStubOpts: &stubOpts{},
+}
+
+type processConfig struct {
+	wait           bool
+	pagesBinary    string
+	listeners      []ListenSpec
+	envs           []string
+	extraArgs      []string
+	gitlabStubOpts *stubOpts
+}
+
+type processOption func(*processConfig)
+
+func withListeners(listeners []ListenSpec) processOption {
+	return func(config *processConfig) {
+		config.listeners = listeners
+	}
+}
+
+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))
+	}
+}
+func withArguments(args []string) processOption {
+	return func(config *processConfig) {
+		config.extraArgs = append(config.extraArgs, args...)
+	}
+}
+
 // makeGitLabPagesAccessStub provides a stub *httptest.Server to check pages_access API call.
 // the result is based on the project id.
 //