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

Prevent unrelated exceptions by requiring rails_helper at .rspec #2579

Closed

Conversation

aeroastro
Copy link

When there is a bug in the code on the load,
RSpec catches the exception, aborts the spec file,
proceeds to the following spec file, and retries require
with partially loaded Rails application.

In some cases, we see many identical errors,
such as NameError, which are noisy but help developers
find the bug. In other cases, we see many unrelated errors,
typically FrozenError on finalized objects,
which push out the original exception raised only from the first spec.

The following comment describes this issue in detail.
thoughtbot/factory_bot_rails#303 (comment)

This patch prevents unrelated exceptions by the following rspec-core feature.
The reason why I edited the .rspec is that this feature is only applicable to
--require option.
rspec/rspec-core#2568

This issue is a relatively well-known issue, and the well-known
workaround is to use --fail-fast when we see many errors.
https://stackoverflow.com/questions/47320772/rails-runtimeerror-cant-modify-frozen-array-when-running-rspec-in-rails

By introducing this patch, we no longer need the workaround.
All we need to do is run bundle exec rspec as usual.

When there is a bug in the code on the load,
RSpec catches the exception, aborts the spec file,
proceeds to the following spec file, and retries `require`
with partially loaded Rails application.

In some cases, we see many identical errors,
such as `NameError`, which are noisy but help developers
find the bug. In other cases, we see many unrelated errors,
typically `FrozenError` on finalized objects,
which push out the original exception raised only from the first spec.

The following comment describes this issue in detail.
thoughtbot/factory_bot_rails#303 (comment)

This patch prevents unrelated exceptions by the following rspec-core feature.
rspec/rspec-core#2568
@JonRowe JonRowe closed this Mar 9, 2022
@JonRowe
Copy link
Member

JonRowe commented Mar 9, 2022

👋 Thanks for taking the time to suggest this, the whole purpose of splitting the spec_helper.rb into spec_helper.rb for plain ruby and rails_helper.rb is to allow not loading Rails when not necessary. This change would completely remove the point of that split so its not something we would want to change.

Individual projects are free to ignore the recommendaton to have two helpers and load just the rails one, but thats not our default.

@aeroastro aeroastro deleted the feature/fail-fast-for-load-error branch March 9, 2022 14:57
@aeroastro
Copy link
Author

Thank you for your prompt reply and the detailed description.
I understood the detailed reason behind the separation of spec_helper.rb and rails_helper.rb.

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.

None yet

2 participants