expect_correction with loop

[bquorning]

In some places, we spec what happens after one auto-correct iteration, and in some places we spec what happens after many auto-correct iterations.

As mentioned in #8003, there is no longer any reason to use helpers like autocorrect_source_with_loop. This PR removes that method and all our uses of it.

---

Before submitting the PR make sure the following are checked:

• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

[bquorning]

This bit is ugly! Is there another way to accomplish the same thing?

[marcandre]

It also won't work after #7868 lands...

[bquorning]

@pirj Maybe you want to take a look at this PR?

[marcandre]

API-wise, I like that it's very convenient; I wonder if it would it better to also spec the intermediate steps?

E.g.

expect_offense(<<~RUBY)
  # ... code with offense
RUBY
expect_correction_offense(<<~RUBY)
  # ... corrected code with offense
RUBY
expect_correction(<<~RUBY)
  # ... final corrected code without offense
RUBY

One advantage I see is that it makes it clear how the final correction is had. Also it specs what happens if someone entered the intermediate code for some reason. Maybe that's too strict or verbose though?

[pirj]

Don't see any flaws 👍

[pirj]

What are the implications of always using loop: true?

[pirj]

rspec execution time didn't change with loop: true (I've actually removed this parameter and removed loop: true around the code.

I've faced an RuboCop::Runner::InfiniteCorrectionLoop and some other spec failures. Is this due to disagreeing cops?

[marcandre]

I still feel this raises questions:

• should we be more specific with spec'ing the intermediate steps?
• or should we spec that a step leads to the expected correction (as @pirj suggests)?
• or should we not always test only the final step?
• why are there infinite loop corrections in our test base?

[bbatsov]

I think it's best to discuss them in a dedicated ticket. This PR improves the status quo, which is why I decided to merge it, but I agree that there's plenty of room for improvement.

[marcandre]

This PR improves the status quo

No doubt about that :-)

I opened #8075 and #8076