Skip to content
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 support be_status style for RSpec/Rails/HttpStatus #1527

Merged
merged 1 commit into from
Apr 18, 2023

Conversation

ydah
Copy link
Member

@ydah ydah commented Dec 23, 2022

Fix: #1148


Before submitting the PR make sure the following are checked:

  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Updated documentation.
  • Added an entry to the CHANGELOG.md if the new code introduces user-observable changes.
  • The build (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:

  • Set VersionChanged: "<<next>>" in config/default.yml.

@ydah ydah force-pushed the feature/1148 branch 4 times, most recently from 94e5ddc to 684e42b Compare January 13, 2023 13:32
@ydah ydah force-pushed the feature/1148 branch 2 times, most recently from d05c7cb to 2c5a1c0 Compare January 20, 2023 06:20
@pirj
Copy link
Member

pirj commented Jan 21, 2023

I'd hold this off until rubocop-rspec-rails extraction.

Copy link
Member

@pirj pirj left a comment

Choose a reason for hiding this comment

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

Wondering what are usage stats for various styles in real-world-rspec repos.

@@ -41,12 +54,13 @@ class HttpStatus < Base
PATTERN

def on_send(node)
Copy link
Member

Choose a reason for hiding this comment

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

It seems this cop has a bias towards the new style, as it inspects have_http_status calls, but doesn't care if be_* are used when configured to use numeric or symbolic styles.

It's not a big problem, since otherwise it would detect false positives in e.g. non-controller/request specs.

Maybe it makes sense to reflect this in our docs somehow.

@pirj
Copy link
Member

pirj commented Jan 24, 2023

EnforcedStyle: symbolic:

57497 files inspected, 413 offenses detected

EnforcedStyle: numeric:

57497 files inspected, 933 offenses detected

EnforcedStyle: be_status:

57497 files inspected, 1346 offenses detected

@ydah
Copy link
Member Author

ydah commented Jan 24, 2023

@pirj Sorry for the late reply. And thanks for checking usage stats for various styles in real-world-rspec repos😀

Comment on lines 11 to 13
# This cop inspects only `have_http_status` calls.
# So, this cop do not check if a method starting with `be_*` is used
# when setting for `EnforcedStyle: symbolic` or `EnforcedStyle: numeric`.
Copy link
Member Author

Choose a reason for hiding this comment

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

lib/rubocop/cop/rspec/rails/http_status.rb Outdated Show resolved Hide resolved
@ydah ydah force-pushed the feature/1148 branch 2 times, most recently from 6315e13 to fb347a3 Compare April 18, 2023 06:20
@bquorning bquorning merged commit 0610b73 into rubocop:master Apr 18, 2023
23 checks passed
@ydah ydah deleted the feature/1148 branch April 18, 2023 06:22
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.

Update RSpec/Rails/HttpStatus to support be_* syntax
3 participants