Don't blindly add lib/ to load path on rspec configuration

[gkop]

This behavior is hardcoded here: https://github.com/rspec/rspec-core/blob/bebc52c9e7c411d4b1951e7b3169eeb8528daab8/lib/rspec/core/configuration.rb#L1271-L1271

And tested here: https://github.com/rspec/rspec-core/blob/bebc52c9e7c411d4b1951e7b3169eeb8528daab8/spec/rspec/core/configuration_spec.rb#L102

I'm sure it made total sense once, for the convenience of testing gems, where the code always lives in lib/.

But it's a big assumption on the part of rspec that this is a desirable behavior. For example, in an ordinary rails 4 / bundler / rspec 3 app, this causes the Bundler.require line in environment.rb to require matching ruby files from #{Rails.root}/lib/ rather than from bundler's gems when executing the rspec binary.

[JonRowe]

But it's a big assumption on the part of rspec that this is a desirable behavior.

Not really it's standard Ruby behaviour.

For example, in an ordinary rails 4 / bundler / rspec 3 app, this causes the Bundler.require line in environment.rb to require matching ruby files from #{Rails.root}/lib/ rather than from bundler's gems when executing the rspec binary.

It shouldn't do, it should cause it to look for #{Rails.root}/lib then bundlers gems, which is the same behaviour Rails itself has...

[gkop]

Hmm, maybe we're looking at it from different sides. Rails gives you nice control over what you want in your load path and when you want to require it. As does rspec generally (for example, the spec_helper.rb file). What precedent are you thinking of for a library to add code that doesn't belong to it to your load path upon initialization?

[JonRowe]

Rails gives you nice control over what you want in your load path and when you want to require it.

Rails does the same thing we do, we just need to do it earlier to get the same behaviour.

What precedent are you thinking of for a library to add code that doesn't belong to it to your load path upon initialization?

./lib is added to your load path in every single rubygem and by Rails, which covers the majority of Ruby code out there, we're for testing your code, thus we follow that convention as if we were something in your ./bin.

[gkop]

What is the desired end result that this behavior yields? I guessed that the behavior was for convenience because when I removed the behavior, the only rspec-core spec that failed was the spec I linked to above that specifically tests for the behavior.

[JonRowe]

For your code and specs to be able to require lib/my_file as require 'my_file' as you would expect.

[JonRowe]

Trust me, us adding lib is not an issue, are you encountering a problem? Because theres probably something else going on, for example using the same name for files in lib as your gems (which is a smell in itself and usually only used for monkey patching)

[JonRowe]

Trust me, us adding lib is not an issue, are you encountering a problem? Because theres probably something else going on, for example using the same name for files in lib as your gems (which is a smell in itself and usually only used for monkey patching)

[gkop]

Right, but this is not always desirable.

To account for my case of using rspec to test a rails app, we could either remove this convenient behavior and force people to opt-in by adding lib/ to their path in spec_helper.rb, or we could give an option to opt-out of this behavior, for example when using rspec-rails.

[JonRowe]

I'm afraid there's no option to opt-out of that behaviour from Rails so it would still happen.

[gkop]

Fortunately, rails adds lib/ to your path after Bundle.require is invoked in application.rb (in a default rails app). We could follow rails' lead in rspec by adding lib/ to your path in an appropriate place in the default spec_helper.rb.

As a framework, rails is entitled to make decisions about what's in lib/. Similarly, rspec as a framework for testing gems would be likewise entitled. But rspec is more than that: you can test whatever you want with rspec, rails apps, scripts, etc.. This assumption about blindly adding lib/ with no recourse, well, it's a bit rude.

[gkop]

(it goes without saying, but lib/ in a gem and lib/ in a rails app mean two completely different things...)

[JonRowe]

(it goes with out saying, but lib/ in a gem and lib/ in a rails app mean two completely different things...)

Not really, they mean the same thing.

This assumption about blindly adding lib/ with no recourse, well, it's a bit rude.

s/bit rude/standard ruby convention

Fortunately, rails adds lib/ to your path after Bundle.require is invoked in application.rb (in a default rails app).

Like I say, this should be entirely harmless, please tell me what the actual issue you have is.

[gkop]

lib/ in a gem is the ruby code constituting the gem. lib/ in a rails app, well, people use it for different stuff, but it never constitutes the app; the bulk of the app lives in /app

The actual issue that prompted this was my picking up a rails app from other developers that included some monkeypatches (yes, monkeypatches are questionable, but people do use them :/). They had the app set up with minitest and everything is hunky dory. But adding the first rspec test and trying to figure out why the load path has been altered meant stepping through the rspec code as I have done.

The current behavior offers no value but for a bit of convenience, and we can re-imagine it in a way that is less mystifying and offers rspec users more control/flexibility.

[JonRowe]

lib/ in a gem is the ruby code constituting the gem.

lib/ in a gem is ruby code constituting things expected to be consumed by another party, or from an executable.

lib/ in a rails app, well, people use it for different stuff, but it never constitutes the app; the bulk of the app lives in /app

lib/ in a rails app is ruby code constituting things expected to be consumed by the app itself, from my perspective thats the same pattern

The current behavior offers no value but for a bit of convenience, and we can re-imagine it in a way that is less mystifying and offers rspec users more control/flexibility.

Your the first person to mention this as causing an issue for them, as such theres no way we'd change the default behaviour that works in 99.999% of cases. I'd argue minitest is behaving surprisingly here as I don't think it's behaviour would necessarily match production usage here.

[gkop]

Fair enough, thanks for your consideration Jon.

[JonRowe]

By the way its worth noting that code you linked to doesn't do what you think it does, it only adds lib to the path when you tell us to require something for you... You could avoid this if you wanted to by simply not using the --require option.

[gkop]

I am not using --require :(

[JonRowe]

You are somewhere, check your .rspec and / or your spec helper for config.requires =

[gkop]

You're right! The default .rspec that rspec-rails gave me has --require spec_helper. I will investigate this.

[myronmarston]

The discussion here made me wonder when RSpec started to do this. I discovered that it dates back to RSpec 1.2.9 in 2009:

https://github.com/dchelimsky/rspec/commit/010d2fd41dcab5df74cd066a210ab485673a1102

It's also prominently documented in our README:

# in spec/calculator_spec.rb
# - RSpec adds ./lib to the $LOAD_PATH
require "calculator"

In the 1.8 days, before the advent of require_relative, there was a common issue where files could be double-required if they were required with different forms (e.g. different relative paths, or one relative and one absolute). The solution was to require everything relative to a $LOAD_PATH entry, and putting lib on the load path facilitated that, helping users to avoid double-require problems. While users certainly could have put lib on the $LOAD_PATH (and still can today), in my experience, most ruby programmers do not like to manually put things on $LOAD_PATH, preferring to allow their tools (e.g. rspec, bundler, rubygems and rails) to setup the load path for them. So yes, it's a convenience, but a valuable one that, as far as I know, had never received any complaints before now.

Making a change to no longer put lib on the $LOAD_PATH would be breaking for most users and as such, is something we'd only consider doing in RSpec 4. That said, I personally like having lib on the $LOAD_PATH and am not convinced it's worth the pain it would cause most users to change. It's not clear to me what problem it's actually causing for you. Can you explain your problem some more? Even though we're unlikely to change it here I'd still like to understand why you find it problematic.

This assumption about blindly adding lib/ with no recourse, well, it's a bit rude.

You do have recourse. Additions to the $LOAD_PATH need not be permanent. If you don't want lib on the $LOAD_PATH, it's trivial to remove it:

$LOAD_PATH.delete_if { |p| File.expand_path(p) == File.expand_path("./lib") }

[gkop]

Thanks for the suggestion and explanation. In my case, moving a single ruby file from lib/ to a new directory lib/monkeypatches/ fixes me up: without lib/monkeypatches/ in my path, bundler requires the gem rather than the monkeypatch.

Out of interest, besides backwards compatibility, what are the other considerations related to my idea to move adding lib/ to the path to the default spec_helper.rb, thus making the behavior explicit and configurable?

[myronmarston]

In my case, moving a single ruby file from lib/ to a new directory lib/monkeypatches/ fixes me up: without lib/monkeypatches/ in my path, bundler requires the gem rather than the monkeypatch.

Thanks for explaining. I must have missed that above.

Out of interest, besides backwards compatibility, what are the other considerations related to my idea to move adding lib/ to the path to the default spec_helper.rb, thus making the behavior explicit and configurable?

Lots of people don't use the generated spec_helper.rb (or even know rspec --init will generate one for you!), so moving it into the generated file is insufficient, IMO. There's also the problem of upgrades: if we moved it from a default to spec_helper.rb in RSpec 4, folks upgrading from RSpec 3 to 4 would have to know to add it themselves after upgrading or they'd get failures. We successfully navigate the RSpec 2 -> 3 transition with many changes but I'm not convinced the benefit here is enough to warrant forcing users to make this change...and we're hoping RSpec 4 will be a much more modest upgrade process :).

Besides, as I said above, the load path is already configurable. It's a global variable you can configure to you hearts content :). It's a matter of whether it makes more sense to make 99% of users configure the load path to put lib on it, or default that on and make the 1% of users who don't want it configure the load path by removing lib from it. The 99/1 split might sound extreme, but given you're the first to complain, that ratio isn't far-fetched.

[gkop]

Makes sense Myron, thanks for the extra info.

[dentarg]

I ran into this. I don't have ./lib in the $LOAD_PATH for my application (I do require "lib/foo" and so on). At one place I made a bad require, leaving of lib/. My tests was all green, but the application could not start.