Java S5786 yields false positives with inheritance or composition

[byb5jdvihbb5tw369x31 (Jeremy)]

Java rule S5786 (added in SONARJAVA-3362) does not account for abstract test classes or interfaces.

Product
SonarQube 8.6.1

Example

import static org.junit.jupiter.api.Assertions.assertNotNull;
import org.junit.jupiter.api.Test;

public abstract class ExampleTestHelper {

    protected abstract Object delegate();

    @Test
    protected void test_toString() {
        assertNotNull(this.delegate().toString());
    }

    protected abstract void test_implSpecific();

}


import static org.junit.jupiter.api.Assertions.assertEquals;
import org.junit.jupiter.api.Test;

class ExampleTest extends ExampleTestHelper {

    @Override
    protected String delegate() {
        return "test";
    }

    @Test
    @Override
    protected void test_implSpecific() {
        assertEquals("test", this.delegate());
    }

}

3 false positives occur in the above example:

• public abstract class ExampleTestHelper → “Remove this ‘public’ modifier.”
• protected void test_toString() → “Remove this ‘protected’ modifier.”
• @Override protected void test_implSpecific() → “Remove this ‘protected’ modifier.”

Reducing visibility for JUnit5 test classes is great, and I want to keep this rule enabled, but need the ability to refactor test code into abstract classes and/or interfaces. Note: the ExampleTestHelper class have been an interface with a default @Test method but still yields the same false-positive.

It would also be great to see a warning when a @Test method is overridden without a @Test annotation.

[Dorian_Burihabwa]

Hello @byb5jdvihbb5tw369x31 and thanks for your post.

Indeed, the rule is tripped up when it sees test code in an abstract class.
As the check is triggered by the presence of @Test annotation on any method the class, there should be some way to (1) ignore abstract classes and interfaces and (2) ignore visibility modifiers on overriding test methods.

However, because the rule only checks one file at a time, visibility modifiers can only be ignored on methods that are explicitly annotated with @Override and @Test.
As a side effect, implementation of test methods declared in interfaces with proper annotation should also be ignored.

A ticket has been created to handle this.

Cheers,

Dorian