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

JUnit 5 computes overrides relationships incorrectly #2390

Closed
tjwilson90 opened this issue Aug 22, 2020 · 7 comments
Closed

JUnit 5 computes overrides relationships incorrectly #2390

tjwilson90 opened this issue Aug 22, 2020 · 7 comments

Comments

@tjwilson90
Copy link

tjwilson90 commented Aug 22, 2020

junit-platform-commons:1.6.0

package foo;

import org.junit.jupiter.api.BeforeEach;

public class Foo {
    @BeforeEach
    final void beforeEach() {
        System.out.println("foo");
    }
}
package bar;

import foo.Foo;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

class Bar extends Foo {
    @BeforeEach
    final void beforeEach() {
        System.out.println("bar");
    }

    @Test
    final void test() { }
}

This outputs

bar

but should output

foo
bar

The reason it does not output foo is because ReflectionUtils.isMethodShadowedBy is used to compute overrides while scanning for all methods in a class hierarchy. That considers only the signature of the method which is not sufficient according to the language specification, https://docs.oracle.com/javase/specs/jls/se8/html/jls-8.html#jls-8.4.8.1

The reason why I care about this is because I want to declare a @BeforeEach method in a super class and always have it execute regardless of what subclasses are doing.

@marcphilipp
Copy link
Member

Tentatively slated for 5.8 M1 solely for the purpose of team discussion.

@sbrannen sbrannen changed the title JUnit5 computes overrides relationships incorrectly JUnit 5 computes overrides relationships incorrectly Sep 13, 2020
@sbrannen
Copy link
Member

This appears to be a duplicate of another issue raised about package private test methods with the same signature but in different packages, though I can't seem to find that issue at the moment.

@marcphilipp
Copy link
Member

Closing with rationale explained in #960 (comment).

@marcphilipp marcphilipp removed this from the 5.8 M1 milestone Oct 9, 2020
@juriad
Copy link

juriad commented May 10, 2022

I disagree with closing this as a duplicate. The referred issue does not deal with packages - it only concerns finding annotations.

Let's imagine two packages and a class in each:

package sup;
public class Sup {
    @BeforeEach
    MODSUP void before() {
        System.out.println("Before in Sup");
    }
}

package sub;
import sup.Sup;

public class Sub extends Sup {
    @BeforeEach
    void before() {
        System.out.println("Before in Sub");
    }

    @Test
    MODSUB void test() {
        System.out.println("Test");
    }
}

We can then write a table of expected behavior based on the modifiers:

Sub \ Sup public protected none private
public Before in Sub
Test
Before in Sub
Test
Before in Sup
Before in Sub
Test (*)
Before in Sub
Test
protected compilation error Before in Sub
Test
Before in Sup
Before in Sub
Test (*)
Before in Sub
Test
none compilation error compilation error Before in Sup
Before in Sub
Test (*)
Before in Sub
Test
private compilation error compilation error Before in Sup
Test (*)
Test (*)

The combinations with (*) instead produce:

Before in Sub
Test

which is inconsistent with documentation:

@beforeeach methods must have a void return type, must not be private, and must not be static.
@beforeeach methods are inherited from superclasses as long as they are not overridden. Furthermore, @beforeeach methods from superclasses will be executed before @beforeeach methods in subclasses.

First - private methods are actually executed too!
Second - when the superclass is in a different package, and its method has a package-private modifier, it is never overridden. Therefore the method in the super class should be executed.

Either my understanding of Java method overriding, or those who sentences in JavaDoc is wrong, or this is really a bug - and it is up to JUnit 5 project to decide whether to fix it by changing the behavior to be consistent with documentation (breaking change), or update the documentation to cover all cases or mention that when methods have the same name the execution is not reliable (similar thing is there already for execution order of multiple methods within one class).

@sbrannen
Copy link
Member

@juriad, this issue was closed 1.5 years ago.

Please open a new issue to discuss your points.

With regard to private methods, those should not be executed. So that sounds like a bug.

With regard to the use of the term "overridden", the documentation should likely be updated to say "overridden or shadowed".

@sbrannen

This comment was marked as outdated.

@sbrannen
Copy link
Member

sbrannen commented Apr 7, 2024

@tjwilson90 and @juriad, please note that this issue has been effectively superseded by the following, which will be included in 5.11 M1 for early testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants