Pmd: [java] False positives for "JUnit tests should include assert() or fail()"

[benmccann]

Affects PMD Version: 6.4.0

Rule: JUnit tests should include assert() or fail()

Description: A large number of issues for our codebase are false positives generated by this rule. See https://app.codacy.com/app/jtablesaw/tablesaw/issues and select the rule under "Pattern"

This is because it PMD not detect asserts that are called indirectly by calling another method first

Code Sample demonstrating the issue:

https://github.com/jtablesaw/tablesaw/blob/ed5cedfa9884133160d43638c56683ca4964e7c1/core/src/test/java/tech/tablesaw/columns/times/PackedLocalTimeTest.java#L170

Running PMD through: Codacy

[matthieu-vergne]

Since the usual vocabulary is about "asserting" stuff, I would suggest to rename your check method to align with this convention. In particular, naming it assertEquivalentTimes would not only solve the warning, it would also be more explicit.

However, the remark remains when you use alternative ways, like when you test an exception is thrown (no assert, just a expected test parameter) or when you use other tools like Mockito (verify methods).

[benmccann]

Ah, I didn't realize it was looking for the text assert. I assumed it was looking to see if one of the junit methods was being called and didn't find it because it was happening indirectly. I agree renaming our method to contain assert is a better name, so will do that

[rdohms]

What about for cases like this:

        mvc.perform(
                post("/endpoint/".concat("not-an-uuid"))
                .contentType(MediaType.APPLICATION_JSON))
                .andExpect(MockMvcResultMatchers.status().isBadRequest());

Is there a plugin i could be using? I'm sorry, i'm new to this and looking for resources.

[matthieu-vergne]

The rule assumes a somehow standard assertion. If you use a specific testing framework, which uses its own terminology, you just can't rely on generic rules like this one. So either you tweak your framework to follow the rule, either you discard the rule. A generic rule cannot afford to deal with any possible specific variants.

Nothing stops you from making your own specific rule, though.

[Fernal73]

A similar flagging of this rule happens when you use the EqualsVerifier.verify method which throws an AssertionError when it fails.

The code appears to be checking for a verify method but it doesn't catch the EqualsVerifier method call.

The call is in this form:

EqualsVerifier.forClass(HighArray.class) .withIgnoredFields("modCount", "lock", "strict") .withRedefinedSuperclass() .withRedefinedSubclass(HighArrayExt.class) .withIgnoredAnnotations(NonNull.class) .verify();

It might be a better idea to make this rule configurable so that users can define methods they discover themselves rather than wait for the rule's code base to be extended. This is what Checkstyle follows with more than a modicum of success.