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 thetesthelpers
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?