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 #292] ensure all kinds of assignments are correctly handled when counting assertions #293

Merged
merged 1 commit into from
Jan 6, 2024
Merged

[Fix #292] ensure all kinds of assignments are correctly handled when counting assertions #293

merged 1 commit into from
Jan 6, 2024

Conversation

G-Rath
Copy link
Contributor

@G-Rath G-Rath commented Jan 2, 2024

Fixes #292
Fixes #295

This ensures that Minitest/MultipleAssertions properly counts assertions when dealing with assignments - I expect (and hope) this is pretty rare in real-world code, but it is technically correct for the cop to be doing and the actual support isn't that complex.

I've only added a single test to ensure all the types of assignment work, and then a test using the standard assignment operator for each different kind of complex node (block, if, rescue, etc) since they all work the same except for mass assignment and it would be very tedious to write all those tests; they could be done through loops but I'm not sure its worth it vs the complexity.

Speaking of mass assignment, that is the fly in the ointment - I've added support for single method and array, and then had it just bail otherwise because I expect this to be the rarest of rare cases and worst case a follow-up PR can be done expanding its support; I know already that this doesn't cover assigning hashes (i.e. x, y => { "value" => assert("why...?") }).


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • 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.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

@G-Rath G-Rath requested a review from koic January 4, 2024 18:12
@koic koic merged commit 1cca1e0 into rubocop:master Jan 6, 2024
13 checks passed
@koic
Copy link
Member

koic commented Jan 6, 2024

Thanks!

@G-Rath G-Rath deleted the check-for-expression branch January 6, 2024 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants