use bin/rspec if present #914

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants

bolandrm commented Jan 5, 2014

Hey,

This updates the rake tasks to use bin/rspec if it's present.

This will make the default rake task compatible with Spring.

Contributor

alindeman commented Jan 5, 2014

Neat. Are there any other examples of other gems doing this? I just want to get a feel for how this works under the covers.

bolandrm commented Jan 5, 2014

Hmm - I wasn't able to find any other examples of gems doing this sort of check. It's not a problem with something like guard, because they allow you to specify the exact rspec command to be used when running tests.

vim-rails does a similar check to see if bin/rake is present: tpope/vim-rails@c8e55b5

You can read a bit more about what the bin file does here: https://github.com/rails/spring#setup

Owner

myronmarston commented Jan 6, 2014

Should this logic move into rspec-core so that the rake task defaults to using bin/rspec if it's present?

bolandrm commented Jan 6, 2014

Up to you guys. If one wasn't working on rails, they may be using bundler binstubs.

Different use case here, but seems valid.

Owner

myronmarston commented Jan 6, 2014

I've personally run into issues with the rspec-core rake task where I used bundler install --standalone --binstubs to install my gems into a bundle subdir of my project, but didn't have an rspec gem install in the system gems, and as a result using the rake task would fail unless I used bundle exec...but the point of my using --standalone was to avoid needing to load bundler at runtime. If the user has an rspec executable at bin/rspec, I think we should use it.

@bolandrm -- are you up for opening an rspec-core PR making the rake task default to this? Then no change will be needed here.

Owner

JonRowe commented Jan 6, 2014

Another option would be to add ./bin to the path before executing rspec thus sidestepping the need for the conditional.

@bolandrm bolandrm referenced this pull request in rspec/rspec-core Jan 6, 2014

Closed

use binstub if present #1241

bolandrm commented Jan 6, 2014

All - PR opened @ rspec/rspec-core#1241

@JonRowe I wasn't able to get the load path working like that. I attempted to add something like $:.unshift File.dirname('./bin') to no avail. Any suggestions?

Owner

myronmarston commented Jan 6, 2014

Another option would be to add ./bin to the path before executing rspec thus sidestepping the need for the conditional.

I think that's a bad idea. If the user's specs do any shelling out, they could get different behavior from bin/rspec vs bin/rake spec, since the modification of the path for the rake task could cause different executables to be used.

Owner

JonRowe commented Jan 6, 2014

Any suggestions?

I meant add ./bin to the system path for the command e.g. PATH=./bin:$PATH rspec (although we'd need to cater for windows to, which i think uses ; instead of :)

I think that's a bad idea.

What if the user is shelling out to things dependant on bundle install? Or git gems? Not using the local bin folder for everything when it is present is more likely to create issues than not using it at all.

Owner

myronmarston commented Jan 6, 2014

What if the user is shelling out to things dependant on bundle install? Or git gems? Not using the local bin folder for everything when it is present is more likely to create issues than not using it at all.

As a user, if I need to run a binary in ./bin, then I would expect to have to take care of that myself. Having rspec modify the path seems terrible for situations like these.

I consider bin/rspec different, because it's an executable owned by rspec, and because the thing that shells out (e.g. the rake task that ships with rspec-core) is also owned by rspec.

Owner

JonRowe commented Jan 6, 2014

Except it's actually going to be a executable owned by bundler in most instances

Owner

myronmarston commented Jan 6, 2014

As things currently stand, I can't use bin/rake spec in most of my projects because I use bundle install --standalone --binstubs and do not install gems in my system gems. I'd like to address that.

Fixing this issue should have no affect on how users write specs that shell out and run binaries. How they manage that is completely up to them.

@bolandrm bolandrm closed this Jan 12, 2014

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