From 1d4667c825bb5d291070bd4463c39a16f950f674 Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Thu, 16 Jun 2022 09:02:52 -0400 Subject: [PATCH] refactor(809): eliminate need to defer CleanupTestRepos() --- api/graphql/graphql_test.go | 3 +-- api/http/git_file_handlers_test.go | 3 +-- bridge/github/export_test.go | 6 ++--- bridge/github/import_integration_test.go | 3 +-- bridge/github/import_test.go | 3 +-- bridge/gitlab/export_test.go | 6 ++--- bridge/gitlab/import_test.go | 3 +-- bug/operation_test.go | 3 +-- cache/repo_cache_test.go | 18 ++++++-------- commands/env_testing.go | 2 ++ commands/select/select_test.go | 3 +-- entity/dag/common_test.go | 6 ++--- entity/dag/entity_actions_test.go | 12 +++------ identity/identity_actions_test.go | 3 +-- identity/identity_test.go | 7 +++--- repository/gogit_test.go | 2 +- repository/gogit_testing.go | 31 ++++++++++++++++-------- repository/mock_repo_test.go | 5 ++-- repository/repo_testing.go | 28 +++------------------ tests/read_bugs_test.go | 6 ++--- 20 files changed, 60 insertions(+), 93 deletions(-) diff --git a/api/graphql/graphql_test.go b/api/graphql/graphql_test.go index 69d96cab7a5c69e7046406c11c177d69b6d560a4..350e489a3829c1573c145ea86fe435c3616cf78b 100644 --- a/api/graphql/graphql_test.go +++ b/api/graphql/graphql_test.go @@ -14,8 +14,7 @@ import ( ) func TestQueries(t *testing.T) { - repo := repository.CreateGoGitTestRepo(false) - defer repository.CleanupTestRepos(repo) + repo := repository.CreateGoGitTestRepo(t, false) random_bugs.FillRepoWithSeed(repo, 10, 42) diff --git a/api/http/git_file_handlers_test.go b/api/http/git_file_handlers_test.go index 68c1542fdbfa96970b2ef7fd6b2307d0256d96f0..736bf75ec2370b0aec486636b303fe3d29570e9b 100644 --- a/api/http/git_file_handlers_test.go +++ b/api/http/git_file_handlers_test.go @@ -19,8 +19,7 @@ import ( ) func TestGitFileHandlers(t *testing.T) { - repo := repository.CreateGoGitTestRepo(false) - defer repository.CleanupTestRepos(repo) + repo := repository.CreateGoGitTestRepo(t, false) mrc := cache.NewMultiRepoCache() repoCache, err := mrc.RegisterDefaultRepository(repo) diff --git a/bridge/github/export_test.go b/bridge/github/export_test.go index 1d1398a9c6b1c2fe473776e2d828ae22a873e500..94664853d42b277132742b8356608b914e02afb2 100644 --- a/bridge/github/export_test.go +++ b/bridge/github/export_test.go @@ -137,8 +137,7 @@ func TestGithubPushPull(t *testing.T) { } // create repo backend - repo := repository.CreateGoGitTestRepo(false) - defer repository.CleanupTestRepos(repo) + repo := repository.CreateGoGitTestRepo(t, false) backend, err := cache.NewRepoCache(repo) require.NoError(t, err) @@ -213,8 +212,7 @@ func TestGithubPushPull(t *testing.T) { fmt.Printf("test repository exported in %f seconds\n", time.Since(start).Seconds()) - repoTwo := repository.CreateGoGitTestRepo(false) - defer repository.CleanupTestRepos(repoTwo) + repoTwo := repository.CreateGoGitTestRepo(t, false) // create a second backend backendTwo, err := cache.NewRepoCache(repoTwo) diff --git a/bridge/github/import_integration_test.go b/bridge/github/import_integration_test.go index 3349f3f5e3b5aa897ab2da4aea7fa9abf7ef71f0..b969f6bd73b8dbb2d89084829f747b6e222ede55 100644 --- a/bridge/github/import_integration_test.go +++ b/bridge/github/import_integration_test.go @@ -32,8 +32,7 @@ func TestGithubImporterIntegration(t *testing.T) { importer.client = &rateLimitHandlerClient{sc: clientMock} // arrange - repo := repository.CreateGoGitTestRepo(false) - defer repository.CleanupTestRepos(repo) + repo := repository.CreateGoGitTestRepo(t, false) backend, err := cache.NewRepoCache(repo) require.NoError(t, err) defer backend.Close() diff --git a/bridge/github/import_test.go b/bridge/github/import_test.go index 324d54211538935236b2db6beb3deb9d65c6b71e..df4989a48f9f179a34786a2d5982cf55eafddd87 100644 --- a/bridge/github/import_test.go +++ b/bridge/github/import_test.go @@ -24,8 +24,7 @@ func TestGithubImporter(t *testing.T) { t.Skip("Env var GITHUB_TOKEN_PRIVATE missing") } - repo := repository.CreateGoGitTestRepo(false) - defer repository.CleanupTestRepos(repo) + repo := repository.CreateGoGitTestRepo(t, false) backend, err := cache.NewRepoCache(repo) require.NoError(t, err) diff --git a/bridge/gitlab/export_test.go b/bridge/gitlab/export_test.go index 422366c1118d656eb788675fed282b3195bab9c3..cca4c6ca5975b1e31ffe8e1755e3690c6cf64831 100644 --- a/bridge/gitlab/export_test.go +++ b/bridge/gitlab/export_test.go @@ -142,8 +142,7 @@ func TestGitlabPushPull(t *testing.T) { } // create repo backend - repo := repository.CreateGoGitTestRepo(false) - defer repository.CleanupTestRepos(repo) + repo := repository.CreateGoGitTestRepo(t, false) backend, err := cache.NewRepoCache(repo) require.NoError(t, err) @@ -215,8 +214,7 @@ func TestGitlabPushPull(t *testing.T) { fmt.Printf("test repository exported in %f seconds\n", time.Since(start).Seconds()) - repoTwo := repository.CreateGoGitTestRepo(false) - defer repository.CleanupTestRepos(repoTwo) + repoTwo := repository.CreateGoGitTestRepo(t, false) // create a second backend backendTwo, err := cache.NewRepoCache(repoTwo) diff --git a/bridge/gitlab/import_test.go b/bridge/gitlab/import_test.go index 1405e43bba6101eee5b7a2dd4bfcf9c2503f6a3e..676e87497e88a3d8278c4de17eb92d329422ff49 100644 --- a/bridge/gitlab/import_test.go +++ b/bridge/gitlab/import_test.go @@ -29,8 +29,7 @@ func TestGitlabImport(t *testing.T) { t.Skip("Env var GITLAB_PROJECT_ID missing") } - repo := repository.CreateGoGitTestRepo(false) - defer repository.CleanupTestRepos(repo) + repo := repository.CreateGoGitTestRepo(t, false) backend, err := cache.NewRepoCache(repo) require.NoError(t, err) diff --git a/bug/operation_test.go b/bug/operation_test.go index 619f2b43785060238cd7a41aca14c0c9b23133f8..c57a1591da49bf3d50f56f0c223b189c08a2b029 100644 --- a/bug/operation_test.go +++ b/bug/operation_test.go @@ -92,8 +92,7 @@ func TestMetadata(t *testing.T) { } func TestID(t *testing.T) { - repo := repository.CreateGoGitTestRepo(false) - defer repository.CleanupTestRepos(repo) + repo := repository.CreateGoGitTestRepo(t, false) repos := []repository.ClockedRepo{ repository.NewMockRepo(), diff --git a/cache/repo_cache_test.go b/cache/repo_cache_test.go index d13fa026048726ddc3826ca440af90dfa2c06791..c01f5478d7d9260af062ffcbb2c84ffe3f2c8f32 100644 --- a/cache/repo_cache_test.go +++ b/cache/repo_cache_test.go @@ -14,8 +14,7 @@ import ( ) func TestCache(t *testing.T) { - repo := repository.CreateGoGitTestRepo(false) - defer repository.CleanupTestRepos(repo) + repo := repository.CreateGoGitTestRepo(t, false) cache, err := NewRepoCache(repo) require.NoError(t, err) @@ -112,8 +111,7 @@ func TestCache(t *testing.T) { } func TestCachePushPull(t *testing.T) { - repoA, repoB, remote := repository.SetupGoGitReposAndRemote() - defer repository.CleanupTestRepos(repoA, repoB, remote) + repoA, repoB, _ := repository.SetupGoGitReposAndRemote(t) cacheA, err := NewRepoCache(repoA) require.NoError(t, err) @@ -171,10 +169,9 @@ func TestCachePushPull(t *testing.T) { } func TestRemove(t *testing.T) { - repo := repository.CreateGoGitTestRepo(false) - remoteA := repository.CreateGoGitTestRepo(true) - remoteB := repository.CreateGoGitTestRepo(true) - defer repository.CleanupTestRepos(repo, remoteA, remoteB) + repo := repository.CreateGoGitTestRepo(t, false) + remoteA := repository.CreateGoGitTestRepo(t, true) + remoteB := repository.CreateGoGitTestRepo(t, true) err := repo.AddRemote("remoteA", remoteA.GetLocalRemote()) require.NoError(t, err) @@ -220,7 +217,7 @@ func TestRemove(t *testing.T) { } func TestCacheEviction(t *testing.T) { - repo := repository.CreateGoGitTestRepo(false) + repo := repository.CreateGoGitTestRepo(t, false) repoCache, err := NewRepoCache(repo) require.NoError(t, err) repoCache.setCacheSize(2) @@ -287,8 +284,7 @@ func TestLongDescription(t *testing.T) { text := strings.Repeat("x", 65536) - repo := repository.CreateGoGitTestRepo(false) - defer repository.CleanupTestRepos(repo) + repo := repository.CreateGoGitTestRepo(t, false) backend, err := NewRepoCache(repo) require.NoError(t, err) diff --git a/commands/env_testing.go b/commands/env_testing.go index 4de66a9db3757d72d541b87d3ff40c493dc5c662..b3c51c69370ffb80f360e6c30fad29084e4c1247 100644 --- a/commands/env_testing.go +++ b/commands/env_testing.go @@ -21,6 +21,8 @@ func newTestEnv(t *testing.T) *testEnv { cwd := t.TempDir() + // r := repository.CreateGoGitTestRepo(t, false) // TODO + repo, err := repository.InitGoGitRepo(cwd, gitBugNamespace) require.NoError(t, err) t.Cleanup(func() { diff --git a/commands/select/select_test.go b/commands/select/select_test.go index 488ab35759726892362e7242fe66d3a8d7d231f9..702700f40f08c0345516bf805250345cfc3418d2 100644 --- a/commands/select/select_test.go +++ b/commands/select/select_test.go @@ -11,8 +11,7 @@ import ( ) func TestSelect(t *testing.T) { - repo := repository.CreateGoGitTestRepo(false) - defer repository.CleanupTestRepos(repo) + repo := repository.CreateGoGitTestRepo(t, false) repoCache, err := cache.NewRepoCache(repo) require.NoError(t, err) diff --git a/entity/dag/common_test.go b/entity/dag/common_test.go index 532e3ff8fe1a6c88f7d851a2d83ed7dce6dfb034..c2177683974caa40050d11b031221ed91a2bbaa1 100644 --- a/entity/dag/common_test.go +++ b/entity/dag/common_test.go @@ -103,9 +103,9 @@ func makeTestContext() (repository.ClockedRepo, identity.Interface, identity.Int } func makeTestContextRemote(t *testing.T) (repository.ClockedRepo, repository.ClockedRepo, repository.ClockedRepo, identity.Interface, identity.Interface, identity.Resolver, Definition) { - repoA := repository.CreateGoGitTestRepo(false) - repoB := repository.CreateGoGitTestRepo(false) - remote := repository.CreateGoGitTestRepo(true) + repoA := repository.CreateGoGitTestRepo(t, false) + repoB := repository.CreateGoGitTestRepo(t, false) + remote := repository.CreateGoGitTestRepo(t, true) err := repoA.AddRemote("remote", remote.GetLocalRemote()) require.NoError(t, err) diff --git a/entity/dag/entity_actions_test.go b/entity/dag/entity_actions_test.go index 45e69c7d9b3067248ce0241c85141e700e06de8e..68aa59b8de79077d2ab76bb2cf8ae602fc968f61 100644 --- a/entity/dag/entity_actions_test.go +++ b/entity/dag/entity_actions_test.go @@ -24,8 +24,7 @@ func allEntities(t testing.TB, bugs <-chan StreamedEntity) []*Entity { } func TestEntityPushPull(t *testing.T) { - repoA, repoB, remote, id1, id2, resolver, def := makeTestContextRemote(t) - defer repository.CleanupTestRepos(repoA, repoB, remote) + repoA, repoB, _, id1, id2, resolver, def := makeTestContextRemote(t) // A --> remote --> B e := New(def) @@ -61,8 +60,7 @@ func TestEntityPushPull(t *testing.T) { } func TestListLocalIds(t *testing.T) { - repoA, repoB, remote, id1, id2, resolver, def := makeTestContextRemote(t) - defer repository.CleanupTestRepos(repoA, repoB, remote) + repoA, repoB, _, id1, id2, resolver, def := makeTestContextRemote(t) // A --> remote --> B e := New(def) @@ -206,8 +204,7 @@ func assertNotEqualRefs(t *testing.T, repoA, repoB repository.RepoData, prefix s } func TestMerge(t *testing.T) { - repoA, repoB, remote, id1, id2, resolver, def := makeTestContextRemote(t) - defer repository.CleanupTestRepos(repoA, repoB, remote) + repoA, repoB, _, id1, id2, resolver, def := makeTestContextRemote(t) // SCENARIO 1 // if the remote Entity doesn't exist locally, it's created @@ -387,8 +384,7 @@ func TestMerge(t *testing.T) { } func TestRemove(t *testing.T) { - repoA, repoB, remote, id1, _, resolver, def := makeTestContextRemote(t) - defer repository.CleanupTestRepos(repoA, repoB, remote) + repoA, _, _, id1, _, resolver, def := makeTestContextRemote(t) e := New(def) e.Append(newOp1(id1, "foo")) diff --git a/identity/identity_actions_test.go b/identity/identity_actions_test.go index 2a5954d6af84983a5b67909da9d5ef93d85ff14a..351fb7a47dc62e7615445a0b142c53662bd51af7 100644 --- a/identity/identity_actions_test.go +++ b/identity/identity_actions_test.go @@ -9,8 +9,7 @@ import ( ) func TestIdentityPushPull(t *testing.T) { - repoA, repoB, remote := repository.SetupGoGitReposAndRemote() - defer repository.CleanupTestRepos(repoA, repoB, remote) + repoA, repoB, _ := repository.SetupGoGitReposAndRemote(t) identity1, err := NewIdentity(repoA, "name1", "email1") require.NoError(t, err) diff --git a/identity/identity_test.go b/identity/identity_test.go index 2cdb4b3653c38a620d1475bee8c972a9034148d4..f0c3bbe969e6d3264c69331a424cc32842cc1505 100644 --- a/identity/identity_test.go +++ b/identity/identity_test.go @@ -245,10 +245,9 @@ func TestJSON(t *testing.T) { } func TestIdentityRemove(t *testing.T) { - repo := repository.CreateGoGitTestRepo(false) - remoteA := repository.CreateGoGitTestRepo(true) - remoteB := repository.CreateGoGitTestRepo(true) - defer repository.CleanupTestRepos(repo, remoteA, remoteB) + repo := repository.CreateGoGitTestRepo(t, false) + remoteA := repository.CreateGoGitTestRepo(t, true) + remoteB := repository.CreateGoGitTestRepo(t, true) err := repo.AddRemote("remoteA", remoteA.GetLocalRemote()) require.NoError(t, err) diff --git a/repository/gogit_test.go b/repository/gogit_test.go index c376de221dc305ad76ea87936393bc719cabf3dd..8179874cbed7934344c6770258522a5369440caf 100644 --- a/repository/gogit_test.go +++ b/repository/gogit_test.go @@ -71,7 +71,7 @@ func TestNewGoGitRepo(t *testing.T) { } func TestGoGitRepo(t *testing.T) { - RepoTest(t, CreateGoGitTestRepo, CleanupTestRepos) + RepoTest(t, CreateGoGitTestRepo) } func TestGoGitRepo_Indexes(t *testing.T) { diff --git a/repository/gogit_testing.go b/repository/gogit_testing.go index 7647c71162edc61221f7a28b17c1e05dc8d37f28..1b39fe5ca8a7c88dc052225c023586a44a879210 100644 --- a/repository/gogit_testing.go +++ b/repository/gogit_testing.go @@ -1,21 +1,26 @@ package repository import ( - "io/ioutil" "log" + "testing" "github.com/99designs/keyring" ) +type TestingT interface { + Cleanup(func()) + Helper() + TempDir() string +} + const namespace = "git-bug" // This is intended for testing only -func CreateGoGitTestRepo(bare bool) TestedRepo { - dir, err := ioutil.TempDir("", "") - if err != nil { - log.Fatal(err) - } +func CreateGoGitTestRepo(t TestingT, bare bool) TestedRepo { + t.Helper() + + dir := t.TempDir() var creator func(string, string) (*GoGitRepo, error) @@ -30,6 +35,10 @@ func CreateGoGitTestRepo(bare bool) TestedRepo { log.Fatal(err) } + t.Cleanup(func() { + repo.Close() + }) + config := repo.LocalConfig() if err := config.StoreString("user.name", "testuser"); err != nil { log.Fatal("failed to set user.name for test repository: ", err) @@ -45,10 +54,12 @@ func CreateGoGitTestRepo(bare bool) TestedRepo { } } -func SetupGoGitReposAndRemote() (repoA, repoB, remote TestedRepo) { - repoA = CreateGoGitTestRepo(false) - repoB = CreateGoGitTestRepo(false) - remote = CreateGoGitTestRepo(true) +func SetupGoGitReposAndRemote(t *testing.T) (repoA, repoB, remote TestedRepo) { + t.Helper() + + repoA = CreateGoGitTestRepo(t, false) + repoB = CreateGoGitTestRepo(t, false) + remote = CreateGoGitTestRepo(t, true) err := repoA.AddRemote("origin", remote.GetLocalRemote()) if err != nil { diff --git a/repository/mock_repo_test.go b/repository/mock_repo_test.go index 12851a80d21d8f271bec336bb809197934a20514..f43e7ea6c537e6545ff03681181b00f9dcd337c7 100644 --- a/repository/mock_repo_test.go +++ b/repository/mock_repo_test.go @@ -5,8 +5,7 @@ import ( ) func TestMockRepo(t *testing.T) { - creator := func(bare bool) TestedRepo { return NewMockRepo() } - cleaner := func(repos ...Repo) {} + creator := func(t TestingT, bare bool) TestedRepo { return NewMockRepo() } - RepoTest(t, creator, cleaner) + RepoTest(t, creator) } diff --git a/repository/repo_testing.go b/repository/repo_testing.go index 0db585cd4736d483d31ee78190a923be69cead05..c5bbe0c42754826aec0f0c5eb03fba47cf6021ff 100644 --- a/repository/repo_testing.go +++ b/repository/repo_testing.go @@ -1,7 +1,6 @@ package repository import ( - "log" "math/rand" "testing" @@ -14,37 +13,16 @@ import ( // TODO: add tests for RepoBleve // TODO: add tests for RepoStorage -func CleanupTestRepos(repos ...Repo) { - var firstErr error - for _, repo := range repos { - if repo, ok := repo.(TestedRepo); ok { - err := repo.EraseFromDisk() - if err != nil { - log.Println(err) - if firstErr == nil { - firstErr = err - } - } - } - } - - if firstErr != nil { - log.Fatal(firstErr) - } -} - -type RepoCreator func(bare bool) TestedRepo -type RepoCleaner func(repos ...Repo) +type RepoCreator func(t TestingT, bare bool) TestedRepo // Test suite for a Repo implementation -func RepoTest(t *testing.T, creator RepoCreator, cleaner RepoCleaner) { +func RepoTest(t *testing.T, creator RepoCreator) { for bare, name := range map[bool]string{ false: "Plain", true: "Bare", } { t.Run(name, func(t *testing.T) { - repo := creator(bare) - defer cleaner(repo) + repo := creator(t, bare) t.Run("Data", func(t *testing.T) { RepoDataTest(t, repo) diff --git a/tests/read_bugs_test.go b/tests/read_bugs_test.go index b198368952b66aed6df0bb2ae4bc45479c5aee67..fd9e994b421c748b8b9f17bb7ab58d13b384eaa7 100644 --- a/tests/read_bugs_test.go +++ b/tests/read_bugs_test.go @@ -9,8 +9,7 @@ import ( ) func TestReadBugs(t *testing.T) { - repo := repository.CreateGoGitTestRepo(false) - defer repository.CleanupTestRepos(repo) + repo := repository.CreateGoGitTestRepo(t, false) random_bugs.FillRepoWithSeed(repo, 15, 42) @@ -23,8 +22,7 @@ func TestReadBugs(t *testing.T) { } func benchmarkReadBugs(bugNumber int, t *testing.B) { - repo := repository.CreateGoGitTestRepo(false) - defer repository.CleanupTestRepos(repo) + repo := repository.CreateGoGitTestRepo(t, false) random_bugs.FillRepoWithSeed(repo, bugNumber, 42) t.ResetTimer()