-
Notifications
You must be signed in to change notification settings - Fork 145
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
New CheckstyleTestMakeupCheck #610
Comments
If it can resolve problems in our code we are ok to have it, even it is very specific to our repo and nobody will use it expect for us. |
Check needs to be changed to understand tests with multiple configurations in 1 test. |
Check was merged. |
@romani Feel free to release sevntu whenever you can. |
Identified at checkstyle/regression-tool#85 (comment) ,
We need a new check to bring uniformity to our UTs in Checkstyle.
There are so many ways to write a test that some conflict with our custom check in regression to read these checks easily.
https://github.com/checkstyle/checkstyle/blob/master/src/test/java/com/puppycrawl/tools/checkstyle/checks/naming/AbstractClassNameCheckTest.java#L66
https://github.com/checkstyle/checkstyle/blob/master/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/DefaultComesLastCheckTest.java#L43
I am currently naming the check
CheckstyleTestMakeupCheck
and started a preview of the check at https://github.com/rnveach/checkstyle/commits/regression_util.We should make a check that only examines methods defined with
@Test
and have averify
method call.The method must have a local variable defining a
DefaultConfiguration
and assigning it with eithercreateModuleConfig(...)
or one of our other standard configuration creation methods.If it calls
addAttribute
on theDefaultConfiguration
the property name must be a string, and the property value must either be: a String, a concatentation of 2 or more strings, a enumerationtoString
in the formXXX.YYY.toString()
,getPath(String)
, or the literalnull
(which is technically not supported in the configuration file).@romani Please review and approve.
Let me know if you can think of anything else to verify.
The text was updated successfully, but these errors were encountered: