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

Added duplicated tests run hint. #46

Merged
merged 1 commit into from
Apr 17, 2022

Conversation

ignacio-chiazzo
Copy link
Contributor

Minitest uses Ruby classes, if a Minitest class inherits from another class, it will also inherit its methods causing Minitest to run the parent's tests twice. In some cases, we want them to run them twice, but most of the time, we don't.

This warning is based on a Rubocop rule for duplicated tests. This blog post and this library contain more information.

Fixes #45

README.adoc Outdated Show resolved Hide resolved
README.adoc Outdated Show resolved Hide resolved
@ignacio-chiazzo ignacio-chiazzo requested a review from andyw8 April 17, 2022 01:43
README.adoc Outdated

[source,ruby]
----
# example
Copy link
Member

@koic koic Apr 17, 2022

Choose a reason for hiding this comment

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

Can you update it?

Suggested change
# example
# bad

Copy link
Contributor Author

@ignacio-chiazzo ignacio-chiazzo Apr 17, 2022

Choose a reason for hiding this comment

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

I prefer not to say bad since it's still a valid but rare use case. thoughts? @koic @andyw8

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe
# bad (unless multiple runs are the intended behavior)

Copy link
Contributor

Choose a reason for hiding this comment

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

(I find the good/bad terminology that the guides use to be not ideal, since they don't really account for nuance or context).

README.adoc Outdated Show resolved Hide resolved
Copy link
Contributor

@andyw8 andyw8 left a comment

Choose a reason for hiding this comment

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

(approved pending @koic's suggestion)

@ignacio-chiazzo ignacio-chiazzo force-pushed the added-duplicated-tests branch 2 times, most recently from c0542fe to b40401e Compare April 17, 2022 16:16
@koic
Copy link
Member

koic commented Apr 17, 2022

Thanks. Can you squash your commits into one?

Minitest uses Ruby classes, if a Minitest class inherits from another class, it will also inherit its methods causing Minitest to run the parent's tests twice. In some cases, we want them to run them twice, but most of the time, we don't.

This commit contains an informative example
This blog post contains more info: https://ignaciochiazzo.medium.com/dont-run-your-ruby-minitest-classes-twice-988645662cdb.
@ignacio-chiazzo ignacio-chiazzo force-pushed the added-duplicated-tests branch from b40401e to df12a46 Compare April 17, 2022 16:26
@ignacio-chiazzo ignacio-chiazzo requested a review from koic April 17, 2022 16:27
@koic koic merged commit bcec2e2 into rubocop:master Apr 17, 2022
@koic
Copy link
Member

koic commented Apr 17, 2022

👍

koic added a commit to rubocop/rubocop-minitest that referenced this pull request Apr 18, 2022
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.

Add warning about inheriting from other tests
3 participants