diff options
author | Pavlo Strokov <pstrokov@gitlab.com> | 2021-10-07 11:31:25 +0300 |
---|---|---|
committer | Pavlo Strokov <pstrokov@gitlab.com> | 2021-10-07 11:31:25 +0300 |
commit | 043bb6e66fa839296d76d5105756a005b2613808 (patch) | |
tree | c090b62ec8f4e2b1b1ae1b53f43975b24fb79093 /internal/git | |
parent | efb14ad0a5a0b23ebd96d3216c07c135258fd24e (diff) | |
parent | 9ef46dc05bbe2ef39838ec973a2e8cf01252d919 (diff) |
Merge branch 'pks-testhelper-goroutine-leakage' into 'master'
testhelper: Enable Goroutine leak checks by default
Closes #3790
See merge request gitlab-org/gitaly!3909
Diffstat (limited to 'internal/git')
21 files changed, 55 insertions, 126 deletions
diff --git a/internal/git/catfile/testhelper_test.go b/internal/git/catfile/testhelper_test.go index 4479adb85..fa977e099 100644 --- a/internal/git/catfile/testhelper_test.go +++ b/internal/git/catfile/testhelper_test.go @@ -2,7 +2,6 @@ package catfile import ( "context" - "os" "testing" "gitlab.com/gitlab-org/gitaly/v14/internal/command" @@ -13,18 +12,7 @@ import ( ) func TestMain(m *testing.M) { - os.Exit(testMain(m)) -} - -func testMain(m *testing.M) int { - defer func() { - testhelper.MustHaveNoChildProcess() - testhelper.MustHaveNoGoroutines() - }() - - cleanup := testhelper.Configure() - defer cleanup() - return m.Run() + testhelper.Run(m) } type repoExecutor struct { diff --git a/internal/git/gitpipe/testhelper_test.go b/internal/git/gitpipe/testhelper_test.go index b3683135f..8b2532840 100644 --- a/internal/git/gitpipe/testhelper_test.go +++ b/internal/git/gitpipe/testhelper_test.go @@ -1,21 +1,11 @@ package gitpipe import ( - "os" "testing" "gitlab.com/gitlab-org/gitaly/v14/internal/testhelper" ) func TestMain(m *testing.M) { - os.Exit(testMain(m)) -} - -func testMain(m *testing.M) int { - defer testhelper.MustHaveNoChildProcess() - - cleanup := testhelper.Configure() - defer cleanup() - - return m.Run() + testhelper.Run(m) } diff --git a/internal/git/gittest/commit_test.go b/internal/git/gittest/commit_test.go index e96520a14..c3ad815ce 100644 --- a/internal/git/gittest/commit_test.go +++ b/internal/git/gittest/commit_test.go @@ -19,6 +19,8 @@ func TestWriteCommit(t *testing.T) { defer cancel() batchCache := catfile.NewCache(cfg) + defer batchCache.Stop() + batch, err := batchCache.BatchProcess(ctx, repo) require.NoError(t, err) diff --git a/internal/git/gittest/testhelper_test.go b/internal/git/gittest/testhelper_test.go index 89d810164..a16e984f6 100644 --- a/internal/git/gittest/testhelper_test.go +++ b/internal/git/gittest/testhelper_test.go @@ -12,14 +12,7 @@ import ( ) func TestMain(m *testing.M) { - os.Exit(testMain(m)) -} - -func testMain(m *testing.M) int { - defer testhelper.MustHaveNoChildProcess() - cleanup := testhelper.Configure() - defer cleanup() - return m.Run() + testhelper.Run(m) } // setup sets up a test configuration and repository. Ideally we'd use our central test helpers to diff --git a/internal/git/helper_test.go b/internal/git/helper_test.go index eb7e925fd..f76065121 100644 --- a/internal/git/helper_test.go +++ b/internal/git/helper_test.go @@ -1,7 +1,6 @@ package git import ( - "os" "testing" "github.com/stretchr/testify/require" @@ -9,14 +8,7 @@ import ( ) func TestMain(m *testing.M) { - os.Exit(testMain(m)) -} - -func testMain(m *testing.M) int { - defer testhelper.MustHaveNoChildProcess() - cleanup := testhelper.Configure() - defer cleanup() - return m.Run() + testhelper.Run(m) } func TestValidateRevision(t *testing.T) { diff --git a/internal/git/housekeeping/testhelper_test.go b/internal/git/housekeeping/testhelper_test.go index 05d4da7f7..f0afe0397 100644 --- a/internal/git/housekeeping/testhelper_test.go +++ b/internal/git/housekeeping/testhelper_test.go @@ -1,18 +1,11 @@ package housekeeping import ( - "os" "testing" "gitlab.com/gitlab-org/gitaly/v14/internal/testhelper" ) func TestMain(m *testing.M) { - os.Exit(testMain(m)) -} - -func testMain(m *testing.M) int { - cleanup := testhelper.Configure() - defer cleanup() - return m.Run() + testhelper.Run(m) } diff --git a/internal/git/localrepo/config_test.go b/internal/git/localrepo/config_test.go index 15c440524..7abc7be08 100644 --- a/internal/git/localrepo/config_test.go +++ b/internal/git/localrepo/config_test.go @@ -31,7 +31,10 @@ func setupRepoConfig(t *testing.T) (Config, string) { repoProto, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0]) gitCmdFactory := git.NewExecCommandFactory(cfg) - repo := New(gitCmdFactory, catfile.NewCache(cfg), repoProto, cfg) + catfileCache := catfile.NewCache(cfg) + t.Cleanup(catfileCache.Stop) + + repo := New(gitCmdFactory, catfileCache, repoProto, cfg) return repo.Config(), repoPath } diff --git a/internal/git/localrepo/objects_test.go b/internal/git/localrepo/objects_test.go index d1d797955..53c420d6b 100644 --- a/internal/git/localrepo/objects_test.go +++ b/internal/git/localrepo/objects_test.go @@ -35,7 +35,9 @@ func setupRepo(t *testing.T, bare bool) (*Repo, string) { } gitCmdFactory := git.NewExecCommandFactory(cfg) - return New(gitCmdFactory, catfile.NewCache(cfg), repoProto, cfg), repoPath + catfileCache := catfile.NewCache(cfg) + t.Cleanup(catfileCache.Stop) + return New(gitCmdFactory, catfileCache, repoProto, cfg), repoPath } type ReaderFunc func([]byte) (int, error) diff --git a/internal/git/localrepo/refs_test.go b/internal/git/localrepo/refs_test.go index 3d3fdcd32..6bdc9a7e1 100644 --- a/internal/git/localrepo/refs_test.go +++ b/internal/git/localrepo/refs_test.go @@ -242,9 +242,12 @@ func TestRepo_GetRemoteReferences(t *testing.T) { annotatedTagOID := text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "annotated-tag")) gitCmdFactory := git.NewExecCommandFactory(cfg) + catfileCache := catfile.NewCache(cfg) + defer catfileCache.Stop() + repo := New( gitCmdFactory, - catfile.NewCache(cfg), + catfileCache, &gitalypb.Repository{StorageName: "default", RelativePath: filepath.Join(relativePath, ".git")}, cfg, ) diff --git a/internal/git/localrepo/remote_test.go b/internal/git/localrepo/remote_test.go index 6b165f0c9..547f0cf90 100644 --- a/internal/git/localrepo/remote_test.go +++ b/internal/git/localrepo/remote_test.go @@ -37,7 +37,10 @@ func setupRepoRemote(t *testing.T, bare bool) (Remote, string) { } gitCmdFactory := git.NewExecCommandFactory(cfg) - return New(gitCmdFactory, catfile.NewCache(cfg), repoProto, cfg).Remote(), repoPath + catfileCache := catfile.NewCache(cfg) + t.Cleanup(catfileCache.Stop) + + return New(gitCmdFactory, catfileCache, repoProto, cfg).Remote(), repoPath } func TestRepo_Remote(t *testing.T) { diff --git a/internal/git/localrepo/repo.go b/internal/git/localrepo/repo.go index 08c996b18..af869a6bb 100644 --- a/internal/git/localrepo/repo.go +++ b/internal/git/localrepo/repo.go @@ -37,7 +37,9 @@ func New(gitCmdFactory git.CommandFactory, catfileCache catfile.Cache, repo repo // dependencies ad-hoc from the given config. func NewTestRepo(t testing.TB, cfg config.Cfg, repo repository.GitRepo) *Repo { gitCmdFactory := git.NewExecCommandFactory(cfg) - return New(gitCmdFactory, catfile.NewCache(cfg), repo, cfg) + catfileCache := catfile.NewCache(cfg) + t.Cleanup(catfileCache.Stop) + return New(gitCmdFactory, catfileCache, repo, cfg) } // Path returns the on-disk path of the repository. diff --git a/internal/git/localrepo/repo_test.go b/internal/git/localrepo/repo_test.go index 8993471e0..c456e21b3 100644 --- a/internal/git/localrepo/repo_test.go +++ b/internal/git/localrepo/repo_test.go @@ -20,7 +20,9 @@ func TestRepo(t *testing.T) { gittest.TestRepository(t, cfg, func(t testing.TB, pbRepo *gitalypb.Repository) git.Repository { t.Helper() gitCmdFactory := git.NewExecCommandFactory(cfg) - return New(gitCmdFactory, catfile.NewCache(cfg), pbRepo, cfg) + catfileCache := catfile.NewCache(cfg) + t.Cleanup(catfileCache.Stop) + return New(gitCmdFactory, catfileCache, pbRepo, cfg) }) } @@ -28,7 +30,9 @@ func TestRepo_Path(t *testing.T) { t.Run("valid repository", func(t *testing.T) { cfg, repoProto, repoPath := testcfg.BuildWithRepo(t) gitCmdFactory := git.NewExecCommandFactory(cfg) - repo := New(gitCmdFactory, catfile.NewCache(cfg), repoProto, cfg) + catfileCache := catfile.NewCache(cfg) + t.Cleanup(catfileCache.Stop) + repo := New(gitCmdFactory, catfileCache, repoProto, cfg) path, err := repo.Path() require.NoError(t, err) @@ -38,7 +42,9 @@ func TestRepo_Path(t *testing.T) { t.Run("deleted repository", func(t *testing.T) { cfg, repoProto, repoPath := testcfg.BuildWithRepo(t) gitCmdFactory := git.NewExecCommandFactory(cfg) - repo := New(gitCmdFactory, catfile.NewCache(cfg), repoProto, cfg) + catfileCache := catfile.NewCache(cfg) + t.Cleanup(catfileCache.Stop) + repo := New(gitCmdFactory, catfileCache, repoProto, cfg) require.NoError(t, os.RemoveAll(repoPath)) @@ -49,7 +55,9 @@ func TestRepo_Path(t *testing.T) { t.Run("non-git repository", func(t *testing.T) { cfg, repoProto, repoPath := testcfg.BuildWithRepo(t) gitCmdFactory := git.NewExecCommandFactory(cfg) - repo := New(gitCmdFactory, catfile.NewCache(cfg), repoProto, cfg) + catfileCache := catfile.NewCache(cfg) + t.Cleanup(catfileCache.Stop) + repo := New(gitCmdFactory, catfileCache, repoProto, cfg) // Recreate the repository as a simple empty directory to simulate // that the repository is in a partially-created state. diff --git a/internal/git/localrepo/testhelper_test.go b/internal/git/localrepo/testhelper_test.go index 224d1566b..38486d705 100644 --- a/internal/git/localrepo/testhelper_test.go +++ b/internal/git/localrepo/testhelper_test.go @@ -1,19 +1,11 @@ package localrepo import ( - "os" "testing" "gitlab.com/gitlab-org/gitaly/v14/internal/testhelper" ) func TestMain(m *testing.M) { - os.Exit(testMain(m)) -} - -func testMain(m *testing.M) int { - defer testhelper.MustHaveNoChildProcess() - cleanup := testhelper.Configure() - defer cleanup() - return m.Run() + testhelper.Run(m) } diff --git a/internal/git/objectpool/clone_test.go b/internal/git/objectpool/clone_test.go index 0720170a0..f3689ef24 100644 --- a/internal/git/objectpool/clone_test.go +++ b/internal/git/objectpool/clone_test.go @@ -23,11 +23,14 @@ func setupObjectPool(t *testing.T) (*ObjectPool, *gitalypb.Repository) { cfg, repo, _ := testcfg.BuildWithRepo(t) gitCommandFactory := git.NewExecCommandFactory(cfg) + catfileCache := catfile.NewCache(cfg) + t.Cleanup(catfileCache.Stop) + pool, err := NewObjectPool( cfg, config.NewLocator(cfg), gitCommandFactory, - catfile.NewCache(cfg), + catfileCache, transaction.NewManager(cfg, backchannel.NewRegistry()), repo.GetStorageName(), gittest.NewObjectPoolName(t), diff --git a/internal/git/objectpool/testhelper_test.go b/internal/git/objectpool/testhelper_test.go index b252b37c1..bf79617d8 100644 --- a/internal/git/objectpool/testhelper_test.go +++ b/internal/git/objectpool/testhelper_test.go @@ -1,7 +1,6 @@ package objectpool import ( - "os" "testing" "gitlab.com/gitlab-org/gitaly/v14/internal/git/hooks" @@ -9,13 +8,8 @@ import ( ) func TestMain(m *testing.M) { - os.Exit(testMain(m)) -} - -func testMain(m *testing.M) int { - defer testhelper.MustHaveNoChildProcess() - cleanup := testhelper.Configure() - defer cleanup() - hooks.Override = "/" - return m.Run() + testhelper.Run(m, testhelper.WithSetup(func() error { + hooks.Override = "/" + return nil + })) } diff --git a/internal/git/packfile/packfile_test.go b/internal/git/packfile/packfile_test.go index c8d412827..ecf56563b 100644 --- a/internal/git/packfile/packfile_test.go +++ b/internal/git/packfile/packfile_test.go @@ -1,7 +1,6 @@ package packfile_test import ( - "os" "path/filepath" "testing" @@ -13,14 +12,7 @@ import ( ) func TestMain(m *testing.M) { - os.Exit(testMain(m)) -} - -func testMain(m *testing.M) int { - defer testhelper.MustHaveNoChildProcess() - cleanup := testhelper.Configure() - defer cleanup() - return m.Run() + testhelper.Run(m) } func TestList(t *testing.T) { diff --git a/internal/git/quarantine/testhelper_test.go b/internal/git/quarantine/testhelper_test.go index 7ad131635..38d896d42 100644 --- a/internal/git/quarantine/testhelper_test.go +++ b/internal/git/quarantine/testhelper_test.go @@ -1,21 +1,11 @@ package quarantine import ( - "os" "testing" "gitlab.com/gitlab-org/gitaly/v14/internal/testhelper" ) func TestMain(m *testing.M) { - os.Exit(testMain(m)) -} - -func testMain(m *testing.M) int { - defer testhelper.MustHaveNoChildProcess() - - cleanup := testhelper.Configure() - defer cleanup() - - return m.Run() + testhelper.Run(m) } diff --git a/internal/git/remoterepo/helper_test.go b/internal/git/remoterepo/helper_test.go index 555b5445d..619ff5b16 100644 --- a/internal/git/remoterepo/helper_test.go +++ b/internal/git/remoterepo/helper_test.go @@ -1,19 +1,11 @@ package remoterepo import ( - "os" "testing" "gitlab.com/gitlab-org/gitaly/v14/internal/testhelper" ) func TestMain(m *testing.M) { - os.Exit(testMain(m)) -} - -func testMain(m *testing.M) int { - defer testhelper.MustHaveNoChildProcess() - cleanup := testhelper.Configure() - defer cleanup() - return m.Run() + testhelper.Run(m) } diff --git a/internal/git/remoterepo/repository_test.go b/internal/git/remoterepo/repository_test.go index ddc63fb11..5bb233fdc 100644 --- a/internal/git/remoterepo/repository_test.go +++ b/internal/git/remoterepo/repository_test.go @@ -32,6 +32,7 @@ func TestRepository(t *testing.T) { deps.GetTxManager(), deps.GetGitCmdFactory(), deps.GetCatfileCache(), + deps.GetConnsPool(), )) gitalypb.RegisterCommitServiceServer(srv, commit.NewServer( deps.GetCfg(), diff --git a/internal/git/stats/testhelper_test.go b/internal/git/stats/testhelper_test.go index ce080ff27..b842dc949 100644 --- a/internal/git/stats/testhelper_test.go +++ b/internal/git/stats/testhelper_test.go @@ -1,19 +1,11 @@ package stats import ( - "os" "testing" "gitlab.com/gitlab-org/gitaly/v14/internal/testhelper" ) func TestMain(m *testing.M) { - os.Exit(testMain(m)) -} - -func testMain(m *testing.M) int { - defer testhelper.MustHaveNoChildProcess() - cleanup := testhelper.Configure() - defer cleanup() - return m.Run() + testhelper.Run(m) } diff --git a/internal/git/updateref/updateref_test.go b/internal/git/updateref/updateref_test.go index c2cf6c0d6..17c87fd16 100644 --- a/internal/git/updateref/updateref_test.go +++ b/internal/git/updateref/updateref_test.go @@ -3,7 +3,6 @@ package updateref import ( "context" "fmt" - "os" "strings" "testing" @@ -17,15 +16,10 @@ import ( ) func TestMain(m *testing.M) { - os.Exit(testMain(m)) -} - -func testMain(m *testing.M) int { - defer testhelper.MustHaveNoChildProcess() - cleanup := testhelper.Configure() - defer cleanup() - hooks.Override = "/" - return m.Run() + testhelper.Run(m, testhelper.WithSetup(func() error { + hooks.Override = "/" + return nil + })) } func setupUpdater(t *testing.T, ctx context.Context) (config.Cfg, *localrepo.Repo, *Updater) { |