rubocop-rspec
rubocop-rspec copied to clipboard
Update RSpec/Rails/HttpStatus to support be_* syntax
Overview
When writing Rails request specs, it'd be handy to auto-correct/enforce the "be_" syntax since it's shorter and easier to read instead of the existing have_http_status
syntax.
Desired Behavior
I'd like to propose we update this cop (or make a new one if that makes sense) so we can promote the following:
# No
it "answers unauthenticated status" do
get api_users_path
expect(response).to have_http_status(:unauthorized)
end
# Yes
it "answers unauthenticated status" do
get api_users_path
expect(response).to be_unauthorized
end
Environment
1.13.0 (using Parser 3.0.1.1, rubocop-ast 1.5.0, running on ruby 3.0.1 arm64-darwin20)
- rubocop-performance 1.11.1
- rubocop-rake 0.5.1
- rubocop-rspec 2.3.0
Sounds reasonable. Would you like to hack on it?
Yeah, it's been a while since I've had to write one of these but probably time I get back into this space. Yeah, might take me a while to get ramped up -- on top of everything else I'm doing -- but will see what I can do. :wink:
Would it be better to add another EnforcedStyle
- be_status
- and maybe make it a default? This will allow users to use old style if they wish to.
It seems that be_*
matchers work with predicate methods however ActionDispatch::TestResponse
doesn't have predicates for all the statuses, but only for most common ones.
- expect(response).to have_http_status(200)
- expect(response).to have_http_status(:ok)
+ expect(response).to be_ok
Research:
$ rg --no-ignore --hidden --count-matches 'have_http_status.200' | wc -l
126 # files, some file have more than one match
$ rg --no-ignore --hidden --count-matches 'have_http_status.:ok' | wc -l
91 # files, some file have more than one match
$ rg --no-ignore --hidden --count-matches 'be_ok' | wc -l
33 # files, some file have more than one match. Just a few are unrelated to responses
$ rg --no-ignore --hidden --count-matches 'be_successful' | wc -l
365 # files, some file have more than one match. All are related to responses
I'd say it could become the default style right away with such a magnitude of adoption.
Mapping in Rack. More - needs to be checked if additional ones work fine with predicate matchers.
I'm uncertain if it should be a new cop or a style for HttpStatus
/HaveHttpStatus
.
It feels that the cop could be strict or relaxed, e.g. suggest be_ok
for 200 or be_successful
for 200/201/202/204.
Should it enforce be_redirect
for all 3xx codes? Or only in the relaxed mode?