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
'RSpec/PredicateMatcher' explicit style conflicts with Rails 'have_http_status' matcher #806
Comments
@chitsaou I believe I don't see a reason not to include - expect(response).to have_http_status(:success)
+ expect(response.status).to be(:success) Which approach would you prefer? |
We should double check for more common It might possibly make sense to add a user-customizable ignore list as well for custom matchers. Also, I added a note to #721 that we should mark this cop as unsafe. |
Also those:
I don't see any common pattern here, probably they should all be excluded. |
Thanks for all of your replies. It's true that there might be other
Wouldn't it be great if we can configure ignore list for this cop? |
@chitsaou #838 has just been released in 1.37.0 allowing you to configure RSpec/PredicateMatcher:
AllowedExplicitMatchers:
- have_http_status
- have_been_made Do you think this is sufficient? On a side note, it would be incredible to add such configuration to corresponding projects (e.g. |
I don't think it's currently possible. For the time being, we will need to pull the list of matchers from popular gems. |
Stumbled across
If you guys are interested, we may add something like: inherit_mode:
merge:
- RSpec/PredicateMatcher/AllowedExplicitMatchers add your settings to a third-party gem and see if it's being pulled. Would you like to hack on it @chitsaou ? |
Background
Our engineering team prefer
expect(object.good?).to be(true)
overexpect(object).to be_good
so that it's easier to search for"good?"
method name across the whole code base.Therefore, we are using the following settings for
RSpec/PredicateMatcher
:However, this cop also finds the rspec-rails
have_http_status
matcher should be changed:Instead of:
The cop would suggest:
Which results in method not found of
Response#has_http_status?
.Questions
Is this the expected behavior? If so, what settings should I use to allow
have_http_status
?Current Workaround
Our current workaround is to use a dirty hack to add
have_http_status
to built-in matchers list?:The text was updated successfully, but these errors were encountered: