Skip to content
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.

Sign up for GitHub

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

[java] Improve UnitTestShouldUse{After,Before}Annotation rules to support JUnit5 and TestNG #5245

Merged
merged 4 commits into from
Oct 24, 2024
Merged

[java] Improve UnitTestShouldUse{After,Before}Annotation rules to support JUnit5 and TestNG #5245

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
33c7377
[java] UnitTestShouldUseBeforeAnnotation: Consider JUnit 5 and TestNG
adangel Oct 3, 2024
9337e5a
[java] UnitTestShouldUseAfterAnnotation: Consider JUnit 5 and TestNG
adangel Oct 3, 2024
f317e8b
Merge branch 'main' into java-unit-test-rules
adangel Oct 18, 2024
a0d4b38
[doc] Update release notes (#5245)
adangel Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
[java] UnitTestShouldUseAfterAnnotation: Consider JUnit 5 and TestNG
  • Loading branch information
@adangel
adangel committed Oct 3, 2024
commit 9337e5a7a21428f92d270c9309b141754e8b7f39
1 change: 1 addition & 0 deletions docs/pages/release_notes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 21 additions & 11 deletions pmd-java/src/main/resources/category/java/bestpractices.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1295,36 +1295,46 @@ public class MyTestCase {
<rule name="UnitTestShouldUseAfterAnnotation"
language="java"
since="4.0"
message="JUnit 4 tests that clean up tests should use the @After annotation, JUnit5 tests should use @AfterEach or @AfterAll"
message="Apply the correct annotation if this method is used to clean up the tests"
class="net.sourceforge.pmd.lang.rule.xpath.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#unittestshoulduseafterannotation">
<description>
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.
</description>
<priority>3</priority>
<properties>
<property name="xpath">
<value>
<![CDATA[
<![CDATA[
//MethodDeclaration[@Name='tearDown' and @Arity=0]
[not(ModifierList/Annotation[
pmd-java:typeIs('org.junit.After')
or pmd-java:typeIs('org.junit.jupiter.api.AfterEach')
or pmd-java:typeIs('org.junit.jupiter.api.AfterAll')
or pmd-java:typeIs('org.testng.annotations.AfterMethod')])]
(: Make sure this is a junit 4 class :)
[../MethodDeclaration[pmd-java:hasAnnotation('org.junit.Test')]]
or pmd-java:typeIs('org.testng.annotations.AfterClass')
or pmd-java:typeIs('org.testng.annotations.AfterMethod')
])]
(: 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')
]]
]]>
</value>
</property>
</properties>
<example>
<![CDATA[
<![CDATA[
public class MyTest {
public void tearDown() {
bad();
Expand Down
197 changes: 126 additions & 71 deletions ...net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnitTestShouldUseAfterAnnotation.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,150 +5,205 @@
xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">

<test-code>
<description>Contains tearDown</description>
<description>JUnit4 test class contains tearDown</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>3</expected-linenumbers>
<code><![CDATA[
import org.junit.Test;
public class Foo {
public void tearDown() {
}
public void tearDown() {}
@Test
public void foo() {
}
public void foo() {}
}
]]></code>
</test-code>

<test-code>
<description>Contains @After tearDown</description>
<description>JUnit4 test class contains tearDown with different signature is ok</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import org.junit.Test;
public class Foo {
public void tearDown(int something) {}
@Test
public void foo() {}
}
]]></code>
</test-code>

<test-code>
<description>JUnit4 test class contains @After tearDown is ok</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import org.junit.Test;
import org.junit.After;
public class Foo {
@After
public void tearDown() {
}
public void tearDown() {}
@Test
public void foo() {
}
public void foo() {}
}
]]></code>
</test-code>

<test-code>
<description>Renamed tearDown</description>
<description>JUnit4 test class contains renamed tearDown is ok</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import org.junit.Test;
import org.junit.After;
public class Foo {
@After
public void clean() {
}
public void clean() {}
@Test
public void foo() {
public void foo() {}
}
]]></code>
</test-code>

<test-code>
<description>Contains tearDown, not a JUnit 4/5 or TestNG test is ok</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
public void tearDown() {}
public void foo() {}
}
]]></code>
</test-code>

<test-code>
<description>[java] JUnit4TestShouldUseBeforeAnnotation false positive when overriding setUp #1592</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import net.sourceforge.pmd.lang.java.rule.bestpractices.UnitTestShouldUseAfterAnnotationTest.BaseTest;

public class AReallyCoolFeatureTest extends BaseTest {
@Override
public void setUp() {
super.setUp();
}

@Override
public void tearDown() {
super.tearDown();
}
}
]]></code>
</test-code>

<test-code>
<description>#1446 False positive with JUnit4TestShouldUseBeforeAnnotation when TestNG is used</description>
<description>TestNG test contains tearDown</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>4</expected-linenumbers>
<code><![CDATA[
import org.testng.annotations.Test;

public class MyTestCase {
public void tearDown() {} // violation expected

@Test
public void myTest() {}
}
]]></code>
</test-code>

<test-code>
<description>TestNG test contains tearDown with different signature is ok (#1446)</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import org.testng.annotations.AfterMethod;
import org.testng.annotations.Test;

public class Foo {
@AfterMethod
public void tearDown(java.lang.reflect.Method m) {
//...
}
public void tearDown(java.lang.reflect.Method m) {}
@Test
public void someTest() {}
public void foo() {}
}
]]></code>
]]></code>
</test-code>

<test-code>
<description>#940 False positive with JUnit4TestShouldUseAfterAnnotation when JUnit5's 'AfterEach' is used</description>
<description>TestNG test contains tearDown with @AfterMethod is ok</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import org.junit.jupiter.api.*;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.Test;

public class Foo {
@AfterEach
public void tearDown() {
//...
}
@Test
public void someTest() {}
public class MyTestCase {
@AfterMethod
public void tearDown() {}

@Test
public void myTest() {}
}
]]></code>
]]></code>
</test-code>

<test-code>
<description>#940 False positive with JUnit4TestShouldUseAfterAnnotation when JUnit5's 'AfterAll' is used</description>
<description>TestNG test contains tearDown with @AfterClass is ok</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import org.junit.jupiter.api.*;
import org.testng.annotations.AfterClass;
import org.testng.annotations.Test;

public class MyTestCase {
@AfterClass
public void tearDown() {}

public class Foo {
@AfterAll
public void tearDown() {
//...
}
@Test
public void someTest() {}
public void myTest() {}
}
]]></code>
]]></code>
</test-code>

<test-code>
<description>Contains tearDown, not a junit 4 test</description>
<expected-problems>0</expected-problems>
<description>JUnit 5 test class contains tearDown</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>4</expected-linenumbers>
<code><![CDATA[
public class Foo {
public void tearDown() {
}
public void foo() {
}
}
]]></code>
import org.junit.jupiter.api.Test;

public class MyTestCase {
public void tearDown() {} // violation expected

@Test
public void myTest() {}
}
]]></code>
</test-code>

<test-code>
<description>Contains tearDown with different signature</description>
<description>JUnit 5 test class contains tearDown with @AfterEach is ok (#940)</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import org.junit.Test;
public class Foo {
public void tearDown(int something) {
}
@Test
public void foo() {
}
}
]]></code>
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;

public class MyTestCase {
@AfterEach
public void tearDown() {}

@Test
public void myTest() {}
}
]]></code>
</test-code>

<test-code>
<description>[java] JUnit4TestShouldUseBeforeAnnotation false positive when overriding setUp #1592</description>
<description>JUnit 5 test class contains tearDown with @AfterAll is ok (#940)</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import net.sourceforge.pmd.lang.java.rule.bestpractices.UnitTestShouldUseAfterAnnotationTest.BaseTest;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Test;

public class AReallyCoolFeatureTest extends BaseTest {
@Override
public void setUp() {
super.setUp();
}
public class MyTestCase {
@AfterAll
public void tearDown() {}

@Override
public void tearDown() {
super.tearDown();
}
@Test
public void myTest() {}
}
]]></code>
]]></code>
</test-code>
</test-data>