Skip to content

add tests with race detector + races fixes #89

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 23, 2019
Merged

add tests with race detector + races fixes #89

merged 1 commit into from
Oct 23, 2019

Conversation

lwsanty
Copy link
Contributor

@lwsanty lwsanty commented Oct 18, 2019 •
edited
Loading

  1. added tests with race detector to CI
  2. fixed race in tests(it was modifying the job while it was processed, @jfontan should we probably handle this case somehow?):
==================
WARNING: DATA RACE
Read at 0x00c00f9960a0 by goroutine 33:
  github.com/src-d/gitcollector/metrics.(*Collector).modifyMetrics()
      /home/lwsanty/goproj/lwsanty/gitcollector/metrics/metrics.go:223 +0x22a
  github.com/src-d/gitcollector/metrics.(*Collector).Start()
      /home/lwsanty/goproj/lwsanty/gitcollector/metrics/metrics.go:154 +0x5b1

Previous write at 0x00c00f9960a0 by goroutine 32:
  github.com/src-d/gitcollector/metrics.testCloseDelayCollector()
      /home/lwsanty/goproj/lwsanty/gitcollector/metrics/metrics_test.go:213 +0x389
  github.com/src-d/gitcollector/metrics.TestClosesWithDelay.func1()
      /home/lwsanty/goproj/lwsanty/gitcollector/metrics/metrics_test.go:184 +0x7e
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:909 +0x199

Goroutine 33 (running) created at:
  github.com/src-d/gitcollector/metrics.testCloseDelayCollector()
      /home/lwsanty/goproj/lwsanty/gitcollector/metrics/metrics_test.go:203 +0x1d1
  github.com/src-d/gitcollector/metrics.TestClosesWithDelay.func1()
      /home/lwsanty/goproj/lwsanty/gitcollector/metrics/metrics_test.go:184 +0x7e
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:909 +0x199

Goroutine 32 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:960 +0x651
  github.com/src-d/gitcollector/metrics.TestClosesWithDelay()
      /home/lwsanty/goproj/lwsanty/gitcollector/metrics/metrics_test.go:183 +0x184
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:909 +0x199
==================
  1. fixed race in Download:
==================
WARNING: DATA RACE
Write at 0x00c0002b8110 by goroutine 25:
  github.com/src-d/gitcollector/downloader.libHas.func1()
      /home/lwsanty/goproj/lwsanty/gitcollector/downloader/download.go:109 +0xfa

Previous write at 0x00c0002b8110 by goroutine 24:
  github.com/src-d/gitcollector/downloader.libHas()
      /home/lwsanty/goproj/lwsanty/gitcollector/downloader/download.go:117 +0x3bc
  github.com/src-d/gitcollector/downloader.Download()
      /home/lwsanty/goproj/lwsanty/gitcollector/downloader/download.go:58 +0x554
  github.com/src-d/gitcollector/downloader.testContextCancelledFail()
      /home/lwsanty/goproj/lwsanty/gitcollector/downloader/download_test.go:239 +0x470
  github.com/src-d/gitcollector/downloader.TestAll.func1()
      /home/lwsanty/goproj/lwsanty/gitcollector/downloader/download_test.go:138 +0x5b
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:909 +0x199

Goroutine 25 (running) created at:
  github.com/src-d/gitcollector/downloader.libHas()
      /home/lwsanty/goproj/lwsanty/gitcollector/downloader/download.go:108 +0x1ae
  github.com/src-d/gitcollector/downloader.Download()
      /home/lwsanty/goproj/lwsanty/gitcollector/downloader/download.go:58 +0x554
  github.com/src-d/gitcollector/downloader.testContextCancelledFail()
      /home/lwsanty/goproj/lwsanty/gitcollector/downloader/download_test.go:239 +0x470
  github.com/src-d/gitcollector/downloader.TestAll.func1()
      /home/lwsanty/goproj/lwsanty/gitcollector/downloader/download_test.go:138 +0x5b
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:909 +0x199

Goroutine 24 (finished) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:960 +0x651
  github.com/src-d/gitcollector/downloader.TestAll()
      /home/lwsanty/goproj/lwsanty/gitcollector/downloader/download_test.go:137 +0x315
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:909 +0x199
==================
  1. fixed race in Download
==================
WARNING: DATA RACE
Read at 0x00c0001f4148 by goroutine 85:
  github.com/src-d/gitcollector/downloader.Download()
      /home/lwsanty/goproj/lwsanty/gitcollector/downloader/download.go:34 +0xd91
  github.com/src-d/gitcollector/library.(*Job).Process()
      /home/lwsanty/goproj/lwsanty/gitcollector/library/job.go:57 +0x87
  github.com/src-d/gitcollector.(*worker).consumeJob.func1()
      /home/lwsanty/goproj/lwsanty/gitcollector/worker.go:61 +0x93

Previous write at 0x00c0001f4148 by goroutine 72:
  github.com/src-d/gitcollector/metrics.triageJob()
      /home/lwsanty/goproj/lwsanty/gitcollector/metrics/metrics.go:359 +0x30e
  github.com/src-d/gitcollector/metrics.(*CollectorByOrg).Discover()
      /home/lwsanty/goproj/lwsanty/gitcollector/metrics/metrics.go:340 +0x5d
  github.com/src-d/gitcollector.(*jobScheduler).Schedule()
      /home/lwsanty/goproj/lwsanty/gitcollector/scheduler.go:113 +0x6ba

Goroutine 85 (running) created at:
  github.com/src-d/gitcollector.(*worker).consumeJob()
      /home/lwsanty/goproj/lwsanty/gitcollector/worker.go:59 +0x206
  github.com/src-d/gitcollector.(*worker).start()
      /home/lwsanty/goproj/lwsanty/gitcollector/worker.go:39 +0xf5
  github.com/src-d/gitcollector.(*WorkerPool).add.func1()
      /home/lwsanty/goproj/lwsanty/gitcollector/worker_pool.go:81 +0x38

Goroutine 72 (running) created at:
  github.com/src-d/gitcollector.(*WorkerPool).Run()
      /home/lwsanty/goproj/lwsanty/gitcollector/worker_pool.go:46 +0xbb
  github.com/src-d/gitcollector/cmd/gitcollector/subcmd.(*DownloadCmd).Execute()
      /home/lwsanty/goproj/lwsanty/gitcollector/cmd/gitcollector/subcmd/download.go:167 +0x12fb
  github.com/src-d/gitcollector/integration.testPostgresSendMetricsSuccess()
      /home/lwsanty/goproj/lwsanty/gitcollector/integration/helper.go:72 +0x159
  github.com/src-d/gitcollector/integration.TestPostgres.func1()
      /home/lwsanty/goproj/lwsanty/gitcollector/integration/postgres_test.go:66 +0x9f
  testing.tRunner()
      /opt/hostedtoolcache/go/1.13.3/x64/src/testing/testing.go:909 +0x199
==================
  • various other races

Signed-off-by: lwsanty lwsanty@gmail.com

@lwsanty lwsanty requested review from jfontan and mcarmonaa October 18, 2019 14:07
@lwsanty lwsanty self-assigned this Oct 18, 2019
@lwsanty lwsanty force-pushed the add-tests-with-race-detector branch 3 times, most recently from 7e87efa to 9c2eeb1 Compare October 18, 2019 15:26
jfontan
jfontan suggested changes Oct 22, 2019
View reviewed changes
library/job.go Outdated
j.Endpoints = endpoints
}

func (j *Job) GetEndpoints() []string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we we are going to have getter and setter I would unexport Endpoints and rename GetEndpoints to Endpoints.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@lwsanty
add tests with race detector + races fixes
862c91c 
Signed-off-by: lwsanty <lwsanty@gmail.com>
@lwsanty lwsanty force-pushed the add-tests-with-race-detector branch from 439b01a to 862c91c Compare October 23, 2019 08:57
@lwsanty lwsanty requested a review from jfontan October 23, 2019 08:57
@jfontan jfontan merged commit 74de6c3 into src-d:master Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
2 more reviewers

@jfontan jfontan jfontan approved these changes

@mcarmonaa mcarmonaa mcarmonaa approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@lwsanty lwsanty

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lwsanty @jfontan @mcarmonaa