From 8ab4681c26204b26f86e19689edde736c23e8f03 Mon Sep 17 00:00:00 2001 From: Grzegorz Dlugoszewski Date: Wed, 8 Jul 2020 13:37:32 +0200 Subject: [PATCH] Refactor repoFinder - Remove io package and move finder to git package - Move tempDir and writeFile into test package. They are only used for testing purposes anyway. - Add a way to specify parent folder for tempDir. Useful for testing nested repos. - Add new test repos with .git/config files --- pkg/get.go | 3 +-- pkg/git/config_test.go | 34 ++++++------------------- pkg/{io/io.go => git/finder.go} | 44 +++++++-------------------------- pkg/git/repo.go | 4 +-- pkg/git/repo_test.go | 3 +-- pkg/list.go | 4 +-- pkg/test/helpers.go | 28 +++++++++++++++------ pkg/test/testrepos.go | 27 ++++++++++++++++++-- 8 files changed, 68 insertions(+), 79 deletions(-) rename pkg/{io/io.go => git/finder.go} (65%) diff --git a/pkg/get.go b/pkg/get.go index f4ab85d..93cf25b 100644 --- a/pkg/get.go +++ b/pkg/get.go @@ -3,7 +3,6 @@ package pkg import ( "fmt" "git-get/pkg/git" - "git-get/pkg/io" "path/filepath" ) @@ -69,7 +68,7 @@ func cloneDumpFile(c *GetCfg) error { } // If target path already exists, skip cloning this repo - if exists, _ := io.Exists(opts.Path); exists { + if exists, _ := git.Exists(opts.Path); exists { continue } diff --git a/pkg/git/config_test.go b/pkg/git/config_test.go index 6a5bc67..d63a7b4 100644 --- a/pkg/git/config_test.go +++ b/pkg/git/config_test.go @@ -1,27 +1,18 @@ package git import ( - "git-get/pkg/io" "git-get/pkg/run" "git-get/pkg/test" - "path/filepath" "testing" ) // cfgStub represents a gitconfig file but instead of using a global one, it creates a temporary git repo and uses its local gitconfig. type cfgStub struct { - repo *test.Repo -} - -func newCfgStub(t *testing.T) *cfgStub { - r := test.RepoEmpty(t) - return &cfgStub{ - repo: r, - } + *test.Repo } func (c *cfgStub) Get(key string) string { - out, err := run.Git("config", "--local", key).OnRepo(c.repo.Path()).AndCaptureLine() + out, err := run.Git("config", "--local", key).OnRepo(c.Path()).AndCaptureLine() if err != nil { return "" } @@ -74,22 +65,13 @@ func TestGitConfig(t *testing.T) { } func makeConfigEmpty(t *testing.T) *cfgStub { - c := newCfgStub(t) - io.Write(filepath.Join(c.repo.Path(), dotgit, "config"), "") - - return c + return &cfgStub{ + Repo: test.RepoWithEmptyConfig(t), + } } func makeConfigValid(t *testing.T) *cfgStub { - c := newCfgStub(t) - - gitconfig := ` - [user] - name = grdl - [gitget] - host = github.com - ` - io.Write(filepath.Join(c.repo.Path(), dotgit, "config"), gitconfig) - - return c + return &cfgStub{ + Repo: test.RepoWithValidConfig(t), + } } diff --git a/pkg/io/io.go b/pkg/git/finder.go similarity index 65% rename from pkg/io/io.go rename to pkg/git/finder.go index 9bfac0d..f65166c 100644 --- a/pkg/io/io.go +++ b/pkg/git/finder.go @@ -1,9 +1,7 @@ -// Package io provides functions to read, write and search files and directories. -package io +package git import ( "fmt" - "io/ioutil" "os" "path/filepath" "strings" @@ -13,36 +11,12 @@ import ( "github.com/pkg/errors" ) -// ErrSkipNode is used as an error indicating that .git directory has been found. +// errSkipNode is used as an error indicating that .git directory has been found. // It's handled by ErrorsCallback to tell the WalkCallback to skip this dir. -var ErrSkipNode = errors.New(".git directory found, skipping this node") +var errSkipNode = errors.New(".git directory found, skipping this node") -// ErrDirectoryAccess indicated a directory doesn't exists or can't be accessed -var ErrDirectoryAccess = errors.New("directory doesn't exist or can't be accessed") - -// TempDir creates a temporary directory for test repos. -func TempDir() (string, error) { - dir, err := ioutil.TempDir("", "git-get-repo-") - if err != nil { - return "", errors.Wrap(err, "failed creating test repo directory") - } - - return dir, nil -} - -// Write writes string content into a file. If file doesn't exists, it will create it. -func Write(path string, content string) error { - file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) - if err != nil { - return errors.Wrapf(err, "failed opening a file for writing %s", path) - } - - _, err = file.Write([]byte(content)) - if err != nil { - errors.Wrapf(err, "Failed writing to a file %s", path) - } - return nil -} +// errDirectoryAccess indicates a directory doesn't exists or can't be accessed +var errDirectoryAccess = errors.New("directory doesn't exist or can't be accessed") // Exists returns true if a directory exists. If it doesn't or the directory can't be accessed it returns an error. func Exists(path string) (bool, error) { @@ -54,12 +28,12 @@ func Exists(path string) (bool, error) { if err != nil { if os.IsNotExist(err) { - return false, ErrDirectoryAccess + return false, errDirectoryAccess } } // Directory exists but can't be accessed - return true, ErrDirectoryAccess + return true, errDirectoryAccess } // RepoFinder finds paths to git repos inside given path. @@ -105,7 +79,7 @@ func (r *RepoFinder) walkCb(path string, ent *godirwalk.Dirent) error { // Do not traverse .git directories if ent.IsDir() && ent.Name() == ".git" { r.repos = append(r.repos, strings.TrimSuffix(path, ".git")) - return ErrSkipNode + return errSkipNode } // Do not traverse directories containing a .git directory if ent.IsDir() { @@ -121,7 +95,7 @@ func (r *RepoFinder) walkCb(path string, ent *godirwalk.Dirent) error { func (r *RepoFinder) errorCb(_ string, err error) godirwalk.ErrorAction { // Skip .git directory and directories we don't have permissions to access // TODO: Will syscall.EACCES work on windows? - if errors.Is(err, ErrSkipNode) || errors.Is(err, syscall.EACCES) { + if errors.Is(err, errSkipNode) || errors.Is(err, syscall.EACCES) { return godirwalk.SkipNode } return godirwalk.Halt diff --git a/pkg/git/repo.go b/pkg/git/repo.go index 78e5c87..ccb3a0d 100644 --- a/pkg/git/repo.go +++ b/pkg/git/repo.go @@ -2,7 +2,6 @@ package git import ( "fmt" - "git-get/pkg/io" "git-get/pkg/run" "net/url" "strconv" @@ -43,8 +42,7 @@ type CloneOpts struct { // Open checks if given path can be accessed and returns a Repo instance pointing to it. func Open(path string) (Repo, error) { - _, err := io.Exists(path) - if err != nil { + if _, err := Exists(path); err != nil { return nil, err } diff --git a/pkg/git/repo_test.go b/pkg/git/repo_test.go index 1f6c909..6bbd29c 100644 --- a/pkg/git/repo_test.go +++ b/pkg/git/repo_test.go @@ -1,7 +1,6 @@ package git import ( - "git-get/pkg/io" "git-get/pkg/test" "reflect" "testing" @@ -10,7 +9,7 @@ import ( func TestOpen(t *testing.T) { _, err := Open("/paththatdoesnotexist/repo") - if err != io.ErrDirectoryAccess { + if err != errDirectoryAccess { t.Errorf("Opening a repo in non existing path should throw an error") } } diff --git a/pkg/list.go b/pkg/list.go index 9abf72a..66706fa 100644 --- a/pkg/list.go +++ b/pkg/list.go @@ -3,7 +3,7 @@ package pkg import ( "fmt" "git-get/pkg/cfg" - "git-get/pkg/io" + "git-get/pkg/git" "git-get/pkg/print" "sort" "strings" @@ -18,7 +18,7 @@ type ListCfg struct { // List executes the "git list" command. func List(c *ListCfg) error { - paths, err := io.NewRepoFinder(c.Root).Find() + paths, err := git.NewRepoFinder(c.Root).Find() if err != nil { return err } diff --git a/pkg/test/helpers.go b/pkg/test/helpers.go index b9c8593..c662b32 100644 --- a/pkg/test/helpers.go +++ b/pkg/test/helpers.go @@ -2,20 +2,26 @@ package test import ( "fmt" - "git-get/pkg/io" "git-get/pkg/run" + "io/ioutil" + "os" "path/filepath" "testing" ) -func (r *Repo) writeFile(filename string, content string) { - path := filepath.Join(r.path, filename) - err := io.Write(path, content) +func (r *Repo) init() { + err := run.Git("init", "--quiet", r.path).AndShutUp() checkFatal(r.t, err) } -func (r *Repo) init() { - err := run.Git("init", "--quiet", r.path).AndShutUp() +// writeFile writes the content string into a file. If file doesn't exists, it will create it. +func (r *Repo) writeFile(filename string, content string) { + path := filepath.Join(r.path, filename) + + file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) + checkFatal(r.t, err) + + _, err = file.Write([]byte(content)) checkFatal(r.t, err) } @@ -45,7 +51,7 @@ func (r *Repo) checkout(name string) { } func (r *Repo) clone() *Repo { - dir, err := io.TempDir() + dir, err := tempDir("") checkFatal(r.t, err) url := fmt.Sprintf("file://%s/.git", r.path) @@ -66,6 +72,14 @@ func (r *Repo) fetch() { checkFatal(r.t, err) } +// tempDir creates a temporary directory inside the parent dir. +// If parent is empty it will use a system default temp dir (usually /tmp). +func tempDir(parent string) (string, error) { + dir, err := ioutil.TempDir(parent, "git-get-repo-") + + return dir, err +} + func checkFatal(t *testing.T, err error) { if err != nil { t.Fatalf("failed making test repo: %+v", err) diff --git a/pkg/test/testrepos.go b/pkg/test/testrepos.go index 873ede6..2143ad2 100644 --- a/pkg/test/testrepos.go +++ b/pkg/test/testrepos.go @@ -1,8 +1,8 @@ package test import ( - "git-get/pkg/io" "os" + "path/filepath" "testing" ) @@ -29,7 +29,7 @@ func (r *Repo) cleanup() { // RepoEmpty creates an empty git repo. func RepoEmpty(t *testing.T) *Repo { - dir, err := io.TempDir() + dir, err := tempDir("") checkFatal(t, err) r := &Repo{ @@ -178,3 +178,26 @@ func RepoWithBranchAheadAndBehind(t *testing.T) *Repo { return r } + +// RepoWithEmptyConfig creates a git repo with empty .git/config file +func RepoWithEmptyConfig(t *testing.T) *Repo { + r := RepoEmpty(t) + r.writeFile(filepath.Join(".git", "config"), "") + + return r +} + +// RepoWithValidConfig creates a git repo with valid content in .git/config file +func RepoWithValidConfig(t *testing.T) *Repo { + r := RepoEmpty(t) + + gitconfig := ` + [user] + name = grdl + [gitget] + host = github.com + ` + r.writeFile(filepath.Join(".git", "config"), gitconfig) + + return r +}