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.

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] Inconsistent results with TestClassWithoutTestCases #929

Closed
phoenix384 opened this issue Feb 20, 2018 · 4 comments · Fixed by #4162
Closed

[java] Inconsistent results with TestClassWithoutTestCases #929

phoenix384 opened this issue Feb 20, 2018 · 4 comments · Fixed by #4162
Labels
a:false-negative PMD doesn't flag a problematic piece of code
Milestone

Comments

@phoenix384
Copy link

phoenix384 commented Feb 20, 2018

Affects PMD Version:
6.0.0

Rule: TestClassWithoutTestCases

Description:
Perhaps this issue might be better place in the pmd-eclipse project - I don't know.
I have a class extending the class TestCase (not junit.framework.TestCase, it's a self-made one) without a test method and there is no warning from PMD.
On a colleague's notebook (same Eclipse version, same PMD version, same Java version, same Project) this class has a warning.
I don't whether there should be a warning (because the class extends a class called TestCase) or not (because it's not the JUnit TestCase class) in this case. I could live with both. But the result should always be the same.
I don't know how to further examine this.

Code Sample demonstrating the issue:


Running PMD through: Eclipse

@adangel
Copy link
Member

adangel commented Feb 20, 2018

@phoenix384
The rule TestClassWithoutTestCases should warn you in your case: Since you have a class, which has "TestCase" in its name (or in your case, extends a class with TestCase in its name), you express the intention, that this class is indeed a class containing test cases. If there are no test methods, then you trigger the rule.

The implementation is however a bit different: We use typeresolution, to determine, whether the class is really a TestCase (e.g. a JUnit 3 test case). If it is not such a specific test case class, then it is ignored.
@jsotuyod This might be actually wrong behavior of the rule TestClassWithoutTestCases. According to the description, we should rely more on naming conventions rather than type resolution. WDYT?

Back to your seen inconsistencies: If typeresolution is not fully configured, the rule falls back to just comparing the class names. This means: on your notebook, PMD is using typeresolution (and hence ignores the class) and on your colleague's notebook, it is not.

Look out in the project properties / workspace configuration for: "Enable using Java Projects Build Path...". Compare these settings - they are probably different on the two notebooks. Enabling this feature should make typeresolution to work correctly, but might raise other issues (see also pmd/pmd-eclipse-plugin#34)

@phoenix384
Copy link
Author

The configuration for "Enable using Java Projects Build Path..." is the same on both machines.

In my opinion the class name should be important to determine whether a class is a test class, since it does not have to extend junit.framework.TestCase.
In case of JUnit4, if I understand the rule's code correctly, a class is a test class if it contains the org.junit.Test annotation. But a class without test methods does not contain this annotation.
So in case of JUnit4 this rule should never work.

@jsotuyod
Copy link
Member

jsotuyod commented Feb 20, 2018

@adangel agreed. The rule should manually check class name, but we also need to review how we determine if the rule is using JUnit 3/4/5 (maybe just looking at imports?); otherwise it would still abort and never report on JUnit 4/5 classes, as stated by @phoenix384

Also of note, this rule manually discards methods from nested classes due to no boundary honoring during child search (related to #904)

@jhovell
Copy link

jhovell commented May 17, 2019

Not sure if this should be a separate issue but should this rule also take into account @ParameterizedTest and other JUnit5 annotations when accounting for test cases?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:false-negative PMD doesn't flag a problematic piece of code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants