Merge pull request #15 from pinpt/code_by_commit

Add CodeByCommit method that returns code information grouped by commit.

e2etests/code/base.go

@@ -1,7 +1,6 @@
 package e2etests
 
 import (
-	"context"
 	"reflect"
 	"testing"
 	"time"
@@ -23,8 +22,8 @@ func NewTest(t *testing.T, repoName string) *Test {
 	return s
 }
 
-func (s *Test) Run(optsp *ripsrc.Opts) []ripsrc.BlameResult {
-	t := s.t
+// cb callback to defer dirs.Remove()
+func (s *Test) Run(optsp *ripsrc.Opts, cb func(*ripsrc.Ripsrc)) {
 	dirs := testutil.UnzipTestRepo(s.repoName)
 	defer dirs.Remove()
 
@@ -33,11 +32,7 @@ func (s *Test) Run(optsp *ripsrc.Opts) []ripsrc.BlameResult {
 		opts = *optsp
 	}
 	opts.RepoDir = dirs.RepoDir
-	res, err := ripsrc.New(opts).CodeSlice(context.Background())
-	if err != nil {
-		t.Fatal("Rip returned error", err)
-	}
-	return res
+	cb(ripsrc.New(opts))
 }
 
 func assertResult(t *testing.T, want, got []ripsrc.BlameResult) {

e2etests/code/basic_test.go

@@ -1,14 +1,52 @@
 package e2etests
 
 import (
+	"context"
 	"testing"
 
 	"github.com/pinpt/ripsrc/ripsrc"
 )
 
+// Test results for a basic repo for both CodeSlice and Code2 calls
 func TestBasic(t *testing.T) {
-	test := NewTest(t, "basic")
-	got := test.Run(nil)
+	var got []ripsrc.BlameResult
+
+	type commit struct {
+		SHA   string
+		Files []ripsrc.BlameResult
+	}
+	var byCommit []commit
+
+	NewTest(t, "basic").Run(nil, func(rip *ripsrc.Ripsrc) {
+		{
+			var err error
+			got, err = rip.CodeSlice(context.Background())
+			if err != nil {
+				t.Fatal(err)
+			}
+		}
+
+		{
+			ch := make(chan ripsrc.CommitCode)
+			done := make(chan bool)
+			go func() {
+				for c := range ch {
+					c2 := commit{}
+					c2.SHA = c.SHA
+					for f := range c.Files {
+						c2.Files = append(c2.Files, f)
+					}
+					byCommit = append(byCommit, c2)
+				}
+				done <- true
+			}()
+			defer func() { <-done }()
+			err := rip.CodeByCommit(context.Background(), ch)
+			if err != nil {
+				t.Fatal(err)
+			}
+		}
+	})
 
 	u1n := "User1"
 	u1e := "[email protected]"
@@ -133,4 +171,19 @@ func TestBasic(t *testing.T) {
 	}
 
 	assertResult(t, want, got)
+
+	if len(byCommit) != 2 {
+		t.Fatal("expecting 2 commits")
+	}
+
+	if byCommit[0].SHA != commit1.SHA {
+		t.Fatal("invalid sha")
+	}
+
+	if len(byCommit[0].Files) != 1 {
+		t.Fatal("invalid files len")
+	}
+
+	assertBlame(t, byCommit[0].Files[0], want[0])
+
 }

e2etests/code/deleted_files_test.go

@@ -1,14 +1,23 @@
 package e2etests
 
 import (
+	"context"
 	"testing"
 
 	"github.com/pinpt/ripsrc/ripsrc"
 )
 
 func TestDeletedFiles(t *testing.T) {
 	test := NewTest(t, "deleted_files")
-	got := test.Run(nil)
+
+	var got []ripsrc.BlameResult
+	test.Run(nil, func(rip *ripsrc.Ripsrc) {
+		var err error
+		got, err = rip.CodeSlice(context.Background())
+		if err != nil {
+			t.Fatal(err)
+		}
+	})
 
 	u1n := "User1"
 	u1e := "[email protected]"

e2etests/code/editing_former_bin_file_test.go

@@ -1,6 +1,7 @@
 package e2etests
 
 import (
+	"context"
 	"testing"
 
 	"github.com/pinpt/ripsrc/ripsrc"
@@ -10,7 +11,15 @@ import (
 // If the file in repo was a binary at some point and then switched to text and was modified, then git log with patches does not contain the full file content. There are 2 options to fix this, either we ignore all files that at some point in history were binary or retrieve the full file content for these cases separately without using log and patches.
 func TestEditingFormerBinFile(t *testing.T) {
 	test := NewTest(t, "editing_former_bin_file")
-	got := test.Run(nil)
+
+	var got []ripsrc.BlameResult
+	test.Run(nil, func(rip *ripsrc.Ripsrc) {
+		var err error
+		got, err = rip.CodeSlice(context.Background())
+		if err != nil {
+			t.Fatal(err)
+		}
+	})
 
 	u1n := "User1"
 	u1e := "[email protected]"

e2etests/code/merge_basic_test.go

@@ -1,14 +1,23 @@
 package e2etests
 
 import (
+	"context"
 	"testing"
 
 	"github.com/pinpt/ripsrc/ripsrc"
 )
 
 func TestMergeBasic(t *testing.T) {
 	test := NewTest(t, "merge_basic")
-	got := test.Run(nil)
+
+	var got []ripsrc.BlameResult
+	test.Run(nil, func(rip *ripsrc.Ripsrc) {
+		var err error
+		got, err = rip.CodeSlice(context.Background())
+		if err != nil {
+			t.Fatal(err)
+		}
+	})
 
 	u1n := "User1"
 	u1e := "[email protected]"

e2etests/code/mutiple_branches_test.go

@@ -1,6 +1,7 @@
 package e2etests
 
 import (
+	"context"
 	"testing"
 
 	"github.com/pinpt/ripsrc/ripsrc"
@@ -9,7 +10,15 @@ import (
 
 func TestMultipleBranches1(t *testing.T) {
 	test := NewTest(t, "multiple_branches")
-	got := test.Run(&ripsrc.Opts{AllBranches: true})
+
+	var got []ripsrc.BlameResult
+	test.Run(&ripsrc.Opts{AllBranches: true}, func(rip *ripsrc.Ripsrc) {
+		var err error
+		got, err = rip.CodeSlice(context.Background())
+		if err != nil {
+			t.Fatal(err)
+		}
+	})
 
 	if len(got) != 3 {
 		t.Fatal("expecting changes for 3 commits")
@@ -28,7 +37,15 @@ func TestMultipleBranches1(t *testing.T) {
 
 func TestMultipleBranchesDisabled(t *testing.T) {
 	test := NewTest(t, "multiple_branches_disabled")
-	got := test.Run(nil)
+
+	var got []ripsrc.BlameResult
+	test.Run(nil, func(rip *ripsrc.Ripsrc) {
+		var err error
+		got, err = rip.CodeSlice(context.Background())
+		if err != nil {
+			t.Fatal(err)
+		}
+	})
 
 	if len(got) != 1 {
 		t.Fatalf("expecting changes for 1 commits, got\n%#+v", got)

ripsrc/code.go

@@ -58,9 +58,40 @@ const (
 	GitFileCommitStatusRemoved = commitmeta.GitFileCommitStatusRemoved
 )
 
+// Code returns code information using one record per file and commit
 func (s *Ripsrc) Code(ctx context.Context, res chan BlameResult) error {
 	defer close(res)
 
+	res2 := make(chan CommitCode)
+	done := make(chan bool)
+
+	go func() {
+		for r := range res2 {
+			for f := range r.Files {
+				res <- f
+			}
+		}
+		done <- true
+	}()
+
+	err := s.CodeByCommit(ctx, res2)
+	<-done
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+type CommitCode struct {
+	SHA   string
+	Files chan BlameResult
+}
+
+// CodeByCommit returns code information using one record per commit that includes records by file
+func (s *Ripsrc) CodeByCommit(ctx context.Context, res chan CommitCode) error {
+	defer close(res)
+
 	err := s.prepareGitExec(ctx)
 	if err != nil {
 		return err
@@ -73,20 +104,26 @@ func (s *Ripsrc) Code(ctx context.Context, res chan BlameResult) error {
 
 	err = s.getCommitInfo(ctx)
 	if err != nil {
-		panic(err)
+		return err
 	}
 
 	gitRes := make(chan process.Result)
 	done := make(chan bool)
 	go func() {
 		for r1 := range gitRes {
+			rc := CommitCode{}
+			rc.SHA = r1.Commit
+			rc.Files = make(chan BlameResult)
+
 			rs, err := s.codeInfoFiles(r1)
 			if err != nil {
 				panic(err)
 			}
+			res <- rc
 			for _, r := range rs {
-				res <- r
+				rc.Files <- r
 			}
+			close(rc.Files)
 		}
 		done <- true
 	}()
@@ -102,12 +139,12 @@ func (s *Ripsrc) Code(ctx context.Context, res chan BlameResult) error {
 	}
 	gitProcessor := process.New(processOpts)
 	err = gitProcessor.Run(gitRes)
+	<-done
+
 	if err != nil {
 		return err
 	}
 
-	<-done
-
 	s.GitProcessTimings = gitProcessor.Timing()
 
 	return nil

ripsrc/history3/incblame/apply_merge2.go

@@ -47,7 +47,10 @@ func ApplyMerge(parents []Blame, diffs []Diff, commit string, fileForDebug strin
 			line := cand[j].Lines[i]
 			// if commit is not the merge commit that means the line appeared from that parent use it in res, in case multiple sources, first will be used
 			if line.Commit != commit {
-				res.Lines[i].Commit = line.Commit
+				// create a copy to avoid mutating original, which leads to race in tests
+				lc := *res.Lines[i]
+				lc.Commit = line.Commit
+				res.Lines[i] = &lc
 				break
 			}
 		}