diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-04-15 11:49:43 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-04-19 10:23:54 +0300 |
commit | ceba1579a711103c56c30e510a07d61444191717 (patch) | |
tree | 56c29af97229b70427da1bf4bc80652c2f3e3f96 | |
parent | f5c607b1ea9a59c35ae47b6b03bdf98ea183a379 (diff) |
testhelper: Convert `WriteExecutable()` to use `t.Cleanup()`
Now that the minimum required Go version is 1.14, we have started to
make use of `t.Cleanup()` to clean up temporary test state. Convert
`WriteExecutable()` and some of its user to use it, too, instead of
returning cleanup functions.
24 files changed, 73 insertions, 126 deletions
diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go index dfad57eea..f0fd73693 100644 --- a/cmd/gitaly-hooks/hooks_test.go +++ b/cmd/gitaly-hooks/hooks_test.go @@ -158,8 +158,7 @@ func testHooksPrePostReceive(t *testing.T, cfg config.Cfg, repo *gitalypb.Reposi for _, hookName := range hookNames { t.Run(fmt.Sprintf("hookName: %s", hookName), func(t *testing.T) { - customHookOutputPath, cleanup := gittest.WriteEnvToCustomHook(t, repoPath, hookName) - defer cleanup() + customHookOutputPath := gittest.WriteEnvToCustomHook(t, repoPath, hookName) gitlabAPI, err := gitalyhook.NewGitlabAPI(cfg.Gitlab, cfg.TLS) require.NoError(t, err) @@ -281,12 +280,10 @@ require 'json' open('%s', 'w') { |f| f.puts(JSON.dump(ARGV)) } `, customHookArgsPath) // write a custom hook to path/to/repo.git/custom_hooks/update.d/dumpargsscript which dumps the args into a tempfile - cleanup = testhelper.WriteExecutable(t, filepath.Join(repoPath, "custom_hooks", "update.d", "dumpargsscript"), []byte(dumpArgsToTempfileScript)) - defer cleanup() + testhelper.WriteExecutable(t, filepath.Join(repoPath, "custom_hooks", "update.d", "dumpargsscript"), []byte(dumpArgsToTempfileScript)) // write a custom hook to path/to/repo.git/custom_hooks/update which dumps the env into a tempfile - customHookOutputPath, cleanup := gittest.WriteEnvToCustomHook(t, repoPath, "update") - defer cleanup() + customHookOutputPath := gittest.WriteEnvToCustomHook(t, repoPath, "update") var stdout, stderr bytes.Buffer @@ -350,8 +347,7 @@ func TestHooksPostReceiveFailed(t *testing.T) { stop := runHookServiceServerWithAPI(t, cfg, gitlabAPI) defer stop() - customHookOutputPath, cleanup := gittest.WriteEnvToCustomHook(t, repoPath, "post-receive") - defer cleanup() + customHookOutputPath := gittest.WriteEnvToCustomHook(t, repoPath, "post-receive") var stdout, stderr bytes.Buffer @@ -460,8 +456,7 @@ func TestHooksNotAllowed(t *testing.T) { cfg.Gitlab.URL = serverURL cfg.Gitlab.SecretFile = testhelper.WriteShellSecretFile(t, cfg.GitlabShell.Dir, "the wrong token") - customHookOutputPath, cleanup := gittest.WriteEnvToCustomHook(t, repoPath, "post-receive") - defer cleanup() + customHookOutputPath := gittest.WriteEnvToCustomHook(t, repoPath, "post-receive") gitlabAPI, err := gitalyhook.NewGitlabAPI(cfg.Gitlab, cfg.TLS) require.NoError(t, err) diff --git a/internal/git/gittest/hooks.go b/internal/git/gittest/hooks.go index dad9bb0a4..058f92f37 100644 --- a/internal/git/gittest/hooks.go +++ b/internal/git/gittest/hooks.go @@ -14,26 +14,27 @@ import ( ) // WriteEnvToCustomHook dumps the env vars that the custom hooks receives to a file -func WriteEnvToCustomHook(t testing.TB, repoPath, hookName string) (string, func()) { +func WriteEnvToCustomHook(t testing.TB, repoPath, hookName string) string { hookOutputTemp, err := ioutil.TempFile("", "") require.NoError(t, err) + t.Cleanup(func() { + assert.NoError(t, os.Remove(hookOutputTemp.Name())) + }) + require.NoError(t, hookOutputTemp.Close()) hookContent := fmt.Sprintf("#!/bin/sh\n/usr/bin/env > %s\n", hookOutputTemp.Name()) - cleanupCustomHook := WriteCustomHook(t, repoPath, hookName, []byte(hookContent)) + WriteCustomHook(t, repoPath, hookName, []byte(hookContent)) - return hookOutputTemp.Name(), func() { - cleanupCustomHook() - assert.NoError(t, os.Remove(hookOutputTemp.Name())) - } + return hookOutputTemp.Name() } // WriteCheckNewObjectExistsHook writes a pre-receive hook which only succeeds // if it can find the object in the quarantine directory. if // GIT_OBJECT_DIRECTORY and GIT_ALTERNATE_OBJECT_DIRECTORIES were not passed // through correctly to the hooks, it will fail -func WriteCheckNewObjectExistsHook(t testing.TB, gitBin, repoPath string) func() { +func WriteCheckNewObjectExistsHook(t testing.TB, gitBin, repoPath string) { hook := fmt.Sprintf(`#!/usr/bin/env ruby STDIN.each_line do |line| new_object = line.split(' ')[1] @@ -42,13 +43,13 @@ STDIN.each_line do |line| end `, gitBin) - return WriteCustomHook(t, repoPath, "pre-receive", []byte(hook)) + WriteCustomHook(t, repoPath, "pre-receive", []byte(hook)) } // WriteCustomHook writes a hook in the repo/path.git/custom_hooks directory -func WriteCustomHook(t testing.TB, repoPath, name string, content []byte) func() { +func WriteCustomHook(t testing.TB, repoPath, name string, content []byte) { fullPath := filepath.Join(repoPath, "custom_hooks", name) - return testhelper.WriteExecutable(t, fullPath, content) + testhelper.WriteExecutable(t, fullPath, content) } // CaptureHookEnv creates a bogus 'update' Git hook to sniff out what diff --git a/internal/git/gittest/protocol.go b/internal/git/gittest/protocol.go index bf45c3f64..d6f8ebb08 100644 --- a/internal/git/gittest/protocol.go +++ b/internal/git/gittest/protocol.go @@ -14,8 +14,9 @@ import ( // EnableGitProtocolV2Support replaces the git binary in config with a wrapper that allows the // protocol to be tested. It returns a function to read the GIT_PROTOCOl environment variable // created by the wrapper script, the modified configuration as well as a cleanup function. -func EnableGitProtocolV2Support(t testing.TB, cfg config.Cfg) (func() string, config.Cfg, testhelper.Cleanup) { +func EnableGitProtocolV2Support(t testing.TB, cfg config.Cfg) (func() string, config.Cfg) { dir, cleanupDir := testhelper.TempDir(t) + t.Cleanup(cleanupDir) gitPath := filepath.Join(dir, "git") envPath := filepath.Join(dir, "git-env") @@ -25,16 +26,13 @@ env | grep ^GIT_PROTOCOL= >>"%s" exec "%s" "$@" `, envPath, cfg.Git.BinPath) - cleanupExe := testhelper.WriteExecutable(t, gitPath, []byte(script)) + testhelper.WriteExecutable(t, gitPath, []byte(script)) cfg.Git.BinPath = gitPath return func() string { - data, err := ioutil.ReadFile(envPath) - require.NoError(t, err) - return string(data) - }, cfg, func() { - cleanupExe() - cleanupDir() - } + data, err := ioutil.ReadFile(envPath) + require.NoError(t, err) + return string(data) + }, cfg } diff --git a/internal/gitaly/hook/postreceive_test.go b/internal/gitaly/hook/postreceive_test.go index a83fe376b..17dd2642f 100644 --- a/internal/gitaly/hook/postreceive_test.go +++ b/internal/gitaly/hook/postreceive_test.go @@ -204,8 +204,7 @@ func TestPostReceive_customHook(t *testing.T) { ctx, cleanup := testhelper.Context() defer cleanup() - cleanup = gittest.WriteCustomHook(t, repoPath, "post-receive", []byte(tc.hook)) - defer cleanup() + gittest.WriteCustomHook(t, repoPath, "post-receive", []byte(tc.hook)) var stdout, stderr bytes.Buffer err = hookManager.PostReceiveHook(ctx, repo, tc.pushOptions, tc.env, strings.NewReader(tc.stdin), &stdout, &stderr) @@ -343,8 +342,7 @@ func TestPostReceive_gitlab(t *testing.T) { hookManager := NewManager(config.NewLocator(cfg), transaction.NewManager(cfg, backchannel.NewRegistry()), &gitlabAPI, cfg) - cleanup = gittest.WriteCustomHook(t, repoPath, "post-receive", []byte("#!/bin/sh\necho hook called\n")) - defer cleanup() + gittest.WriteCustomHook(t, repoPath, "post-receive", []byte("#!/bin/sh\necho hook called\n")) var stdout, stderr bytes.Buffer err = hookManager.PostReceiveHook(ctx, repo, tc.pushOptions, tc.env, strings.NewReader(tc.changes), &stdout, &stderr) diff --git a/internal/gitaly/hook/prereceive_test.go b/internal/gitaly/hook/prereceive_test.go index 1bcf28815..48537a048 100644 --- a/internal/gitaly/hook/prereceive_test.go +++ b/internal/gitaly/hook/prereceive_test.go @@ -165,8 +165,7 @@ func TestPrereceive_customHooks(t *testing.T) { ctx, cleanup := testhelper.Context() defer cleanup() - cleanup = gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte(tc.hook)) - defer cleanup() + gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte(tc.hook)) var stdout, stderr bytes.Buffer err = hookManager.PreReceiveHook(ctx, repo, tc.pushOptions, tc.env, strings.NewReader(tc.stdin), &stdout, &stderr) @@ -307,8 +306,7 @@ func TestPrereceive_gitlab(t *testing.T) { hookManager := NewManager(config.NewLocator(cfg), transaction.NewManager(cfg, backchannel.NewRegistry()), &gitlabAPI, cfg) - cleanup = gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte("#!/bin/sh\necho called\n")) - defer cleanup() + gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte("#!/bin/sh\necho called\n")) var stdout, stderr bytes.Buffer err = hookManager.PreReceiveHook(ctx, repo, nil, tc.env, strings.NewReader(tc.changes), &stdout, &stderr) diff --git a/internal/gitaly/hook/transactions_test.go b/internal/gitaly/hook/transactions_test.go index 624618d2a..b7c8f3707 100644 --- a/internal/gitaly/hook/transactions_test.go +++ b/internal/gitaly/hook/transactions_test.go @@ -52,8 +52,7 @@ func TestHookManager_stopCalled(t *testing.T) { defer cleanup() for _, hook := range []string{"pre-receive", "update", "post-receive"} { - cleanup = gittest.WriteCustomHook(t, repoPath, hook, []byte("#!/bin/sh\nexit 1\n")) - defer cleanup() + gittest.WriteCustomHook(t, repoPath, hook, []byte("#!/bin/sh\nexit 1\n")) } preReceiveFunc := func(t *testing.T) error { diff --git a/internal/gitaly/hook/update_test.go b/internal/gitaly/hook/update_test.go index 5711898d1..896f57b22 100644 --- a/internal/gitaly/hook/update_test.go +++ b/internal/gitaly/hook/update_test.go @@ -191,8 +191,7 @@ func TestUpdate_customHooks(t *testing.T) { ctx, cleanup := testhelper.Context() defer cleanup() - cleanup = gittest.WriteCustomHook(t, repoPath, "update", []byte(tc.hook)) - defer cleanup() + gittest.WriteCustomHook(t, repoPath, "update", []byte(tc.hook)) var stdout, stderr bytes.Buffer err = hookManager.UpdateHook(ctx, repo, tc.reference, tc.oldHash, tc.newHash, tc.env, &stdout, &stderr) diff --git a/internal/gitaly/server/auth_test.go b/internal/gitaly/server/auth_test.go index c08b637a3..f5fd1b1f7 100644 --- a/internal/gitaly/server/auth_test.go +++ b/internal/gitaly/server/auth_test.go @@ -335,10 +335,9 @@ func TestAuthBeforeLimit(t *testing.T) { }(gitalyauth.TokenValidityDuration()) gitalyauth.SetTokenValidityDuration(5 * time.Second) - cleanupCustomHook := gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte(fmt.Sprintf(`#!/bin/bash + gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte(fmt.Sprintf(`#!/bin/bash sleep %vs `, gitalyauth.TokenValidityDuration().Seconds()))) - t.Cleanup(cleanupCustomHook) errChan := make(chan error) diff --git a/internal/gitaly/service/hook/pre_receive_test.go b/internal/gitaly/service/hook/pre_receive_test.go index 8fc1b7fdf..750cdca12 100644 --- a/internal/gitaly/service/hook/pre_receive_test.go +++ b/internal/gitaly/service/hook/pre_receive_test.go @@ -292,11 +292,10 @@ func TestPreReceiveHook_CustomHookErrors(t *testing.T) { customHookReturnCode := int32(128) customHookReturnMsg := "custom hook error" - cleanup = gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte(fmt.Sprintf(`#!/bin/bash + gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte(fmt.Sprintf(`#!/bin/bash echo '%s' 1>&2 exit %d `, customHookReturnMsg, customHookReturnCode))) - defer cleanup() gitlabConfig := config.Gitlab{ URL: srv.URL, @@ -419,8 +418,7 @@ func TestPreReceiveHook_Primary(t *testing.T) { secretFilePath := filepath.Join(tmpDir, ".gitlab_shell_secret") testhelper.WriteShellSecretFile(t, tmpDir, "token") - cleanup = gittest.WriteCustomHook(t, testRepoPath, "pre-receive", []byte(fmt.Sprintf("#!/bin/bash\nexit %d", tc.hookExitCode))) - defer cleanup() + gittest.WriteCustomHook(t, testRepoPath, "pre-receive", []byte(fmt.Sprintf("#!/bin/bash\nexit %d", tc.hookExitCode))) gitlabAPI, err := gitalyhook.NewGitlabAPI(config.Gitlab{ URL: srv.URL, diff --git a/internal/gitaly/service/hook/update_test.go b/internal/gitaly/service/hook/update_test.go index 93553e15c..8daeb702d 100644 --- a/internal/gitaly/service/hook/update_test.go +++ b/internal/gitaly/service/hook/update_test.go @@ -59,11 +59,10 @@ func TestUpdate_CustomHooks(t *testing.T) { } errorMsg := "error123" - cleanup := gittest.WriteCustomHook(t, repoPath, "update", []byte(fmt.Sprintf(`#!/bin/bash + gittest.WriteCustomHook(t, repoPath, "update", []byte(fmt.Sprintf(`#!/bin/bash echo %s 1>&2 exit 1 `, errorMsg))) - defer cleanup() stream, err := client.UpdateHook(ctx, &req) require.NoError(t, err) diff --git a/internal/gitaly/service/operations/branches_test.go b/internal/gitaly/service/operations/branches_test.go index 570369f8f..1d0ecbff9 100644 --- a/internal/gitaly/service/operations/branches_test.go +++ b/internal/gitaly/service/operations/branches_test.go @@ -249,8 +249,7 @@ func testSuccessfulGitHooksForUserCreateBranchRequest(t *testing.T, ctx context. t.Run(hookName, func(t *testing.T) { defer testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "branch", "-D", branchName) - hookOutputTempPath, cleanup := gittest.WriteEnvToCustomHook(t, repoPath, hookName) - defer cleanup() + hookOutputTempPath := gittest.WriteEnvToCustomHook(t, repoPath, hookName) response, err := client.UserCreateBranch(ctx, request) require.NoError(t, err) @@ -352,8 +351,7 @@ func TestFailedUserCreateBranchDueToHooks(t *testing.T) { hookContent := []byte("#!/bin/sh\nprintenv | paste -sd ' ' -\nexit 1") for _, hookName := range gitlabPreHooks { - remove := gittest.WriteCustomHook(t, repoPath, hookName, hookContent) - defer remove() + gittest.WriteCustomHook(t, repoPath, hookName, hookContent) response, err := client.UserCreateBranch(ctx, request) require.Nil(t, err) @@ -497,8 +495,7 @@ func TestSuccessfulGitHooksForUserDeleteBranchRequest(t *testing.T) { t.Run(hookName, func(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "branch", branchNameInput) - hookOutputTempPath, cleanup := gittest.WriteEnvToCustomHook(t, repoPath, hookName) - defer cleanup() + hookOutputTempPath := gittest.WriteEnvToCustomHook(t, repoPath, hookName) _, err := client.UserDeleteBranch(ctx, request) require.NoError(t, err) @@ -658,8 +655,7 @@ func TestFailedUserDeleteBranchDueToHooks(t *testing.T) { for _, hookName := range gitlabPreHooks { t.Run(hookName, func(t *testing.T) { - remove := gittest.WriteCustomHook(t, repoPath, hookName, hookContent) - defer remove() + gittest.WriteCustomHook(t, repoPath, hookName, hookContent) response, err := client.UserDeleteBranch(ctx, request) require.NoError(t, err) @@ -730,8 +726,7 @@ func TestBranchHookOutput(t *testing.T) { User: testhelper.TestUser, } - remove := gittest.WriteCustomHook(t, repoPath, hookName, []byte(testCase.hookContent)) - defer remove() + gittest.WriteCustomHook(t, repoPath, hookName, []byte(testCase.hookContent)) createResponse, err := client.UserCreateBranch(ctx, createRequest) require.NoError(t, err) diff --git a/internal/gitaly/service/operations/cherry_pick_test.go b/internal/gitaly/service/operations/cherry_pick_test.go index 63ebff85c..a6f87b25b 100644 --- a/internal/gitaly/service/operations/cherry_pick_test.go +++ b/internal/gitaly/service/operations/cherry_pick_test.go @@ -199,8 +199,7 @@ func testServerUserCherryPickSuccessfulGitHooksFeatured(t *testing.T, ctx contex var hookOutputFiles []string for _, hookName := range GitlabHooks { - hookOutputTempPath, cleanup := gittest.WriteEnvToCustomHook(t, repoPath, hookName) - defer cleanup() + hookOutputTempPath := gittest.WriteEnvToCustomHook(t, repoPath, hookName) hookOutputFiles = append(hookOutputFiles, hookOutputTempPath) } @@ -371,8 +370,7 @@ func testServerUserCherryPickFailedWithPreReceiveErrorFeatured(t *testing.T, ctx for _, hookName := range GitlabPreHooks { t.Run(hookName, func(t *testing.T) { - remove := gittest.WriteCustomHook(t, repoPath, hookName, hookContent) - defer remove() + gittest.WriteCustomHook(t, repoPath, hookName, hookContent) response, err := client.UserCherryPick(ctx, request) require.NoError(t, err) diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go index 4e7ae0a89..61cd00faa 100644 --- a/internal/gitaly/service/operations/commit_files_test.go +++ b/internal/gitaly/service/operations/commit_files_test.go @@ -1352,8 +1352,7 @@ func TestFailedUserCommitFilesRequestDueToHooks(t *testing.T) { for _, hookName := range GitlabPreHooks { t.Run(hookName, func(t *testing.T) { - remove := gittest.WriteCustomHook(t, repoPath, hookName, hookContent) - defer remove() + gittest.WriteCustomHook(t, repoPath, hookName, hookContent) stream, err := client.UserCommitFiles(ctx) require.NoError(t, err) diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go index c2aa9554e..b70e79892 100644 --- a/internal/gitaly/service/operations/merge_test.go +++ b/internal/gitaly/service/operations/merge_test.go @@ -62,8 +62,7 @@ func TestSuccessfulMerge(t *testing.T) { defer os.Remove(outputFile.Name()) script := fmt.Sprintf("#!/bin/sh\n(cat && env) >%s \n", outputFile.Name()) - cleanup := gittest.WriteCustomHook(t, repoPath, hook, []byte(script)) - defer cleanup() + gittest.WriteCustomHook(t, repoPath, hook, []byte(script)) hookTempfiles[i] = outputFile.Name() } @@ -372,8 +371,7 @@ func TestFailedMergeDueToHooks(t *testing.T) { for _, hookName := range gitlabPreHooks { t.Run(hookName, func(t *testing.T) { - remove := gittest.WriteCustomHook(t, repoPath, hookName, hookContent) - defer remove() + gittest.WriteCustomHook(t, repoPath, hookName, hookContent) mergeBidi, err := client.UserMergeBranch(ctx) require.NoError(t, err) @@ -549,8 +547,7 @@ func TestFailedUserFFBranchDueToHooks(t *testing.T) { for _, hookName := range gitlabPreHooks { t.Run(hookName, func(t *testing.T) { - remove := gittest.WriteCustomHook(t, repoPath, hookName, hookContent) - defer remove() + gittest.WriteCustomHook(t, repoPath, hookName, hookContent) resp, err := client.UserFFBranch(ctx, request) require.Nil(t, err) @@ -920,8 +917,7 @@ func TestUserMergeToRefIgnoreHooksRequest(t *testing.T) { for _, hookName := range gitlabPreHooks { t.Run(hookName, func(t *testing.T) { - remove := gittest.WriteCustomHook(t, repoPath, hookName, hookContent) - defer remove() + gittest.WriteCustomHook(t, repoPath, hookName, hookContent) resp, err := client.UserMergeToRef(ctx, request) require.NoError(t, err) diff --git a/internal/gitaly/service/operations/rebase_test.go b/internal/gitaly/service/operations/rebase_test.go index 0c23b24de..fe130d6c5 100644 --- a/internal/gitaly/service/operations/rebase_test.go +++ b/internal/gitaly/service/operations/rebase_test.go @@ -43,10 +43,8 @@ func testSuccessfulUserRebaseConfirmableRequest(t *testing.T, cfg config.Cfg, ru rebaseStream, err := client.UserRebaseConfirmable(ctx) require.NoError(t, err) - preReceiveHookOutputPath, removePreReceive := gittest.WriteEnvToCustomHook(t, repoPath, "pre-receive") - postReceiveHookOutputPath, removePostReceive := gittest.WriteEnvToCustomHook(t, repoPath, "post-receive") - defer removePreReceive() - defer removePostReceive() + preReceiveHookOutputPath := gittest.WriteEnvToCustomHook(t, repoPath, "pre-receive") + postReceiveHookOutputPath := gittest.WriteEnvToCustomHook(t, repoPath, "post-receive") headerRequest := buildHeaderRequest(repoProto, testhelper.TestUser, "1", rebaseBranchName, branchSha, repoCopyProto, "master") headerRequest.GetHeader().GitPushOptions = pushOptions @@ -331,8 +329,7 @@ func testFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T, cf for i, hookName := range GitlabPreHooks { t.Run(hookName, func(t *testing.T) { - remove := gittest.WriteCustomHook(t, repoPath, hookName, hookContent) - defer remove() + gittest.WriteCustomHook(t, repoPath, hookName, hookContent) rebaseStream, err := client.UserRebaseConfirmable(ctx) require.NoError(t, err) diff --git a/internal/gitaly/service/operations/revert_test.go b/internal/gitaly/service/operations/revert_test.go index 5e539690c..84266ce0f 100644 --- a/internal/gitaly/service/operations/revert_test.go +++ b/internal/gitaly/service/operations/revert_test.go @@ -304,8 +304,7 @@ func testServerUserRevertSuccessfulGitHooksFeatured(t *testing.T, ctx context.Co var hookOutputFiles []string for _, hookName := range GitlabHooks { - hookOutputTempPath, cleanup := gittest.WriteEnvToCustomHook(t, repoPath, hookName) - defer cleanup() + hookOutputTempPath := gittest.WriteEnvToCustomHook(t, repoPath, hookName) hookOutputFiles = append(hookOutputFiles, hookOutputTempPath) } @@ -419,8 +418,7 @@ func testServerUserRevertFailedDueToPreReceiveErrorFeatured(t *testing.T, ctx co for _, hookName := range GitlabPreHooks { t.Run(hookName, func(t *testing.T) { - remove := gittest.WriteCustomHook(t, repoPath, hookName, hookContent) - defer remove() + gittest.WriteCustomHook(t, repoPath, hookName, hookContent) response, err := client.UserRevert(ctx, request) require.NoError(t, err) diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go index 9a516f4ee..d51ca29f6 100644 --- a/internal/gitaly/service/operations/tags_test.go +++ b/internal/gitaly/service/operations/tags_test.go @@ -78,8 +78,7 @@ func testSuccessfulGitHooksForUserDeleteTagRequest(t *testing.T, ctx context.Con t.Run(hookName, func(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "tag", tagNameInput) - hookOutputTempPath, cleanup := gittest.WriteEnvToCustomHook(t, repoPath, hookName) - defer cleanup() + hookOutputTempPath := gittest.WriteEnvToCustomHook(t, repoPath, hookName) _, err := client.UserDeleteTag(ctx, request) require.NoError(t, err) @@ -224,8 +223,7 @@ func testSuccessfulUserCreateTagRequest(t *testing.T, ctx context.Context) { "pre-receive": fmt.Sprintf("#!/bin/sh\n%s %s \"$@\"", preReceiveHook, testCase.expectedObjectType), "update": fmt.Sprintf("#!/bin/sh\n%s %s \"$@\"", updateHook, testCase.expectedObjectType), } { - hookCleanup := gittest.WriteCustomHook(t, repoPath, hook, []byte(content)) - defer hookCleanup() + gittest.WriteCustomHook(t, repoPath, hook, []byte(content)) } request := &gitalypb.UserCreateTagRequest{ @@ -495,8 +493,7 @@ func testSuccessfulUserCreateTagRequestAnnotatedLightweightDisambiguation(t *tes "pre-receive": fmt.Sprintf("#!/bin/sh\n%s %s \"$@\"", preReceiveHook, testCase.objType), "update": fmt.Sprintf("#!/bin/sh\n%s %s \"$@\"", updateHook, testCase.objType), } { - hookCleanup := gittest.WriteCustomHook(t, repoPath, hook, []byte(content)) - defer hookCleanup() + gittest.WriteCustomHook(t, repoPath, hook, []byte(content)) } tagName := "what-will-it-be" @@ -681,8 +678,7 @@ func TestSuccessfulUserCreateTagRequestToNonCommit(t *testing.T) { "pre-receive": fmt.Sprintf("#!/bin/sh\n%s %s \"$@\"", preReceiveHook, testCase.expectedObjectType), "update": fmt.Sprintf("#!/bin/sh\n%s %s \"$@\"", updateHook, testCase.expectedObjectType), } { - hookCleanup := gittest.WriteCustomHook(t, repoPath, hook, []byte(content)) - defer hookCleanup() + gittest.WriteCustomHook(t, repoPath, hook, []byte(content)) } request := &gitalypb.UserCreateTagRequest{ @@ -763,8 +759,7 @@ func TestSuccessfulUserCreateTagNestedTags(t *testing.T) { "pre-receive": fmt.Sprintf("#!/bin/sh\n%s %s \"$@\"", preReceiveHook, hookObjectType), "update": fmt.Sprintf("#!/bin/sh\n%s %s \"$@\"", updateHook, hookObjectType), } { - hookCleanup := gittest.WriteCustomHook(t, repoPath, hook, []byte(content)) - defer hookCleanup() + gittest.WriteCustomHook(t, repoPath, hook, []byte(content)) } targetObject := testCase.targetObject @@ -998,8 +993,7 @@ func TestSuccessfulGitHooksForUserCreateTagRequest(t *testing.T) { t.Run(hookName, func(t *testing.T) { defer testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "tag", "-d", tagName) - hookOutputTempPath, cleanup := gittest.WriteEnvToCustomHook(t, repoPath, hookName) - defer cleanup() + hookOutputTempPath := gittest.WriteEnvToCustomHook(t, repoPath, hookName) response, err := client.UserCreateTag(ctx, request) require.NoError(t, err) @@ -1106,8 +1100,7 @@ func testFailedUserDeleteTagDueToHooks(t *testing.T, ctx context.Context) { for _, hookName := range gitlabPreHooks { t.Run(hookName, func(t *testing.T) { - remove := gittest.WriteCustomHook(t, repoPath, hookName, hookContent) - defer remove() + gittest.WriteCustomHook(t, repoPath, hookName, hookContent) response, err := client.UserDeleteTag(ctx, request) require.Nil(t, err) @@ -1135,8 +1128,7 @@ func TestFailedUserCreateTagDueToHooks(t *testing.T) { hookContent := []byte("#!/bin/sh\necho GL_ID=$GL_ID\nexit 1") for _, hookName := range gitlabPreHooks { - remove := gittest.WriteCustomHook(t, repoPath, hookName, hookContent) - defer remove() + gittest.WriteCustomHook(t, repoPath, hookName, hookContent) response, err := client.UserCreateTag(ctx, request) require.Nil(t, err) @@ -1375,8 +1367,7 @@ func testTagHookOutput(t *testing.T, ctx context.Context) { User: testhelper.TestUser, } - remove := gittest.WriteCustomHook(t, repoPath, hookName, []byte(testCase.hookContent)) - defer remove() + gittest.WriteCustomHook(t, repoPath, hookName, []byte(testCase.hookContent)) createResponse, err := client.UserCreateTag(ctx, createRequest) require.NoError(t, err) diff --git a/internal/gitaly/service/operations/update_branches_test.go b/internal/gitaly/service/operations/update_branches_test.go index d2c37cb39..2db6e757f 100644 --- a/internal/gitaly/service/operations/update_branches_test.go +++ b/internal/gitaly/service/operations/update_branches_test.go @@ -188,8 +188,7 @@ func testSuccessfulGitHooksForUserUpdateBranchRequestFeatured(t *testing.T, ctx testRepo, testRepoPath, cleanupFn := gittest.CloneRepoAtStorage(t, cfg.Storages[0], "repo") defer cleanupFn() - hookOutputTempPath, cleanup := gittest.WriteEnvToCustomHook(t, testRepoPath, hookName) - defer cleanup() + hookOutputTempPath := gittest.WriteEnvToCustomHook(t, testRepoPath, hookName) request := &gitalypb.UserUpdateBranchRequest{ Repository: testRepo, @@ -230,8 +229,7 @@ func testFailedUserUpdateBranchDueToHooksFeatured(t *testing.T, ctx context.Cont hookContent := []byte("#!/bin/sh\nprintenv | paste -sd ' ' - >&2\nexit 1") for _, hookName := range gitlabPreHooks { - remove := gittest.WriteCustomHook(t, repoPath, hookName, hookContent) - defer remove() + gittest.WriteCustomHook(t, repoPath, hookName, hookContent) response, err := client.UserUpdateBranch(ctx, request) require.Nil(t, err) diff --git a/internal/gitaly/service/smarthttp/inforefs_test.go b/internal/gitaly/service/smarthttp/inforefs_test.go index 3de658dc2..4cbf80479 100644 --- a/internal/gitaly/service/smarthttp/inforefs_test.go +++ b/internal/gitaly/service/smarthttp/inforefs_test.go @@ -96,8 +96,7 @@ func TestSuccessfulInfoRefsUploadPackWithGitConfigOptions(t *testing.T) { func TestSuccessfulInfoRefsUploadPackWithGitProtocol(t *testing.T) { cfg, repo, _ := testcfg.BuildWithRepo(t) - readProtocol, cfg, restore := gittest.EnableGitProtocolV2Support(t, cfg) - defer restore() + readProtocol, cfg := gittest.EnableGitProtocolV2Support(t, cfg) serverSocketPath, stop := runSmartHTTPServer(t, cfg) defer stop() diff --git a/internal/gitaly/service/smarthttp/receive_pack_test.go b/internal/gitaly/service/smarthttp/receive_pack_test.go index 89cac847c..cc62ba23e 100644 --- a/internal/gitaly/service/smarthttp/receive_pack_test.go +++ b/internal/gitaly/service/smarthttp/receive_pack_test.go @@ -113,8 +113,7 @@ func TestSuccessfulReceivePackRequestWithGitProtocol(t *testing.T) { testhelper.ConfigureGitalyHooksBin(t, cfg) - readProto, cfg, restore := gittest.EnableGitProtocolV2Support(t, cfg) - defer restore() + readProto, cfg := gittest.EnableGitProtocolV2Support(t, cfg) serverSocketPath, stop := runSmartHTTPServer(t, cfg) defer stop() @@ -412,8 +411,7 @@ func TestPostReceivePackToHooks(t *testing.T) { }) defer cleanup() - cleanup = gittest.WriteCheckNewObjectExistsHook(t, cfg.Git.BinPath, testRepoPath) - defer cleanup() + gittest.WriteCheckNewObjectExistsHook(t, cfg.Git.BinPath, testRepoPath) ctx, cancel := testhelper.Context() defer cancel() diff --git a/internal/gitaly/service/smarthttp/upload_pack_test.go b/internal/gitaly/service/smarthttp/upload_pack_test.go index a2aa66601..f640a0cd4 100644 --- a/internal/gitaly/service/smarthttp/upload_pack_test.go +++ b/internal/gitaly/service/smarthttp/upload_pack_test.go @@ -172,8 +172,7 @@ func TestUploadPackRequestWithGitProtocol(t *testing.T) { defer cancel() cfg, repo, _ := testcfg.BuildWithRepo(t) - readProto, cfg, restore := gittest.EnableGitProtocolV2Support(t, cfg) - defer restore() + readProto, cfg := gittest.EnableGitProtocolV2Support(t, cfg) serverSocketPath, stop := runSmartHTTPServer(t, cfg) defer stop() @@ -249,8 +248,7 @@ func TestUploadPackWithPackObjectsHook(t *testing.T) { // out. In the best case we'd have just printed the error to stderr and // check the return error message. But it's unfortunately not // transferred back. - cleanup = testhelper.WriteExecutable(t, filepath.Join(cfg.BinDir, "gitaly-hooks"), []byte(hookScript)) - defer cleanup() + testhelper.WriteExecutable(t, filepath.Join(cfg.BinDir, "gitaly-hooks"), []byte(hookScript)) serverSocketPath, stop := runSmartHTTPServer(t, cfg) defer stop() diff --git a/internal/gitaly/service/ssh/receive_pack_test.go b/internal/gitaly/service/ssh/receive_pack_test.go index 86a98fe06..48aaccad2 100644 --- a/internal/gitaly/service/ssh/receive_pack_test.go +++ b/internal/gitaly/service/ssh/receive_pack_test.go @@ -147,8 +147,7 @@ func TestReceivePackPushSuccessWithGitProtocol(t *testing.T) { testhelper.ConfigureGitalySSHBin(t, cfg) testhelper.ConfigureGitalyHooksBin(t, cfg) - readProto, cfg, restore := gittest.EnableGitProtocolV2Support(t, cfg) - defer restore() + readProto, cfg := gittest.EnableGitProtocolV2Support(t, cfg) serverSocketPath, stop := runSSHServer(t, cfg) defer stop() @@ -262,8 +261,7 @@ func TestSSHReceivePackToHooks(t *testing.T) { glID = "key-123" ) - readProto, cfg, restore := gittest.EnableGitProtocolV2Support(t, cfg) - defer restore() + readProto, cfg := gittest.EnableGitProtocolV2Support(t, cfg) tempGitlabShellDir, cleanup := testhelper.TempDir(t) defer cleanup() @@ -290,8 +288,7 @@ func TestSSHReceivePackToHooks(t *testing.T) { cfg.Gitlab.URL = serverURL cfg.Gitlab.SecretFile = filepath.Join(tempGitlabShellDir, ".gitlab_shell_secret") - cleanup = gittest.WriteCheckNewObjectExistsHook(t, cfg.Git.BinPath, cloneDetails.RemoteRepoPath) - defer cleanup() + gittest.WriteCheckNewObjectExistsHook(t, cfg.Git.BinPath, cloneDetails.RemoteRepoPath) serverSocketPath, stop := runSSHServer(t, cfg) defer stop() diff --git a/internal/gitaly/service/ssh/upload_pack_test.go b/internal/gitaly/service/ssh/upload_pack_test.go index c3d015a6e..ff3a007aa 100644 --- a/internal/gitaly/service/ssh/upload_pack_test.go +++ b/internal/gitaly/service/ssh/upload_pack_test.go @@ -426,8 +426,7 @@ func TestUploadPackCloneSuccessWithGitProtocol(t *testing.T) { for _, tc := range tests { t.Run(tc.desc, func(t *testing.T) { - readProto, cfg, restore := gittest.EnableGitProtocolV2Support(t, cfg) - defer restore() + readProto, cfg := gittest.EnableGitProtocolV2Support(t, cfg) serverSocketPath, stop := runSSHServer(t, cfg) defer stop() diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go index f0b06217e..7bb67e36f 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -277,15 +277,15 @@ func TempDir(t testing.TB) (string, func()) { type Cleanup func() // WriteExecutable ensures that the parent directory exists, and writes an executable with provided content -func WriteExecutable(t testing.TB, path string, content []byte) func() { +func WriteExecutable(t testing.TB, path string, content []byte) { dir := filepath.Dir(path) require.NoError(t, os.MkdirAll(dir, 0755)) require.NoError(t, ioutil.WriteFile(path, content, 0755)) - return func() { + t.Cleanup(func() { assert.NoError(t, os.RemoveAll(dir)) - } + }) } // ModifyEnvironment will change an environment variable and return a func suitable |