Merge pull request #988 from MichaelMure/fix-cache-test

Michael Muré created 2 years ago

cache: fix tests?

Change summary

bridge/gitlab/event.go        |   9 +
cache/repo_cache.go           |  16 +-
cache/repo_cache_test.go      | 162 +++++++++++++++---------------------
commands/bridge/bridge_new.go |   2 
commands/select/select.go     |   8 
entities/identity/identity.go |   6 
entities/identity/version.go  |   6 
entity/dag/entity_actions.go  |  24 ++--
8 files changed, 104 insertions(+), 129 deletions(-)

Detailed changes

bridge/gitlab/event.go

@@ -5,8 +5,9 @@ import (
 	"strings"
 	"time"
 
-	"github.com/MichaelMure/git-bug/util/text"
 	"github.com/xanzy/go-gitlab"
+
+	"github.com/MichaelMure/git-bug/util/text"
 )
 
 // Event represents a unified GitLab event (note, label or state event).
@@ -204,9 +205,9 @@ func SortedEvents(inputs ...<-chan Event) chan Event {
 }
 
 // getNewTitle parses body diff given by gitlab api and return it final form
-// examples: "changed title from **fourth issue** to **fourth issue{+ changed+}**"
-//           "changed title from **fourth issue{- changed-}** to **fourth issue**"
-// because Gitlab
+// examples:
+// - "changed title from **fourth issue** to **fourth issue{+ changed+}**"
+// - "changed title from **fourth issue{- changed-}** to **fourth issue**"
 func getNewTitle(diff string) string {
 	newTitle := strings.Split(diff, "** to **")[1]
 	newTitle = strings.Replace(newTitle, "{+", "", -1)

cache/repo_cache.go

@@ -40,14 +40,14 @@ type cacheMgmt interface {
 
 // 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

cache/repo_cache_test.go

@@ -29,104 +29,89 @@ func TestCache(t *testing.T) {
 		return count
 	}
 
-	var (
-		cache *RepoCache
-		bug1  *BugCache
-		iden1 *IdentityCache
-	)
-
-	t.Run("entity operations on cache", func(t *testing.T) {
-		cache, err := NewRepoCacheNoEvents(repo)
-		require.NoError(t, err)
-
-		// Create, set and get user identity
-		iden1, err = cache.Identities().New("René Descartes", "[e-mail]")
-		require.NoError(t, err)
-		err = cache.SetUserIdentity(iden1)
-		require.NoError(t, err)
-		userIden, err := cache.GetUserIdentity()
-		require.NoError(t, err)
-		require.Equal(t, iden1.Id(), userIden.Id())
-
-		// it's possible to create two identical identities
-		iden2, err := cache.Identities().New("René Descartes", "[e-mail]")
-		require.NoError(t, err)
+	cache, err := NewRepoCacheNoEvents(repo)
+	require.NoError(t, err)
 
-		// Two identical identities yield a different id
-		require.NotEqual(t, iden1.Id(), iden2.Id())
+	// Create, set and get user identity
+	iden1, err := cache.Identities().New("René Descartes", "[e-mail]")
+	require.NoError(t, err)
+	err = cache.SetUserIdentity(iden1)
+	require.NoError(t, err)
+	userIden, err := cache.GetUserIdentity()
+	require.NoError(t, err)
+	require.Equal(t, iden1.Id(), userIden.Id())
 
-		// There is now two identities in the cache
-		require.Len(t, cache.Identities().AllIds(), 2)
-		require.Len(t, cache.identities.excerpts, 2)
-		require.Len(t, cache.identities.cached, 2)
-		require.Equal(t, uint64(2), indexCount(t, identity.Namespace))
-		require.Equal(t, uint64(0), indexCount(t, bug.Namespace))
+	// it's possible to create two identical identities
+	iden2, err := cache.Identities().New("René Descartes", "[e-mail]")
+	require.NoError(t, err)
 
-		// Create a bug
-		bug1, _, err = cache.Bugs().New("title", "message")
-		require.NoError(t, err)
+	// Two identical identities yield a different id
+	require.NotEqual(t, iden1.Id(), iden2.Id())
 
-		// It's possible to create two identical bugs
-		bug2, _, err := cache.Bugs().New("title", "marker")
-		require.NoError(t, err)
+	// There is now two identities in the cache
+	require.Len(t, cache.Identities().AllIds(), 2)
+	require.Len(t, cache.identities.excerpts, 2)
+	require.Len(t, cache.identities.cached, 2)
+	require.Equal(t, uint64(2), indexCount(t, identity.Namespace))
+	require.Equal(t, uint64(0), indexCount(t, bug.Namespace))
 
-		// two identical bugs yield a different id
-		require.NotEqual(t, bug1.Id(), bug2.Id())
+	// Create a bug
+	bug1, _, err := cache.Bugs().New("title", "message")
+	require.NoError(t, err)
 
-		// There is now two bugs in the cache
-		require.Len(t, cache.Bugs().AllIds(), 2)
-		require.Len(t, cache.bugs.excerpts, 2)
-		require.Len(t, cache.bugs.cached, 2)
-		require.Equal(t, uint64(2), indexCount(t, identity.Namespace))
-		require.Equal(t, uint64(2), indexCount(t, bug.Namespace))
+	// It's possible to create two identical bugs
+	bug2, _, err := cache.Bugs().New("title", "marker")
+	require.NoError(t, err)
 
-		// Resolving
-		_, err = cache.Identities().Resolve(iden1.Id())
-		require.NoError(t, err)
-		_, err = cache.Identities().ResolveExcerpt(iden1.Id())
-		require.NoError(t, err)
-		_, err = cache.Identities().ResolvePrefix(iden1.Id().String()[:10])
-		require.NoError(t, err)
+	// two identical bugs yield a different id
+	require.NotEqual(t, bug1.Id(), bug2.Id())
 
-		_, err = cache.Bugs().Resolve(bug1.Id())
-		require.NoError(t, err)
-		_, err = cache.Bugs().ResolveExcerpt(bug1.Id())
-		require.NoError(t, err)
-		_, err = cache.Bugs().ResolvePrefix(bug1.Id().String()[:10])
-		require.NoError(t, err)
+	// There is now two bugs in the cache
+	require.Len(t, cache.Bugs().AllIds(), 2)
+	require.Len(t, cache.bugs.excerpts, 2)
+	require.Len(t, cache.bugs.cached, 2)
+	require.Equal(t, uint64(2), indexCount(t, identity.Namespace))
+	require.Equal(t, uint64(2), indexCount(t, bug.Namespace))
 
-		// Querying
-		q, err := query.Parse("status:open author:descartes sort:edit-asc")
-		require.NoError(t, err)
-		res, err := cache.Bugs().Query(q)
-		require.NoError(t, err)
-		require.Len(t, res, 2)
+	// Resolving
+	_, err = cache.Identities().Resolve(iden1.Id())
+	require.NoError(t, err)
+	_, err = cache.Identities().ResolveExcerpt(iden1.Id())
+	require.NoError(t, err)
+	_, err = cache.Identities().ResolvePrefix(iden1.Id().String()[:10])
+	require.NoError(t, err)
 
-		q, err = query.Parse("status:open marker") // full-text search
-		require.NoError(t, err)
-		res, err = cache.Bugs().Query(q)
-		require.NoError(t, err)
-		require.Len(t, res, 1)
+	_, err = cache.Bugs().Resolve(bug1.Id())
+	require.NoError(t, err)
+	_, err = cache.Bugs().ResolveExcerpt(bug1.Id())
+	require.NoError(t, err)
+	_, err = cache.Bugs().ResolvePrefix(bug1.Id().String()[:10])
+	require.NoError(t, err)
 
-		// Close
-		require.NoError(t, cache.Close())
-		require.Empty(t, cache.bugs.cached)
-		require.Empty(t, cache.bugs.excerpts)
-		require.Empty(t, cache.identities.cached)
-		require.Empty(t, cache.identities.excerpts)
-	})
+	// Querying
+	q, err := query.Parse("status:open author:descartes sort:edit-asc")
+	require.NoError(t, err)
+	res, err := cache.Bugs().Query(q)
+	require.NoError(t, err)
+	require.Len(t, res, 2)
 
-	t.Run("closed caches are empty", func(t *testing.T) {
-		t.Skip()
-		cache, err := NewRepoCacheNoEvents(repo)
-		require.NoError(t, err)
+	q, err = query.Parse("status:open marker") // full-text search
+	require.NoError(t, err)
+	res, err = cache.Bugs().Query(q)
+	require.NoError(t, err)
+	require.Len(t, res, 1)
 
-		require.Empty(t, cache.identities.excerpts)
-	})
+	// Close
+	require.NoError(t, cache.Close())
+	require.Empty(t, cache.bugs.cached)
+	require.Empty(t, cache.bugs.excerpts)
+	require.Empty(t, cache.identities.cached)
+	require.Empty(t, cache.identities.excerpts)
 
 	// 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 = createTestRepoCacheNoEvents(t, repo)
+	cache, err = NewRepoCacheNoEvents(repo)
+	require.NoError(t, err)
 
 	require.Len(t, cache.bugs.cached, 0)
 	require.Len(t, cache.bugs.excerpts, 2)
@@ -136,7 +121,7 @@ func TestCache(t *testing.T) {
 	require.Equal(t, uint64(2), indexCount(t, bug.Namespace))
 
 	// Resolving load from the disk
-	_, err := cache.Identities().Resolve(iden1.Id())
+	_, err = cache.Identities().Resolve(iden1.Id())
 	require.NoError(t, err)
 	_, err = cache.Identities().ResolveExcerpt(iden1.Id())
 	require.NoError(t, err)
@@ -352,14 +337,3 @@ func createTestRepoCacheNoEvents(t *testing.T, repo repository.TestedRepo) *Repo
 
 	return cache
 }
-
-func indexCount(t *testing.T, repo repository.TestedRepo, name string) uint64 {
-	t.Helper()
-
-	idx, err := repo.GetIndex(name)
-	require.NoError(t, err)
-	count, err := idx.DocCount()
-	require.NoError(t, err)
-
-	return count
-}

commands/bridge/bridge_new.go

@@ -33,7 +33,7 @@ func newBridgeNewCommand() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "new",
 		Short: "Configure a new bridge",
-		Long: `	Configure a new bridge by passing flags or/and using interactive terminal prompts. You can avoid all the terminal prompts by passing all the necessary flags to configure your bridge.`,
+		Long:  "Configure a new bridge by passing flags or/and using interactive terminal prompts. You can avoid all the terminal prompts by passing all the necessary flags to configure your bridge.",
 		Example: `# Interactive example
 [1]: github
 [2]: gitlab

commands/select/select.go

@@ -39,10 +39,10 @@ type Resolver[CacheT cache.CacheEntity] interface {
 // line. If it fails, it falls back to the select mechanism.
 //
 // Returns:
-// - the entity if any
-// - the new list of command line arguments with the entity prefix removed if it
-//   has been used
-// - an error if the process failed
+//   - the entity if any
+//   - the new list of command line arguments with the entity prefix removed if it
+//     has been used
+//   - an error if the process failed
 func Resolve[CacheT cache.CacheEntity](repo *cache.RepoCache,
 	typename string, namespace string, resolver Resolver[CacheT],
 	args []string) (CacheT, []string, error) {

entities/identity/identity.go

@@ -383,9 +383,9 @@ func (i *Identity) NeedCommit() bool {
 // cleaning would be required.
 //
 // An alternative approach would be to have a determinist rebase:
-// - any commits present in both local and remote version would be kept, never changed.
-// - newer commits would be merged in a linear chain of commits, ordered based on the
-//   Lamport time
+//   - any commits present in both local and remote version would be kept, never changed.
+//   - newer commits would be merged in a linear chain of commits, ordered based on the
+//     Lamport time
 //
 // However, this approach leave the possibility, in the case of a compromised crypto keys,
 // of forging a new version with a bogus Lamport time to be inserted before a legit version,

entities/identity/version.go

@@ -15,8 +15,8 @@ import (
 )
 
 // 1: original format
-// 2: Identity Ids are generated from the first version serialized data instead of from the first git commit
-//    + Identity hold multiple lamport clocks from other entities, instead of just bug edit
+// 2: Identity Ids are generated from the first version serialized data instead of from the first git
+// commit + Identity hold multiple lamport clocks from other entities, instead of just bug edit
 const formatVersion = 2
 
 // version is a complete set of information about an Identity at a point in time.
@@ -31,7 +31,7 @@ type version struct {
 	unixTime int64
 
 	// The set of keys valid at that time, from this version onward, until they get removed
-	// in a new version. This allow to have multiple key for the same identity (e.g. one per
+	// in a new version. This allows to have multiple key for the same identity (e.g. one per
 	// device) as well as revoke key.
 	keys []*Key
 

entity/dag/entity_actions.go

@@ -53,18 +53,18 @@ func Pull[EntityT entity.Interface](def Definition, wrapper func(e *Entity) Enti
 // MergeAll will merge all the available remote Entity:
 //
 // Multiple scenario exist:
-// 1. if the remote Entity doesn't exist locally, it's created
-//    --> emit entity.MergeStatusNew
-// 2. if the remote and local Entity have the same state, nothing is changed
-//    --> emit entity.MergeStatusNothing
-// 3. if the local Entity has new commits but the remote don't, nothing is changed
-//    --> emit entity.MergeStatusNothing
-// 4. if the remote has new commit, the local bug is updated to match the same history
-//    (fast-forward update)
-//    --> emit entity.MergeStatusUpdated
-// 5. if both local and remote Entity have new commits (that is, we have a concurrent edition),
-//    a merge commit with an empty operationPack is created to join both branch and form a DAG.
-//    --> emit entity.MergeStatusUpdated
+//  1. if the remote Entity doesn't exist locally, it's created
+//     --> emit entity.MergeStatusNew
+//  2. if the remote and local Entity have the same state, nothing is changed
+//     --> emit entity.MergeStatusNothing
+//  3. if the local Entity has new commits but the remote don't, nothing is changed
+//     --> emit entity.MergeStatusNothing
+//  4. if the remote has new commit, the local bug is updated to match the same history
+//     (fast-forward update)
+//     --> emit entity.MergeStatusUpdated
+//  5. if both local and remote Entity have new commits (that is, we have a concurrent edition),
+//     a merge commit with an empty operationPack is created to join both branch and form a DAG.
+//     --> emit entity.MergeStatusUpdated
 //
 // Note: an author is necessary for the case where a merge commit is created, as this commit will
 // have an author and may be signed if a signing key is available.