rspec-style-guide icon indicating copy to clipboard operation
rspec-style-guide copied to clipboard

Don't require all `spec/support` files by default

Open andyw8 opened this issue 5 years ago • 5 comments

This is specific to rspec-rails:

The generator originally created a helper which required everything in support/**/*.rb. This was changed in https://github.com/rspec/rspec-rails/pull/1137 but I still find many projects which require all the files, either because it's an old project, or because the advice was ignored.

andyw8 avatar Apr 28 '19 19:04 andyw8

Is this specific to rspec-rails, or does bare RSpec require support files? I see this in RSpec's own test suite:

require 'rspec/support/spec'

pirj avatar Apr 29 '19 08:04 pirj

Oh, interesting.

It seems RSpec own suite started using that in https://github.com/rspec/rspec-core/commit/82bf93e6e2eac8a20d6d34c6b0a8f6ccb874628a (March 2010), which was far before https://github.com/rspec/rspec-rails/pull/1137.

I suspect many most non-Rails projects don't do this, but I think it would be fine to have this advice apply for everywhere.

andyw8 avatar Apr 30 '19 00:04 andyw8

I think this advice really only makes sense if you have a large number of support files. The trouble with selectively requireing any files in ruby is that if multiple files need the dependency, there's no way to enforce that they all individually require it as long as the first one that happens to be loaded requires it.

Therefore, you may not notice that you've missed the appropriate require in some files. This means that if you load only that one spec (that is supposed to be able to run fast now), it won't work at all. I think the maintenance of taking a selective require approach in ruby should only be undertaken if your test suite is painfully slow. Maybe it makes sense for most rails apps--I'm not sure. I always require everything before running my tests to avoid this problem but all the projects I work on are small-to-medium size. I think this recommendation should be project-size dependent or at least list this maintenance caveat.

dgollahon avatar Apr 30 '19 00:04 dgollahon

Fair point. I'll think about how to phrase it.

andyw8 avatar Apr 30 '19 00:04 andyw8

There's also when_first_matching_example_defined:

RSpec.configure do |config|
  config.when_first_matching_example_defined(:db) do
    require "support/db"
  end
end

Together with define_derived_metadata:

RSpec.configure do |config|
  # Tag all groups and examples in the spec/model directory with :db => true
  config.define_derived_metadata(:file_path => %r{/spec/model/}) do |metadata|
    metadata[:db] = true
  end
end 

it provides a quite elegant (though magical) way to lazy-load support dependencies.

pirj avatar Apr 30 '19 16:04 pirj