rubocop / rubocop-minitest

Code style checking for Minitest files.
https://docs.rubocop.org/rubocop-minitest
MIT License
144 stars 44 forks source link

[Fix #301] Add new Minitest/Focus cop #302

Closed jaredmoody closed 9 months ago

jaredmoody commented 9 months ago

Checks for focused tests in the formats supported by minitest-focus.

I decided not to add an auto-corrector because IME it gets in the way when using editor auto-formatting. When focusing a test and then saving the file while working on the focused test, the focus gets removed, running everything.

However, I could add this if others disagree.

Hat tip to @NewAlexandria for the original PR.


Before submitting the PR make sure the following are checked:

NewAlexandria commented 9 months ago

yea, thanks for the PR. I just have not had time to come back to it and sort out the tests.

NewAlexandria commented 9 months ago

is there a good guide or doc on write an auto-correct method? I'm not sure it's a good idea in this case, but I'd like to know.

IMO it may not be a good things to autocorrect because many, including me, leave -A on as part of save and commits. That would make it hard to use focus the way it's intended to for local testing. I guess, i.e., I'd want to make the autocorrection of focus only happen for specific environments.

koic commented 9 months ago

The focus method that is not modifying the test method should not trigger warnings (e.g., calls to focus method alone). Can you add tests for this case and update the implementation accordingly?

koic commented 9 months ago

I decided not to add an auto-corrector because IME it gets in the way when using editor auto-formatting. When focusing a test and then saving the file while working on the focused test, the focus gets removed, running everything.

While it has not been released yet, the next release of RuboCop will enable control through --editor-mode. Cops set with AutoCorrect: contextual will not autocorrect when --editor-mode is used. https://github.com/rubocop/rubocop/pull/12682

jaredmoody commented 9 months ago

The focus method that is not modifying the test method should not trigger warnings (e.g., calls to focus method alone). Can you add tests for this case and update the implementation accordingly?

Per the README, minitest-focus allows focusing by directly - by placing the focus on the same line, or indirectly, on the previous line, which sets a "trap", focusing the next test. I added an additional "bad" example showing both focus possibilities.

While it has not been released yet, the next release of RuboCop will enable control through --editor-mode. Cops set with AutoCorrect: contextual will not autocorrect when --editor-mode is used.

This is cool! I think an autocorrect for this is lower value, since usually only a single test will be focused, but I'll take a stab at autocorrecting. Is there anything I need to do to set this cop to AutoCorrect: contextual?

jaredmoody commented 9 months ago

I've added auto-correction and tests are passing, but as I've never written one of these before, I don't know if I did it the best way, suggestions welcome.

koic commented 9 months ago

Per the README, minitest-focus allows focusing by directly - by placing the focus on the same line, or indirectly, on the previous line, which sets a "trap", focusing the next test. I added an additional "bad" example showing both focus possibilities.

Oops! I was mistaken 🙇 You're correct!

Is there anything I need to do to set this cop to AutoCorrect: contextual?

Since AutoCorrect: contextual has not been released yet, it's fine to keep the current behavior of autocorrection as is. It can be addressed later once supported.

koic commented 9 months ago

Thanks!