Merge pull request #8 from grdl/io_pkg_refactor

io package refactoring

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
 		}
 

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),
+	}
 }

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,14 +79,15 @@ 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() {
 		_, err := os.Stat(filepath.Join(path, ".git"))
 		if err == nil {
 			r.repos = append(r.repos, strings.TrimSuffix(path, ".git"))
-			return ErrSkipNode
+			return errSkipNode
 		}
 	}
 	return nil
@@ -121,7 +96,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

pkg/git/finder_test.go

@@ -0,0 +1,90 @@
+package git
+
+import (
+	"git-get/pkg/test"
+	"testing"
+)
+
+func TestFinder(t *testing.T) {
+	tests := []struct {
+		name       string
+		reposMaker func(*testing.T) string
+		want       int
+	}{
+		{
+			name:       "no repos",
+			reposMaker: makeNoRepos,
+			want:       0,
+		},
+		{
+			name:       "single repos",
+			reposMaker: makeSingleRepo,
+			want:       1,
+		},
+		{
+			name:       "single nested repo",
+			reposMaker: makeNestedRepo,
+			want:       1,
+		},
+		{
+			name:       "multiple nested repo",
+			reposMaker: makeMultipleNestedRepos,
+			want:       2,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			root := test.reposMaker(t)
+
+			finder := NewRepoFinder(root)
+			paths, _ := finder.Find()
+
+			if len(paths) != test.want {
+				t.Errorf("expected %d; got %d", test.want, len(paths))
+			}
+		})
+	}
+}
+
+func makeNoRepos(t *testing.T) string {
+	root := test.TempDir(t, "")
+
+	return root
+}
+
+func makeSingleRepo(t *testing.T) string {
+	root := test.TempDir(t, "")
+
+	test.RepoEmptyInDir(t, root)
+
+	return root
+}
+
+func makeNestedRepo(t *testing.T) string {
+	// a repo with single nested repo should still be counted as one beacause finder doesn't traverse inside nested repos
+	root := test.TempDir(t, "")
+
+	r := test.RepoEmptyInDir(t, root)
+	test.RepoEmptyInDir(t, r.Path())
+
+	return root
+}
+
+func makeMultipleNestedRepos(t *testing.T) string {
+	root := test.TempDir(t, "")
+
+	// create two repos inside root - should be counted as 2
+	repo1 := test.RepoEmptyInDir(t, root)
+	repo2 := test.RepoEmptyInDir(t, root)
+
+	// created repos nested inside two parent roots - shouldn't be counted
+	test.RepoEmptyInDir(t, repo1.Path())
+	test.RepoEmptyInDir(t, repo1.Path())
+	test.RepoEmptyInDir(t, repo2.Path())
+
+	// create a empty dir inside root - shouldn't be counted
+	test.TempDir(t, root)
+
+	return root
+}

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
 	}
 

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")
 	}
 }

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
 	}

pkg/test/helpers.go

@@ -2,16 +2,28 @@ 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)
-	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(t *testing.T, parent string) string {
+	dir, err := ioutil.TempDir(parent, "git-get-repo-")
+	checkFatal(t, err)
+
+	// Automatically remove temp dir when the test is over.
+	t.Cleanup(func() {
+		err := os.RemoveAll(dir)
+		if err != nil {
+			t.Errorf("failed removing test repo %s", dir)
+		}
+	})
+
+	return dir
 }
 
 func (r *Repo) init() {
@@ -19,6 +31,17 @@ func (r *Repo) init() {
 	checkFatal(r.t, err)
 }
 
+// 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)
+}
+
 func (r *Repo) stageFile(path string) {
 	err := run.Git("add", path).OnRepo(r.path).AndShutUp()
 	checkFatal(r.t, err)
@@ -45,11 +68,10 @@ func (r *Repo) checkout(name string) {
 }
 
 func (r *Repo) clone() *Repo {
-	dir, err := io.TempDir()
-	checkFatal(r.t, err)
+	dir := TempDir(r.t, "")
 
 	url := fmt.Sprintf("file://%s/.git", r.path)
-	err = run.Git("clone", url, dir).AndShutUp()
+	err := run.Git("clone", url, dir).AndShutUp()
 	checkFatal(r.t, err)
 
 	clone := &Repo{
@@ -57,7 +79,6 @@ func (r *Repo) clone() *Repo {
 		t:    r.t,
 	}
 
-	clone.t.Cleanup(r.cleanup)
 	return clone
 }
 

pkg/test/testrepos.go

@@ -1,8 +1,7 @@
 package test
 
 import (
-	"git-get/pkg/io"
-	"os"
+	"path/filepath"
 	"testing"
 )
 
@@ -13,32 +12,23 @@ type Repo struct {
 	t    *testing.T
 }
 
-// Path returs path to a repository.
+// Path returns path to a repository.
 func (r *Repo) Path() string {
 	return r.path
 }
 
-// TODO: this should be a method of a tempDir, not a repo
-// Automatically remove test repo when the test is over.
-func (r *Repo) cleanup() {
-	err := os.RemoveAll(r.path)
-	if err != nil {
-		r.t.Errorf("failed removing test repo directory %s", r.path)
-	}
-}
-
 // RepoEmpty creates an empty git repo.
 func RepoEmpty(t *testing.T) *Repo {
-	dir, err := io.TempDir()
-	checkFatal(t, err)
+	return RepoEmptyInDir(t, "")
+}
 
+// RepoEmptyInDir creates an empty git repo inside a given parent dir.
+func RepoEmptyInDir(t *testing.T, parent string) *Repo {
 	r := &Repo{
-		path: dir,
+		path: TempDir(t, parent),
 		t:    t,
 	}
 
-	t.Cleanup(r.cleanup)
-
 	r.init()
 	return r
 }
@@ -178,3 +168,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
+}