From 1d4667c825bb5d291070bd4463c39a16f950f674 Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Thu, 16 Jun 2022 09:02:52 -0400 Subject: [PATCH 1/5] 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() From f3d316d16cc9fadd90faaaf77becbb82f1e09367 Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Thu, 16 Jun 2022 19:45:51 -0400 Subject: [PATCH 2/5] test(809): remove remaining calls to InitGoRepo in tests Note that examples still need to shown how a developer would use the library. --- commands/env_testing.go | 11 ++--------- repository/gogit_test.go | 31 ++++++------------------------- repository/gogit_testing.go | 17 +++++++++++++++++ 3 files changed, 25 insertions(+), 34 deletions(-) diff --git a/commands/env_testing.go b/commands/env_testing.go index b3c51c69370ffb80f360e6c30fad29084e4c1247..48807725bcb757bd9f05159f1b987211b55c72f2 100644 --- a/commands/env_testing.go +++ b/commands/env_testing.go @@ -19,15 +19,8 @@ type testEnv struct { func newTestEnv(t *testing.T) *testEnv { t.Helper() - cwd := t.TempDir() - - // r := repository.CreateGoGitTestRepo(t, false) // TODO - - repo, err := repository.InitGoGitRepo(cwd, gitBugNamespace) - require.NoError(t, err) - t.Cleanup(func() { - require.NoError(t, repo.Close()) - }) + repo := repository.CreateGoGitTestRepo(t, false) + cwd := repository.RepoDir(t, repo) buf := new(bytes.Buffer) diff --git a/repository/gogit_test.go b/repository/gogit_test.go index 8179874cbed7934344c6770258522a5369440caf..5d3378b70ee1479bdbfc7f8c3dd480eb01d60154 100644 --- a/repository/gogit_test.go +++ b/repository/gogit_test.go @@ -1,8 +1,6 @@ package repository import ( - "io/ioutil" - "os" "path" "path/filepath" "testing" @@ -13,26 +11,14 @@ import ( func TestNewGoGitRepo(t *testing.T) { // Plain - plainRoot, err := ioutil.TempDir("", "") - require.NoError(t, err) - t.Cleanup(func() { - require.NoError(t, os.RemoveAll(plainRoot)) - }) - - plainRepo, err := InitGoGitRepo(plainRoot, namespace) - require.NoError(t, err) + plainRepo := CreateGoGitTestRepo(t, false) + plainRoot := RepoDir(t, plainRepo) require.NoError(t, plainRepo.Close()) plainGitDir := filepath.Join(plainRoot, ".git") // Bare - bareRoot, err := ioutil.TempDir("", "") - require.NoError(t, err) - t.Cleanup(func() { - require.NoError(t, os.RemoveAll(bareRoot)) - }) - - bareRepo, err := InitBareGoGitRepo(bareRoot, namespace) - require.NoError(t, err) + bareRepo := CreateGoGitTestRepo(t, true) + bareRoot := RepoDir(t, bareRepo) require.NoError(t, bareRepo.Close()) bareGitDir := bareRoot @@ -75,13 +61,8 @@ func TestGoGitRepo(t *testing.T) { } func TestGoGitRepo_Indexes(t *testing.T) { - plainRoot := t.TempDir() - - repo, err := InitGoGitRepo(plainRoot, namespace) - require.NoError(t, err) - t.Cleanup(func() { - require.NoError(t, repo.Close()) - }) + repo := CreateGoGitTestRepo(t, false) + plainRoot := RepoDir(t, repo) // Can create indices indexA, err := repo.GetBleveIndex("a") diff --git a/repository/gogit_testing.go b/repository/gogit_testing.go index 1b39fe5ca8a7c88dc052225c023586a44a879210..01be54dea2f170c929512eeca076087ff9443a43 100644 --- a/repository/gogit_testing.go +++ b/repository/gogit_testing.go @@ -2,6 +2,8 @@ package repository import ( "log" + "path/filepath" + "strings" "testing" "github.com/99designs/keyring" @@ -73,3 +75,18 @@ func SetupGoGitReposAndRemote(t *testing.T) (repoA, repoB, remote TestedRepo) { return repoA, repoB, remote } + +func RepoDir(t *testing.T, repo TestedRepo) string { + t.Helper() + + dir := repo.GetLocalRemote() + if strings.HasSuffix(dir, ".git") { + dir, _ = filepath.Split(dir) + } + + if dir[len(dir)-1] == filepath.Separator { + dir = dir[:len(dir)-1] + } + + return dir +} From 52c724a03334dda9df30dbac22cb22a04cbe4260 Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Tue, 21 Jun 2022 06:48:28 -0400 Subject: [PATCH 3/5] test: use testing.TB as common interface --- repository/gogit_testing.go | 8 +------- repository/mock_repo_test.go | 2 +- repository/repo_testing.go | 2 +- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/repository/gogit_testing.go b/repository/gogit_testing.go index 01be54dea2f170c929512eeca076087ff9443a43..3e6f72fcb82cc7e266486ec6168e1d161496844a 100644 --- a/repository/gogit_testing.go +++ b/repository/gogit_testing.go @@ -9,17 +9,11 @@ import ( "github.com/99designs/keyring" ) -type TestingT interface { - Cleanup(func()) - Helper() - TempDir() string -} - const namespace = "git-bug" // This is intended for testing only -func CreateGoGitTestRepo(t TestingT, bare bool) TestedRepo { +func CreateGoGitTestRepo(t testing.TB, bare bool) TestedRepo { t.Helper() dir := t.TempDir() diff --git a/repository/mock_repo_test.go b/repository/mock_repo_test.go index f43e7ea6c537e6545ff03681181b00f9dcd337c7..66fad7b65cd50fe7d298f69af9922cbc34a60d64 100644 --- a/repository/mock_repo_test.go +++ b/repository/mock_repo_test.go @@ -5,7 +5,7 @@ import ( ) func TestMockRepo(t *testing.T) { - creator := func(t TestingT, bare bool) TestedRepo { return NewMockRepo() } + creator := func(t testing.TB, bare bool) TestedRepo { return NewMockRepo() } RepoTest(t, creator) } diff --git a/repository/repo_testing.go b/repository/repo_testing.go index c5bbe0c42754826aec0f0c5eb03fba47cf6021ff..5d51d23ffe96ae3a11d2d08644ac1ea5c6660a91 100644 --- a/repository/repo_testing.go +++ b/repository/repo_testing.go @@ -13,7 +13,7 @@ import ( // TODO: add tests for RepoBleve // TODO: add tests for RepoStorage -type RepoCreator func(t TestingT, bare bool) TestedRepo +type RepoCreator func(t testing.TB, bare bool) TestedRepo // Test suite for a Repo implementation func RepoTest(t *testing.T, creator RepoCreator) { From 6fc6a0f0ab8ce8c7b6ec510d1d7557a6e7cd6835 Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Wed, 22 Jun 2022 10:38:09 -0400 Subject: [PATCH 4/5] test(809): remove reliance on repo's filesystem --- commands/add_test.go | 10 +++++----- commands/env_testing.go | 3 --- commands/rm_test.go | 2 +- commands/user_create_test.go | 16 +++++++++------- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/commands/add_test.go b/commands/add_test.go index 63eda06e3043f45be04dd4d8f33986419ee86d97..077995a6df5f5f9c2f8de37c4830fb6743671e89 100644 --- a/commands/add_test.go +++ b/commands/add_test.go @@ -7,10 +7,10 @@ import ( "github.com/stretchr/testify/require" ) -func newTestEnvUserAndBug(t *testing.T) (*testEnv, string, string) { +func newTestEnvAndBug(t *testing.T) (*testEnv, string) { t.Helper() - testEnv, userID := newTestEnvAndUser(t) + testEnv, _ := newTestEnvAndUser(t) opts := addOptions{ title: "this is a bug title", message: "this is a bug message", @@ -23,10 +23,10 @@ func newTestEnvUserAndBug(t *testing.T) (*testEnv, string, string) { bugID := strings.Split(testEnv.out.String(), " ")[0] testEnv.out.Reset() - return testEnv, userID, bugID + return testEnv, bugID } func TestAdd(t *testing.T) { - _, _, user := newTestEnvUserAndBug(t) - require.Regexp(t, "^[0-9A-Fa-f]{7}$", user) + _, bugID := newTestEnvAndBug(t) + require.Regexp(t, "^[0-9A-Fa-f]{7}$", bugID) } diff --git a/commands/env_testing.go b/commands/env_testing.go index 48807725bcb757bd9f05159f1b987211b55c72f2..1493a190d59b6ef9c58163523f9f1b0f477eebab 100644 --- a/commands/env_testing.go +++ b/commands/env_testing.go @@ -12,7 +12,6 @@ import ( type testEnv struct { env *Env - cwd string out *bytes.Buffer } @@ -20,7 +19,6 @@ func newTestEnv(t *testing.T) *testEnv { t.Helper() repo := repository.CreateGoGitTestRepo(t, false) - cwd := repository.RepoDir(t, repo) buf := new(bytes.Buffer) @@ -37,7 +35,6 @@ func newTestEnv(t *testing.T) *testEnv { out: out{Writer: buf}, err: out{Writer: buf}, }, - cwd: cwd, out: buf, } } diff --git a/commands/rm_test.go b/commands/rm_test.go index 5d4e7cca1007d336e891b96c0550c2da501d3182..0156bbd4d618ceb2c0dc470146998907811a3935 100644 --- a/commands/rm_test.go +++ b/commands/rm_test.go @@ -7,7 +7,7 @@ import ( ) func TestRm(t *testing.T) { - testEnv, _, bugID := newTestEnvUserAndBug(t) + testEnv, bugID := newTestEnvAndBug(t) exp := "bug " + bugID + " removed\n" diff --git a/commands/user_create_test.go b/commands/user_create_test.go index 223e7ec3fc1e9e38e07bba614f9f8c10396728cf..08958344fad1ee520c9183b2c84e1bb8116f39ca 100644 --- a/commands/user_create_test.go +++ b/commands/user_create_test.go @@ -1,21 +1,25 @@ package commands import ( - "path/filepath" "strings" "testing" "github.com/stretchr/testify/require" ) +const ( + testUserName = "John Doe" + testUserEmail = "jdoe@example.com" +) + func newTestEnvAndUser(t *testing.T) (*testEnv, string) { t.Helper() testEnv := newTestEnv(t) opts := createUserOptions{ - name: "John Doe", - email: "jdoe@example.com", + name: testUserName, + email: testUserEmail, avatarURL: "", nonInteractive: true, } @@ -29,8 +33,6 @@ func newTestEnvAndUser(t *testing.T) (*testEnv, string) { } func TestUserCreateCommand(t *testing.T) { - testEnv, userID := newTestEnvAndUser(t) - - require.FileExists(t, filepath.Join(testEnv.cwd, ".git", "refs", "identities", userID)) - require.FileExists(t, filepath.Join(testEnv.cwd, ".git", "git-bug", "identity-cache")) + _, userID := newTestEnvAndUser(t) + require.Regexp(t, "[0-9a-f]{64}", userID) } From 323dd0e293fb965743269fe52c8335b3d7a7bb94 Mon Sep 17 00:00:00 2001 From: Steve Moyer Date: Wed, 22 Jun 2022 10:46:52 -0400 Subject: [PATCH 5/5] test(809): do not export function that returns GoGit filesystem --- repository/gogit_test.go | 6 +++--- repository/gogit_testing.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/repository/gogit_test.go b/repository/gogit_test.go index 5d3378b70ee1479bdbfc7f8c3dd480eb01d60154..a3de0a03da2a6e0cc10fc2a7ecade11c783e7462 100644 --- a/repository/gogit_test.go +++ b/repository/gogit_test.go @@ -12,13 +12,13 @@ import ( func TestNewGoGitRepo(t *testing.T) { // Plain plainRepo := CreateGoGitTestRepo(t, false) - plainRoot := RepoDir(t, plainRepo) + plainRoot := goGitRepoDir(t, plainRepo) require.NoError(t, plainRepo.Close()) plainGitDir := filepath.Join(plainRoot, ".git") // Bare bareRepo := CreateGoGitTestRepo(t, true) - bareRoot := RepoDir(t, bareRepo) + bareRoot := goGitRepoDir(t, bareRepo) require.NoError(t, bareRepo.Close()) bareGitDir := bareRoot @@ -62,7 +62,7 @@ func TestGoGitRepo(t *testing.T) { func TestGoGitRepo_Indexes(t *testing.T) { repo := CreateGoGitTestRepo(t, false) - plainRoot := RepoDir(t, repo) + plainRoot := goGitRepoDir(t, repo) // Can create indices indexA, err := repo.GetBleveIndex("a") diff --git a/repository/gogit_testing.go b/repository/gogit_testing.go index 3e6f72fcb82cc7e266486ec6168e1d161496844a..afbb917f4c3302c6da8cf0dd688bf8c9e0e83dc0 100644 --- a/repository/gogit_testing.go +++ b/repository/gogit_testing.go @@ -70,7 +70,7 @@ func SetupGoGitReposAndRemote(t *testing.T) (repoA, repoB, remote TestedRepo) { return repoA, repoB, remote } -func RepoDir(t *testing.T, repo TestedRepo) string { +func goGitRepoDir(t *testing.T, repo TestedRepo) string { t.Helper() dir := repo.GetLocalRemote()