Refactor Repo code and tests

- Add Repo as a separate type
- Merge together repo status and branches status
- Load status on repo opening

pkg/branch.go

@@ -1,91 +0,0 @@
-package pkg
-
-import (
-	git "github.com/libgit2/git2go/v30"
-	"github.com/pkg/errors"
-)
-
-type BranchStatus struct {
-	Name        string
-	IsRemote    bool
-	HasUpstream bool
-	NeedsPull   bool
-	NeedsPush   bool
-	Ahead       int
-	Behind      int
-}
-
-func Branches(repo *git.Repository) (map[string]BranchStatus, error) {
-	iter, err := repo.NewBranchIterator(git.BranchAll)
-	if err != nil {
-		return nil, errors.Wrap(err, "Failed creating branch iterator")
-	}
-
-	var branches []*git.Branch
-	err = iter.ForEach(func(branch *git.Branch, branchType git.BranchType) error {
-		branches = append(branches, branch)
-		return nil
-	})
-
-	if err != nil {
-		return nil, errors.Wrap(err, "Failed iterating over branches")
-	}
-
-	statuses := make(map[string]BranchStatus)
-	for _, branch := range branches {
-		status, err := NewBranchStatus(repo, branch)
-		if err != nil {
-			// TODO: Handle error. We should tell user that we couldn't read status of that branch but probably shouldn't exit
-			continue
-		}
-		statuses[status.Name] = status
-	}
-
-	return statuses, nil
-}
-
-func NewBranchStatus(repo *git.Repository, branch *git.Branch) (BranchStatus, error) {
-	var status BranchStatus
-
-	name, err := branch.Name()
-	if err != nil {
-		return status, errors.Wrap(err, "Failed getting branch name")
-	}
-	status.Name = name
-
-	// If branch is a remote one, return immediately. Upstream can only be found for local branches.
-	if branch.IsRemote() {
-		status.IsRemote = true
-		return status, nil
-	}
-
-	upstream, err := branch.Upstream()
-	if err != nil && !git.IsErrorCode(err, git.ErrNotFound) {
-		return status, errors.Wrap(err, "Failed getting branch upstream")
-	}
-
-	// If there's no upstream, return immediately. Ahead/Behind can only be found when upstream exists.
-	if upstream == nil {
-		return status, nil
-	}
-
-	status.HasUpstream = true
-
-	ahead, behind, err := repo.AheadBehind(branch.Target(), upstream.Target())
-	if err != nil {
-		return status, errors.Wrap(err, "Failed getting ahead/behind information")
-	}
-
-	status.Ahead = ahead
-	status.Behind = behind
-
-	if ahead > 0 {
-		status.NeedsPush = true
-	}
-
-	if behind > 0 {
-		status.NeedsPull = true
-	}
-
-	return status, nil
-}

pkg/branch_test.go

@@ -1,93 +0,0 @@
-package pkg
-
-import (
-	"reflect"
-	"testing"
-)
-
-func TestNewLocalBranch(t *testing.T) {
-	repo := newTestRepo(t)
-
-	createFile(t, repo, "file")
-	stageFile(t, repo, "file")
-	createCommit(t, repo, "Initial commit")
-	branch := createBranch(t, repo, "branch")
-
-	status, err := NewBranchStatus(repo, branch)
-	checkFatal(t, err)
-
-	want := BranchStatus{
-		Name:        "branch",
-		IsRemote:    false,
-		HasUpstream: false,
-		NeedsPull:   false,
-		NeedsPush:   false,
-		Ahead:       0,
-		Behind:      0,
-	}
-
-	if status != want {
-		t.Errorf("Wrong branch status, got %+v; want %+v", status, want)
-	}
-}
-
-func TestClonedBranches(t *testing.T) {
-	origin := newTestRepo(t)
-	createFile(t, origin, "file")
-	stageFile(t, origin, "file")
-	createCommit(t, origin, "Initial commit")
-
-	createBranch(t, origin, "branch")
-
-	repo, err := CloneRepo(origin.Path(), newTempDir(t))
-	checkFatal(t, err)
-
-	createBranch(t, repo, "local")
-
-	checkoutBranch(t, repo, "branch")
-	createFile(t, repo, "anotherFile")
-	stageFile(t, repo, "anotherFile")
-	createCommit(t, repo, "Second commit")
-
-	branches, err := Branches(repo)
-	checkFatal(t, err)
-
-	var tests = []struct {
-		got  BranchStatus
-		want BranchStatus
-	}{
-		{branches["master"], BranchStatus{
-			Name:        "master",
-			IsRemote:    false,
-			HasUpstream: true,
-		}},
-		{branches["origin/master"], BranchStatus{
-			Name:        "origin/master",
-			IsRemote:    true,
-			HasUpstream: false,
-		}},
-		{branches["branch"], BranchStatus{
-			Name:        "branch",
-			IsRemote:    false,
-			HasUpstream: true,
-			Ahead:       1,
-			NeedsPush:   true,
-		}},
-		{branches["origin/branch"], BranchStatus{
-			Name:        "origin/branch",
-			IsRemote:    true,
-			HasUpstream: false,
-		}},
-		{branches["local"], BranchStatus{
-			Name:        "local",
-			IsRemote:    false,
-			HasUpstream: false,
-		}},
-	}
-
-	for _, test := range tests {
-		if !reflect.DeepEqual(test.got, test.want) {
-			t.Errorf("Wrong branch status, got %+v; want %+v", test.got, test.want)
-		}
-	}
-}

pkg/repo.go

@@ -6,7 +6,12 @@ import (
 	git "github.com/libgit2/git2go/v30"
 )
 
-func CloneRepo(url string, path string) (*git.Repository, error) {
+type Repo struct {
+	repo   *git.Repository
+	Status *RepoStatus
+}
+
+func CloneRepo(url string, path string) error {
 	options := &git.CloneOptions{
 		CheckoutOpts:         nil,
 		FetchOptions:         nil,
@@ -15,18 +20,50 @@ func CloneRepo(url string, path string) (*git.Repository, error) {
 		RemoteCreateCallback: nil,
 	}
 
-	repo, err := git.Clone(url, path, options)
+	_, err := git.Clone(url, path, options)
-	return repo, errors.Wrap(err, "Failed cloning repo")
+	if err != nil {
+		return errors.Wrap(err, "Failed cloning repo")
+	}
+	return nil
 }
 
-func Fetch(repo *git.Repository) error {
+func OpenRepo(path string) (*Repo, error) {
-	remotes, err := repo.Remotes.List()
+	r, err := git.OpenRepository(path)
 	if err != nil {
-		return errors.Wrap(err, "Failed listing remotes")
+		return nil, errors.Wrap(err, "Failed opening repo")
 	}
 
-	for _, r := range remotes {
+	repoStatus, err := loadStatus(r)
-		remote, err := repo.Remotes.Lookup(r)
+	if err != nil {
+		return nil, err
+	}
+
+	repo := &Repo{
+		repo:   r,
+		Status: repoStatus,
+	}
+
+	return repo, nil
+}
+
+func (r *Repo) Reload() error {
+	status, err := loadStatus(r.repo)
+	if err != nil {
+		return err
+	}
+
+	r.Status = status
+	return nil
+}
+
+func (r *Repo) Fetch() error {
+	remoteNames, err := r.repo.Remotes.List()
+	if err != nil {
+		return errors.Wrap(err, "Failed listing remoteNames")
+	}
+
+	for _, name := range remoteNames {
+		remote, err := r.repo.Remotes.Lookup(name)
 		if err != nil {
 			return errors.Wrap(err, "Failed looking up remote")
 		}

pkg/repo_test.go

@@ -10,14 +10,16 @@ func TestFetch(t *testing.T) {
 	createCommit(t, origin, "Initial commit")
 
 	// Clone the origin repo
-	repo, err := CloneRepo(origin.Path(), newTempDir(t))
+	dir := newTempDir(t)
+	err := CloneRepo(origin.Path(), dir)
+	checkFatal(t, err)
+
+	// Open cloned repo and load its status
+	repo, err := OpenRepo(dir)
 	checkFatal(t, err)
 
 	// Check cloned status. It should not be behind origin
-	status, err := NewRepoStatus(repo.Workdir())
+	if repo.Status.Branches["master"].Behind != 0 {
-	checkFatal(t, err)
-
-	if status.BranchStatuses["master"].Behind != 0 {
 		t.Errorf("Master should not be behind")
 	}
 
@@ -27,16 +29,17 @@ func TestFetch(t *testing.T) {
 	createCommit(t, origin, "Second commit")
 
 	// Fetch cloned repo and check the status again
-	err = Fetch(repo)
+	err = repo.Fetch()
-	status, err = NewRepoStatus(repo.Workdir())
+	checkFatal(t, err)
+	err = repo.Reload()
 	checkFatal(t, err)
 
 	// Cloned master should now be 1 commit behind origin
-	if status.BranchStatuses["master"].Behind != 1 {
+	if repo.Status.Branches["master"].Behind != 1 {
 		t.Errorf("Master should be 1 commit behind")
 	}
 
-	if status.BranchStatuses["master"].Ahead != 0 {
+	if repo.Status.Branches["master"].Ahead != 0 {
 		t.Errorf("Master should not be ahead")
 	}
 }

pkg/status.go

@@ -8,20 +8,32 @@ import (
 type RepoStatus struct {
 	HasUntrackedFiles     bool
 	HasUncommittedChanges bool
-	BranchStatuses        map[string]BranchStatus
+	Branches              map[string]BranchStatus
 }
 
-func NewRepoStatus(path string) (RepoStatus, error) {
+type BranchStatus struct {
-	var status RepoStatus
+	Name        string
+	IsRemote    bool
+	HasUpstream bool
+	NeedsPull   bool
+	NeedsPush   bool
+	Ahead       int
+	Behind      int
+}
 
-	repo, err := git.OpenRepository(path)
+func loadStatus(r *git.Repository) (*RepoStatus, error) {
+	entries, err := statusEntries(r)
 	if err != nil {
-		return status, errors.Wrap(err, "Failed opening repository")
+		return nil, err
 	}
 
-	entries, err := statusEntries(repo)
+	branches, err := branches(r)
 	if err != nil {
-		return status, errors.Wrap(err, "Failed getting repository status")
+		return nil, err
+	}
+
+	status := &RepoStatus{
+		Branches: branches,
 	}
 
 	for _, entry := range entries {
@@ -33,29 +45,23 @@ func NewRepoStatus(path string) (RepoStatus, error) {
 		}
 	}
 
-	branchStatuses, err := Branches(repo)
-	if err != nil {
-		return status, errors.Wrap(err, "Failed getting branches statuses")
-	}
-	status.BranchStatuses = branchStatuses
-
 	return status, nil
 }
 
-func statusEntries(repo *git.Repository) ([]git.StatusEntry, error) {
+func statusEntries(r *git.Repository) ([]git.StatusEntry, error) {
 	opts := &git.StatusOptions{
 		Show:  git.StatusShowIndexAndWorkdir,
 		Flags: git.StatusOptIncludeUntracked,
 	}
 
-	status, err := repo.StatusList(opts)
+	status, err := r.StatusList(opts)
 	if err != nil {
-		return nil, errors.Wrap(err, "Failed getting repository status")
+		return nil, errors.Wrap(err, "Failed getting repository status list")
 	}
 
 	entryCount, err := status.EntryCount()
 	if err != nil {
-		return nil, errors.Wrap(err, "Failed getting repository status count")
+		return nil, errors.Wrap(err, "Failed getting repository status list count")
 	}
 
 	var entries []git.StatusEntry
@@ -70,3 +76,78 @@ func statusEntries(repo *git.Repository) ([]git.StatusEntry, error) {
 
 	return entries, nil
 }
+
+func branches(r *git.Repository) (map[string]BranchStatus, error) {
+	iter, err := r.NewBranchIterator(git.BranchAll)
+	if err != nil {
+		return nil, errors.Wrap(err, "Failed creating branch iterator")
+	}
+
+	var branches []*git.Branch
+	err = iter.ForEach(func(branch *git.Branch, branchType git.BranchType) error {
+		branches = append(branches, branch)
+		return nil
+	})
+
+	if err != nil {
+		return nil, errors.Wrap(err, "Failed iterating over branches")
+	}
+
+	statuses := make(map[string]BranchStatus)
+	for _, branch := range branches {
+		status, err := branchStatus(branch)
+		if err != nil {
+			// TODO: Handle error. We should tell user that we couldn't read status of that branch but probably shouldn't exit
+			continue
+		}
+		statuses[status.Name] = status
+	}
+
+	return statuses, nil
+}
+
+func branchStatus(branch *git.Branch) (BranchStatus, error) {
+	var status BranchStatus
+
+	name, err := branch.Name()
+	if err != nil {
+		return status, errors.Wrap(err, "Failed getting branch name")
+	}
+	status.Name = name
+
+	// If branch is a remote one, return immediately. Upstream can only be found for local branches.
+	if branch.IsRemote() {
+		status.IsRemote = true
+		return status, nil
+	}
+
+	upstream, err := branch.Upstream()
+	if err != nil && !git.IsErrorCode(err, git.ErrNotFound) {
+		return status, errors.Wrap(err, "Failed getting branch upstream")
+	}
+
+	// If there's no upstream, return immediately. Ahead/Behind can only be found when upstream exists.
+	if upstream == nil {
+		return status, nil
+	}
+
+	status.HasUpstream = true
+
+	ahead, behind, err := branch.Owner().AheadBehind(branch.Target(), upstream.Target())
+	if err != nil {
+		return status, errors.Wrap(err, "Failed getting ahead/behind information")
+	}
+
+	status.Ahead = ahead
+	status.Behind = behind
+
+	if ahead > 0 {
+		status.NeedsPush = true
+	}
+
+	if behind > 0 {
+		status.NeedsPull = true
+	}
+
+	return status, nil
+}

pkg/status_test.go

@@ -17,13 +17,13 @@ func TestStatusWithEmptyRepo(t *testing.T) {
 		t.Errorf("Empty repo should have no status entries")
 	}
 
-	status, err := NewRepoStatus(repo.Workdir())
+	status, err := loadStatus(repo)
 	checkFatal(t, err)
 
-	want := RepoStatus{
+	want := &RepoStatus{
 		HasUntrackedFiles:     false,
 		HasUncommittedChanges: false,
-		BranchStatuses:        status.BranchStatuses,
+		Branches:              status.Branches,
 	}
 
 	if !reflect.DeepEqual(status, want) {
@@ -46,13 +46,13 @@ func TestStatusWithUntrackedFile(t *testing.T) {
 		t.Errorf("Invalid status, got %d; want %d", entries[0].Status, git.StatusWtNew)
 	}
 
-	status, err := NewRepoStatus(repo.Workdir())
+	status, err := loadStatus(repo)
 	checkFatal(t, err)
 
-	want := RepoStatus{
+	want := &RepoStatus{
 		HasUntrackedFiles:     true,
 		HasUncommittedChanges: false,
-		BranchStatuses:        status.BranchStatuses,
+		Branches:              status.Branches,
 	}
 
 	if !reflect.DeepEqual(status, want) {
@@ -84,13 +84,13 @@ func TestStatusWithStagedFile(t *testing.T) {
 		t.Errorf("Invalid status, got %d; want %d", entries[0].Status, git.StatusIndexNew)
 	}
 
-	status, err := NewRepoStatus(repo.Workdir())
+	status, err := loadStatus(repo)
 	checkFatal(t, err)
 
-	want := RepoStatus{
+	want := &RepoStatus{
 		HasUntrackedFiles:     false,
 		HasUncommittedChanges: true,
-		BranchStatuses:        status.BranchStatuses,
+		Branches:              status.Branches,
 	}
 
 	if !reflect.DeepEqual(status, want) {
@@ -111,13 +111,13 @@ func TestStatusWithSingleCommit(t *testing.T) {
 		t.Errorf("Repo with no uncommitted files should have no status entries")
 	}
 
-	status, err := NewRepoStatus(repo.Workdir())
+	status, err := loadStatus(repo)
 	checkFatal(t, err)
 
-	want := RepoStatus{
+	want := &RepoStatus{
 		HasUntrackedFiles:     false,
 		HasUncommittedChanges: false,
-		BranchStatuses:        status.BranchStatuses,
+		Branches:              status.Branches,
 	}
 
 	if !reflect.DeepEqual(status, want) {
@@ -141,13 +141,13 @@ func TestStatusWithMultipleCommits(t *testing.T) {
 		t.Errorf("Repo with no uncommitted files should have no status entries")
 	}
 
-	status, err := NewRepoStatus(repo.Workdir())
+	status, err := loadStatus(repo)
 	checkFatal(t, err)
 
-	want := RepoStatus{
+	want := &RepoStatus{
 		HasUntrackedFiles:     false,
 		HasUncommittedChanges: false,
-		BranchStatuses:        status.BranchStatuses,
+		Branches:              status.Branches,
 	}
 
 	if !reflect.DeepEqual(status, want) {
@@ -159,19 +159,111 @@ func TestStatusCloned(t *testing.T) {
 	origin := newTestRepo(t)
 	dir := newTempDir(t)
 
-	repo, err := CloneRepo(origin.Path(), dir)
+	err := CloneRepo(origin.Path(), dir)
+	checkFatal(t, err)
+	repo, err := OpenRepo(dir)
 	checkFatal(t, err)
 
-	status, err := NewRepoStatus(repo.Workdir())
+	status, err := loadStatus(repo.repo)
 	checkFatal(t, err)
 
-	want := RepoStatus{
+	want := &RepoStatus{
 		HasUntrackedFiles:     false,
 		HasUncommittedChanges: false,
-		BranchStatuses:        status.BranchStatuses,
+		Branches:              status.Branches,
 	}
 
 	if !reflect.DeepEqual(status, want) {
 		t.Errorf("Wrong repo status, got %+v; want %+v", status, want)
 	}
 }
+
+func TestBranchNewLocal(t *testing.T) {
+	repo := newTestRepo(t)
+
+	createFile(t, repo, "file")
+	stageFile(t, repo, "file")
+	createCommit(t, repo, "Initial commit")
+	branch := createBranch(t, repo, "branch")
+
+	status, err := branchStatus(branch)
+	checkFatal(t, err)
+
+	want := BranchStatus{
+		Name:        "branch",
+		IsRemote:    false,
+		HasUpstream: false,
+		NeedsPull:   false,
+		NeedsPush:   false,
+		Ahead:       0,
+		Behind:      0,
+	}
+
+	if status != want {
+		t.Errorf("Wrong branch status, got %+v; want %+v", status, want)
+	}
+}
+
+func TestBranchCloned(t *testing.T) {
+	origin := newTestRepo(t)
+	createFile(t, origin, "file")
+	stageFile(t, origin, "file")
+	createCommit(t, origin, "Initial commit")
+
+	createBranch(t, origin, "branch")
+
+	dir := newTempDir(t)
+	err := CloneRepo(origin.Path(), dir)
+	checkFatal(t, err)
+	repo, err := OpenRepo(dir)
+	checkFatal(t, err)
+
+	createBranch(t, repo.repo, "local")
+
+	checkoutBranch(t, repo.repo, "branch")
+	createFile(t, repo.repo, "anotherFile")
+	stageFile(t, repo.repo, "anotherFile")
+	createCommit(t, repo.repo, "Second commit")
+
+	err = repo.Reload()
+	checkFatal(t, err)
+
+	var tests = []struct {
+		got  BranchStatus
+		want BranchStatus
+	}{
+		{repo.Status.Branches["master"], BranchStatus{
+			Name:        "master",
+			IsRemote:    false,
+			HasUpstream: true,
+		}},
+		{repo.Status.Branches["origin/master"], BranchStatus{
+			Name:        "origin/master",
+			IsRemote:    true,
+			HasUpstream: false,
+		}},
+		{repo.Status.Branches["branch"], BranchStatus{
+			Name:        "branch",
+			IsRemote:    false,
+			HasUpstream: true,
+			Ahead:       1,
+			NeedsPush:   true,
+		}},
+		{repo.Status.Branches["origin/branch"], BranchStatus{
+			Name:        "origin/branch",
+			IsRemote:    true,
+			HasUpstream: false,
+		}},
+		{repo.Status.Branches["local"], BranchStatus{
+			Name:        "local",
+			IsRemote:    false,
+			HasUpstream: false,
+		}},
+	}
+
+	for _, test := range tests {
+		if !reflect.DeepEqual(test.got, test.want) {
+			t.Errorf("Wrong branch status, got %+v; want %+v", test.got, test.want)
+		}
+	}
+}