Only load example group modules when they're used #2742
Conversation
Currently the example group modules for all spec types are always loaded, regardless of which type of spec is being run. Some of those modules force parts of the user's application to load; for example, RSpec::Rails::HelperExampleGroup includes ActionView::TestCase::Behavior and loading ActionView::TestCase loads all of the application's helpers. By autoloading these modules and including them lazily the first time the corresponding spec type is defined, we can avoid loading parts of the user's application unnecessarily. Co-authored-by: Iliana Hadzhiatanasova <[email protected]>
| config.include RSpec::Rails::RoutingExampleGroup, type: :routing | ||
| config.include RSpec::Rails::ViewExampleGroup, type: :view | ||
| config.include RSpec::Rails::FeatureExampleGroup, type: :feature | ||
| config.define_derived_metadata(type: :controller) do |
There was a problem hiding this comment.
I originally wanted to use when_first_matching_example_defined here, but that hook doesn't fire until the first call to it, and some methods from the example group module could be called before then (e.g. render_views).
If this PR is accepted, I propose adding a new when_first_matching_example_group_defined hook to rspec-core and using it here instead.
| end | ||
|
|
||
| config.define_derived_metadata(type: :routing) do | ||
| require "action_controller/test_case" |
There was a problem hiding this comment.
This is necessary because RSpec::Rails::RoutingExampleGroup depends on ActionDispatch::Assertions::RoutingAssertions, which references ActionController::TestRequest without requiring the file where it's defined:
https://github.com/rails/rails/blob/v7.1.3.2/actionpack/lib/action_dispatch/testing/assertions/routing.rb#L242
I've fixed that upstream in rails/rails@5307976, but for compatibility with all existing Rails versions we need to keep eager loading ActionController::TestCase here.
|
Anything we can do to load things only when needed gets a plus from me, however I'm slightly concerned the build is failing because our snippets are exposing the change in whats loaded by default (in the case of 3.2 its just minitest being missing) which makes me wonder if this is a breaking change from a users perspective but I'm willing to be convinced its not if its easy to fix the build. The suggested improvement to rspec-core is also fine, I think the helpers you are using were actually invented for rspec-rails ironically (for the define derived types options) |
Previously ActiveSupport::TestCase was always loaded, so minitest would be required here: https://github.com/rails/rails/blob/v7.1.3.2/activesupport/lib/active_support/test_case.rb#L3
|
Previously The most straightforward fix for that is to continue to load minitest by requiring it explicitly; I've pushed a commit to do that. |
| @@ -291,6 +291,7 @@ def in_inferring_type_from_location_environment | |||
| end | |||
|
|
|||
| it "metadata `type: :request` sets up request example groups" do | |||
| require "rspec/rails/example/request_example_group" | |||
There was a problem hiding this comment.
I was going to comment "This could cause order dependent failures, we typically do such requires in a sub process to avoid posioning the env" but its made me think prehaps we should be doing that all over now if we lazy load so that we don't accidentally depend on a module load order that we just happen to have in our specs 😧
Currently the example group modules for all spec types are always loaded, regardless of which type of spec is being run. Some of those modules force parts of the user's application to load; for example,
RSpec::Rails::HelperExampleGroupincludesActionView::TestCase::Behavior, and loadingActionView::TestCaseloads all of the application's helpers.By autoloading these modules and including them lazily the first time the corresponding spec type is defined, we can avoid loading parts of the user's application unnecessarily.
Co-authored by @ilianah 🍐