Add RuboCop to unify `have_gitlab_http_status(:symbol)`
What does this MR do?
This MR adds a have_http_status
usages in specs.
It also discourages the usage of numeric HTTP status codes in
have_gitlab_http_status
.
See the proposal issue #196163 (closed)
Note that this MR adds this cop disabled. Future MRs will enable it per directory (e.g. spec
, ee/spec
) to prevent huge MRs.
See !17705 (3e7142fb) to see how the future change will look like.
Bad
expect(response).to have_http_status(200)
expect(response).to have_http_status(:ok)
expect(response).to have_gitlab_http_status(200)
Good
expect(response).to have_gitlab_http_status(:ok)
Screenshots
Examples
have_gitlab_http_status
and symbolic codes
Prefer Inspecting 1 file
C
Offenses:
spec/support/helpers/rack_attack_spec_helpers.rb:31:25: C: RSpec/HaveGitlabHttpStatus: Use have_gitlab_http_status instead of have_http_status. Prefer named HTTP status :too_many_requests over its numeric representation 429. https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#have_gitlab_http_status
expect(response).to have_http_status(429)
^^^^^^^^^^^^^^^^^^^^^
1 file inspected, 1 offense detected
Unknown HTTP status code
$ be rubocop --only RSpec/HaveGitlabHttpStatus spec/support/helpers/rack_attack_spec_helpers.rb
Inspecting 1 file
C
Offenses:
spec/support/helpers/rack_attack_spec_helpers.rb:31:25: C: RSpec/HaveGitlabHttpStatus: HTTP status 418 is unknown. Please provide a valid one or disable this cop. https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#have_gitlab_http_status
expect(response).to have_http_status(418)
^^^^^^^^^^^^^^^^^^^^^
1 file inspected, 1 offense detected
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation created/updated or follow-up review issue created -
Code review guidelines - [-] Merge request performance guidelines
-
Style guides - [-] Database guides
- [-] Separation of EE specific content
Performance and Testing
- [-] Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
- [-] Tested in all supported browsers
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
- [-] Label as security and @ mention
@gitlab-com/gl-security/appsec
- [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
- [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by Peter Leitzen