Skip to content
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.

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

Package not usable with parallel tests #9

Closed
jnwhiteh opened this issue Oct 11, 2014 · 5 comments · Fixed by #21
Closed

Package not usable with parallel tests #9

jnwhiteh opened this issue Oct 11, 2014 · 5 comments · Fixed by #21
Milestone

Comments

@jnwhiteh
Copy link

There is currently a data race when using go-sqlmock in two tests in parallel due to the way the API is currently designed. There is only ever one "mock" database and all operations are done on this. This means two tests cannot use this package in parallel. This is evidenced from the following data race in a package I am developing. I'm going to take a stab and changing the API to accommodate this.

==================
WARNING: DATA RACE
Read by goroutine 10:
  github.com/DATA-DOG/go-sqlmock.ExpectQuery()
      /Users/jnwhiteh/go/src/github.com/DATA-DOG/go-sqlmock/sqlmock.go:155 +0x138
  github.com/jnwhiteh/migration.setupVersioned()
      /Users/jnwhiteh/go/src/github.com/jnwhiteh/migration/migration_test.go:35 +0x22a
  github.com/jnwhiteh/migration.TestDowngradesUnsupported()
      /Users/jnwhiteh/go/src/github.com/jnwhiteh/migration/migration_test.go:61 +0x73
  testing.tRunner()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/testing/testing.go:422 +0x10f

Previous write by goroutine 8:
  github.com/DATA-DOG/go-sqlmock.(*conn).Close()
      /Users/jnwhiteh/go/src/github.com/DATA-DOG/go-sqlmock/connection.go:24 +0x266
  database/sql.(*driverConn).finalClose()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/database/sql/sql.go:310 +0x175
  database/sql.finalCloser.(database/sql.finalClose)·fm()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/database/sql/sql.go:398 +0x57
  database/sql.(*DB).Close()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/database/sql/sql.go:487 +0x6fa
  github.com/jnwhiteh/migration.TestFailingToGetCurrentVersion()
      /Users/jnwhiteh/go/src/github.com/jnwhiteh/migration/migration_test.go:54 +0x331
  testing.tRunner()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/testing/testing.go:422 +0x10f

Goroutine 10 (running) created at:
  testing.RunTests()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/testing/testing.go:504 +0xb46
  testing.Main()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/testing/testing.go:435 +0xa2
  main.main()
      github.com/jnwhiteh/migration/_test/_testmain.go:61 +0xdc

Goroutine 8 (finished) created at:
  testing.RunTests()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/testing/testing.go:504 +0xb46
  testing.Main()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/testing/testing.go:435 +0xa2
  main.main()
      github.com/jnwhiteh/migration/_test/_testmain.go:61 +0xdc
==================
==================
WARNING: DATA RACE
Write by goroutine 10:
  github.com/DATA-DOG/go-sqlmock.ExpectQuery()
      /Users/jnwhiteh/go/src/github.com/DATA-DOG/go-sqlmock/sqlmock.go:156 +0x309
  github.com/jnwhiteh/migration.setupVersioned()
      /Users/jnwhiteh/go/src/github.com/jnwhiteh/migration/migration_test.go:35 +0x22a
  github.com/jnwhiteh/migration.TestDowngradesUnsupported()
      /Users/jnwhiteh/go/src/github.com/jnwhiteh/migration/migration_test.go:61 +0x73
  testing.tRunner()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/testing/testing.go:422 +0x10f

Previous write by goroutine 8:
  github.com/DATA-DOG/go-sqlmock.(*conn).Close()
      /Users/jnwhiteh/go/src/github.com/DATA-DOG/go-sqlmock/connection.go:25 +0x2d0
  database/sql.(*driverConn).finalClose()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/database/sql/sql.go:310 +0x175
  database/sql.finalCloser.(database/sql.finalClose)·fm()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/database/sql/sql.go:398 +0x57
  database/sql.(*DB).Close()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/database/sql/sql.go:487 +0x6fa
  github.com/jnwhiteh/migration.TestFailingToGetCurrentVersion()
      /Users/jnwhiteh/go/src/github.com/jnwhiteh/migration/migration_test.go:54 +0x331
  testing.tRunner()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/testing/testing.go:422 +0x10f

Goroutine 10 (running) created at:
  testing.RunTests()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/testing/testing.go:504 +0xb46
  testing.Main()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/testing/testing.go:435 +0xa2
  main.main()
      github.com/jnwhiteh/migration/_test/_testmain.go:61 +0xdc

Goroutine 8 (finished) created at:
  testing.RunTests()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/testing/testing.go:504 +0xb46
  testing.Main()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/testing/testing.go:435 +0xa2
  main.main()
      github.com/jnwhiteh/migration/_test/_testmain.go:61 +0xdc
==================
PASS
Found 2 data race(s)
exit status 66
FAIL    github.com/jnwhiteh/migration   1.105s
@l3pp4rd
Copy link
Member

l3pp4rd commented Oct 12, 2014

Hi, yes indeed, currently the api is designed to use one mock connection, that could be improved without interfering with the public interface. In meantime, I cannot promise to take a look soon, but if you are working in this area, would be great if you could submit a pull request. cheers

@wongak
Copy link

wongak commented Oct 16, 2014

@jnwhiteh Could you do me a favor and see if my PR would solve your problem? If you are also tackling this issue: what approach did you take? Cheers

@jnwhiteh
Copy link
Author

@wongak I added some comments to your pull request, lets continue that discussion there. If useful, I can share the branch I'm working on.

@l3pp4rd
Copy link
Member

l3pp4rd commented Aug 26, 2015

thanks @jnwhiteh and @wongak for the discussion and your branch to support concurrency. The refactor branch is now covering all the issues which the library had in initial version. It will be merged as soon as the README and examples are updated, cheers

l3pp4rd added a commit that referenced this issue Aug 28, 2015
* c600769 do not require a connection name, unique dsn is generated
* 1b20b9c update travis
* 1097b6a add comments for godoc documentation
* c142a95 fix golint reported issues
l3pp4rd added a commit that referenced this issue Aug 28, 2015
concurrency support, closes #20 and closes #9 and closes #15
@jnwhiteh
Copy link
Author

Thanks for the update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants