Skip to content

Commit 9c2eeb1

Browse files
lwsantylwsanty
committed
add tests with race detector + races fixes
Signed-off-by: lwsanty <[email protected]>
1 parent e398e4f commit 9c2eeb1

File tree

12 files changed

+65
-39
lines changed

12 files changed

+65
-39
lines changed

‎.github/workflows/push_pull.yml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,11 @@ jobs:
3333
bash <(curl -s https://codecov.io/bash) -f coverage.txt -cF $os,$go
3434
env:
3535
CODECOV_TOKEN: be731f61-6ca9-42b0-8f1a-59f4a0b28c0d
36+
37+
- name: Test races
38+
run: |
39+
make test-race
40+
env:
41+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
42+
43+

‎Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ CI_REPOSITORY ?= https://github.com/src-d/ci.git
1010
CI_BRANCH ?= v1
1111
CI_PATH ?= .ci
1212
MAKEFILE := $(CI_PATH)/Makefile.main
13+
TEST_RACE ?= true
1314
$(MAKEFILE):
1415
git clone --quiet --depth 1 -b $(CI_BRANCH) $(CI_REPOSITORY) $(CI_PATH);
1516
-include $(MAKEFILE)

‎downloader/download.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ var (
3131
func Download(ctx context.Context, job *library.Job) error {
3232
logger := job.Logger.New(log.Fields{"job": "download", "id": job.ID})
3333
if job.Type != library.JobDownload ||
34-
len(job.Endpoints) == 0 ||
34+
len(job.GetEndpoints()) == 0 ||
3535
job.Lib == nil ||
3636
job.TempFS == nil {
3737
err := ErrNotDownloadJob.New()
@@ -46,7 +46,7 @@ func Download(ctx context.Context, job *library.Job) error {
4646
return err
4747
}
4848

49-
endpoint := job.Endpoints[0]
49+
endpoint := job.GetEndpoints()[0]
5050
logger = logger.New(log.Fields{"url": endpoint})
5151

5252
repoID, err := library.NewRepositoryID(endpoint)
@@ -113,8 +113,7 @@ func libHas(
113113
select {
114114
case <-done:
115115
case <-ctx.Done():
116-
ok = false
117-
err = ctx.Err()
116+
return false, "", ctx.Err()
118117
}
119118

120119
return ok, locID, err

‎go.mod‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ require (
2929
github.com/sirupsen/logrus v1.4.2 // indirect
3030
github.com/src-d/envconfig v1.0.0 // indirect
3131
github.com/src-d/go-borges v0.0.0-20190704083038-44867e8f2a2a
32-
github.com/stretchr/testify v1.3.0
32+
github.com/stretchr/testify v1.4.0
3333
github.com/x-cray/logrus-prefixed-formatter v0.5.2 // indirect
3434
golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4 // indirect
3535
golang.org/x/net v0.0.0-20190628185345-da137c7871d7 // indirect
@@ -41,5 +41,6 @@ require (
4141
gopkg.in/src-d/go-errors.v1 v1.0.0
4242
gopkg.in/src-d/go-git.v4 v4.12.0
4343
gopkg.in/src-d/go-log.v1 v1.0.2
44+
gopkg.in/yaml.v2 v2.2.4 // indirect
4445
gotest.tools v2.2.0+incompatible // indirect
4546
)

‎go.sum‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1
123123
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
124124
github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
125125
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
126+
github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
127+
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
126128
github.com/x-cray/logrus-prefixed-formatter v0.5.2 h1:00txxvfBM9muc0jiLIEAkAcIMJzfthRT6usrui8uGmg=
127129
github.com/x-cray/logrus-prefixed-formatter v0.5.2/go.mod h1:2duySbKsL6M18s5GU7VPsoEPHyzalCE06qoARUCeBBE=
128130
github.com/xanzy/ssh-agent v0.2.0 h1:Adglfbi5p9Z0BmK2oKU9nTG+zKfniSfnaMYB+ULd+Ro=
@@ -214,5 +216,7 @@ gopkg.in/warnings.v0 v0.1.2/go.mod h1:jksf8JmL6Qr/oQM2OXTHunEvvTAsrWBLb6OOjuVWRN
214216
gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
215217
gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
216218
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
219+
gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I=
220+
gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
217221
gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo=
218222
gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw=

‎library/job.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package library
22

33
import (
44
"context"
5+
"sync"
56

67
"github.com/src-d/gitcollector"
78
"github.com/src-d/go-borges"
@@ -31,6 +32,7 @@ const (
3132

3233
// Job represents a gitcollector.Job to perform a task on a borges.Library.
3334
type Job struct {
35+
mu sync.Mutex
3436
ID string
3537
Type JobType
3638
Lib borges.Library
@@ -48,6 +50,21 @@ var _ gitcollector.Job = (*Job)(nil)
4850
// JobFn represents the task to be performed by a Job.
4951
type JobFn func(context.Context, *Job) error
5052

53+
// TODO: we should probably secure other fiels
54+
func (j *Job) SetEndpoints(endpoints []string) {
55+
j.mu.Lock()
56+
defer j.mu.Unlock()
57+
58+
j.Endpoints = endpoints
59+
}
60+
61+
func (j *Job) GetEndpoints() []string {
62+
j.mu.Lock()
63+
defer j.mu.Unlock()
64+
65+
return j.Endpoints
66+
}
67+
5168
// Process implements the Job interface.
5269
func (j *Job) Process(ctx context.Context) error {
5370
if j.ProcessFn == nil {

‎library/job_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func TestJobScheduleFn(t *testing.T) {
2222
mu.Lock()
2323
defer mu.Unlock()
2424

25-
got = append(got, j.Endpoints[0])
25+
got = append(got, j.GetEndpoints()[0])
2626
return nil
2727
}
2828
)
@@ -56,7 +56,7 @@ func TestDownloadJobScheduleFn(t *testing.T) {
5656
mu.Lock()
5757
defer mu.Unlock()
5858

59-
got = append(got, j.Endpoints[0])
59+
got = append(got, j.GetEndpoints()[0])
6060
return nil
6161
}
6262
)
@@ -89,7 +89,7 @@ func TestUpdateJobScheduleFn(t *testing.T) {
8989
mu.Lock()
9090
defer mu.Unlock()
9191

92-
got = append(got, j.Endpoints[0])
92+
got = append(got, j.GetEndpoints()[0])
9393
return nil
9494
}
9595
)

‎metrics/metrics.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -225,11 +225,11 @@ func (c *Collector) modifyMetrics(job *library.Job, kind int) error {
225225
break
226226
}
227227

228-
for range job.Endpoints {
228+
for range job.GetEndpoints() {
229229
c.successUpdateCount++
230230
}
231231
case failKind:
232-
for range job.Endpoints {
232+
for range job.GetEndpoints() {
233233
c.failCount++
234234
}
235235
case discoverKind:
@@ -351,16 +351,16 @@ func (c *CollectorByOrg) Discover(job gitcollector.Job) {
351351
func triageJob(job gitcollector.Job) map[string]*library.Job {
352352
organizations := map[string]*library.Job{}
353353
lj, _ := job.(*library.Job)
354-
for _, ep := range lj.Endpoints {
354+
for _, ep := range lj.GetEndpoints() {
355355
org := library.GetOrgFromEndpoint(ep)
356356
j, ok := organizations[org]
357357
if !ok {
358358
j = &(*lj)
359-
j.Endpoints = []string{}
359+
j.SetEndpoints([]string{})
360360
organizations[org] = j
361361
}
362362

363-
j.Endpoints = append(j.Endpoints, ep)
363+
j.SetEndpoints(append(j.GetEndpoints(), ep))
364364
}
365365

366366
return organizations

‎metrics/metrics_test.go‎

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -201,17 +201,10 @@ func testCloseDelayCollector(t *testing.T, c closeDelayCase) {
201201
})
202202

203203
go mc.Start()
204-
205-
job := &library.Job{
206-
Type: library.JobDownload,
207-
Endpoints: []string{"ep"},
208-
}
209-
210204
time.Sleep(c.delay)
211205

212-
mc.Success(job)
213-
job.Type = library.JobUpdate
214-
mc.Success(job)
206+
mc.Success(getJob(library.JobDownload))
207+
mc.Success(getJob(library.JobUpdate))
215208
mc.Stop(c.immediate)
216209

217210
require.Equal(t, c.expCounter, counter)
@@ -240,20 +233,23 @@ func testFailedSend(t *testing.T, stopImmediate bool) {
240233

241234
go mc.Start()
242235

243-
job := &library.Job{
244-
Type: library.JobDownload,
245-
Endpoints: []string{"ep"},
246-
}
247-
248-
mc.Success(job)
249-
job.Type = library.JobUpdate
250-
mc.Success(job)
236+
mc.Success(getJob(library.JobDownload))
237+
mc.Success(getJob(library.JobUpdate))
251238
mc.Stop(stopImmediate)
252239

253240
// TODO maybe we can stabilize it?
254241
if stopImmediate {
255-
require.True(t, mc.successUpdateCount <= 2, "expected: <= 2, got: %v", mc.successUpdateCount)
242+
require.LessOrEqual(t, mc.successDownloadCount, uint64(1))
243+
require.LessOrEqual(t, mc.successUpdateCount, uint64(1))
256244
} else {
257-
require.Equal(t, uint64(2), mc.successUpdateCount)
245+
require.Equal(t, uint64(1), mc.successDownloadCount)
246+
require.Equal(t, uint64(1), mc.successUpdateCount)
247+
}
248+
}
249+
250+
func getJob(jobType int) *library.Job {
251+
return &library.Job{
252+
Type: library.JobType(jobType),
253+
Endpoints: []string{"ep"},
258254
}
259255
}

‎provider/github_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func TestGitHub(t *testing.T) {
6666
job, ok := j.(*library.Job)
6767
req.True(ok)
6868
req.True(job.Type == library.JobDownload)
69-
req.Len(job.Endpoints, 1)
70-
req.True(strings.Contains(job.Endpoints[0], org))
69+
req.Len(job.GetEndpoints(), 1)
70+
req.True(strings.Contains(job.GetEndpoints()[0], org))
7171
}
7272
}

0 commit comments

Comments
 (0)