-
Notifications
You must be signed in to change notification settings - Fork 39
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
Introduce JUnitNullaryParameterizedTestDeclaration
check
#817
Conversation
...ib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitParameterizedMethodDeclaration.java
Outdated
Show resolved
Hide resolved
Looks good. All 4 mutations in this change were killed.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
c9fe3c1
to
9af2385
Compare
Looks good. All 4 mutations in this change were killed.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
9af2385
to
30e9220
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebased and added a commit; the check now also suggests a fix. Nice idea for a check @Venorcis!
Suggested commit message:
Introduce `JUnitNullaryParameterizedTestDeclaration` check (#817)
While there, slightly simplify the `AutowiredConstructor` check.
...ib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitParameterizedMethodDeclaration.java
Outdated
Show resolved
Hide resolved
...ib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitParameterizedMethodDeclaration.java
Outdated
Show resolved
Hide resolved
...ib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitParameterizedMethodDeclaration.java
Outdated
Show resolved
Hide resolved
Looks good. All 14 mutations in this change were killed.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
Thanks @Stephan202, all LGTM! 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes LGTM! Nice idea for a check 🚀 !
*/ | ||
@AutoService(BugChecker.class) | ||
@BugPattern( | ||
summary = "Nullary JUnit test methods need not be parameterized", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably a personal thingy, but I do not really like "need not be" phrase. Maybe we can change to "should not".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's you ;)
But will add a commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😄 haha nice. Will merge once 🟢 !
private static final long serialVersionUID = 1L; | ||
private static final MultiMatcher<MethodTree, AnnotationTree> IS_PARAMETERIZED_TEST = | ||
annotations(AT_LEAST_ONE, isType("org.junit.jupiter.params.ParameterizedTest")); | ||
private static final Matcher<AnnotationTree> IS_ARGUMENT_SOURCE = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes me think of the MoreJUnitMatchers#HAS_METHOD_SOURCE
(link) but this usage is slightly different is fine as-is and we don't need to consider any changes right now. Anyway, wanted to highlight it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, fine to move such things only when a second use case comes up 👍
30e9220
to
71bd150
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebased and added a commit. @rickie feel free to merge once built.
private static final long serialVersionUID = 1L; | ||
private static final MultiMatcher<MethodTree, AnnotationTree> IS_PARAMETERIZED_TEST = | ||
annotations(AT_LEAST_ONE, isType("org.junit.jupiter.params.ParameterizedTest")); | ||
private static final Matcher<AnnotationTree> IS_ARGUMENT_SOURCE = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, fine to move such things only when a second use case comes up 👍
*/ | ||
@AutoService(BugChecker.class) | ||
@BugPattern( | ||
summary = "Nullary JUnit test methods need not be parameterized", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's you ;)
But will add a commit.
Looks good. All 14 mutations in this change were killed.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
71bd150
to
29d70a7
Compare
Kudos, SonarCloud Quality Gate passed! |
Looks good. All 14 mutations in this change were killed.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
JUnitParameterizedMethodDeclaration
bug checkerJUnitNullaryParameterizedTestDeclaration
check
Flag uses of
@ParameterizedTest
on methods that don't have any parameters.