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

Disable Spring by default when running executable - #301 #468

Merged
merged 1 commit into from Dec 2, 2015

Conversation

soberstadt
Copy link
Contributor

I don't know very much about rake tasks, so I don't know how to fix if it is running with rake, but this fixes bundle exec parallel_rspec.

@grosser
Copy link
Owner

grosser commented Nov 30, 2015

this only fixes rspec :)

I'd say put it intoParallelTests::CLI#run ... ideally with a test that assets that it is getting set

@soberstadt
Copy link
Contributor Author

Ah, I didn't realize this was an issue on all of the suites. I'll look into that.

@soberstadt
Copy link
Contributor Author

I updated the code to set the var in the CLI class, and tested it in practice and it works for both the executable and the rake task. But now I am struggling to understand your tests and where I should check if it was set. The test I wrote won't pass.

@grosser
Copy link
Owner

grosser commented Nov 30, 2015

I can fix up the tests, thx for verifying :)

@soberstadt
Copy link
Contributor Author

Thanks for encouragement and contributions in OSS!

@grosser grosser merged commit e06f7eb into grosser:master Dec 2, 2015
@grosser
Copy link
Owner

grosser commented Dec 2, 2015

2.2.0 is out! :D

@betesh
Copy link

betesh commented Dec 2, 2015

This breaks all my tests. I was using parallel_tests with spring and everything was working fine with a little help from #309 (comment)

I don't see any documentation warning uses about this breaking change, and I don't see from the code how it's possible to override it.

@grosser
Copy link
Owner

grosser commented Dec 3, 2015

why is it breaking your tests ?
can you post some backtraces or instructions to reproduce ?

The 309 patch is hard to generalize since it would need to reconnect all
your data sources (redis/memcached etc) ... it's a nice hack for super fast
tests though ... so I like the idea of supporting it ... for example by
doing DISABLE_SPRING||=1 and you could set it to '' or '0' to prevent that
override ...

On Wed, Dec 2, 2015 at 11:03 AM, betesh notifications@github.com wrote:

This breaks all my tests. I was using parallel_tests with spring and
everything was working fine with a little help from #309 (comment)
#309 (comment)

I would think if a default is going to change, there's at least be a
documented way to override it.


Reply to this email directly or view it on GitHub
#468 (comment)
.

@grosser
Copy link
Owner

grosser commented Dec 3, 2015

PR #469

On Thu, Dec 3, 2015 at 8:28 AM, Michael Grosser michael@grosser.it wrote:

why is it breaking your tests ?
can you post some backtraces or instructions to reproduce ?

The 309 patch is hard to generalize since it would need to reconnect all
your data sources (redis/memcached etc) ... it's a nice hack for super fast
tests though ... so I like the idea of supporting it ... for example by
doing DISABLE_SPRING||=1 and you could set it to '' or '0' to prevent that
override ...

On Wed, Dec 2, 2015 at 11:03 AM, betesh notifications@github.com wrote:

This breaks all my tests. I was using parallel_tests with spring and
everything was working fine with a little help from #309 (comment)
#309 (comment)

I would think if a default is going to change, there's at least be a
documented way to override it.


Reply to this email directly or view it on GitHub
#468 (comment)
.

@betesh
Copy link

betesh commented Dec 4, 2015

That should work--I'll give it a shot tomorrow.

The backtraces wouldn't really tell you anything--it's just clear from them that different processes are using the same database because I get a lot of Mysql2 errors complaining about UNIQUE INDEX constraints. When I run the same suite with parallel_tests but without spring, everything passes.

This reminds me of another suggestions--it would be great if when running tests (rspec in my case), the output would mention the TEST_ENV_NUMBER somewhere, and maybe also the DB name. Maybe a custom RSpec formatter would work for this, but I suspect there's a more general solution that will work for any test framework.

It would give the user a quick verification that everything is working correctly--running in different processes, with different DB's.

@grosser
Copy link
Owner

grosser commented Dec 4, 2015

why would they use the same database if spring was disabled ?

running with --verbose should show the TEST_ENV_NUMBER ... showing the
database is a bit trickier and needs some sort of AR integration

On Thu, Dec 3, 2015 at 4:09 PM, betesh notifications@github.com wrote:

That should work--I'll give it a shot tomorrow.

The backtraces wouldn't really tell you anything--it's just clear from
them that different processes are using the same database because I get a
lot of Mysql2 errors complaining about UNIQUE INDEX constraints. When I run
the same suite with parallel_tests but without spring, everything passes.

This reminds me of another suggestions--it would be great if when running
tests (rspec in my case), the output would mention the TEST_ENV_NUMBER
somewhere, and maybe also the DB name. Maybe a custom RSpec formatter would
work for this, but I suspect there's a more general solution that will work
for any test framework.

It would give the user a quick verification that everything is working
correctly--running in different processes, with different DB's.


Reply to this email directly or view it on GitHub
#468 (comment)
.

@betesh
Copy link

betesh commented Dec 4, 2015

why would they use the same database if spring was disabled ?

This is a good question, and I don't know the answer. It's especially odd considering then when I just don't use spring at all, tests pass but when I run the command with spring and let parallel_tests disable it, tests fail.

Maybe by the time parallel tests is loaded, some of spring has already been loaded and executed and only part of its behavior ends up being disabled. That is wild speculation on my part but nothing else comes to mind.

Regardless, I just tested with #469 and it works.

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

Successfully merging this pull request may close these issues.

None yet

3 participants