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 new Minitest/NonPublicTestMethod cop #229

Merged
merged 1 commit into from
Jan 22, 2023

Conversation

fatkodima
Copy link
Contributor

Closes #220.

Tested on rails/rails - no offenses there.
Does anyone know other large OSS projects with minitest tests in it?

@koic
Copy link
Member

koic commented Jan 18, 2023

First, it should be enough if it is confirmed with rails/rails repo 👍 Can you update this cop name to NonPublicTestMethod? Because "non public" has a naming track record.
https://github.com/rubocop/rubocop/blob/v1.43.0/lib/rubocop/cop/mixin/def_node.rb#L12

@@ -158,6 +158,11 @@ Minitest/GlobalExpectations:
VersionAdded: '0.7'
VersionChanged: '0.26'

Minitest/HiddenTestMethod:
Description: 'Detects hidden (marked as `private` or `protected`) test methods.'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Description: 'Detects hidden (marked as `private` or `protected`) test methods.'
Description: 'Detects non public (marked as `private` or `protected`) test methods.'

@@ -158,6 +158,11 @@ Minitest/GlobalExpectations:
VersionAdded: '0.7'
VersionChanged: '0.26'

Minitest/HiddenTestMethod:
Description: 'Detects hidden (marked as `private` or `protected`) test methods.'
Enabled: pending
Copy link
Member

Choose a reason for hiding this comment

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

I think it's probably Severity: warning.

Suggested change
Enabled: pending
Enabled: pending
Severity: warning

module RuboCop
module Cop
module Minitest
# Detects hidden (marked as `private` or `protected`) test methods. Minitest runs only
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Detects hidden (marked as `private` or `protected`) test methods. Minitest runs only
# Detects non `public` (marked as `private` or `protected`) test methods. Minitest runs only

include MinitestExplorationHelpers
include DefNode

MSG = 'Hidden test method detected. Make it public for it to run.'
Copy link
Member

@koic koic Jan 18, 2023

Choose a reason for hiding this comment

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

Suggested change
MSG = 'Hidden test method detected. Make it public for it to run.'
MSG = 'Non `public` test method detected. Make it `public` for it to run.'

@fatkodima
Copy link
Contributor Author

Updated.

@fatkodima fatkodima changed the title Add new Minitest/HiddenTestMethod cop Add new Minitest/NonPublicTestMethod cop Jan 18, 2023
@andyw8
Copy link
Contributor

andyw8 commented Jan 18, 2023

I ran this on Shopify's main app. No errors, and it caught a good number of unintentionally private tests.

@fatkodima
Copy link
Contributor Author

Cool! Glad it works. Thank you for checking ❤️

@koic koic merged commit 94238e3 into rubocop:master Jan 22, 2023
@koic
Copy link
Member

koic commented Jan 22, 2023

Thanks!

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.

New cop: Minitest/PrivateTestMethod
3 participants