Merge pull request #234 from vmarkovtsev/master

Change -covermode from count to atomic and add -race

.appveyor.yml

@@ -23,7 +23,7 @@ build_script:
 
 test_script:
   - go get -v -t -d gopkg.in/src-d/hercules.v9/...
-  - go test -v -tags disable_babelfish gopkg.in/src-d/hercules.v9/...
+  - go test -v -race -tags disable_babelfish gopkg.in/src-d/hercules.v9/...
 
 artifacts:
   - name: hercules.win64.zip

.travis.yml

@@ -65,7 +65,7 @@ script:
   - golint -set_exit_status $(go list ./... | grep -v /vendor/)
   - flake8
   - go test -coverpkg=all -v -coverprofile=coverage.txt -covermode=count gopkg.in/src-d/hercules.v9/... && sed -i '/cmd\/hercules\|core.go/d' coverage.txt
-  - go test -race gopkg.in/src-d/hercules.v9/... # run race test separately to preserve -covermode=count
+  - # race checks increase the elapsed time by 10 minutes, we run them only in AppVeyor
   - $GOPATH/bin/hercules version
   - $GOPATH/bin/hercules --burndown --couples --devs --quiet --pb https://github.com/src-d/hercules > 1.pb
   - cp 1.pb 2.pb

Dockerfile

@@ -33,7 +33,7 @@ echo\n\' > /browser && \
     pip3 install --no-cache-dir --no-build-isolation -r /root/src/gopkg.in/src-d/hercules.v9/requirements.txt https://github.com/mind/wheels/releases/download/tf1.7-cpu/tensorflow-1.7.0-cp36-cp36m-linux_x86_64.whl && \
     rm -rf /root/* && \
     apt-get remove -y software-properties-common golang-1.10-go python3-dev libyaml-dev libxml2-dev curl git make unzip g++ && \
-    apt-get remove -y *-doc *-man && \
+    apt-get remove -qy *-doc *-man && \
     rm -rf /usr/share/doc /usr/share/man && \
     apt-get autoremove -y && \
     rm -rf /var/lib/apt/lists/* && \

internal/plumbing/diff.go

@@ -2,6 +2,7 @@ package plumbing
 
 import (
 	"strings"
+	"time"
 
 	"github.com/sergi/go-diff/diffmatchpatch"
 	"gopkg.in/src-d/go-git.v4"
@@ -128,6 +129,7 @@ func (diff *FileDiff) Consume(deps map[string]interface{}) (map[string]interface
 			// git/git 4f7770c87ce3c302e1639a7737a6d2531fe4b160 fetch-pack.c is invalid UTF-8
 			strFrom, strTo := string(blobFrom.Data), string(blobTo.Data)
 			dmp := diffmatchpatch.New()
+			dmp.DiffTimeout = time.Hour
 			src, dst, _ := dmp.DiffLinesToRunes(stripWhitespace(strFrom, diff.WhitespaceIgnore), stripWhitespace(strTo, diff.WhitespaceIgnore))
 			diffs := dmp.DiffMainRunes(src, dst, false)
 

internal/plumbing/renames.go

@@ -6,6 +6,7 @@ import (
 	"sort"
 	"strings"
 	"sync"
+	"time"
 	"unicode/utf8"
 
 	"github.com/sergi/go-diff/diffmatchpatch"
@@ -332,8 +333,8 @@ func (ra *RenameAnalysis) Consume(deps map[string]interface{}) (map[string]inter
 	}
 	// run two functions in parallel, and take the result from the one which finished earlier
 	wg.Add(2)
-	go func() { matchA() }()
-	go func() { matchB() }()
+	go matchA()
+	go matchB()
 	wg.Wait()
 	var matches object.Changes
 	select {
@@ -403,6 +404,7 @@ func (ra *RenameAnalysis) blobsAreClose(blob1 *CachedBlob, blob2 *CachedBlob) (b
 	// compute the line-by-line diff, then the char-level diffs of the del-ins blocks
 	// yes, this algorithm is greedy and not exact
 	dmp := diffmatchpatch.New()
+	dmp.DiffTimeout = time.Hour
 	srcLineRunes, dstLineRunes, _ := dmp.DiffLinesToRunes(src, dst)
 	// the third returned value, []string, is the mapping from runes to lines
 	// we cannot use it because it is approximate and has string collisions
@@ -426,6 +428,7 @@ func (ra *RenameAnalysis) blobsAreClose(blob1 *CachedBlob, blob2 *CachedBlob) (b
 				if internal.Max(srcPositions[posSrc]-srcPositions[prevPosSrc],
 					dstPositions[nextPosDst]-dstPositions[posDst]) < RenameAnalysisByteDiffSizeThreshold {
 					localDmp := diffmatchpatch.New()
+					localDmp.DiffTimeout = time.Hour
 					localSrc := src[srcPositions[prevPosSrc]:srcPositions[posSrc]]
 					localDst := dst[dstPositions[posDst]:dstPositions[nextPosDst]]
 					localDiffs := localDmp.DiffMainRunes(

internal/plumbing/renames_test.go

@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path"
+	"sync"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -192,6 +193,54 @@ func TestRenameAnalysisSortRenameCandidates(t *testing.T) {
 	assert.Equal(t, candidates[1], 1)
 }
 
+func TestBlobsAreCloseFlakyBug(t *testing.T) {
+	gitBlob1, err := test.Repository.BlobObject(plumbing.NewHash(
+		"29c9fafd6a2fae8cd20298c3f60115bc31a4c0f2"))
+	if err != nil {
+		t.Fatalf("get 29c9fafd6a2fae8cd20298c3f60115bc31a4c0f2 %v", err)
+	}
+	gitBlob2, err := test.Repository.BlobObject(plumbing.NewHash(
+		"baa64828831d174f40140e4b3cfa77d1e917a2c1"))
+	if err != nil {
+		t.Fatalf("get baa64828831d174f40140e4b3cfa77d1e917a2c1 %v", err)
+	}
+	blob1 := &CachedBlob{*gitBlob1, nil}
+	blob2 := &CachedBlob{*gitBlob2, nil}
+	err = blob1.Cache()
+	if err != nil {
+		t.Fatalf("read 29c9fafd6a2fae8cd20298c3f60115bc31a4c0f2 %v", err)
+	}
+	err = blob2.Cache()
+	if err != nil {
+		t.Fatalf("read baa64828831d174f40140e4b3cfa77d1e917a2c1 %v", err)
+	}
+	wg := sync.WaitGroup{}
+	gr := 10 // number of concurrent goroutines
+	wg.Add(gr)
+	for i := 0; i < gr; i++ {
+		go func() {
+			ra := fixtureRenameAnalysis()
+			ra.SimilarityThreshold = 37
+			result, err := ra.blobsAreClose(blob1, blob2)
+			assert.Nil(t, err)
+			assert.True(t, result)
+			result, err = ra.blobsAreClose(blob2, blob1)
+			assert.Nil(t, err)
+			assert.True(t, result)
+
+			ra.SimilarityThreshold = 39
+			result, err = ra.blobsAreClose(blob1, blob2)
+			assert.Nil(t, err)
+			assert.False(t, result)
+			result, err = ra.blobsAreClose(blob2, blob1)
+			assert.Nil(t, err)
+			assert.False(t, result)
+			wg.Done()
+		}()
+	}
+	wg.Wait()
+}
+
 func TestBlobsAreCloseText(t *testing.T) {
 	blob1 := &CachedBlob{Data: []byte("hello, world!")}
 	blob2 := &CachedBlob{Data: []byte("hello, world?")}

internal/plumbing/uast/diff_refiner_test.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"path"
 	"testing"
+	"time"
 	"unicode/utf8"
 
 	"github.com/sergi/go-diff/diffmatchpatch"
@@ -70,6 +71,7 @@ func TestFileDiffRefinerConsume(t *testing.T) {
 	bytes2, err := ioutil.ReadFile(path.Join("..", "..", "test_data", "2.java"))
 	assert.Nil(t, err)
 	dmp := diffmatchpatch.New()
+	dmp.DiffTimeout = time.Hour
 	src, dst, _ := dmp.DiffLinesToRunes(string(bytes1), string(bytes2))
 	state := map[string]interface{}{}
 	fileDiffs := map[string]plumbing.FileDiffData{}
@@ -112,6 +114,7 @@ func TestFileDiffRefinerConsumeNoUast(t *testing.T) {
 	bytes2, err := ioutil.ReadFile(path.Join("..", "..", "test_data", "2.java"))
 	assert.Nil(t, err)
 	dmp := diffmatchpatch.New()
+	dmp.DiffTimeout = time.Hour
 	src, dst, _ := dmp.DiffLinesToRunes(string(bytes1), string(bytes2))
 	state := map[string]interface{}{}
 	fileDiffs := map[string]plumbing.FileDiffData{}

leaves/shotness_test.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"path"
 	"testing"
+	"time"
 
 	"github.com/gogo/protobuf/proto"
 	"github.com/sergi/go-diff/diffmatchpatch"
@@ -30,7 +31,7 @@ func fixtureShotness() *ShotnessAnalysis {
 
 func TestShotnessMeta(t *testing.T) {
 	sh := &ShotnessAnalysis{}
-	sh.Initialize(test.Repository)
+	assert.Nil(t, sh.Initialize(test.Repository))
 	assert.NotNil(t, sh.nodes)
 	assert.NotNil(t, sh.files)
 	assert.Equal(t, sh.Name(), "Shotness")
@@ -41,13 +42,13 @@ func TestShotnessMeta(t *testing.T) {
 	assert.Len(t, sh.ListConfigurationOptions(), 2)
 	assert.Equal(t, sh.ListConfigurationOptions()[0].Name, ConfigShotnessXpathStruct)
 	assert.Equal(t, sh.ListConfigurationOptions()[1].Name, ConfigShotnessXpathName)
-	sh.Configure(nil)
+	assert.Nil(t, sh.Configure(nil))
 	assert.Equal(t, sh.XpathStruct, DefaultShotnessXpathStruct)
 	assert.Equal(t, sh.XpathName, DefaultShotnessXpathName)
 	facts := map[string]interface{}{}
 	facts[ConfigShotnessXpathStruct] = "xpath!"
 	facts[ConfigShotnessXpathName] = "another!"
-	sh.Configure(facts)
+	assert.Nil(t, sh.Configure(facts))
 	assert.Equal(t, sh.XpathStruct, "xpath!")
 	assert.Equal(t, sh.XpathName, "another!")
 	features := sh.Features()
@@ -90,6 +91,7 @@ func bakeShotness(t *testing.T, eraseEndPosition bool) (*ShotnessAnalysis, Shotn
 	bytes2, err := ioutil.ReadFile(path.Join("..", "internal", "test_data", "2.java"))
 	assert.Nil(t, err)
 	dmp := diffmatchpatch.New()
+	dmp.DiffTimeout = time.Hour
 	src, dst, _ := dmp.DiffLinesToRunes(string(bytes1), string(bytes2))
 	state := map[string]interface{}{}
 	state[core.DependencyCommit] = &object.Commit{}
@@ -144,6 +146,7 @@ func TestShotnessConsume(t *testing.T) {
 	bytes2, err := ioutil.ReadFile(path.Join("..", "internal", "test_data", "2.java"))
 	assert.Nil(t, err)
 	dmp := diffmatchpatch.New()
+	dmp.DiffTimeout = time.Hour
 	src, dst, _ := dmp.DiffLinesToRunes(string(bytes1), string(bytes2))
 	state := map[string]interface{}{}
 	state[core.DependencyCommit] = &object.Commit{}
@@ -239,7 +242,7 @@ func TestShotnessConsumeNoEnd(t *testing.T) {
 func TestShotnessSerializeText(t *testing.T) {
 	sh, result := bakeShotness(t, false)
 	buffer := &bytes.Buffer{}
-	sh.Serialize(result, false, buffer)
+	assert.Nil(t, sh.Serialize(result, false, buffer))
 	assert.Equal(t, buffer.String(), `  - name: testAddEntry
     file: test.java
     internal_role: uast:FunctionGroup
@@ -318,7 +321,7 @@ func TestShotnessSerializeText(t *testing.T) {
 func TestShotnessSerializeBinary(t *testing.T) {
 	sh, result := bakeShotness(t, false)
 	buffer := &bytes.Buffer{}
-	sh.Serialize(result, true, buffer)
+	assert.Nil(t, sh.Serialize(result, true, buffer))
 	message := pb.ShotnessAnalysisResults{}
 	err := proto.Unmarshal(buffer.Bytes(), &message)
 	assert.Nil(t, err)