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

Add NoAssertions cop #124

Merged
merged 1 commit into from
Apr 20, 2021
Merged

Add NoAssertions cop #124

merged 1 commit into from
Apr 20, 2021

Conversation

ghiculescu
Copy link
Contributor

@ghiculescu ghiculescu commented Mar 15, 2021

Fixes #123

@ghiculescu ghiculescu force-pushed the AnyAssetions-cop branch 3 times, most recently from 5ff630c to 3371f33 Compare March 15, 2021 19:12
@ghiculescu ghiculescu marked this pull request as ready for review March 15, 2021 19:12
config/default.yml Outdated Show resolved Hide resolved
@andyw8
Copy link
Contributor

andyw8 commented Mar 15, 2021

I posted a question in the issue that I think still needs to be addressed:

Another thing to keep in mind is that the assertion may happen in a method called by the test, in which case it wouldn't be possible to catch through static analysis.

@ghiculescu
Copy link
Contributor Author

I posted a question in the issue that I think still needs to be addressed:

Another thing to keep in mind is that the assertion may happen in a method called by the test, in which case it wouldn't be possible to catch through static analysis.

I don't have any good ideas for this, sorry for not addressing it directly. The MultipleAssertions cop would have the same problem.

I guess this is where rubocop:disable comes in. It doesn't feel like great practice to me to have no assertions inside your test.

config/default.yml Outdated Show resolved Hide resolved
config/default.yml Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@ghiculescu ghiculescu force-pushed the AnyAssetions-cop branch 2 times, most recently from 5aad9f0 to 2099d41 Compare April 19, 2021 15:23
@ghiculescu
Copy link
Contributor Author

Thanks @koic

I made all those changes.

CHANGELOG.md Show resolved Hide resolved
@ghiculescu ghiculescu changed the title Add AnyAssertions cop Add NoAssertionMethods cop Apr 19, 2021
@ghiculescu ghiculescu changed the title Add NoAssertionMethods cop Add NoAssertions cop Apr 20, 2021
@koic koic merged commit 21d519e into rubocop:master Apr 20, 2021
@koic
Copy link
Member

koic commented Apr 20, 2021

Great! Thanks @ghiculescu!

@ghiculescu ghiculescu deleted the AnyAssetions-cop branch April 20, 2021 15:17
ghiculescu added a commit to ghiculescu/rubocop-minitest that referenced this pull request Apr 20, 2021
Should have tested rubocop#124 more thoroughly
ghiculescu added a commit to ghiculescu/rubocop-minitest that referenced this pull request Apr 20, 2021
Should have tested rubocop#124 more thoroughly
ghiculescu added a commit to ghiculescu/rubocop-minitest that referenced this pull request Apr 21, 2021
Should have tested rubocop#124 more thoroughly
@andyw8
Copy link
Contributor

andyw8 commented Feb 12, 2024

Here's another approach to catching tests without assertions:

https://railsatscale.com/2024-01-25-catching-assertionless-tests

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.

Could we make MultipleAssertions support minimums too?
3 participants