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

False positive in Rails/DynamicFindBy #778

Closed
annaswims opened this issue Sep 15, 2022 · 0 comments · Fixed by #785
Closed

False positive in Rails/DynamicFindBy #778

annaswims opened this issue Sep 15, 2022 · 0 comments · Fixed by #785

Comments

@annaswims
Copy link

annaswims commented Sep 15, 2022

Expected behavior

no violation

Actual behavior

 C: [Correctable] Rails/DynamicFindBy: Use find_by instead of dynamic find_by_id.
      page.find_by_id("appraisal_appraisal").click

page is a Capybara::Session, so find_by_id so it's not related to the ActiveRecord stuff that rubocop is actually looking for

Steps to reproduce the problem

a Capybara test with
page.find_by_id("my_dom_id").click

RuboCop version

1.36.0 (using Parser 3.1.2.1, rubocop-ast 1.21.0, running on ruby 2.7.5) [arm64-darwin21]
  - rubocop-rails 2.15.2
  - rubocop-rspec 2.13.1
koic added a commit to koic/rubocop-rails that referenced this issue Sep 21, 2022
Fixes rubocop#778.

This PR fixes a false positive for `Rails/DynamicFindBy`
when using `page.find_by_id` as a Capybara testing API.
This default setting emphasizes suppressing false positive over false negative.
koic added a commit to koic/rubocop-rails that referenced this issue Sep 21, 2022
Fixes rubocop#778.

This PR fixes a false positive for `Rails/DynamicFindBy`
when using `page.find_by_id` as a Capybara testing API.
This default setting emphasizes suppressing false positive over false negative.
@koic koic closed this as completed in #785 Sep 30, 2022
@koic koic closed this as completed in 449162b Sep 30, 2022
koic added a commit that referenced this issue Sep 30, 2022
…c_find_by

[Fix #778] Fix a false positive for `Rails/DynamicFindBy`
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 a pull request may close this issue.

1 participant