Skip to content

Comments

Disable RSpec monkeypatching#8563

Merged
zachmargolis merged 6 commits intomainfrom
margolis-rspec-disable-monkeypatching
Jun 9, 2023
Merged

Disable RSpec monkeypatching#8563
zachmargolis merged 6 commits intomainfrom
margolis-rspec-disable-monkeypatching

Conversation

@zachmargolis
Copy link
Contributor

@zachmargolis zachmargolis commented Jun 8, 2023

This is not urgent by any means, but it's something that was on my mind, so I figured I'd give converting our base a shot

  • There's a few files I moved some examples to be indented/scoped, so I'd recommend ?w=1 to hide whitespace changes on this PR

Methodology:

git grep -l -E "^shared_examples" -- '*.rb'  | xargs perl -p -i -e 's/^shared_examples/RSpec.shared_examples/g'
git grep -l -E "^describe" -- '*.rb'  | xargs perl -p -i -e 's/^describe/RSpec.describe/g'
git grep -l -E "^feature" -- '*.rb'  | xargs perl -p -i -e 's/^feature/RSpec.feature/g' --            

Syntax changes:
- top-level describe, shared_examples are now RSpec.describe, RSpec.shared_examples

changelog: Internal, Source code, Update RSpec syntax
@zachmargolis zachmargolis requested a review from a team June 8, 2023 22:10
it 'includes before_actions from IdvSession' do
expect(subject).to have_actions(:before, :redirect_if_sp_context_needed)
RSpec.describe Idv::PhoneErrorsController do
shared_examples_for 'an idv phone errors controller action' do
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is one of the files that got indented, recommend ?w=1 to see clearer version of changes here

@@ -13,6 +13,7 @@

RSpec.configure do |config|
# see more settings at spec/rails_helper.rb
config.disable_monkey_patching!
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aduth
Copy link
Contributor

aduth commented Jun 9, 2023

Can you describe the motivation? Is it that the global methods are bad? 🤔

@zachmargolis
Copy link
Contributor Author

Can you describe the motivation? Is it that the global methods are bad? 🤔

Yeah it's using an option to let RSpec be a "better-behaved" gem. Since this is in specs, not in prod, it's pretty minimal. It also made more of a difference with the old "should" style expectations, which relied a lot on monkeypatching (1.should eq 2) and the new "expect" expectations (expect(1).to eq(2)) already move away from a lot of those

@zachmargolis zachmargolis merged commit f4e84ac into main Jun 9, 2023
@zachmargolis zachmargolis deleted the margolis-rspec-disable-monkeypatching branch June 9, 2023 18:31
@jmhooper jmhooper mentioned this pull request Jun 13, 2023
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.

3 participants