gittest: Automatically close HTTPServer after test

The `gittest.HTTPServer()` function sets up an HTTP server that is
getting served by git-http-backend(1) so that clients can clone from it.
While this function returns a cleanup function, none of its callers use
it in any special way, but only ever close it in a deferred function
call.

Refactor the code to not return the close function anymore and use
`testing.TB.Cleanup` instead.

7 files changed, 20 insertions, 42 deletions

diff --git a/internal/git/gittest/http_server.go b/internal/git/gittest/http_server.go
index eb78dcade..ef3fd14fd 100644
--- a/internal/git/gittest/http_server.go
+++ b/internal/git/gittest/http_server.go
@@ -17,7 +17,7 @@ import (
 // HTTPServer starts an HTTP server with git-http-backend(1) as CGI handler. The repository is
 // prepared such that git-http-backend(1) will serve it by creating the "git-daemon-export-ok" magic
 // file.
-func HTTPServer(tb testing.TB, ctx context.Context, gitCmdFactory git.CommandFactory, repoPath string, middleware func(http.ResponseWriter, *http.Request, http.Handler)) (int, func() error) {
+func HTTPServer(tb testing.TB, ctx context.Context, gitCmdFactory git.CommandFactory, repoPath string, middleware func(http.ResponseWriter, *http.Request, http.Handler)) int {
 	require.NoError(tb, os.WriteFile(filepath.Join(repoPath, "git-daemon-export-ok"), nil, 0o644))
 
 	listener, err := net.Listen("tcp", "127.0.0.1:0")
@@ -36,7 +36,11 @@ func HTTPServer(tb testing.TB, ctx context.Context, gitCmdFactory git.CommandFac
 			"GIT_CONFIG_VALUE_0=true",
 		}, gitExecEnv.EnvironmentVariables...),
 	}
-	s := http.Server{Handler: gitHTTPBackend}
+
+	s := &http.Server{Handler: gitHTTPBackend}
+	tb.Cleanup(func() {
+		testhelper.MustClose(tb, s)
+	})
 
 	if middleware != nil {
 		s.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -44,7 +48,7 @@ func HTTPServer(tb testing.TB, ctx context.Context, gitCmdFactory git.CommandFac
 		})
 	}
 
-	go testhelper.MustServe(tb, &s, listener)
+	go testhelper.MustServe(tb, s, listener)
 
-	return listener.Addr().(*net.TCPAddr).Port, s.Close
+	return listener.Addr().(*net.TCPAddr).Port
 }
diff --git a/internal/git/stats/http_clone_test.go b/internal/git/stats/http_clone_test.go
index eb818c124..65b53d6d9 100644
--- a/internal/git/stats/http_clone_test.go
+++ b/internal/git/stats/http_clone_test.go
@@ -20,10 +20,7 @@ func TestClone(t *testing.T) {
 	gitCmdFactory := gittest.NewCommandFactory(t, cfg)
 	ctx := testhelper.Context(t)
 
-	serverPort, stopGitServer := gittest.HTTPServer(t, ctx, gitCmdFactory, repoPath, nil)
-	defer func() {
-		require.NoError(t, stopGitServer())
-	}()
+	serverPort := gittest.HTTPServer(t, ctx, gitCmdFactory, repoPath, nil)
 
 	clone, err := PerformHTTPClone(ctx, fmt.Sprintf("http://localhost:%d/%s", serverPort, filepath.Base(repoPath)), "", "", false)
 	require.NoError(t, err, "perform analysis clone")
@@ -86,7 +83,7 @@ func TestCloneWithAuth(t *testing.T) {
 
 	authWasChecked := false
 
-	serverPort, stopGitServer := gittest.HTTPServer(t, ctx, gitCmdFactory, repoPath, func(w http.ResponseWriter, r *http.Request, next http.Handler) {
+	serverPort := gittest.HTTPServer(t, ctx, gitCmdFactory, repoPath, func(w http.ResponseWriter, r *http.Request, next http.Handler) {
 		authWasChecked = true
 
 		actualUser, actualPassword, ok := r.BasicAuth()
@@ -96,9 +93,6 @@ func TestCloneWithAuth(t *testing.T) {
 
 		next.ServeHTTP(w, r)
 	})
-	defer func() {
-		require.NoError(t, stopGitServer())
-	}()
 
 	_, err := PerformHTTPClone(
 		ctx,
diff --git a/internal/git/stats/http_push_test.go b/internal/git/stats/http_push_test.go
index 49531722a..9cbf9a384 100644
--- a/internal/git/stats/http_push_test.go
+++ b/internal/git/stats/http_push_test.go
@@ -26,10 +26,7 @@ func TestPerformHTTPPush(t *testing.T) {
 	gitCmdFactory := gittest.NewCommandFactory(t, cfg)
 	ctx := testhelper.Context(t)
 
-	serverPort, stopGitServer := gittest.HTTPServer(t, ctx, gitCmdFactory, targetRepoPath, nil)
-	defer func() {
-		require.NoError(t, stopGitServer())
-	}()
+	serverPort := gittest.HTTPServer(t, ctx, gitCmdFactory, targetRepoPath, nil)
 	url := fmt.Sprintf("http://localhost:%d/%s", serverPort, filepath.Base(targetRepoPath))
 
 	for _, tc := range []struct {
diff --git a/internal/gitaly/service/remote/find_remote_root_ref_test.go b/internal/gitaly/service/remote/find_remote_root_ref_test.go
index 45a2b1edb..1b7026020 100644
--- a/internal/gitaly/service/remote/find_remote_root_ref_test.go
+++ b/internal/gitaly/service/remote/find_remote_root_ref_test.go
@@ -31,8 +31,7 @@ func TestFindRemoteRootRefSuccess(t *testing.T) {
 		secret = "mysecret"
 	)
 
-	port, stopGitServer := gittest.HTTPServer(t, ctx, gitCmdFactory, repoPath, newGitRequestValidationMiddleware(host, secret))
-	defer func() { require.NoError(t, stopGitServer()) }()
+	port := gittest.HTTPServer(t, ctx, gitCmdFactory, repoPath, newGitRequestValidationMiddleware(host, secret))
 
 	originURL := fmt.Sprintf("http://127.0.0.1:%d/%s", port, filepath.Base(repoPath))
 
diff --git a/internal/gitaly/service/repository/create_repository_from_url_test.go b/internal/gitaly/service/repository/create_repository_from_url_test.go
index cc3aaed26..820615854 100644
--- a/internal/gitaly/service/repository/create_repository_from_url_test.go
+++ b/internal/gitaly/service/repository/create_repository_from_url_test.go
@@ -35,10 +35,7 @@ func TestCreateRepositoryFromURL_successful(t *testing.T) {
 
 	user := "username123"
 	password := "password321localhost"
-	port, stopGitServer := gitServerWithBasicAuth(t, ctx, gitCmdFactory, user, password, repoPath)
-	defer func() {
-		require.NoError(t, stopGitServer())
-	}()
+	port := gitServerWithBasicAuth(t, ctx, gitCmdFactory, user, password, repoPath)
 
 	url := fmt.Sprintf("http://%s:%s@localhost:%d/%s", user, password, port, filepath.Base(repoPath))
 
@@ -75,10 +72,7 @@ func TestCreateRepositoryFromURL_successfulWithOptionalParameters(t *testing.T)
 
 	user := "username123"
 	password := "password321localhost"
-	port, stopGitServer := gitServerWithBasicAuth(t, ctx, gitCmdFactory, user, password, repoPath)
-	defer func() {
-		require.NoError(t, stopGitServer())
-	}()
+	port := gitServerWithBasicAuth(t, ctx, gitCmdFactory, user, password, repoPath)
 
 	url := fmt.Sprintf("http://%s:%s@localhost:%d/%s", user, password, port, filepath.Base(repoPath))
 	host := "www.example.com"
@@ -337,7 +331,7 @@ func TestServer_CloneFromURLCommand_withMirror(t *testing.T) {
 	require.Error(t, cmd.Wait())
 }
 
-func gitServerWithBasicAuth(tb testing.TB, ctx context.Context, gitCmdFactory git.CommandFactory, user, pass, repoPath string) (int, func() error) {
+func gitServerWithBasicAuth(tb testing.TB, ctx context.Context, gitCmdFactory git.CommandFactory, user, pass, repoPath string) int {
 	return gittest.HTTPServer(tb, ctx, gitCmdFactory, repoPath, basicAuthMiddleware(tb, user, pass))
 }
 
diff --git a/internal/gitaly/service/repository/fetch_bundle_test.go b/internal/gitaly/service/repository/fetch_bundle_test.go
index 81293b39b..e85ea24b5 100644
--- a/internal/gitaly/service/repository/fetch_bundle_test.go
+++ b/internal/gitaly/service/repository/fetch_bundle_test.go
@@ -73,7 +73,6 @@ func TestServer_FetchBundle_transaction(t *testing.T) {
 
 	ctx := testhelper.Context(t)
 	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
-	gitCmdFactory := gittest.NewCommandFactory(t, cfg)
 	testcfg.BuildGitalyHooks(t, cfg)
 
 	hookManager := &mockHookManager{}
@@ -84,8 +83,6 @@ func TestServer_FetchBundle_transaction(t *testing.T) {
 	gittest.BundleRepo(t, cfg, repoPath, bundlePath)
 
 	hookManager.Reset()
-	_, stopGitServer := gittest.HTTPServer(t, ctx, gitCmdFactory, repoPath, nil)
-	defer func() { require.NoError(t, stopGitServer()) }()
 
 	ctx, err := txinfo.InjectTransaction(ctx, 1, "node", true)
 	require.NoError(t, err)
diff --git a/internal/gitaly/service/repository/fetch_remote_test.go b/internal/gitaly/service/repository/fetch_remote_test.go
index 61ea7b92d..41960bf19 100644
--- a/internal/gitaly/service/repository/fetch_remote_test.go
+++ b/internal/gitaly/service/repository/fetch_remote_test.go
@@ -193,8 +193,7 @@ func TestFetchRemote_withDefaultRefmaps(t *testing.T) {
 	})
 	targetRepo := localrepo.NewTestRepo(t, cfg, targetRepoProto)
 
-	port, stopGitServer := gittest.HTTPServer(t, ctx, gitCmdFactory, sourceRepoPath, nil)
-	defer func() { require.NoError(t, stopGitServer()) }()
+	port := gittest.HTTPServer(t, ctx, gitCmdFactory, sourceRepoPath, nil)
 
 	require.NoError(t, sourceRepo.UpdateRef(ctx, "refs/heads/foobar", "refs/heads/master", ""))
 
@@ -236,8 +235,7 @@ func TestFetchRemote_transaction(t *testing.T) {
 	targetCfg, targetRepoProto, targetRepoPath := testcfg.BuildWithRepo(t)
 	targetGitCmdFactory := gittest.NewCommandFactory(t, targetCfg)
 
-	port, stopGitServer := gittest.HTTPServer(t, ctx, targetGitCmdFactory, targetRepoPath, nil)
-	defer func() { require.NoError(t, stopGitServer()) }()
+	port := gittest.HTTPServer(t, ctx, targetGitCmdFactory, targetRepoPath, nil)
 
 	ctx, err := txinfo.InjectTransaction(ctx, 1, "node", true)
 	require.NoError(t, err)
@@ -263,8 +261,7 @@ func TestFetchRemote_prune(t *testing.T) {
 	cfg, _, sourceRepoPath, client := setupRepositoryService(t, ctx)
 	gitCmdFactory := gittest.NewCommandFactory(t, cfg)
 
-	port, stopGitServer := gittest.HTTPServer(t, ctx, gitCmdFactory, sourceRepoPath, nil)
-	defer func() { require.NoError(t, stopGitServer()) }()
+	port := gittest.HTTPServer(t, ctx, gitCmdFactory, sourceRepoPath, nil)
 
 	remoteURL := fmt.Sprintf("http://127.0.0.1:%d/%s", port, filepath.Base(sourceRepoPath))
 
@@ -349,8 +346,7 @@ func TestFetchRemote_force(t *testing.T) {
 	divergingBranchOID := gittest.WriteCommit(t, cfg, sourceRepoPath, gittest.WithBranch("b1"))
 	divergingTagOID := gittest.WriteCommit(t, cfg, sourceRepoPath, gittest.WithBranch("b2"))
 
-	port, stopGitServer := gittest.HTTPServer(t, ctx, gitCmdFactory, sourceRepoPath, nil)
-	defer func() { require.NoError(t, stopGitServer()) }()
+	port := gittest.HTTPServer(t, ctx, gitCmdFactory, sourceRepoPath, nil)
 
 	remoteURL := fmt.Sprintf("http://127.0.0.1:%d/%s", port, filepath.Base(sourceRepoPath))
 
@@ -836,7 +832,7 @@ func TestFetchRemote_pooledRepository(t *testing.T) {
 			// can observe the reference negotiation and check whether alternate refs
 			// are announced or not.
 			var requestBuffer bytes.Buffer
-			port, stop := gittest.HTTPServer(t, ctx, gitCmdFactory, remoteRepoPath, func(responseWriter http.ResponseWriter, request *http.Request, handler http.Handler) {
+			port := gittest.HTTPServer(t, ctx, gitCmdFactory, remoteRepoPath, func(responseWriter http.ResponseWriter, request *http.Request, handler http.Handler) {
 				closer := request.Body
 				defer testhelper.MustClose(t, closer)
 
@@ -844,9 +840,6 @@ func TestFetchRemote_pooledRepository(t *testing.T) {
 
 				handler.ServeHTTP(responseWriter, request)
 			})
-			defer func() {
-				require.NoError(t, stop())
-			}()
 
 			// Perform the fetch.
 			_, err := client.FetchRemote(ctx, &gitalypb.FetchRemoteRequest{