diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2187.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2187.json index 2d78c41fc28..3e0c2f6a447 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2187.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2187.json @@ -2,5 +2,5 @@ "ruleKey": "S2187", "hasTruePositives": true, "falseNegatives": 13, - "falsePositives": 0 + "falsePositives": 1 } diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/NoTestInTestClassCheckSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/NoTestInTestClassCheckSample.java index 70a248bc6e8..463faebea20 100644 --- a/java-checks-test-sources/default/src/main/files/non-compiling/checks/NoTestInTestClassCheckSample.java +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/NoTestInTestClassCheckSample.java @@ -121,6 +121,33 @@ public class TestNGClassTest { // Noncompliant private void test1() { } public static void foo() {} } + +@org.testng.annotations.Test +class TestNGClassTestWithMethodAnnotated { // compliant, with testng when the class is annotated with @Test all public methods are considered as tests + // non public methods can also be added to tests with the @Test annotation + @org.testng.annotations.Test + void myMethod(){ + + } +} + +@org.testng.annotations.Test +class TestNGClassWithUnkownAnnotation { + @Unkown + void myMethod(){ + + } +} + +@org.testng.annotations.Test +class TestNGClassWithWrongAnnotation { // Noncompliant + @Override + void myMethod(){ + + } +} + + @org.testng.annotations.Test(groups ="integration") public abstract class AbstractIntegrationTest2{ } diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckTestNgTest.java b/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckTestNgTest.java new file mode 100644 index 00000000000..5b5fd8d0f84 --- /dev/null +++ b/java-checks-test-sources/default/src/test/java/checks/tests/NoTestInTestClassCheckTestNgTest.java @@ -0,0 +1,38 @@ +package checks.tests; + +import org.testng.annotations.Test; + +class TestNGTest { + @Test + void foo() { + } +} + +@Test +class TestNgFooTest { + public void test1() { + } + + public void test2() { + } +} + +@Test +class TestNGClassTest { // Noncompliant + public int field; + private void test1() { } + public static void foo() {} +} + +@Test +class TestNGClassTestUseAnnotation { + + @Test + void myMethod(){ + + } +} + +@Test(groups ="integration") +abstract class AbstractIntegrationTest2{ +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java b/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java index 551d4c07c83..b91e3942563 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/tests/NoTestInTestClassCheck.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.function.Predicate; import java.util.regex.Pattern; import java.util.stream.Stream; import org.apache.commons.lang3.StringUtils; @@ -44,6 +45,7 @@ public class NoTestInTestClassCheck extends IssuableSubscriptionVisitor { public static final String ARCH_UNIT_RUNNER = "ArchUnitRunner"; public static final String ARCH_UNIT_ANALYZE_CLASSES = "com.tngtech.archunit.junit.AnalyzeClasses"; public static final String ARCH_UNIT_TEST = "com.tngtech.archunit.junit.ArchTest"; + private static final String TEST_NG_TEST = "org.testng.annotations.Test"; private static final List PACT_UNIT_TEST = Arrays.asList("au.com.dius.pact.provider.junit.State", "au.com.dius.pact.provider.junitsupport.State"); @@ -75,7 +77,7 @@ public void visitNode(Tree tree) { private void resetAnnotationCache() { Arrays.asList(testFieldAnnotations, testMethodAnnotations, seenAnnotations).forEach(Set::clear); - testMethodAnnotations.addAll(Arrays.asList("org.junit.Test", "org.testng.annotations.Test", "org.junit.jupiter.api.Test")); + testMethodAnnotations.addAll(Arrays.asList("org.junit.Test", TEST_NG_TEST, "org.junit.jupiter.api.Test")); } private void checkClass(ClassTree classTree) { @@ -93,7 +95,7 @@ private void checkClass(ClassTree classTree) { Symbol.TypeSymbol classSymbol = classTree.symbol(); Stream members = getAllMembers(classSymbol, checkRunWith(classSymbol, "Enclosed")); IdentifierTree simpleName = classTree.simpleName(); - if (classSymbol.metadata().isAnnotatedWith("org.testng.annotations.Test")) { + if (classSymbol.metadata().isAnnotatedWith(TEST_NG_TEST)) { checkTestNGmembers(simpleName, members); } else { boolean isJunit3TestClass = classSymbol.type().isSubtypeOf("junit.framework.TestCase"); @@ -124,7 +126,22 @@ private static boolean isArchUnitTestClass(Symbol.TypeSymbol classSymbol) { } private void checkTestNGmembers(IdentifierTree className, Stream members) { - if (members.noneMatch(member -> member.isMethodSymbol() && member.isPublic() && !member.isStatic() && !"".equals(member.name()))) { + Predicate isTestNgAnnotation = ann -> { + Type type = ann.symbol().type(); + return type.isUnknown() || type.is(TEST_NG_TEST); + }; + Predicate isTestMethod = member -> { + if(!member.isMethodSymbol()){ + return false; + } + + // we know member is a method. + boolean annotatedWithTest = member.metadata().annotations().stream().anyMatch(isTestNgAnnotation); + boolean publicMethod = member.isPublic() && !member.isStatic() && !"".equals(member.name()); + return annotatedWithTest || publicMethod; + }; + + if (members.noneMatch(isTestMethod)) { reportClass(className); } } diff --git a/java-checks/src/test/java/org/sonar/java/checks/tests/NoTestInTestClassCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/tests/NoTestInTestClassCheckTest.java index 9643da4c8f7..78f236f6a8b 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/tests/NoTestInTestClassCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/tests/NoTestInTestClassCheckTest.java @@ -85,4 +85,12 @@ void pactUnit() { .withCheck(new NoTestInTestClassCheck()) .verifyIssues(); } + + @Test + void testNg() { + CheckVerifier.newVerifier() + .onFile(testCodeSourcesPath("checks/tests/NoTestInTestClassCheckTestNgTest.java")) + .withCheck(new NoTestInTestClassCheck()) + .verifyIssues(); + } }