From 33c737718c0ff7a02739e8c7c2a1e026d17d0ec0 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 3 Oct 2024 17:01:24 +0200 Subject: [PATCH 1/3] [java] UnitTestShouldUseBeforeAnnotation: Consider JUnit 5 and TestNG --- docs/pages/release_notes.md | 3 + .../resources/category/java/bestpractices.xml | 28 ++- .../xml/UnitTestShouldUseBeforeAnnotation.xml | 213 +++++++++++------- 3 files changed, 159 insertions(+), 85 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 0068c0480d..645aa4f485 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -16,6 +16,9 @@ This is a {{ site.pmd.release_type }} release. ### 🌟 Rule Changes +#### Changed Rules +* {% rule java/bestpractices/UnitTestShouldUseBeforeAnnotation %} (Java Best Practices) now also considers JUnit 5 and TestNG tests. + #### Renamed Rules Several rules for unit testing have been renamed to better reflect their actual scope. Lots of them were called after JUnit / JUnit 4, even when they applied to JUnit 5 and / or TestNG. diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 2698fbab73..6786399cea 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -1342,16 +1342,20 @@ public class MyTest2 { - This rule detects methods called `setUp()` that are not properly annotated as a setup method. - This is primarily intended to assist in upgrading from JUnit 3, where setup methods were required to be called `setUp()`. - To a lesser extent, this may help detect omissions even under newer JUnit versions, as long as you are following this convention to name the methods. +This rule detects methods called `setUp()` that are not properly annotated as a setup method. +This is primarily intended to assist in upgrading from JUnit 3, where setup methods were required to be called `setUp()`. +To a lesser extent, this may help detect omissions even under newer JUnit versions or under TestNG, +as long as you are following this convention to name the methods. - JUnit 4 will only execute methods annotated with `@Before` before all tests. - JUnit 5 introduced `@BeforeEach` and `@BeforeAll` annotations to execute methods before each test or before all tests in the class, respectively. +* JUnit 4 will only execute methods annotated with `@Before` before all tests. +* JUnit 5 introduced `@BeforeEach` and `@BeforeAll` annotations to execute methods before each test or before all + tests in the class, respectively. +* TestNG provides the annotations `@BeforeMethod` and `@BeforeClass` to execute methods before each test or before + tests in the class, respectively. 3 @@ -1363,9 +1367,15 @@ public class MyTest2 { pmd-java:typeIs('org.junit.Before') or pmd-java:typeIs('org.junit.jupiter.api.BeforeEach') or pmd-java:typeIs('org.junit.jupiter.api.BeforeAll') - or pmd-java:typeIs('org.testng.annotations.BeforeMethod')])] - (: Make sure this is a junit 4 class :) - [../MethodDeclaration[pmd-java:hasAnnotation('org.junit.Test')]] + or pmd-java:typeIs('org.testng.annotations.BeforeMethod') + or pmd-java:typeIs('org.testng.annotations.BeforeClass') + ])] + (: Make sure this is a JUnit 4/5 or TestNG class :) + [../MethodDeclaration[ + pmd-java:hasAnnotation('org.junit.Test') + or pmd-java:hasAnnotation('org.junit.jupiter.api.Test') + or pmd-java:hasAnnotation('org.testng.annotations.Test') + ]] ]]> diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnitTestShouldUseBeforeAnnotation.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnitTestShouldUseBeforeAnnotation.xml index 10d6fbc524..d9ae22cfd4 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnitTestShouldUseBeforeAnnotation.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnitTestShouldUseBeforeAnnotation.xml @@ -5,8 +5,9 @@ xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd"> - Contains setUp + JUnit4 Test class contains setUp 1 + 3 - Contains @setUp + JUnit4 Test class with Before is ok 0 - Renamed setup + JUnit4 Test class with renamed setup using Before is ok 0 - #1446 False positive with JUnit4TestShouldUseBeforeAnnotation when TestNG is used + Contains setUp, not a JUnit 4/5/TestNG test 0 + + + + JUnit4 Test class contains setUp with different signature is ok + 0 + - - - - #940 False positive with JUnit4TestShouldUseBeforeAnnotation when JUnit5's 'BeforeEach' is used - 0 - - - - - #940 False positive with JUnit4TestShouldUseBeforeAnnotation when JUnit5's 'BeforeAll' is used - 0 - - - - Contains setUp, not a junit 4 test - 0 - - - - Contains setUp with different signature - 0 - +]]> @@ -173,4 +118,120 @@ public class AReallyCoolFeatureTest extends BaseTest { } ]]> + + + TestNG class contains setUp + 1 + 4 + + + + + TestNG class contains setUp with different signature is ok (#1446) + 0 + + + + + TestNG with @BeforeMethod is ok (#1446) + 0 + + + + + TestNG with @BeforeClass is ok + 0 + + + + + JUnit5 Test class contains setUp + 1 + 4 + + + + + JUnit5 Test class with BeforeEach is ok (#940) + 0 + + + + + JUnit5 Test class with BeforeAll is ok (#940) + 0 + + From 9337e5a7a21428f92d270c9309b141754e8b7f39 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 3 Oct 2024 17:18:04 +0200 Subject: [PATCH 2/3] [java] UnitTestShouldUseAfterAnnotation: Consider JUnit 5 and TestNG --- docs/pages/release_notes.md | 1 + .../resources/category/java/bestpractices.xml | 32 ++- .../xml/UnitTestShouldUseAfterAnnotation.xml | 227 +++++++++++------- 3 files changed, 163 insertions(+), 97 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 645aa4f485..d5d1238fcc 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -17,6 +17,7 @@ This is a {{ site.pmd.release_type }} release. ### 🌟 Rule Changes #### Changed Rules +* {% rule java/bestpractices/UnitTestShouldUseAfterAnnotation %} (Java Best Practices) now also considers JUnit 5 and TestNG tests. * {% rule java/bestpractices/UnitTestShouldUseBeforeAnnotation %} (Java Best Practices) now also considers JUnit 5 and TestNG tests. #### Renamed Rules diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 6786399cea..cd51508131 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -1295,36 +1295,46 @@ public class MyTestCase { - This rule detects methods called `tearDown()` that are not properly annotated as a cleanup method. - This is primarily intended to assist in upgrading from JUnit 3, where tear down methods were required to be called `tearDown()`. - To a lesser extent, this may help detect omissions under newer JUnit versions, as long as you are following this convention to name the methods. +This rule detects methods called `tearDown()` that are not properly annotated as a cleanup method. +This is primarily intended to assist in upgrading from JUnit 3, where tear down methods were required to be called `tearDown()`. +To a lesser extent, this may help detect omissions even under newer JUnit versions or under TestNG, +as long as you are following this convention to name the methods. - JUnit 4 will only execute methods annotated with `@After` after running each test. - JUnit 5 introduced `@AfterEach` and `@AfterAll` annotations to execute methods after each test or after all tests in the class, respectively. +* JUnit 4 will only execute methods annotated with `@After` after running each test. +* JUnit 5 introduced `@AfterEach` and `@AfterAll` annotations to execute methods after each test or after + all tests in the class, respectively. +* TestNG provides the annotations `@AfterMethod` and `@AfterClass` to execute methods after each test or after + tests in the class, respectively. 3 - - - Contains tearDown + JUnit4 test class contains tearDown 1 + 3 - Contains @After tearDown + JUnit4 test class contains tearDown with different signature is ok + 0 + + + + + JUnit4 test class contains @After tearDown is ok 0 - Renamed tearDown + JUnit4 test class contains renamed tearDown is ok 0 - #1446 False positive with JUnit4TestShouldUseBeforeAnnotation when TestNG is used - 0 - - - - - #940 False positive with JUnit4TestShouldUseAfterAnnotation when JUnit5's 'AfterEach' is used - 0 - - - - - #940 False positive with JUnit4TestShouldUseAfterAnnotation when JUnit5's 'AfterAll' is used - 0 - - - - Contains tearDown, not a junit 4 test + Contains tearDown, not a JUnit 4/5 or TestNG test is ok 0 - - - Contains tearDown with different signature - 0 - @@ -151,4 +91,119 @@ public class AReallyCoolFeatureTest extends BaseTest { } ]]> + + + TestNG test contains tearDown + 1 + 4 + + + + + TestNG test contains tearDown with different signature is ok (#1446) + 0 + + + + + TestNG test contains tearDown with @AfterMethod is ok + 0 + + + + + TestNG test contains tearDown with @AfterClass is ok + 0 + + + + + JUnit 5 test class contains tearDown + 1 + 4 + + + + + JUnit 5 test class contains tearDown with @AfterEach is ok (#940) + 0 + + + + + JUnit 5 test class contains tearDown with @AfterAll is ok (#940) + 0 + + From a0d4b38b5349d27bd4762080131e0362d2fe26da Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 18 Oct 2024 16:16:54 +0200 Subject: [PATCH 3/3] [doc] Update release notes (#5245) --- docs/pages/release_notes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 5c5329437e..088b0f9d06 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -55,6 +55,7 @@ The old rule names still work but are deprecated. * [#4965](https://github.com/pmd/pmd/pull/4965): Fix #4532: \[java] Rename JUnit rules with overly restrictive names - [Juan Martín Sotuyo Dodero](https://github.com/jsotuyod) (@jsotuyod) * [#5225](https://github.com/pmd/pmd/pull/5225): Fix #5067: \[java] CloseResource: False positive for FileSystems.getDefault() - [Lukas Gräf](https://github.com/lukasgraef) (@lukasgraef) * [#5241](https://github.com/pmd/pmd/pull/5241): Ignore javacc code in coverage report - [Juan Martín Sotuyo Dodero](https://github.com/jsotuyod) (@jsotuyod) +* [#5245](https://github.com/pmd/pmd/pull/5245): \[java] Improve UnitTestShouldUse{After,Before}Annotation rules to support JUnit5 and TestNG - [Andreas Dangel](https://github.com/adangel) (@adangel) * [#5258](https://github.com/pmd/pmd/pull/5258): Ignore generated antlr classes in coverage reports - [Juan Martín Sotuyo Dodero](https://github.com/jsotuyod) (@jsotuyod) * [#5264](https://github.com/pmd/pmd/pull/5264): Fix #5261: \[java] Fix NPE with empty pattern list - [Clément Fournier](https://github.com/oowekyala) (@oowekyala) * [#5269](https://github.com/pmd/pmd/pull/5269): Fix #5253: \[java] Support Boolean wrapper class for BooleanGetMethodName rule - [Aryant Tripathi](https://github.com/Aryant-Tripathi) (@Aryant-Tripathi)