Skip to content

Replace defer with t.Cleanup in tests

At least split into 2 MRs

  • unit tests
  • acceptance tests

But smaller chunks would be great and easier to review

The following discussion from !546 (merged) should be addressed:

  • @jaime started a discussion: (+1 comment)

    suggestion: thinking if we should use t.Cleanup and extract this into the testhelpers

    git diff --color=always --exit-code
diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go
index d03407a52..d2f219c83 100644
--- a/internal/auth/auth_test.go
+++ b/internal/auth/auth_test.go
@@ -15,6 +15,7 @@ import (

 	"gitlab.com/gitlab-org/gitlab-pages/internal/request"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/source"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers"
 )

 func createTestAuth(t *testing.T, internalServer string, publicServer string) *Auth {
@@ -211,7 +212,7 @@ func testTryAuthenticateWithCodeAndState(t *testing.T, https bool) {
 	require.Equal(t, true, auth.TryAuthenticate(result, r, source.NewMockSource()))

 	res := result.Result()
-	defer res.Body.Close()
+	testhelpers.Close(t, res.Body)

 	require.Equal(t, http.StatusFound, result.Code)
 	require.Equal(t, "https://pages.gitlab-example.com/project/", result.Header().Get("Location"))
diff --git a/internal/testhelpers/testhelpers.go b/internal/testhelpers/testhelpers.go
index 3ec97a79c..46d4932ca 100644
--- a/internal/testhelpers/testhelpers.go
+++ b/internal/testhelpers/testhelpers.go
@@ -2,6 +2,7 @@ package testhelpers

 import (
 	"fmt"
+	"io"
 	"mime"
 	"net/http"
 	"net/http/httptest"
@@ -68,6 +69,15 @@ func ToFileProtocol(t *testing.T, path string) string {
 	return fmt.Sprintf("file://%s/%s", wd, path)
 }

+// Close a closer like response.Body as part of testing.T.Cleanup
+func Close(t *testing.T, c io.Closer) {
+	t.Helper()
+
+	t.Cleanup(func() {
+		require.NoError(t, c.Close())
+	})
+}
+
 // Getwd must return current working directory
 func Getwd(t *testing.T) string {
 	t.Helper()

    WDYT?