Skip to content

Commit

Permalink
Hardcode JUnit4TestNotRun:DoNotRequireRunWith=true.
Browse files Browse the repository at this point in the history
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=297796637
  • Loading branch information
graememorgan authored and cpovirk committed Feb 28, 2020
1 parent 9a3bc86 commit a0da207
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import static com.google.errorprone.matchers.JUnitMatchers.isJUnit4TestClass;
import static com.google.errorprone.matchers.JUnitMatchers.isJunit3TestCase;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.enclosingClass;
import static com.google.errorprone.matchers.Matchers.hasModifier;
import static com.google.errorprone.matchers.Matchers.methodHasParameters;
import static com.google.errorprone.matchers.Matchers.methodReturns;
Expand All @@ -34,7 +33,6 @@

import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.ProvidesFix;
import com.google.errorprone.ErrorProneFlags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
Expand Down Expand Up @@ -70,18 +68,6 @@ public class JUnit4TestNotRun extends BugChecker implements ClassTreeMatcher {
private static final String TEST_ANNOTATION = "@Test ";
private static final String IGNORE_ANNOTATION = "@Ignore ";

private static final Matcher<MethodTree> OLD_POSSIBLE_TEST_METHOD =
allOf(
hasModifier(PUBLIC),
methodReturns(VOID_TYPE),
methodHasParameters(),
not(JUnitMatchers::hasJUnitAnnotation),
// Use old logic to determine if this is a JUnit4 test class.
// Only classes with @RunWith, don't check for other @Test methods.
not(enclosingClass(hasModifier(Modifier.ABSTRACT))),
not(enclosingClass(JUnitMatchers.isTestCaseDescendant)),
enclosingClass(JUnitMatchers.hasJUnit4TestRunner));

private static final Matcher<MethodTree> POSSIBLE_TEST_METHOD =
allOf(
hasModifier(PUBLIC),
Expand All @@ -92,26 +78,18 @@ public class JUnit4TestNotRun extends BugChecker implements ClassTreeMatcher {
private static final Matcher<Tree> NOT_STATIC = not(hasModifier(STATIC));


private final boolean improvedHeuristic;

public JUnit4TestNotRun(ErrorProneFlags flags) {
improvedHeuristic = flags.getBoolean("JUnit4TestNotRun:DoNotRequireRunWith").orElse(true);
}

@Override
public Description matchClass(ClassTree tree, VisitorState state) {
if (improvedHeuristic && !isJUnit4TestClass.matches(tree, state)) {
if (!isJUnit4TestClass.matches(tree, state)) {
return NO_MATCH;
}
Matcher<MethodTree> matcher =
improvedHeuristic ? POSSIBLE_TEST_METHOD : OLD_POSSIBLE_TEST_METHOD;
Map<MethodSymbol, MethodTree> suspiciousMethods = new HashMap<>();
for (Tree member : tree.getMembers()) {
if (!(member instanceof MethodTree) || isSuppressed(member)) {
continue;
}
MethodTree methodTree = (MethodTree) member;
if (matcher.matches(methodTree, state) && !isSuppressed(tree)) {
if (POSSIBLE_TEST_METHOD.matches(methodTree, state) && !isSuppressed(tree)) {
suspiciousMethods.put(getSymbol(methodTree), methodTree);
}
}
Expand Down Expand Up @@ -142,19 +120,15 @@ public Void visitMethodInvocation(
* <li>The method is public, void, and has no parameters;
* <li>the method is not already annotated with {@code @Test}, {@code @Before}, {@code @After},
* {@code @BeforeClass}, or {@code @AfterClass};
* <li>the enclosing class appears to be intended to run in JUnit4, that is:
* <ol type="a">
* <li>it is non-abstract,
* <li>it does not extend JUnit3 {@code TestCase},
* <li>it has an {@code @RunWith} annotation or at least one other method annotated
* {@code @Test};
* </ol>
* <li>and, the method appears to be a test method, that is:
* <ol type="a">
* <li>or, the method body contains a method call with a name that contains "assert",
* "verify", "check", "fail", or "expect".
* </ol>
* </ol>
*
* Assumes that we have reason to believe we're in a test class (i.e. has a {@code RunWith}
* annotation; has other {@code @Test} methods, etc).
*/
private Optional<Description> handleMethod(MethodTree methodTree, VisitorState state) {
// Method appears to be a JUnit 3 test case (name prefixed with "test"), probably a test.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@

package com.google.errorprone.bugpatterns;

import com.google.common.collect.ImmutableList;
import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers;
import com.google.errorprone.CompilationTestHelper;
import com.google.errorprone.ErrorProneFlags;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand All @@ -33,8 +31,7 @@ public class JUnit4TestNotRunTest {
CompilationTestHelper.newInstance(JUnit4TestNotRun.class, getClass());

private final BugCheckerRefactoringTestHelper refactoringHelper =
BugCheckerRefactoringTestHelper.newInstance(
new JUnit4TestNotRun(ErrorProneFlags.empty()), getClass());
BugCheckerRefactoringTestHelper.newInstance(new JUnit4TestNotRun(), getClass());

@Test
public void testPositiveCase1() {
Expand Down Expand Up @@ -317,7 +314,6 @@ public void shouldNotDetectMethodsOnAbstractClass() {
"public abstract class Test {",
" public void testDoSomething() {}",
"}")
.setArgs(ImmutableList.of("-XepOpt:JUnit4TestNotRun:DoNotRequireRunWith=false"))
.doTest();
}

Expand Down Expand Up @@ -505,15 +501,13 @@ public void suppression() {
public void testNegativeCase1() {
compilationHelper
.addSourceFile("JUnit4TestNotRunNegativeCase1.java")
.setArgs(ImmutableList.of("-XepOpt:JUnit4TestNotRun:DoNotRequireRunWith=false"))
.doTest();
}

@Test
public void testNegativeCase2() {
compilationHelper
.addSourceFile("JUnit4TestNotRunNegativeCase2.java")
.setArgs(ImmutableList.of("-XepOpt:JUnit4TestNotRun:DoNotRequireRunWith=false"))
.doTest();
}

Expand All @@ -526,7 +520,6 @@ public void testNegativeCase3() {
public void testNegativeCase4() {
compilationHelper
.addSourceFile("JUnit4TestNotRunNegativeCase4.java")
.setArgs(ImmutableList.of("-XepOpt:JUnit4TestNotRun:DoNotRequireRunWith=false"))
.doTest();
}

Expand Down

0 comments on commit a0da207

Please sign in to comment.