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

make "spring test" support any number of arguments #102

Merged
merged 2 commits into from
Apr 12, 2013

Conversation

aspiers
Copy link

@aspiers aspiers commented Mar 17, 2013

This obsoletes #101.

spring test needs to accept multiple arguments if guard-minitest is to ever have a chance of supporting spring. Specs are automatically pruned from the command-line with a warning, since loading specs will stop the unit tests from running.

It's also useful for it to support zero arguments, in which case it does the sensible thing and defaults to running all unit tests. Additionally, this is consistent with the behaviour of spring rspec.

@aspiers
Copy link
Author

aspiers commented Mar 17, 2013

Hold tight - I'm working on tests for this ...

@aspiers
Copy link
Author

aspiers commented Mar 17, 2013

Weird, Ruby 2.0 doesn't like my use of Enumerable#partition :-( Looking into it ...

@aspiers
Copy link
Author

aspiers commented Mar 17, 2013

Ah, run_test was a bad choice of helper method :-) Fixing ...

@aspiers
Copy link
Author

aspiers commented Mar 17, 2013

guard/guard-minitest#57 adds support for spring based off this pull request.

ARGV.replace args
path = File.expand_path(args.first)
if args.empty?
args = [ 'test' ]
Copy link
Member

Choose a reason for hiding this comment

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

please use ['test'] to be consistent with the existing code.

Copy link
Author

Choose a reason for hiding this comment

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

Sure, will adjust.

@aspiers
Copy link
Author

aspiers commented Apr 6, 2013

If you're here at RubyManor maybe we could meet for a quick chat about this? I'll try to split the input validation commit out into a different pull request so that we can get the other (non-contentious) commits merged at least.

@aspiers
Copy link
Author

aspiers commented Apr 8, 2013

OK, the contentious change is no longer part of this pull request. I also rebased against latest master. Please can you review/merge this and then we can address the remaining issues separately? Cheers!

@@ -148,7 +148,7 @@ def spring_test_command

teardown do
app_run "#{spring} stop"
File.write(@test, @test_contents)
File.write(@test, @test_contents) unless @test.empty?
Copy link
Member

Choose a reason for hiding this comment

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

what's this for?

Copy link
Author

Choose a reason for hiding this comment

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

It's a remnant from the test you asked to be dropped. I guess we can ditch it now, although it still makes sense for if we wanted to re-introduce a similar test in the future.

@jonleighton
Copy link
Member

Sorry, I was at Ruby Manor but I didn't see your message

@aspiers
Copy link
Author

aspiers commented Apr 10, 2013

I'll tidy up this stuff soon and re-push.

Adam Spiers added 2 commits April 11, 2013 00:49
"spring test" needs to accept multiple arguments if guard-minitest is
to ever have a chance of supporting spring.
With no arguments, it now defaults to running all test-unit tests it
can find, i.e. files matching test/**/*_test.rb.  This is more useful
and arguably more intuitive too.

We need to remove test/performance/browsing_test.rb from the Rails app
being tested, otherwise "spring test" attempts to run this when it
iterates over test/**/*_test.rb, causing a failure due to ruby-prof
not being installed.

Signed-off-by: Adam Spiers <[email protected]>
@aspiers
Copy link
Author

aspiers commented Apr 11, 2013

OK, another attempt...

jonleighton added a commit that referenced this pull request Apr 12, 2013
make "spring test" support any number of arguments
@jonleighton jonleighton merged commit 5e03447 into rails:master Apr 12, 2013
@aspiers aspiers deleted the test-multi-args branch April 17, 2013 17:04
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.

2 participants