Merge pull request #9 from mcuadros/blame-cleanup

blame code reorganization, and mutting the test

Author: mcuadros

blame/blame.go renamed to blame.go

@@ -6,7 +6,7 @@
 //
 // This package also provides a pretty print function to output the
 // results of a blame in a similar format to the git-blame command.
-package blame
+package git
 
 import (
 	"bytes"
@@ -16,14 +16,18 @@ import (
 	"strings"
 	"unicode/utf8"
 
-	"gopkg.in/src-d/go-git.v2"
 	"gopkg.in/src-d/go-git.v2/core"
 	"gopkg.in/src-d/go-git.v2/diff"
-	"gopkg.in/src-d/go-git.v2/revlist"
 )
 
-// Blame returns the last commit that modified each line of a file in
-// a repository.
+type Blame struct {
+	Path  string
+	Rev   core.Hash
+	Lines []*line
+}
+
+// Blame returns the last commit that modified each line of a file in a
+// repository.
 //
 // The file to blame is identified by the input arguments: repo, commit and path.
 // The output is a slice of commits, one for each line in the file.
@@ -66,19 +70,9 @@ import (
 //    1. Add memoization between revlist and assign.
 //
 //    2. It is using much more memory than needed, see the TODOs below.
-
-type Blame struct {
-	Repo  string
-	Path  string
-	Rev   string
-	Lines []*line
-}
-
-func New(repo *git.Repository, path string, commit *git.Commit) (*Blame, error) {
-	// init the internal blame struct
+func (c *Commit) Blame(path string) (*Blame, error) {
 	b := new(blame)
-	b.repo = repo
-	b.fRev = commit
+	b.fRev = c
 	b.path = path
 
 	// get all the file revisions
@@ -104,9 +98,8 @@ func New(repo *git.Repository, path string, commit *git.Commit) (*Blame, error)
 	}
 
 	return &Blame{
-		Repo:  repo.URL,
 		Path:  path,
-		Rev:   commit.Hash.String(),
+		Rev:   c.Hash,
 		Lines: lines,
 	}, nil
 }
@@ -123,7 +116,7 @@ func newLine(author, text string) *line {
 	}
 }
 
-func newLines(contents []string, commits []*git.Commit) ([]*line, error) {
+func newLines(contents []string, commits []*Commit) ([]*line, error) {
 	if len(contents) != len(commits) {
 		return nil, errors.New("contents and commits have different length")
 	}
@@ -138,18 +131,18 @@ func newLines(contents []string, commits []*git.Commit) ([]*line, error) {
 // this struct is internally used by the blame function to hold its
 // inputs, outputs and state.
 type blame struct {
-	repo  *git.Repository // the repo holding the history of the file to blame
-	path  string          // the path of the file to blame
-	fRev  *git.Commit     // the commit of the final revision of the file to blame
-	revs  revlist.Revs    // the chain of revisions affecting the the file to blame
-	data  []string        // the contents of the file across all its revisions
-	graph [][]*git.Commit // the graph of the lines in the file across all the revisions TODO: not all commits are needed, only the current rev and the prev
+	path  string      // the path of the file to blame
+	fRev  *Commit     // the commit of the final revision of the file to blame
+	revs  []*Commit   // the chain of revisions affecting the the file to blame
+	data  []string    // the contents of the file across all its revisions
+	graph [][]*Commit // the graph of the lines in the file across all the revisions TODO: not all commits are needed, only the current rev and the prev
 }
 
 // calculte the history of a file "path", starting from commit "from", sorted by commit date.
 func (b *blame) fillRevs() error {
 	var err error
-	b.revs, err = revlist.NewRevs(b.repo, b.fRev, b.path)
+
+	b.revs, err = b.fRev.References(b.path)
 	if err != nil {
 		return err
 	}
@@ -158,7 +151,7 @@ func (b *blame) fillRevs() error {
 
 // build graph of a file from its revision history
 func (b *blame) fillGraphAndData() error {
-	b.graph = make([][]*git.Commit, len(b.revs))
+	b.graph = make([][]*Commit, len(b.revs))
 	b.data = make([]string, len(b.revs)) // file contents in all the revisions
 	// for every revision of the file, starting with the first
 	// one...
@@ -169,15 +162,15 @@ func (b *blame) fillGraphAndData() error {
 			return nil
 		}
 		b.data[i] = file.Contents()
-		nLines := git.CountLines(b.data[i])
+		nLines := countLines(b.data[i])
 		// create a node for each line
-		b.graph[i] = make([]*git.Commit, nLines)
+		b.graph[i] = make([]*Commit, nLines)
 		// assign a commit to each node
 		// if this is the first revision, then the node is assigned to
 		// this first commit.
 		if i == 0 {
 			for j := 0; j < nLines; j++ {
-				b.graph[i][j] = (*git.Commit)(b.revs[i])
+				b.graph[i][j] = (*Commit)(b.revs[i])
 			}
 		} else {
 			// if this is not the first commit, then assign to the old
@@ -191,11 +184,11 @@ func (b *blame) fillGraphAndData() error {
 
 // sliceGraph returns a slice of commits (one per line) for a particular
 // revision of a file (0=first revision).
-func (b *blame) sliceGraph(i int) []*git.Commit {
+func (b *blame) sliceGraph(i int) []*Commit {
 	fVs := b.graph[i]
-	result := make([]*git.Commit, 0, len(fVs))
+	result := make([]*Commit, 0, len(fVs))
 	for _, v := range fVs {
-		c := git.Commit(*v)
+		c := Commit(*v)
 		result = append(result, &c)
 	}
 	return result
@@ -209,7 +202,7 @@ func (b *blame) assignOrigin(c, p int) {
 	sl := -1 // source line
 	dl := -1 // destination line
 	for h := range hunks {
-		hLines := git.CountLines(hunks[h].Text)
+		hLines := countLines(hunks[h].Text)
 		for hl := 0; hl < hLines; hl++ {
 			switch {
 			case hunks[h].Type == 0:
@@ -218,7 +211,7 @@ func (b *blame) assignOrigin(c, p int) {
 				b.graph[c][dl] = b.graph[p][sl]
 			case hunks[h].Type == 1:
 				dl++
-				b.graph[c][dl] = (*git.Commit)(b.revs[c])
+				b.graph[c][dl] = (*Commit)(b.revs[c])
 			case hunks[h].Type == -1:
 				sl++
 			default:
@@ -255,7 +248,7 @@ func (b *blame) GoString() string {
 }
 
 // utility function to pretty print the author.
-func prettyPrintAuthor(c *git.Commit) string {
+func prettyPrintAuthor(c *Commit) string {
 	return fmt.Sprintf("%s %s", c.Author.Name, c.Author.When.Format("2006-01-02"))
 }
 

blame/blame_test.go renamed to blame_test.go

@@ -1,38 +1,25 @@
-package blame
+package git
 
 import (
-	"bytes"
 	"os"
-	"testing"
 
-	"gopkg.in/src-d/go-git.v2"
 	"gopkg.in/src-d/go-git.v2/core"
 	"gopkg.in/src-d/go-git.v2/formats/packfile"
 
 	. "gopkg.in/check.v1"
 )
 
-func Test(t *testing.T) { TestingT(t) }
-
-type SuiteCommon struct {
-	repos map[string]*git.Repository
+type BlameCommon struct {
+	repos map[string]*Repository
 }
 
-var _ = Suite(&SuiteCommon{})
-
-var fixtureRepos = [...]struct {
-	url      string
-	packfile string
-}{
-	{"https://github.com/tyba/git-fixture.git", "../formats/packfile/fixtures/git-fixture.ofs-delta"},
-	{"https://github.com/spinnaker/spinnaker.git", "../formats/packfile/fixtures/spinnaker-spinnaker.pack"},
-}
+var _ = Suite(&BlameCommon{})
 
 // create the repositories of the fixtures
-func (s *SuiteCommon) SetUpSuite(c *C) {
-	s.repos = make(map[string]*git.Repository, 0)
+func (s *BlameCommon) SetUpSuite(c *C) {
+	s.repos = make(map[string]*Repository, 0)
 	for _, fixRepo := range fixtureRepos {
-		repo := git.NewPlainRepository()
+		repo := NewPlainRepository()
 		repo.URL = fixRepo.url
 
 		d, err := os.Open(fixRepo.packfile)
@@ -60,7 +47,7 @@ type blameTest struct {
 	blames []string // the commits blamed for each line
 }
 
-func (s *SuiteCommon) mockBlame(t blameTest, c *C) (blame *Blame) {
+func (s *BlameCommon) mockBlame(t blameTest, c *C) (blame *Blame) {
 	repo, ok := s.repos[t.repo]
 	c.Assert(ok, Equals, true)
 
@@ -85,16 +72,15 @@ func (s *SuiteCommon) mockBlame(t blameTest, c *C) (blame *Blame) {
 	}
 
 	return &Blame{
-		Repo:  t.repo,
 		Path:  t.path,
-		Rev:   t.rev,
+		Rev:   core.NewHash(t.rev),
 		Lines: blamedLines,
 	}
 }
 
 // run a blame on all the suite's tests
-func (s *SuiteCommon) TestBlame(c *C) {
-	for i, t := range blameTests {
+func (s *BlameCommon) TestBlame(c *C) {
+	for _, t := range blameTests {
 		expected := s.mockBlame(t, c)
 
 		repo, ok := s.repos[t.repo]
@@ -103,22 +89,12 @@ func (s *SuiteCommon) TestBlame(c *C) {
 		commit, err := repo.Commit(core.NewHash(t.rev))
 		c.Assert(err, IsNil)
 
-		obtained, err := New(repo, t.path, commit)
-		c.Assert(err, IsNil, Commentf("subtest %d", i))
-
-		c.Assert(obtained, DeepEquals, expected, Commentf("subtest %d: %s",
-			i, sideBySide(obtained, expected)))
+		obtained, err := commit.Blame(t.path)
+		c.Assert(err, IsNil)
+		c.Assert(obtained, DeepEquals, expected)
 	}
 }
 
-func sideBySide(output, expected *Blame) string {
-	var buf bytes.Buffer
-	buf.WriteString(output.Repo)
-	buf.WriteString("    ")
-	buf.WriteString(expected.Repo)
-	return buf.String()
-}
-
 // utility function to avoid writing so many repeated commits
 func repeat(s string, n int) []string {
 	if n < 0 {

clients/ssh/git_upload_pack_test.go

@@ -25,7 +25,6 @@ const (
 )
 
 func (s *SuiteRemote) TestConnect(c *C) {
-	fmt.Println("TestConnect")
 	r := NewGitUploadPackService()
 	c.Assert(r.Connect(fixRepo), Equals, ErrAuthRequired)
 }
@@ -59,7 +58,6 @@ func (c *sshAgentConn) close() error {
 }
 
 func (s *SuiteRemote) TestConnectWithPublicKeysCallback(c *C) {
-	fmt.Println("TestConnectWithPublicKeysCallback")
 	agent, err := newSSHAgentConn()
 	c.Assert(err, IsNil)
 	defer func() { c.Assert(agent.close(), IsNil) }()
@@ -72,19 +70,16 @@ func (s *SuiteRemote) TestConnectWithPublicKeysCallback(c *C) {
 }
 
 func (s *SuiteRemote) TestConnectBadVcs(c *C) {
-	fmt.Println("TestConnectBadVcs")
 	r := NewGitUploadPackService()
 	c.Assert(r.ConnectWithAuth(fixRepoBadVcs, nil), ErrorMatches, fmt.Sprintf(".*%s.*", fixRepoBadVcs))
 }
 
 func (s *SuiteRemote) TestConnectNonGit(c *C) {
-	fmt.Println("TestConnectNonGit")
 	r := NewGitUploadPackService()
 	c.Assert(r.ConnectWithAuth(fixRepoNonGit, nil), Equals, ErrUnsupportedVCS)
 }
 
 func (s *SuiteRemote) TestConnectNonGithub(c *C) {
-	fmt.Println("TestConnectNonGit")
 	r := NewGitUploadPackService()
 	c.Assert(r.ConnectWithAuth(fixGitRepoNonGithub, nil), Equals, ErrUnsupportedRepo)
 }
@@ -97,14 +92,12 @@ func (*mockAuth) Name() string   { return "" }
 func (*mockAuth) String() string { return "" }
 
 func (s *SuiteRemote) TestConnectWithAuthWrongType(c *C) {
-	fmt.Println("TestConnectWithAuthWrongType")
 	r := NewGitUploadPackService()
 	c.Assert(r.ConnectWithAuth(fixRepo, &mockAuth{}), Equals, ErrInvalidAuthMethod)
 	c.Assert(r.connected, Equals, false)
 }
 
 func (s *SuiteRemote) TestAlreadyConnected(c *C) {
-	fmt.Println("TestAlreadyConnected")
 	agent, err := newSSHAgentConn()
 	c.Assert(err, IsNil)
 	defer func() { c.Assert(agent.close(), IsNil) }()
@@ -117,7 +110,6 @@ func (s *SuiteRemote) TestAlreadyConnected(c *C) {
 }
 
 func (s *SuiteRemote) TestDisconnect(c *C) {
-	fmt.Println("TestDisconnect")
 	agent, err := newSSHAgentConn()
 	c.Assert(err, IsNil)
 	defer func() { c.Assert(agent.close(), IsNil) }()
@@ -129,13 +121,11 @@ func (s *SuiteRemote) TestDisconnect(c *C) {
 }
 
 func (s *SuiteRemote) TestDisconnectedWhenNonConnected(c *C) {
-	fmt.Println("TestDisconnectedWhenNonConnected")
 	r := NewGitUploadPackService()
 	c.Assert(r.Disconnect(), Equals, ErrNotConnected)
 }
 
 func (s *SuiteRemote) TestAlreadyDisconnected(c *C) {
-	fmt.Println("TestAlreadyDisconnected")
 	agent, err := newSSHAgentConn()
 	c.Assert(err, IsNil)
 	defer func() { c.Assert(agent.close(), IsNil) }()
@@ -148,7 +138,6 @@ func (s *SuiteRemote) TestAlreadyDisconnected(c *C) {
 }
 
 func (s *SuiteRemote) TestServeralConnections(c *C) {
-	fmt.Println("TestServeralConnections")
 	agent, err := newSSHAgentConn()
 	c.Assert(err, IsNil)
 	defer func() { c.Assert(agent.close(), IsNil) }()
@@ -169,14 +158,12 @@ func (s *SuiteRemote) TestServeralConnections(c *C) {
 }
 
 func (s *SuiteRemote) TestInfoNotConnected(c *C) {
-	fmt.Println("TestInfoNotConnected")
 	r := NewGitUploadPackService()
 	_, err := r.Info()
 	c.Assert(err, Equals, ErrNotConnected)
 }
 
 func (s *SuiteRemote) TestDefaultBranch(c *C) {
-	fmt.Println("TestDefaultBranch")
 	agent, err := newSSHAgentConn()
 	c.Assert(err, IsNil)
 	defer func() { c.Assert(agent.close(), IsNil) }()
@@ -191,7 +178,6 @@ func (s *SuiteRemote) TestDefaultBranch(c *C) {
 }
 
 func (s *SuiteRemote) TestCapabilities(c *C) {
-	fmt.Println("TestCapabilities")
 	agent, err := newSSHAgentConn()
 	c.Assert(err, IsNil)
 	defer func() { c.Assert(agent.close(), IsNil) }()
@@ -206,7 +192,6 @@ func (s *SuiteRemote) TestCapabilities(c *C) {
 }
 
 func (s *SuiteRemote) TestFetchNotConnected(c *C) {
-	fmt.Println("TestFetchNotConnected")
 	r := NewGitUploadPackService()
 	pr := &common.GitUploadPackRequest{}
 	pr.Want(core.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5"))
@@ -215,7 +200,6 @@ func (s *SuiteRemote) TestFetchNotConnected(c *C) {
 }
 
 func (s *SuiteRemote) TestFetch(c *C) {
-	fmt.Println("TestFetch")
 	agent, err := newSSHAgentConn()
 	c.Assert(err, IsNil)
 	defer func() { c.Assert(agent.close(), IsNil) }()