From aa23d8de5d8b3e044ef050163c8ad5da604ae90d Mon Sep 17 00:00:00 2001 From: Steve Moyer <smoyer1@selesy.com> Date: Sat, 1 Oct 2022 12:52:27 -0400 Subject: [PATCH] feat(849): currate stdout and stderr in "core" Resolves #849 References #828 --- api/graphql/graphql_test.go | 7 ++++- api/http/git_file_handlers_test.go | 6 +++- bridge/github/export_test.go | 8 +++-- bridge/github/import_integration_test.go | 5 ++-- bridge/github/import_test.go | 5 ++-- bridge/gitlab/export_test.go | 9 +++--- bridge/gitlab/import_test.go | 5 ++-- cache/multi_repo_cache.go | 20 +++++++------ cache/repo_cache.go | 34 +++++++++++---------- cache/repo_cache_test.go | 38 +++++++++++++----------- cache/repo_cache_testing.go | 24 +++++++++++++++ commands/env.go | 2 +- commands/env_testing.go | 8 ++--- commands/select/select_test.go | 7 +++-- commands/webui.go | 2 +- 15 files changed, 114 insertions(+), 66 deletions(-) create mode 100644 cache/repo_cache_testing.go diff --git a/api/graphql/graphql_test.go b/api/graphql/graphql_test.go index 2ddfb314..c87f5843 100644 --- a/api/graphql/graphql_test.go +++ b/api/graphql/graphql_test.go @@ -1,6 +1,7 @@ package graphql import ( + "bytes" "testing" "github.com/99designs/gqlgen/client" @@ -18,8 +19,10 @@ func TestQueries(t *testing.T) { random_bugs.FillRepoWithSeed(repo, 10, 42) + stderr := &bytes.Buffer{} + mrc := cache.NewMultiRepoCache() - _, err := mrc.RegisterDefaultRepository(repo) + _, err := mrc.RegisterDefaultRepository(repo, stderr) require.NoError(t, err) handler := NewHandler(mrc, nil) @@ -216,4 +219,6 @@ func TestQueries(t *testing.T) { err = c.Post(query, &resp) assert.NoError(t, err) + + require.Equal(t, cache.ExpectedCacheInitializationMessage, stderr.String()) } diff --git a/api/http/git_file_handlers_test.go b/api/http/git_file_handlers_test.go index 736bf75e..04acd2a9 100644 --- a/api/http/git_file_handlers_test.go +++ b/api/http/git_file_handlers_test.go @@ -21,8 +21,10 @@ import ( func TestGitFileHandlers(t *testing.T) { repo := repository.CreateGoGitTestRepo(t, false) + stderr := &bytes.Buffer{} + mrc := cache.NewMultiRepoCache() - repoCache, err := mrc.RegisterDefaultRepository(repo) + repoCache, err := mrc.RegisterDefaultRepository(repo, stderr) require.NoError(t, err) author, err := repoCache.NewIdentity("test identity", "test@test.org") @@ -87,4 +89,6 @@ func TestGitFileHandlers(t *testing.T) { assert.Equal(t, http.StatusOK, w.Code) assert.Equal(t, data.Bytes(), w.Body.Bytes()) + + require.Equal(t, cache.ExpectedCacheInitializationMessage, stderr.String()) } diff --git a/bridge/github/export_test.go b/bridge/github/export_test.go index de2d3f34..8b7c9b25 100644 --- a/bridge/github/export_test.go +++ b/bridge/github/export_test.go @@ -142,8 +142,7 @@ func TestGithubPushPull(t *testing.T) { // create repo backend repo := repository.CreateGoGitTestRepo(t, false) - backend, err := cache.NewRepoCache(repo) - require.NoError(t, err) + backend, stderr := cache.NewTestRepoCache(t, repo) // set author identity login := "identity-test" @@ -212,13 +211,14 @@ func TestGithubPushPull(t *testing.T) { require.NoError(t, result.Err) } require.NoError(t, err) + require.Empty(t, stderr.String()) fmt.Printf("test repository exported in %f seconds\n", time.Since(start).Seconds()) repoTwo := repository.CreateGoGitTestRepo(t, false) // create a second backend - backendTwo, err := cache.NewRepoCache(repoTwo) + backendTwo, stderr := cache.NewTestRepoCache(t, repoTwo) require.NoError(t, err) importer := &githubImporter{} @@ -276,6 +276,8 @@ func TestGithubPushPull(t *testing.T) { // TODO: maybe more tests to ensure bug final state }) } + + require.Empty(t, stderr.String()) } func generateRepoName() string { diff --git a/bridge/github/import_integration_test.go b/bridge/github/import_integration_test.go index 50cbd5c8..cfbf9702 100644 --- a/bridge/github/import_integration_test.go +++ b/bridge/github/import_integration_test.go @@ -34,11 +34,9 @@ func TestGithubImporterIntegration(t *testing.T) { // arrange repo := repository.CreateGoGitTestRepo(t, false) - backend, err := cache.NewRepoCache(repo) - require.NoError(t, err) + backend, stderr := cache.NewTestRepoCache(t, repo) defer backend.Close() interrupt.RegisterCleaner(backend.Close) - require.NoError(t, err) // act events, err := importer.ImportAll(context.Background(), backend, time.Time{}) @@ -71,6 +69,7 @@ func TestGithubImporterIntegration(t *testing.T) { ops4 := b4.Snapshot().Operations require.Equal(t, "edited", ops4[1].(*bug.EditCommentOperation).Message) + require.Empty(t, stderr.String()) } func setupExpectations(t *testing.T, mock *mocks.Client) { diff --git a/bridge/github/import_test.go b/bridge/github/import_test.go index 5575de98..0e6c6ac8 100644 --- a/bridge/github/import_test.go +++ b/bridge/github/import_test.go @@ -28,8 +28,7 @@ func TestGithubImporter(t *testing.T) { repo := repository.CreateGoGitTestRepo(t, false) - backend, err := cache.NewRepoCache(repo) - require.NoError(t, err) + backend, stderr := cache.NewTestRepoCache(t, repo) defer backend.Close() interrupt.RegisterCleaner(backend.Close) @@ -208,4 +207,6 @@ func TestGithubImporter(t *testing.T) { } }) } + + require.Empty(t, stderr.String()) } diff --git a/bridge/gitlab/export_test.go b/bridge/gitlab/export_test.go index e9f3ae75..28cbf25d 100644 --- a/bridge/gitlab/export_test.go +++ b/bridge/gitlab/export_test.go @@ -148,8 +148,7 @@ func TestGitlabPushPull(t *testing.T) { // create repo backend repo := repository.CreateGoGitTestRepo(t, false) - backend, err := cache.NewRepoCache(repo) - require.NoError(t, err) + backend, stderr := cache.NewTestRepoCache(t, repo) // set author identity login := "test-identity" @@ -215,14 +214,14 @@ func TestGitlabPushPull(t *testing.T) { require.NoError(t, result.Err) } require.NoError(t, err) + require.Empty(t, stderr.String()) fmt.Printf("test repository exported in %f seconds\n", time.Since(start).Seconds()) repoTwo := repository.CreateGoGitTestRepo(t, false) // create a second backend - backendTwo, err := cache.NewRepoCache(repoTwo) - require.NoError(t, err) + backendTwo, stderr := cache.NewTestRepoCache(t, repoTwo) importer := &gitlabImporter{} err = importer.Init(ctx, backend, core.Configuration{ @@ -279,6 +278,8 @@ func TestGitlabPushPull(t *testing.T) { // TODO: maybe more tests to ensure bug final state }) } + + require.Empty(t, stderr.String()) } func generateRepoName() string { diff --git a/bridge/gitlab/import_test.go b/bridge/gitlab/import_test.go index d98da4ef..587c3226 100644 --- a/bridge/gitlab/import_test.go +++ b/bridge/gitlab/import_test.go @@ -33,8 +33,7 @@ func TestGitlabImport(t *testing.T) { repo := repository.CreateGoGitTestRepo(t, false) - backend, err := cache.NewRepoCache(repo) - require.NoError(t, err) + backend, stderr := cache.NewTestRepoCache(t, repo) defer backend.Close() interrupt.RegisterCleaner(backend.Close) @@ -164,4 +163,6 @@ func TestGitlabImport(t *testing.T) { } }) } + + require.Empty(t, stderr.String()) } diff --git a/cache/multi_repo_cache.go b/cache/multi_repo_cache.go index 659cd5e6..ac0495db 100644 --- a/cache/multi_repo_cache.go +++ b/cache/multi_repo_cache.go @@ -2,6 +2,7 @@ package cache import ( "fmt" + "io" "github.com/MichaelMure/git-bug/repository" ) @@ -21,8 +22,8 @@ func NewMultiRepoCache() *MultiRepoCache { } // RegisterRepository register a named repository. Use this for multi-repo setup -func (c *MultiRepoCache) RegisterRepository(ref string, repo repository.ClockedRepo) (*RepoCache, error) { - r, err := NewRepoCache(repo) +func (c *MultiRepoCache) RegisterRepository(ref string, repo repository.ClockedRepo, stderr io.Writer) (*RepoCache, error) { + r, err := NewRepoCache(repo, stderr) if err != nil { return nil, err } @@ -32,14 +33,15 @@ func (c *MultiRepoCache) RegisterRepository(ref string, repo repository.ClockedR } // RegisterDefaultRepository register a unnamed repository. Use this for mono-repo setup -func (c *MultiRepoCache) RegisterDefaultRepository(repo repository.ClockedRepo) (*RepoCache, error) { - r, err := NewRepoCache(repo) - if err != nil { - return nil, err - } +func (c *MultiRepoCache) RegisterDefaultRepository(repo repository.ClockedRepo, stderr io.Writer) (*RepoCache, error) { + return c.RegisterRepository(defaultRepoName, repo, stderr) + // r, err := NewRepoCache(repo) + // if err != nil { + // return nil, err + // } - c.repos[defaultRepoName] = r - return r, nil + // c.repos[defaultRepoName] = r + // return r, nil } // DefaultRepo retrieve the default repository diff --git a/cache/repo_cache.go b/cache/repo_cache.go index 71abf968..09d30ee9 100644 --- a/cache/repo_cache.go +++ b/cache/repo_cache.go @@ -30,14 +30,14 @@ var _ repository.RepoKeyring = &RepoCache{} // RepoCache is a cache for a Repository. This cache has multiple functions: // -// 1. After being loaded, a Bug is kept in memory in the cache, allowing for fast -// access later. -// 2. The cache maintain in memory and on disk a pre-digested excerpt for each bug, -// allowing for fast querying the whole set of bugs without having to load -// them individually. -// 3. The cache guarantee that a single instance of a Bug is loaded at once, avoiding -// loss of data that we could have with multiple copies in the same process. -// 4. The same way, the cache maintain in memory a single copy of the loaded identities. +// 1. After being loaded, a Bug is kept in memory in the cache, allowing for fast +// access later. +// 2. The cache maintain in memory and on disk a pre-digested excerpt for each bug, +// allowing for fast querying the whole set of bugs without having to load +// them individually. +// 3. The cache guarantee that a single instance of a Bug is loaded at once, avoiding +// loss of data that we could have with multiple copies in the same process. +// 4. The same way, the cache maintain in memory a single copy of the loaded identities. // // The cache also protect the on-disk data by locking the git repository for its // own usage, by writing a lock file. Of course, normal git operations are not @@ -71,13 +71,16 @@ type RepoCache struct { // the user identity's id, if known userIdentityId entity.Id + + // the io.Writer where messages to (human) users should be written + stderr io.Writer } -func NewRepoCache(r repository.ClockedRepo) (*RepoCache, error) { - return NewNamedRepoCache(r, "") +func NewRepoCache(r repository.ClockedRepo, stderr io.Writer) (*RepoCache, error) { + return NewNamedRepoCache(r, "", stderr) } -func NewNamedRepoCache(r repository.ClockedRepo, name string) (*RepoCache, error) { +func NewNamedRepoCache(r repository.ClockedRepo, name string, stderr io.Writer) (*RepoCache, error) { c := &RepoCache{ repo: r, name: name, @@ -85,6 +88,7 @@ func NewNamedRepoCache(r repository.ClockedRepo, name string) (*RepoCache, error bugs: make(map[entity.Id]*BugCache), loadedBugs: NewLRUIdCache(), identities: make(map[entity.Id]*IdentityCache), + stderr: stderr, } c.resolvers = makeResolvers(c) @@ -173,7 +177,7 @@ func (c *RepoCache) Close() error { } func (c *RepoCache) buildCache() error { - _, _ = fmt.Fprintf(os.Stderr, "Building identity cache... ") + _, _ = fmt.Fprintf(c.stderr, "Building identity cache... ") c.identitiesExcerpts = make(map[entity.Id]*IdentityExcerpt) @@ -187,9 +191,9 @@ func (c *RepoCache) buildCache() error { c.identitiesExcerpts[i.Identity.Id()] = NewIdentityExcerpt(i.Identity) } - _, _ = fmt.Fprintln(os.Stderr, "Done.") + _, _ = fmt.Fprintln(c.stderr, "Done.") - _, _ = fmt.Fprintf(os.Stderr, "Building bug cache... ") + _, _ = fmt.Fprintf(c.stderr, "Building bug cache... ") c.bugExcerpts = make(map[entity.Id]*BugExcerpt) @@ -214,7 +218,7 @@ func (c *RepoCache) buildCache() error { } } - _, _ = fmt.Fprintln(os.Stderr, "Done.") + _, _ = fmt.Fprintln(c.stderr, "Done.") return nil } diff --git a/cache/repo_cache_test.go b/cache/repo_cache_test.go index a9557ff0..c77db52d 100644 --- a/cache/repo_cache_test.go +++ b/cache/repo_cache_test.go @@ -15,9 +15,7 @@ import ( func TestCache(t *testing.T) { repo := repository.CreateGoGitTestRepo(t, false) - - cache, err := NewRepoCache(repo) - require.NoError(t, err) + cache, stderr := NewTestRepoCache(t, repo) // Create, set and get user identity iden1, err := cache.NewIdentity("René Descartes", "rene@descartes.fr") @@ -85,10 +83,12 @@ func TestCache(t *testing.T) { require.Empty(t, cache.identities) require.Empty(t, cache.identitiesExcerpts) + // There should be no output to stderr + require.Empty(t, stderr.String()) + // Reload, only excerpt are loaded, but as we need to load the identities used in the bugs // to check the signatures, we also load the identity used above - cache, err = NewRepoCache(repo) - require.NoError(t, err) + cache, stderr = NewTestRepoCache(t, repo) require.Empty(t, cache.bugs) require.Len(t, cache.identities, 1) require.Len(t, cache.bugExcerpts, 2) @@ -108,16 +108,14 @@ func TestCache(t *testing.T) { require.NoError(t, err) _, err = cache.ResolveBugPrefix(bug1.Id().String()[:10]) require.NoError(t, err) + + require.Empty(t, stderr.String()) } func TestCachePushPull(t *testing.T) { repoA, repoB, _ := repository.SetupGoGitReposAndRemote(t) - - cacheA, err := NewRepoCache(repoA) - require.NoError(t, err) - - cacheB, err := NewRepoCache(repoB) - require.NoError(t, err) + cacheA, stderrA := NewTestRepoCache(t, repoA) + cacheB, stderrB := NewTestRepoCache(t, repoB) // Create, set and get user identity reneA, err := cacheA.NewIdentity("René Descartes", "rene@descartes.fr") @@ -166,6 +164,9 @@ func TestCachePushPull(t *testing.T) { require.NoError(t, err) require.Len(t, cacheA.AllBugsIds(), 2) + + require.Empty(t, stderrA.String()) + require.Empty(t, stderrB.String()) } func TestRemove(t *testing.T) { @@ -179,8 +180,7 @@ func TestRemove(t *testing.T) { err = repo.AddRemote("remoteB", remoteB.GetLocalRemote()) require.NoError(t, err) - repoCache, err := NewRepoCache(repo) - require.NoError(t, err) + repoCache, stderr := NewTestRepoCache(t, repo) rene, err := repoCache.NewIdentity("René Descartes", "rene@descartes.fr") require.NoError(t, err) @@ -214,12 +214,13 @@ func TestRemove(t *testing.T) { _, err = repoCache.ResolveBug(b1.Id()) assert.Error(t, bug.ErrBugNotExist, err) + + require.Empty(t, stderr.String()) } func TestCacheEviction(t *testing.T) { repo := repository.CreateGoGitTestRepo(t, false) - repoCache, err := NewRepoCache(repo) - require.NoError(t, err) + repoCache, stderr := NewTestRepoCache(t, repo) repoCache.setCacheSize(2) require.Equal(t, 2, repoCache.maxLoadedBugs) @@ -267,6 +268,8 @@ func TestCacheEviction(t *testing.T) { checkBugPresence(t, repoCache, bug3, true) require.Equal(t, 2, repoCache.loadedBugs.Len()) require.Equal(t, 2, len(repoCache.bugs)) + + require.Empty(t, stderr.String()) } func checkBugPresence(t *testing.T, cache *RepoCache, bug *BugCache, presence bool) { @@ -286,12 +289,13 @@ func TestLongDescription(t *testing.T) { repo := repository.CreateGoGitTestRepo(t, false) - backend, err := NewRepoCache(repo) - require.NoError(t, err) + backend, stderr := NewTestRepoCache(t, repo) i, err := backend.NewIdentity("René Descartes", "rene@descartes.fr") require.NoError(t, err) _, _, err = backend.NewBugRaw(i, time.Now().Unix(), text, text, nil, nil) require.NoError(t, err) + + require.Empty(t, stderr.String()) } diff --git a/cache/repo_cache_testing.go b/cache/repo_cache_testing.go new file mode 100644 index 00000000..f4dc18bc --- /dev/null +++ b/cache/repo_cache_testing.go @@ -0,0 +1,24 @@ +package cache + +import ( + "bytes" + "testing" + + "github.com/MichaelMure/git-bug/repository" + "github.com/stretchr/testify/require" +) + +const ExpectedCacheInitializationMessage = "Building identity cache... Done.\nBuilding bug cache... Done.\n" + +func NewTestRepoCache(t *testing.T, repo repository.TestedRepo) (*RepoCache, *bytes.Buffer) { + t.Helper() + + stderr := &bytes.Buffer{} + cache, err := NewRepoCache(repo, stderr) + require.NoError(t, err) + require.Equal(t, ExpectedCacheInitializationMessage, stderr.String()) + + stderr.Reset() + + return cache, stderr +} diff --git a/commands/env.go b/commands/env.go index 11b91c4b..2afb6d1c 100644 --- a/commands/env.go +++ b/commands/env.go @@ -97,7 +97,7 @@ func loadBackend(env *Env) func(*cobra.Command, []string) error { return err } - env.backend, err = cache.NewRepoCache(env.repo) + env.backend, err = cache.NewRepoCache(env.repo, env.err) if err != nil { return err } diff --git a/commands/env_testing.go b/commands/env_testing.go index 1493a190..25329b5b 100644 --- a/commands/env_testing.go +++ b/commands/env_testing.go @@ -4,10 +4,9 @@ import ( "bytes" "testing" - "github.com/stretchr/testify/require" - "github.com/MichaelMure/git-bug/cache" "github.com/MichaelMure/git-bug/repository" + "github.com/stretchr/testify/require" ) type testEnv struct { @@ -22,10 +21,11 @@ func newTestEnv(t *testing.T) *testEnv { buf := new(bytes.Buffer) - backend, err := cache.NewRepoCache(repo) - require.NoError(t, err) + backend, stderr := cache.NewTestRepoCache(t, repo) t.Cleanup(func() { backend.Close() + + require.Empty(t, stderr.String()) }) return &testEnv{ diff --git a/commands/select/select_test.go b/commands/select/select_test.go index 702700f4..2745d0fb 100644 --- a/commands/select/select_test.go +++ b/commands/select/select_test.go @@ -13,10 +13,9 @@ import ( func TestSelect(t *testing.T) { repo := repository.CreateGoGitTestRepo(t, false) - repoCache, err := cache.NewRepoCache(repo) - require.NoError(t, err) + repoCache, stderr := cache.NewTestRepoCache(t, repo) - _, _, err = ResolveBug(repoCache, []string{}) + _, _, err := ResolveBug(repoCache, []string{}) require.Equal(t, ErrNoValidId, err) err = Select(repoCache, "invalid") @@ -76,4 +75,6 @@ func TestSelect(t *testing.T) { // Resolve without a pattern should error again after clearing the selected bug _, _, err = ResolveBug(repoCache, []string{}) require.Error(t, err) + + require.Empty(t, stderr.String()) } diff --git a/commands/webui.go b/commands/webui.go index 758a153b..c8badbf4 100644 --- a/commands/webui.go +++ b/commands/webui.go @@ -104,7 +104,7 @@ func runWebUI(env *Env, opts webUIOptions) error { } mrc := cache.NewMultiRepoCache() - _, err := mrc.RegisterDefaultRepository(env.repo) + _, err := mrc.RegisterDefaultRepository(env.repo, env.err) if err != nil { return err } -- GitLab