-
-
Notifications
You must be signed in to change notification settings - Fork 278
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
Add configuration option AllowNested
to RSpec/MultipleExpectations
#1810
Conversation
expect_no_offenses(<<~RUBY) | ||
describe Foo do | ||
it 'uses expect with block twice' do | ||
expect { my_code }.to raise_error(MyErrorType) do |error| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would we count multiple nested expectations? As one or as multiple?
This really depends, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would happen if there will be two expectations inside the block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I understand. This is not an offense if there are multiple expectations in a block of the same block if it is completely nested, so instead, we need to count the number of expectations in the same block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. And this is getting interesting when the Max is above one.
So
ex is one
ex { ex } is one
ex { ex }
ex { ex }
is two
ex {
ex
ex
}
is two
etc
does this make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. Now that I look at it, I feel like adding this option makes it more confusing. Maybe AllowNested isn't such a good approach. I think I'll close this PR, but what do you think of @pirj ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. And the ticket as well - i see some comments expressing doubts from my past self
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and welcome back to your usual level of commitment! ❤️
6ea1d53
to
05dcaad
Compare
Fix: #379
Before submitting the PR make sure the following are checked:
master
(if not - rebase it).CHANGELOG.md
if the new code introduces user-observable changes.bundle exec rake
) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).If you have modified an existing cop's configuration options:
VersionChanged: "<<next>>"
inconfig/default.yml
.