From 88ad7e606f1cbf9e47b968a208e3510f7f9a81c5 Mon Sep 17 00:00:00 2001 From: Michael Muré Date: Tue, 23 Jun 2020 18:02:54 +0200 Subject: repository: remove tie to Bug, improved and reusable testing - allow the creation of arbitrary Lamport clocks, freeing the way to new entities and removing Bug specific (upper layer) code. - generalize the memory-only and persisted Lamport clocks behind a common interface - rework the tests to provide reusable testing code for a Repo, a Clock, a Config, opening a path to add a new Repo implementation more easily - test previously untested components with those new tests Note: one problem found during this endeavor is that `identity.Version` also need to store one time + Lamport time for each other Entity (Bug, config, PR ...). This could possibly done without breaking change but it would be much easier to wait for https://github.com/MichaelMure/git-bug-migration to happen. --- repository/config_git.go | 221 ------------------------------------------ repository/config_mem.go | 8 ++ repository/config_mem_test.go | 7 ++ repository/config_testing.go | 63 ++++++++++++ repository/git.go | 202 ++++++++++++++------------------------ repository/git_config.go | 221 ++++++++++++++++++++++++++++++++++++++++++ repository/git_test.go | 61 +----------- repository/git_testing.go | 44 +-------- repository/mock_repo.go | 45 +++------ repository/mock_repo_test.go | 10 ++ repository/repo.go | 54 +++++------ repository/repo_testing.go | 70 +++++++++++++ repository/tree_entry_test.go | 8 +- 13 files changed, 501 insertions(+), 513 deletions(-) delete mode 100644 repository/config_git.go create mode 100644 repository/config_mem_test.go create mode 100644 repository/config_testing.go create mode 100644 repository/git_config.go create mode 100644 repository/mock_repo_test.go create mode 100644 repository/repo_testing.go (limited to 'repository') diff --git a/repository/config_git.go b/repository/config_git.go deleted file mode 100644 index 987cf195..00000000 --- a/repository/config_git.go +++ /dev/null @@ -1,221 +0,0 @@ -package repository - -import ( - "fmt" - "regexp" - "strconv" - "strings" - "time" - - "github.com/blang/semver" - "github.com/pkg/errors" -) - -var _ Config = &gitConfig{} - -type gitConfig struct { - repo *GitRepo - localityFlag string -} - -func newGitConfig(repo *GitRepo, global bool) *gitConfig { - localityFlag := "--local" - if global { - localityFlag = "--global" - } - return &gitConfig{ - repo: repo, - localityFlag: localityFlag, - } -} - -// StoreString store a single key/value pair in the config of the repo -func (gc *gitConfig) StoreString(key string, value string) error { - _, err := gc.repo.runGitCommand("config", gc.localityFlag, "--replace-all", key, value) - return err -} - -func (gc *gitConfig) StoreBool(key string, value bool) error { - return gc.StoreString(key, strconv.FormatBool(value)) -} - -func (gc *gitConfig) StoreTimestamp(key string, value time.Time) error { - return gc.StoreString(key, strconv.Itoa(int(value.Unix()))) -} - -// ReadAll read all key/value pair matching the key prefix -func (gc *gitConfig) ReadAll(keyPrefix string) (map[string]string, error) { - stdout, err := gc.repo.runGitCommand("config", gc.localityFlag, "--includes", "--get-regexp", keyPrefix) - - // / \ - // / ! \ - // ------- - // - // There can be a legitimate error here, but I see no portable way to - // distinguish them from the git error that say "no matching value exist" - if err != nil { - return nil, nil - } - - lines := strings.Split(stdout, "\n") - - result := make(map[string]string, len(lines)) - - for _, line := range lines { - if strings.TrimSpace(line) == "" { - continue - } - - parts := strings.SplitN(line, " ", 2) - result[parts[0]] = parts[1] - } - - return result, nil -} - -func (gc *gitConfig) ReadString(key string) (string, error) { - stdout, err := gc.repo.runGitCommand("config", gc.localityFlag, "--includes", "--get-all", key) - - // / \ - // / ! \ - // ------- - // - // There can be a legitimate error here, but I see no portable way to - // distinguish them from the git error that say "no matching value exist" - if err != nil { - return "", ErrNoConfigEntry - } - - lines := strings.Split(stdout, "\n") - - if len(lines) == 0 { - return "", ErrNoConfigEntry - } - if len(lines) > 1 { - return "", ErrMultipleConfigEntry - } - - return lines[0], nil -} - -func (gc *gitConfig) ReadBool(key string) (bool, error) { - val, err := gc.ReadString(key) - if err != nil { - return false, err - } - - return strconv.ParseBool(val) -} - -func (gc *gitConfig) ReadTimestamp(key string) (time.Time, error) { - value, err := gc.ReadString(key) - if err != nil { - return time.Time{}, err - } - return ParseTimestamp(value) -} - -func (gc *gitConfig) rmSection(keyPrefix string) error { - _, err := gc.repo.runGitCommand("config", gc.localityFlag, "--remove-section", keyPrefix) - return err -} - -func (gc *gitConfig) unsetAll(keyPrefix string) error { - _, err := gc.repo.runGitCommand("config", gc.localityFlag, "--unset-all", keyPrefix) - return err -} - -// return keyPrefix section -// example: sectionFromKey(a.b.c.d) return a.b.c -func sectionFromKey(keyPrefix string) string { - s := strings.Split(keyPrefix, ".") - if len(s) == 1 { - return keyPrefix - } - - return strings.Join(s[:len(s)-1], ".") -} - -// rmConfigs with git version lesser than 2.18 -func (gc *gitConfig) rmConfigsGitVersionLT218(keyPrefix string) error { - // try to remove key/value pair by key - err := gc.unsetAll(keyPrefix) - if err != nil { - return gc.rmSection(keyPrefix) - } - - m, err := gc.ReadAll(sectionFromKey(keyPrefix)) - if err != nil { - return err - } - - // if section doesn't have any left key/value remove the section - if len(m) == 0 { - return gc.rmSection(sectionFromKey(keyPrefix)) - } - - return nil -} - -// RmConfigs remove all key/value pair matching the key prefix -func (gc *gitConfig) RemoveAll(keyPrefix string) error { - // starting from git 2.18.0 sections are automatically deleted when the last existing - // key/value is removed. Before 2.18.0 we should remove the section - // see https://github.com/git/git/blob/master/Documentation/RelNotes/2.18.0.txt#L379 - lt218, err := gc.gitVersionLT218() - if err != nil { - return errors.Wrap(err, "getting git version") - } - - if lt218 { - return gc.rmConfigsGitVersionLT218(keyPrefix) - } - - err = gc.unsetAll(keyPrefix) - if err != nil { - return gc.rmSection(keyPrefix) - } - - return nil -} - -func (gc *gitConfig) gitVersion() (*semver.Version, error) { - versionOut, err := gc.repo.runGitCommand("version") - if err != nil { - return nil, err - } - return parseGitVersion(versionOut) -} - -func parseGitVersion(versionOut string) (*semver.Version, error) { - // extract the version and truncate potential bad parts - // ex: 2.23.0.rc1 instead of 2.23.0-rc1 - r := regexp.MustCompile(`(\d+\.){1,2}\d+`) - - extracted := r.FindString(versionOut) - if extracted == "" { - return nil, fmt.Errorf("unreadable git version %s", versionOut) - } - - version, err := semver.Make(extracted) - if err != nil { - return nil, err - } - - return &version, nil -} - -func (gc *gitConfig) gitVersionLT218() (bool, error) { - version, err := gc.gitVersion() - if err != nil { - return false, err - } - - version218string := "2.18.0" - gitVersion218, err := semver.Make(version218string) - if err != nil { - return false, err - } - - return version.LT(gitVersion218), nil -} 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/git_config.go b/repository/git_config.go new file mode 100644 index 00000000..987cf195 --- /dev/null +++ b/repository/git_config.go @@ -0,0 +1,221 @@ +package repository + +import ( + "fmt" + "regexp" + "strconv" + "strings" + "time" + + "github.com/blang/semver" + "github.com/pkg/errors" +) + +var _ Config = &gitConfig{} + +type gitConfig struct { + repo *GitRepo + localityFlag string +} + +func newGitConfig(repo *GitRepo, global bool) *gitConfig { + localityFlag := "--local" + if global { + localityFlag = "--global" + } + return &gitConfig{ + repo: repo, + localityFlag: localityFlag, + } +} + +// StoreString store a single key/value pair in the config of the repo +func (gc *gitConfig) StoreString(key string, value string) error { + _, err := gc.repo.runGitCommand("config", gc.localityFlag, "--replace-all", key, value) + return err +} + +func (gc *gitConfig) StoreBool(key string, value bool) error { + return gc.StoreString(key, strconv.FormatBool(value)) +} + +func (gc *gitConfig) StoreTimestamp(key string, value time.Time) error { + return gc.StoreString(key, strconv.Itoa(int(value.Unix()))) +} + +// ReadAll read all key/value pair matching the key prefix +func (gc *gitConfig) ReadAll(keyPrefix string) (map[string]string, error) { + stdout, err := gc.repo.runGitCommand("config", gc.localityFlag, "--includes", "--get-regexp", keyPrefix) + + // / \ + // / ! \ + // ------- + // + // There can be a legitimate error here, but I see no portable way to + // distinguish them from the git error that say "no matching value exist" + if err != nil { + return nil, nil + } + + lines := strings.Split(stdout, "\n") + + result := make(map[string]string, len(lines)) + + for _, line := range lines { + if strings.TrimSpace(line) == "" { + continue + } + + parts := strings.SplitN(line, " ", 2) + result[parts[0]] = parts[1] + } + + return result, nil +} + +func (gc *gitConfig) ReadString(key string) (string, error) { + stdout, err := gc.repo.runGitCommand("config", gc.localityFlag, "--includes", "--get-all", key) + + // / \ + // / ! \ + // ------- + // + // There can be a legitimate error here, but I see no portable way to + // distinguish them from the git error that say "no matching value exist" + if err != nil { + return "", ErrNoConfigEntry + } + + lines := strings.Split(stdout, "\n") + + if len(lines) == 0 { + return "", ErrNoConfigEntry + } + if len(lines) > 1 { + return "", ErrMultipleConfigEntry + } + + return lines[0], nil +} + +func (gc *gitConfig) ReadBool(key string) (bool, error) { + val, err := gc.ReadString(key) + if err != nil { + return false, err + } + + return strconv.ParseBool(val) +} + +func (gc *gitConfig) ReadTimestamp(key string) (time.Time, error) { + value, err := gc.ReadString(key) + if err != nil { + return time.Time{}, err + } + return ParseTimestamp(value) +} + +func (gc *gitConfig) rmSection(keyPrefix string) error { + _, err := gc.repo.runGitCommand("config", gc.localityFlag, "--remove-section", keyPrefix) + return err +} + +func (gc *gitConfig) unsetAll(keyPrefix string) error { + _, err := gc.repo.runGitCommand("config", gc.localityFlag, "--unset-all", keyPrefix) + return err +} + +// return keyPrefix section +// example: sectionFromKey(a.b.c.d) return a.b.c +func sectionFromKey(keyPrefix string) string { + s := strings.Split(keyPrefix, ".") + if len(s) == 1 { + return keyPrefix + } + + return strings.Join(s[:len(s)-1], ".") +} + +// rmConfigs with git version lesser than 2.18 +func (gc *gitConfig) rmConfigsGitVersionLT218(keyPrefix string) error { + // try to remove key/value pair by key + err := gc.unsetAll(keyPrefix) + if err != nil { + return gc.rmSection(keyPrefix) + } + + m, err := gc.ReadAll(sectionFromKey(keyPrefix)) + if err != nil { + return err + } + + // if section doesn't have any left key/value remove the section + if len(m) == 0 { + return gc.rmSection(sectionFromKey(keyPrefix)) + } + + return nil +} + +// RmConfigs remove all key/value pair matching the key prefix +func (gc *gitConfig) RemoveAll(keyPrefix string) error { + // starting from git 2.18.0 sections are automatically deleted when the last existing + // key/value is removed. Before 2.18.0 we should remove the section + // see https://github.com/git/git/blob/master/Documentation/RelNotes/2.18.0.txt#L379 + lt218, err := gc.gitVersionLT218() + if err != nil { + return errors.Wrap(err, "getting git version") + } + + if lt218 { + return gc.rmConfigsGitVersionLT218(keyPrefix) + } + + err = gc.unsetAll(keyPrefix) + if err != nil { + return gc.rmSection(keyPrefix) + } + + return nil +} + +func (gc *gitConfig) gitVersion() (*semver.Version, error) { + versionOut, err := gc.repo.runGitCommand("version") + if err != nil { + return nil, err + } + return parseGitVersion(versionOut) +} + +func parseGitVersion(versionOut string) (*semver.Version, error) { + // extract the version and truncate potential bad parts + // ex: 2.23.0.rc1 instead of 2.23.0-rc1 + r := regexp.MustCompile(`(\d+\.){1,2}\d+`) + + extracted := r.FindString(versionOut) + if extracted == "" { + return nil, fmt.Errorf("unreadable git version %s", versionOut) + } + + version, err := semver.Make(extracted) + if err != nil { + return nil, err + } + + return &version, nil +} + +func (gc *gitConfig) gitVersionLT218() (bool, error) { + version, err := gc.gitVersion() + if err != nil { + return false, err + } + + version218string := "2.18.0" + gitVersion218, err := semver.Make(version218string) + if err != nil { + return false, err + } + + return version.LT(gitVersion218), nil +} 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) } } -- cgit