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

Fix infinite correction loop in RSpec/ExampleWording #1438

Merged
merged 1 commit into from Oct 24, 2022

Conversation

pirj
Copy link
Member

@pirj pirj commented Oct 24, 2022

fixes #1433

Since we don't have any corrections for an insufficient example wording like "works", we keep "replacing" the description with itself.

NOTE: expect_no_corrections was passing. But RuboCop was (falsely?) detecting that a change has been made, and was re-running the cop over and over.


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).

@pirj pirj self-assigned this Oct 24, 2022
Copy link
Member

@ydah ydah left a comment

Choose a reason for hiding this comment

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

Should we add a test for explicitly being expect_no_corrections?
Also, it seems like it would be a better to mention in the documentation the cases that are not autocorrected. How about it?

else
check_and_handle_insufficient_examples(description_node)
elsif insufficient_docstring?(description_node)
add_offense(docstring(description_node),
Copy link
Member

Choose a reason for hiding this comment

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

How about adding next if text(node) == replacement_text(node) in the auto-correction?
RuboCop thinks correction was made because the autocorrect didn't return nil, even though the correction resulted in replacing with the same value

Copy link
Member Author

Choose a reason for hiding this comment

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

It feels a bit hacky, for two reasons.
For "should do something" we know we have a correction, but we'll be making this check that will also return false.
For "works" we know we don't have a correction, so the check will be true.
I'm shortcutting (in a good way) here to skip autocorrection for "works", along with this check.

@pirj
Copy link
Member Author

pirj commented Oct 24, 2022

Should we add a test for explicitly being expect_no_corrections?

@ydah This expectation was passing before the fix - no surprise, it was replacing "works" with "works". But expect_no_corrections, apparently doesn't check if changes were made, but if the source before and after are identical.

I deliberately decided not to add this expectation because it's deceiving, at least not in the scope of this PR.

Don't trust a spec you didn't see fail™

@ydah
Copy link
Member

ydah commented Oct 24, 2022

Don't trust a spec you didn't see fail™

Indeed it is. Thank you.

@pirj pirj merged commit 5b1a370 into master Oct 24, 2022
@pirj pirj deleted the fix-infinite-correction-loop branch October 24, 2022 21:18
bquorning pushed a commit that referenced this pull request Oct 25, 2022
Fix infinite correction loop in RSpec/ExampleWording
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.

Infinite loop caused by RSpec/ExampleWording
3 participants