Skip to content

Commit 4384ed8

Browse files
graememorganError Prone Team
authored andcommitted
JUnit4TestNotRun: remove the heuristic requirement that the method under consideration contains some kind of assertion.
This is presumably an effort to reduce false positives, but it leads to a huge number of false negatives. PiperOrigin-RevId: 775710089
1 parent 0d2f69c commit 4384ed8

File tree

2 files changed

+11
-6
lines changed

2 files changed

+11
-6
lines changed

core/src/main/java/com/google/errorprone/bugpatterns/JUnit4TestNotRun.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737

3838
import com.google.common.collect.ImmutableSet;
3939
import com.google.errorprone.BugPattern;
40+
import com.google.errorprone.ErrorProneFlags;
4041
import com.google.errorprone.VisitorState;
4142
import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
4243
import com.google.errorprone.fixes.SuggestedFix;
@@ -119,8 +120,13 @@ private static boolean isParameterAnnotation(AnnotationTree annotation, VisitorS
119120

120121
private static final Matcher<Tree> NOT_STATIC = not(hasModifier(STATIC));
121122

123+
private final boolean removeAssertionRequirement;
124+
122125
@Inject
123-
JUnit4TestNotRun() {}
126+
JUnit4TestNotRun(ErrorProneFlags flags) {
127+
this.removeAssertionRequirement =
128+
flags.getBoolean("JUnit4TestNotRun:RemoveAssertionRequirement").orElse(true);
129+
}
124130

125131
@Override
126132
public Description matchClass(ClassTree tree, VisitorState state) {
@@ -212,7 +218,8 @@ private Optional<Description> handleMethod(MethodTree methodTree, VisitorState s
212218

213219
// Method non-static and contains call(s) to testing method, probably a test,
214220
// unless it is called elsewhere in the class, in which case it is a helper method.
215-
if (NOT_STATIC.matches(methodTree, state) && containsTestMethod(methodTree)) {
221+
if (NOT_STATIC.matches(methodTree, state)
222+
&& (removeAssertionRequirement || containsTestMethod(methodTree))) {
216223
return Optional.of(describeFixes(methodTree, state));
217224
}
218225
return Optional.empty();

core/src/test/java/com/google/errorprone/bugpatterns/JUnit4TestNotRunTest.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ public void shouldDoSomething() {
307307
}
308308

309309
@Test
310-
public void noTestKeyword_notATest() {
310+
public void noAssertions_isATestAnyway() {
311311
compilationHelper
312312
.addSourceLines(
313313
"Test.java",
@@ -318,6 +318,7 @@ public void noTestKeyword_notATest() {
318318
319319
@RunWith(JUnit4.class)
320320
public class Test {
321+
// BUG: Diagnostic contains:
321322
public void shouldDoSomething() {
322323
Collections.sort(Collections.<Integer>emptyList());
323324
}
@@ -728,9 +729,6 @@ public void negativeCase3() {
728729
*/
729730
@RunWith(JUnit4.class)
730731
public class JUnit4TestNotRunNegativeCase3 {
731-
// Doesn't begin with "test", and doesn't contain any assertion-like method invocations.
732-
public void thisIsATest() {}
733-
734732
// Isn't public.
735733
void testTest1() {}
736734

0 commit comments

Comments
 (0)