Merge pull request #411 from MichaelMure/repo-rework

Repository rework

12 files changed, 280 insertions, 292 deletions

diff --git a/repository/config_mem.go b/repository/config_mem.go
index 5ce577ac..9725e8d5 100644
--- a/repository/config_mem.go
+++ b/repository/config_mem.go
@@ -1,6 +1,7 @@
 package repository
 
 import (
+	"fmt"
 	"strconv"
 	"strings"
 	"time"
@@ -77,10 +78,17 @@ func (mc *MemConfig) ReadTimestamp(key string) (time.Time, error) {
 
 // RmConfigs remove all key/value pair matching the key prefix
 func (mc *MemConfig) RemoveAll(keyPrefix string) error {
+	found := false
 	for key := range mc.config {
 		if strings.HasPrefix(key, keyPrefix) {
 			delete(mc.config, key)
+			found = true
 		}
 	}
+
+	if !found {
+		return fmt.Errorf("section not found")
+	}
+
 	return nil
 }
diff --git a/repository/config_mem_test.go b/repository/config_mem_test.go
new file mode 100644
index 00000000..d9c33851
--- /dev/null
+++ b/repository/config_mem_test.go
@@ -0,0 +1,7 @@
+package repository
+
+import "testing"
+
+func TestNewMemConfig(t *testing.T) {
+	testConfig(t, NewMemConfig())
+}
diff --git a/repository/config_testing.go b/repository/config_testing.go
new file mode 100644
index 00000000..25639d59
--- /dev/null
+++ b/repository/config_testing.go
@@ -0,0 +1,63 @@
+package repository
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func testConfig(t *testing.T, config Config) {
+	err := config.StoreString("section.key", "value")
+	assert.NoError(t, err)
+
+	val, err := config.ReadString("section.key")
+	assert.NoError(t, err)
+	assert.Equal(t, "value", val)
+
+	err = config.StoreString("section.true", "true")
+	assert.NoError(t, err)
+
+	val2, err := config.ReadBool("section.true")
+	assert.NoError(t, err)
+	assert.Equal(t, true, val2)
+
+	configs, err := config.ReadAll("section")
+	assert.NoError(t, err)
+	assert.Equal(t, map[string]string{
+		"section.key":  "value",
+		"section.true": "true",
+	}, configs)
+
+	err = config.RemoveAll("section.true")
+	assert.NoError(t, err)
+
+	configs, err = config.ReadAll("section")
+	assert.NoError(t, err)
+	assert.Equal(t, map[string]string{
+		"section.key": "value",
+	}, configs)
+
+	_, err = config.ReadBool("section.true")
+	assert.Equal(t, ErrNoConfigEntry, err)
+
+	err = config.RemoveAll("section.nonexistingkey")
+	assert.Error(t, err)
+
+	err = config.RemoveAll("section.key")
+	assert.NoError(t, err)
+
+	_, err = config.ReadString("section.key")
+	assert.Equal(t, ErrNoConfigEntry, err)
+
+	err = config.RemoveAll("nonexistingsection")
+	assert.Error(t, err)
+
+	err = config.RemoveAll("section")
+	assert.Error(t, err)
+
+	_, err = config.ReadString("section.key")
+	assert.Error(t, err)
+
+	err = config.RemoveAll("section.key")
+	assert.Error(t, err)
+}
diff --git a/repository/git.go b/repository/git.go
index 1eb3b8b5..e4cb57e6 100644
--- a/repository/git.go
+++ b/repository/git.go
@@ -8,30 +8,25 @@ import (
 	"os/exec"
 	"path"
 	"strings"
-
-	"github.com/pkg/errors"
+	"sync"
 
 	"github.com/MichaelMure/git-bug/util/git"
 	"github.com/MichaelMure/git-bug/util/lamport"
 )
 
 const (
-	createClockFile = "/git-bug/create-clock"
-	editClockFile   = "/git-bug/edit-clock"
-)
-
-var (
-	// ErrNotARepo is the error returned when the git repo root wan't be found
-	ErrNotARepo = errors.New("not a git repository")
+	clockPath = "git-bug"
 )
 
 var _ ClockedRepo = &GitRepo{}
+var _ TestedRepo = &GitRepo{}
 
 // GitRepo represents an instance of a (local) git repository.
 type GitRepo struct {
-	Path        string
-	createClock *lamport.Persisted
-	editClock   *lamport.Persisted
+	path string
+
+	clocksMutex sync.Mutex
+	clocks      map[string]lamport.Clock
 }
 
 // LocalConfig give access to the repository scoped configuration
@@ -46,19 +41,14 @@ func (repo *GitRepo) GlobalConfig() Config {
 
 // Run the given git command with the given I/O reader/writers, returning an error if it fails.
 func (repo *GitRepo) runGitCommandWithIO(stdin io.Reader, stdout, stderr io.Writer, args ...string) error {
-	repopath := repo.Path
-	if repopath == ".git" {
-		// seeduvax> trangely the git command sometimes fail for very unknown
-		// reason wihtout this replacement.
-		// observed with rev-list command when git-bug is called from git
-		// hook script, even the same command with same args runs perfectly
-		// when called directly from the same hook script.
-		repopath = ""
-	}
-	// fmt.Printf("[%s] Running git %s\n", repopath, strings.Join(args, " "))
+	// make sure that the working directory for the command
+	// always exist, in particular when running "git init".
+	path := strings.TrimSuffix(repo.path, ".git")
+
+	// fmt.Printf("[%s] Running git %s\n", path, strings.Join(args, " "))
 
 	cmd := exec.Command("git", args...)
-	cmd.Dir = repopath
+	cmd.Dir = path
 	cmd.Stdin = stdin
 	cmd.Stdout = stdout
 	cmd.Stderr = stderr
@@ -93,8 +83,11 @@ func (repo *GitRepo) runGitCommand(args ...string) (string, error) {
 
 // NewGitRepo determines if the given working directory is inside of a git repository,
 // and returns the corresponding GitRepo instance if it is.
-func NewGitRepo(path string, witnesser Witnesser) (*GitRepo, error) {
-	repo := &GitRepo{Path: path}
+func NewGitRepo(path string, clockLoaders []ClockLoader) (*GitRepo, error) {
+	repo := &GitRepo{
+		path:   path,
+		clocks: make(map[string]lamport.Clock),
+	}
 
 	// Check the repo and retrieve the root path
 	stdout, err := repo.runGitCommand("rev-parse", "--git-dir")
@@ -107,28 +100,22 @@ func NewGitRepo(path string, witnesser Witnesser) (*GitRepo, error) {
 	}
 
 	// Fix the path to be sure we are at the root
-	repo.Path = stdout
-
-	err = repo.LoadClocks()
-
-	if err != nil {
-		// No clock yet, trying to initialize them
-		err = repo.createClocks()
-		if err != nil {
-			return nil, err
+	repo.path = stdout
+
+	for _, loader := range clockLoaders {
+		allExist := true
+		for _, name := range loader.Clocks {
+			if _, err := repo.getClock(name); err != nil {
+				allExist = false
+			}
 		}
 
-		err = witnesser(repo)
-		if err != nil {
-			return nil, err
+		if !allExist {
+			err = loader.Witnesser(repo)
+			if err != nil {
+				return nil, err
+			}
 		}
-
-		err = repo.WriteClocks()
-		if err != nil {
-			return nil, err
-		}
-
-		return repo, nil
 	}
 
 	return repo, nil
@@ -136,13 +123,12 @@ func NewGitRepo(path string, witnesser Witnesser) (*GitRepo, error) {
 
 // InitGitRepo create a new empty git repo at the given path
 func InitGitRepo(path string) (*GitRepo, error) {
-	repo := &GitRepo{Path: path + "/.git"}
-	err := repo.createClocks()
-	if err != nil {
-		return nil, err
+	repo := &GitRepo{
+		path:   path + "/.git",
+		clocks: make(map[string]lamport.Clock),
 	}
 
-	_, err = repo.runGitCommand("init", path)
+	_, err := repo.runGitCommand("init", path)
 	if err != nil {
 		return nil, err
 	}
@@ -152,13 +138,12 @@ func InitGitRepo(path string) (*GitRepo, error) {
 
 // InitBareGitRepo create a new --bare empty git repo at the given path
 func InitBareGitRepo(path string) (*GitRepo, error) {
-	repo := &GitRepo{Path: path}
-	err := repo.createClocks()
-	if err != nil {
-		return nil, err
+	repo := &GitRepo{
+		path:   path,
+		clocks: make(map[string]lamport.Clock),
 	}
 
-	_, err = repo.runGitCommand("init", "--bare", path)
+	_, err := repo.runGitCommand("init", "--bare", path)
 	if err != nil {
 		return nil, err
 	}
@@ -168,7 +153,7 @@ func InitBareGitRepo(path string) (*GitRepo, error) {
 
 // GetPath returns the path to the repo.
 func (repo *GitRepo) GetPath() string {
-	return repo.Path
+	return repo.path
 }
 
 // GetUserName returns the name the the user has used to configure git
@@ -385,93 +370,56 @@ func (repo *GitRepo) GetTreeHash(commit git.Hash) (git.Hash, error) {
 	return git.Hash(stdout), nil
 }
 
-// AddRemote add a new remote to the repository
-// Not in the interface because it's only used for testing
-func (repo *GitRepo) AddRemote(name string, url string) error {
-	_, err := repo.runGitCommand("remote", "add", name, url)
-
-	return err
-}
-
-func (repo *GitRepo) createClocks() error {
-	createPath := path.Join(repo.Path, createClockFile)
-	createClock, err := lamport.NewPersisted(createPath)
-	if err != nil {
-		return err
+// GetOrCreateClock return a Lamport clock stored in the Repo.
+// If the clock doesn't exist, it's created.
+func (repo *GitRepo) GetOrCreateClock(name string) (lamport.Clock, error) {
+	c, err := repo.getClock(name)
+	if err == nil {
+		return c, nil
 	}
-
-	editPath := path.Join(repo.Path, editClockFile)
-	editClock, err := lamport.NewPersisted(editPath)
-	if err != nil {
-		return err
+	if err != ErrClockNotExist {
+		return nil, err
 	}
 
-	repo.createClock = createClock
-	repo.editClock = editClock
+	repo.clocksMutex.Lock()
+	defer repo.clocksMutex.Unlock()
 
-	return nil
-}
-
-// LoadClocks read the clocks values from the on-disk repo
-func (repo *GitRepo) LoadClocks() error {
-	createClock, err := lamport.LoadPersisted(repo.GetPath() + createClockFile)
-	if err != nil {
-		return err
-	}
+	p := path.Join(repo.path, clockPath, name+"-clock")
 
-	editClock, err := lamport.LoadPersisted(repo.GetPath() + editClockFile)
+	c, err = lamport.NewPersistedClock(p)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
-	repo.createClock = createClock
-	repo.editClock = editClock
-	return nil
+	repo.clocks[name] = c
+	return c, nil
 }
 
-// WriteClocks write the clocks values into the repo
-func (repo *GitRepo) WriteClocks() error {
-	err := repo.createClock.Write()
-	if err != nil {
-		return err
-	}
+func (repo *GitRepo) getClock(name string) (lamport.Clock, error) {
+	repo.clocksMutex.Lock()
+	defer repo.clocksMutex.Unlock()
 
-	err = repo.editClock.Write()
-	if err != nil {
-		return err
+	if c, ok := repo.clocks[name]; ok {
+		return c, nil
 	}
 
-	return nil
-}
-
-// CreateTime return the current value of the creation clock
-func (repo *GitRepo) CreateTime() lamport.Time {
-	return repo.createClock.Time()
-}
-
-// CreateTimeIncrement increment the creation clock and return the new value.
-func (repo *GitRepo) CreateTimeIncrement() (lamport.Time, error) {
-	return repo.createClock.Increment()
-}
+	p := path.Join(repo.path, clockPath, name+"-clock")
 
-// EditTime return the current value of the edit clock
-func (repo *GitRepo) EditTime() lamport.Time {
-	return repo.editClock.Time()
-}
-
-// EditTimeIncrement increment the edit clock and return the new value.
-func (repo *GitRepo) EditTimeIncrement() (lamport.Time, error) {
-	return repo.editClock.Increment()
+	c, err := lamport.LoadPersistedClock(p)
+	if err == nil {
+		repo.clocks[name] = c
+		return c, nil
+	}
+	if err == lamport.ErrClockNotExist {
+		return nil, ErrClockNotExist
+	}
+	return nil, err
 }
 
-// WitnessCreate witness another create time and increment the corresponding clock
-// if needed.
-func (repo *GitRepo) WitnessCreate(time lamport.Time) error {
-	return repo.createClock.Witness(time)
-}
+// AddRemote add a new remote to the repository
+// Not in the interface because it's only used for testing
+func (repo *GitRepo) AddRemote(name string, url string) error {
+	_, err := repo.runGitCommand("remote", "add", name, url)
 
-// WitnessEdit witness another edition time and increment the corresponding clock
-// if needed.
-func (repo *GitRepo) WitnessEdit(time lamport.Time) error {
-	return repo.editClock.Witness(time)
+	return err
 }
diff --git a/repository/config_git.go b/repository/git_config.go
index 987cf195..987cf195 100644
--- a/repository/config_git.go
+++ b/repository/git_config.go
diff --git a/repository/git_test.go b/repository/git_test.go
index de442e39..1b36fd4c 100644
--- a/repository/git_test.go
+++ b/repository/git_test.go
@@ -3,65 +3,8 @@ package repository
 
 import (
 	"testing"
-
-	"github.com/stretchr/testify/assert"
 )
 
-func TestConfig(t *testing.T) {
-	repo := CreateTestRepo(false)
-	defer CleanupTestRepos(t, repo)
-
-	err := repo.LocalConfig().StoreString("section.key", "value")
-	assert.NoError(t, err)
-
-	val, err := repo.LocalConfig().ReadString("section.key")
-	assert.NoError(t, err)
-	assert.Equal(t, "value", val)
-
-	err = repo.LocalConfig().StoreString("section.true", "true")
-	assert.NoError(t, err)
-
-	val2, err := repo.LocalConfig().ReadBool("section.true")
-	assert.NoError(t, err)
-	assert.Equal(t, true, val2)
-
-	configs, err := repo.LocalConfig().ReadAll("section")
-	assert.NoError(t, err)
-	assert.Equal(t, configs, map[string]string{
-		"section.key":  "value",
-		"section.true": "true",
-	})
-
-	err = repo.LocalConfig().RemoveAll("section.true")
-	assert.NoError(t, err)
-
-	configs, err = repo.LocalConfig().ReadAll("section")
-	assert.NoError(t, err)
-	assert.Equal(t, configs, map[string]string{
-		"section.key": "value",
-	})
-
-	_, err = repo.LocalConfig().ReadBool("section.true")
-	assert.Equal(t, ErrNoConfigEntry, err)
-
-	err = repo.LocalConfig().RemoveAll("section.nonexistingkey")
-	assert.Error(t, err)
-
-	err = repo.LocalConfig().RemoveAll("section.key")
-	assert.NoError(t, err)
-
-	_, err = repo.LocalConfig().ReadString("section.key")
-	assert.Equal(t, ErrNoConfigEntry, err)
-
-	err = repo.LocalConfig().RemoveAll("nonexistingsection")
-	assert.Error(t, err)
-
-	err = repo.LocalConfig().RemoveAll("section")
-	assert.Error(t, err)
-
-	_, err = repo.LocalConfig().ReadString("section.key")
-	assert.Error(t, err)
-
-	err = repo.LocalConfig().RemoveAll("section.key")
-	assert.Error(t, err)
+func TestGitRepo(t *testing.T) {
+	RepoTest(t, CreateTestRepo, CleanupTestRepos)
 }
diff --git a/repository/git_testing.go b/repository/git_testing.go
index 37a15d93..5ae4ccc9 100644
--- a/repository/git_testing.go
+++ b/repository/git_testing.go
@@ -3,21 +3,16 @@ package repository
 import (
 	"io/ioutil"
 	"log"
-	"os"
-	"strings"
-	"testing"
 )
 
 // This is intended for testing only
 
-func CreateTestRepo(bare bool) *GitRepo {
+func CreateTestRepo(bare bool) TestedRepo {
 	dir, err := ioutil.TempDir("", "")
 	if err != nil {
 		log.Fatal(err)
 	}
 
-	// fmt.Println("Creating repo:", dir)
-
 	var creator func(string) (*GitRepo, error)
 
 	if bare {
@@ -42,38 +37,7 @@ func CreateTestRepo(bare bool) *GitRepo {
 	return repo
 }
 
-func CleanupTestRepos(t testing.TB, repos ...Repo) {
-	var firstErr error
-	for _, repo := range repos {
-		path := repo.GetPath()
-		if strings.HasSuffix(path, "/.git") {
-			// for a normal repository (not --bare), we want to remove everything
-			// including the parent directory where files are checked out
-			path = strings.TrimSuffix(path, "/.git")
-
-			// Testing non-bare repo should also check path is
-			// only .git (i.e. ./.git), but doing so, we should
-			// try to remove the current directory and hav some
-			// trouble. In the present case, this case should not
-			// occur.
-			// TODO consider warning or error when path == ".git"
-		}
-		// fmt.Println("Cleaning repo:", path)
-		err := os.RemoveAll(path)
-		if err != nil {
-			log.Println(err)
-			if firstErr == nil {
-				firstErr = err
-			}
-		}
-	}
-
-	if firstErr != nil {
-		t.Fatal(firstErr)
-	}
-}
-
-func SetupReposAndRemote(t testing.TB) (repoA, repoB, remote *GitRepo) {
+func SetupReposAndRemote() (repoA, repoB, remote TestedRepo) {
 	repoA = CreateTestRepo(false)
 	repoB = CreateTestRepo(false)
 	remote = CreateTestRepo(true)
@@ -82,12 +46,12 @@ func SetupReposAndRemote(t testing.TB) (repoA, repoB, remote *GitRepo) {
 
 	err := repoA.AddRemote("origin", remoteAddr)
 	if err != nil {
-		t.Fatal(err)
+		log.Fatal(err)
 	}
 
 	err = repoB.AddRemote("origin", remoteAddr)
 	if err != nil {
-		t.Fatal(err)
+		log.Fatal(err)
 	}
 
 	return repoA, repoB, remote
diff --git a/repository/mock_repo.go b/repository/mock_repo.go
index 89d0f395..04d9355e 100644
--- a/repository/mock_repo.go
+++ b/repository/mock_repo.go
@@ -9,6 +9,7 @@ import (
 )
 
 var _ ClockedRepo = &mockRepoForTest{}
+var _ TestedRepo = &mockRepoForTest{}
 
 // mockRepoForTest defines an instance of Repo that can be used for testing.
 type mockRepoForTest struct {
@@ -18,8 +19,7 @@ type mockRepoForTest struct {
 	trees        map[git.Hash]string
 	commits      map[git.Hash]commit
 	refs         map[string]git.Hash
-	createClock  lamport.Clock
-	editClock    lamport.Clock
+	clocks       map[string]lamport.Clock
 }
 
 type commit struct {
@@ -35,8 +35,7 @@ func NewMockRepoForTest() *mockRepoForTest {
 		trees:        make(map[git.Hash]string),
 		commits:      make(map[git.Hash]commit),
 		refs:         make(map[string]git.Hash),
-		createClock:  lamport.NewClock(),
-		editClock:    lamport.NewClock(),
+		clocks:       make(map[string]lamport.Clock),
 	}
 }
 
@@ -213,36 +212,16 @@ func (r *mockRepoForTest) GetTreeHash(commit git.Hash) (git.Hash, error) {
 	panic("implement me")
 }
 
-func (r *mockRepoForTest) LoadClocks() error {
-	return nil
-}
-
-func (r *mockRepoForTest) WriteClocks() error {
-	return nil
-}
-
-func (r *mockRepoForTest) CreateTime() lamport.Time {
-	return r.createClock.Time()
-}
-
-func (r *mockRepoForTest) CreateTimeIncrement() (lamport.Time, error) {
-	return r.createClock.Increment(), nil
-}
-
-func (r *mockRepoForTest) EditTime() lamport.Time {
-	return r.editClock.Time()
-}
-
-func (r *mockRepoForTest) EditTimeIncrement() (lamport.Time, error) {
-	return r.editClock.Increment(), nil
-}
+func (r *mockRepoForTest) GetOrCreateClock(name string) (lamport.Clock, error) {
+	if c, ok := r.clocks[name]; ok {
+		return c, nil
+	}
 
-func (r *mockRepoForTest) WitnessCreate(time lamport.Time) error {
-	r.createClock.Witness(time)
-	return nil
+	c := lamport.NewMemClock()
+	r.clocks[name] = c
+	return c, nil
 }
 
-func (r *mockRepoForTest) WitnessEdit(time lamport.Time) error {
-	r.editClock.Witness(time)
-	return nil
+func (r *mockRepoForTest) AddRemote(name string, url string) error {
+	panic("implement me")
 }
diff --git a/repository/mock_repo_test.go b/repository/mock_repo_test.go
new file mode 100644
index 00000000..b56b94f2
--- /dev/null
+++ b/repository/mock_repo_test.go
@@ -0,0 +1,10 @@
+package repository
+
+import "testing"
+
+func TestMockRepo(t *testing.T) {
+	creator := func(bare bool) TestedRepo { return NewMockRepoForTest() }
+	cleaner := func(repos ...Repo) {}
+
+	RepoTest(t, creator, cleaner)
+}
diff --git a/repository/repo.go b/repository/repo.go
index e8517508..561fc1c5 100644
--- a/repository/repo.go
+++ b/repository/repo.go
@@ -13,6 +13,10 @@ import (
 var (
 	ErrNoConfigEntry       = errors.New("no config entry for the given key")
 	ErrMultipleConfigEntry = errors.New("multiple config entry for the given key")
+	// ErrNotARepo is the error returned when the git repo root wan't be found
+	ErrNotARepo = errors.New("not a git repository")
+	// ErrClockNotExist is the error returned when a clock can't be found
+	ErrClockNotExist = errors.New("clock doesn't exist")
 )
 
 // RepoConfig access the configuration of a repository
@@ -97,36 +101,22 @@ type Repo interface {
 type ClockedRepo interface {
 	Repo
 
-	// LoadClocks read the clocks values from the on-disk repo
-	LoadClocks() error
-
-	// WriteClocks write the clocks values into the repo
-	WriteClocks() error
-
-	// CreateTime return the current value of the creation clock
-	CreateTime() lamport.Time
-
-	// CreateTimeIncrement increment the creation clock and return the new value.
-	CreateTimeIncrement() (lamport.Time, error)
-
-	// EditTime return the current value of the edit clock
-	EditTime() lamport.Time
-
-	// EditTimeIncrement increment the edit clock and return the new value.
-	EditTimeIncrement() (lamport.Time, error)
-
-	// WitnessCreate witness another create time and increment the corresponding
-	// clock if needed.
-	WitnessCreate(time lamport.Time) error
-
-	// WitnessEdit witness another edition time and increment the corresponding
-	// clock if needed.
-	WitnessEdit(time lamport.Time) error
+	// GetOrCreateClock return a Lamport clock stored in the Repo.
+	// If the clock doesn't exist, it's created.
+	GetOrCreateClock(name string) (lamport.Clock, error)
 }
 
-// Witnesser is a function that will initialize the clocks of a repo
-// from scratch
-type Witnesser func(repo ClockedRepo) error
+// ClockLoader hold which logical clock need to exist for an entity and
+// how to create them if they don't.
+type ClockLoader struct {
+	// Clocks hold the name of all the clocks this loader deal with.
+	// Those clocks will be checked when the repo load. If not present or broken,
+	// Witnesser will be used to create them.
+	Clocks []string
+	// Witnesser is a function that will initialize the clocks of a repo
+	// from scratch
+	Witnesser func(repo ClockedRepo) error
+}
 
 func prepareTreeEntries(entries []TreeEntry) bytes.Buffer {
 	var buffer bytes.Buffer
@@ -158,3 +148,11 @@ func readTreeEntries(s string) ([]TreeEntry, error) {
 
 	return casted, nil
 }
+
+// TestedRepo is an extended ClockedRepo with function for testing only
+type TestedRepo interface {
+	ClockedRepo
+
+	// AddRemote add a new remote to the repository
+	AddRemote(name string, url string) error
+}
diff --git a/repository/repo_testing.go b/repository/repo_testing.go
new file mode 100644
index 00000000..43bbaf14
--- /dev/null
+++ b/repository/repo_testing.go
@@ -0,0 +1,70 @@
+package repository
+
+import (
+	"log"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func CleanupTestRepos(repos ...Repo) {
+	var firstErr error
+	for _, repo := range repos {
+		path := repo.GetPath()
+		if strings.HasSuffix(path, "/.git") {
+			// for a normal repository (not --bare), we want to remove everything
+			// including the parent directory where files are checked out
+			path = strings.TrimSuffix(path, "/.git")
+
+			// Testing non-bare repo should also check path is
+			// only .git (i.e. ./.git), but doing so, we should
+			// try to remove the current directory and hav some
+			// trouble. In the present case, this case should not
+			// occur.
+			// TODO consider warning or error when path == ".git"
+		}
+		// fmt.Println("Cleaning repo:", path)
+		err := os.RemoveAll(path)
+		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)
+
+// Test suite for a Repo implementation
+func RepoTest(t *testing.T, creator RepoCreator, cleaner RepoCleaner) {
+	t.Run("Read/Write data", func(t *testing.T) {
+		repo := creator(false)
+		defer cleaner(repo)
+
+		data := []byte("hello")
+
+		h, err := repo.StoreData(data)
+		require.NoError(t, err)
+		assert.NotEmpty(t, h)
+
+		read, err := repo.ReadData(h)
+		require.NoError(t, err)
+		assert.Equal(t, data, read)
+	})
+
+	t.Run("Local config", func(t *testing.T) {
+		repo := creator(false)
+		defer cleaner(repo)
+
+		testConfig(t, repo.LocalConfig())
+	})
+}
diff --git a/repository/tree_entry_test.go b/repository/tree_entry_test.go
index 58da075c..78f58ff6 100644
--- a/repository/tree_entry_test.go
+++ b/repository/tree_entry_test.go
@@ -3,11 +3,12 @@ package repository
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
 	"github.com/MichaelMure/git-bug/util/git"
 )
 
 func TestTreeEntryFormat(t *testing.T) {
-
 	entries := []TreeEntry{
 		{Blob, git.Hash("a85730cf5287d40a1e32d3a671ba2296c73387cb"), "name"},
 		{Tree, git.Hash("a85730cf5287d40a1e32d3a671ba2296c73387cb"), "name"},
@@ -26,10 +27,7 @@ func TestTreeEntryParse(t *testing.T) {
 
 	for _, line := range lines {
 		_, err := ParseTreeEntry(line)
-
-		if err != nil {
-			t.Fatal(err)
-		}
+		assert.NoError(t, err)
 	}
 
 }