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

@BeforeAll method in super class skipped when it has same name as a @BeforeEach method in subclass #3498

Closed
hjohn opened this issue Oct 9, 2023 · 6 comments · Fixed by #3500

Comments

@hjohn
Copy link

hjohn commented Oct 9, 2023

Steps to reproduce

Given:

package a;

import org.junit.jupiter.api.BeforeAll;

public abstract class A {

    @BeforeAll
    static void before() {
        System.out.println("A");
    }
}

and

package b;

import a.A;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

public class B extends A {

    @BeforeEach
    void before() {
        System.out.println("B");
    }

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

Executing the test in B prints: B and Hi there. Note the absence of A: the @BeforeAll method was not called. Note that the non-static before method in B does not override the static method (it is in a different package), nor does it conflict with it. Renaming the before method in B or A fixes the problem.

This can easily happen when you have an abstract base class in some utility package, and you are extending it to build tests. If you happen to name a method in this test class that conflicts with a package-private method in the base class, things do not work as expected.

Workaround:

Ensure the names in the base class are sufficiently unique that they're unlikely to have similar names in a subclass.

Deliverables

  • A fix for this problem. It probably is a problem with annotation scanning not being thorough enough and not taking this special case into account.
@sbrannen sbrannen changed the title @BeforeAll method in super class skipped when it has same name as a @BeforeEach method in subclass @BeforeAll method in super class skipped when it has same name as a @BeforeEach method in subclass Oct 9, 2023
@sbrannen sbrannen added this to the 5.11 M1 milestone Oct 9, 2023
@sbrannen
Copy link
Member

sbrannen commented Oct 9, 2023

Thanks for raising the issue.

I have confirmed that this is a bug.

We'll look into it.

@sbrannen
Copy link
Member

sbrannen commented Oct 9, 2023

At a quick glance, the cause of the issue appears to be due to the fact that org.junit.platform.commons.util.ReflectionUtils.streamMethods(...) first invokes findAllMethodsInHierarchy(...) and then filters based on the isAnnotated(...) predicate afterwards.

Whereas, for such use cases, we would need to apply the predicate while searching the hierarchy for methods. If we did that, b.B.before() would not "hide" a.A.before() which is currently the case.

@sbrannen sbrannen self-assigned this Oct 9, 2023
sbrannen added a commit to sbrannen/junit5 that referenced this issue Oct 9, 2023
@sbrannen
Copy link
Member

sbrannen commented Oct 9, 2023

Update: I put together a proof-of-concept fix in #3500.

@marcphilipp marcphilipp modified the milestones: 5.11 M1, 5.10.1 Oct 13, 2023
sbrannen added a commit to sbrannen/junit5 that referenced this issue Oct 30, 2023
Prior to this commit, findMethods() and streamMethods() in
ReflectionSupport as well as findAnnotatedMethods() in
AnnotationSupport first searched for all methods in the type hierarchy
and then applied the user-supplied predicate (or "is annotated?"
predicate) afterwards.

This resulted in methods in subclasses incorrectly "shadowing"
package-private methods in superclasses (in a different package) even
if the predicate would otherwise exclude the method in such a subclass.

For example, given a superclass that declares a package-private static
@⁠BeforeAll "before()" method and a subclass (in a different package)
that declares a @⁠BeforeEach "before()" method, when JUnit Jupiter
looked up @⁠BeforeAll methods for the subclass, the @⁠BeforeAll
"before()" method in the superclass was not found because the
@⁠BeforeEach "before()" method shadowed it based solely on the method
signature, ignoring the type of annotation sought.

To address that, this commit modifies the internal search algorithms in
ReflectionUtils so that method predicates are applied while searching
the hierarchy for methods.

Closes junit-team#3498
Closes junit-team#3500
sbrannen added a commit that referenced this issue Oct 30, 2023
Prior to this commit, findMethods() and streamMethods() in
ReflectionSupport as well as findAnnotatedMethods() in
AnnotationSupport first searched for all methods in the type hierarchy
and then applied the user-supplied predicate (or "is annotated?"
predicate) afterwards.

This resulted in methods in subclasses incorrectly "shadowing"
package-private methods in superclasses (in a different package) even
if the predicate would otherwise exclude the method in such a subclass.

For example, given a superclass that declares a package-private static
@⁠BeforeAll "before()" method and a subclass (in a different package)
that declares a @⁠BeforeEach "before()" method, when JUnit Jupiter
looked up @⁠BeforeAll methods for the subclass, the @⁠BeforeAll
"before()" method in the superclass was not found because the
@⁠BeforeEach "before()" method shadowed it based solely on the method
signature, ignoring the type of annotation sought.

To address that, this commit modifies the internal search algorithms in
ReflectionUtils so that method predicates are applied while searching
the hierarchy for methods.

Closes #3498
Closes #3500
sbrannen added a commit to sbrannen/junit5 that referenced this issue Nov 3, 2023
This commit includes a fix with two simple test classes that
demonstrate the issue.

TODO:

- add "formal" tests in ReflectionUtilsTests and AnnotationUtilsTests
- add release note entries

See junit-team#3498
Closes junit-team#3532
sbrannen added a commit to sbrannen/junit5 that referenced this issue Nov 4, 2023
Prior to this commit, findFields() and streamFields() in
ReflectionSupport as well as findAnnotatedFields() and
findAnnotatedFieldValues() in AnnotationSupport first searched for all
fields in the type hierarchy and then applied the user-supplied
predicate (or "is annotated?" predicate) afterwards.

This resulted in fields in subclasses incorrectly "shadowing"
package-private fields in superclasses (in a different package) even if
the predicate would otherwise exclude the field in such a subclass.

For example, given a superclass that declares a package-private static
@⁠TempDir "tempDir" field and a subclass (in a different package) that
declares a @⁠TempDir "tempDir" field, when JUnit Jupiter looked up
@⁠TempDir fields for the subclass, the @⁠TempDir "tempDir" field in the
superclass was not found because the @⁠TempDir "tempDir" field shadowed
it based solely on the field signature, ignoring the type of annotation
sought.

To address that, this commit modifies the internal search algorithms in
ReflectionUtils so that field predicates are applied while searching
the hierarchy for fields.

TODO:

- add release note entries

See junit-team#3498
Closes junit-team#3532
sbrannen added a commit to sbrannen/junit5 that referenced this issue Nov 4, 2023
Prior to this commit, findFields() and streamFields() in
ReflectionSupport as well as findAnnotatedFields() and
findAnnotatedFieldValues() in AnnotationSupport first searched for all
fields in the type hierarchy and then applied the user-supplied
predicate (or "is annotated?" predicate) afterwards.

This resulted in fields in subclasses incorrectly "shadowing"
package-private fields in superclasses (in a different package) even if
the predicate would otherwise exclude the field in such a subclass.

For example, given a superclass that declares a package-private static
@⁠TempDir "tempDir" field and a subclass (in a different package) that
declares a @⁠TempDir "tempDir" field, when JUnit Jupiter looked up
@⁠TempDir fields for the subclass, the @⁠TempDir "tempDir" field in the
superclass was not found because the @⁠TempDir "tempDir" field shadowed
it based solely on the field signature, ignoring the type of annotation
sought.

To address that, this commit modifies the internal search algorithms in
ReflectionUtils so that field predicates are applied while searching
the hierarchy for fields.

TODO:

- add release note entries

See junit-team#3498
Closes junit-team#3532
sbrannen added a commit to sbrannen/junit5 that referenced this issue Nov 4, 2023
Prior to this commit, findFields() and streamFields() in
ReflectionSupport as well as findAnnotatedFields() and
findAnnotatedFieldValues() in AnnotationSupport first searched for all
fields in the type hierarchy and then applied the user-supplied
predicate (or "is annotated?" predicate) afterwards.

This resulted in fields in subclasses incorrectly "shadowing"
package-private fields in superclasses (in a different package) even if
the predicate would otherwise exclude the field in such a subclass.

For example, given a superclass that declares a package-private static
@⁠TempDir "tempDir" field and a subclass (in a different package) that
declares a @⁠TempDir "tempDir" field, when JUnit Jupiter looked up
@⁠TempDir fields for the subclass, the @⁠TempDir "tempDir" field in the
superclass was not found because the @⁠TempDir "tempDir" field shadowed
it based solely on the field signature, ignoring the type of annotation
sought.

To address that, this commit modifies the internal search algorithms in
ReflectionUtils so that field predicates are applied while searching
the hierarchy for fields.

See junit-team#3498
Closes junit-team#3532
Closes junit-team#3533
sbrannen added a commit that referenced this issue Nov 4, 2023
Prior to this commit, findFields() and streamFields() in
ReflectionSupport as well as findAnnotatedFields() and
findAnnotatedFieldValues() in AnnotationSupport first searched for all
fields in the type hierarchy and then applied the user-supplied
predicate (or "is annotated?" predicate) afterwards.

This resulted in fields in subclasses incorrectly "shadowing"
package-private fields in superclasses (in a different package) even if
the predicate would otherwise exclude the field in such a subclass.

For example, given a superclass that declares a package-private static
@⁠TempDir "tempDir" field and a subclass (in a different package) that
declares a @⁠TempDir "tempDir" field, when JUnit Jupiter looked up
@⁠TempDir fields for the subclass, the @⁠TempDir "tempDir" field in the
superclass was not found because the @⁠TempDir "tempDir" field shadowed
it based solely on the field signature, ignoring the type of annotation
sought.

To address that, this commit modifies the internal search algorithms in
ReflectionUtils so that field predicates are applied while searching
the hierarchy for fields.

See #3498
Closes #3532
Closes #3533
sbrannen added a commit that referenced this issue Nov 4, 2023
Prior to this commit, findFields() and streamFields() in
ReflectionSupport as well as findAnnotatedFields() and
findAnnotatedFieldValues() in AnnotationSupport first searched for all
fields in the type hierarchy and then applied the user-supplied
predicate (or "is annotated?" predicate) afterwards.

This resulted in fields in subclasses incorrectly "shadowing"
package-private fields in superclasses (in a different package) even if
the predicate would otherwise exclude the field in such a subclass.

For example, given a superclass that declares a package-private static
@⁠TempDir "tempDir" field and a subclass (in a different package) that
declares a @⁠TempDir "tempDir" field, when JUnit Jupiter looked up
@⁠TempDir fields for the subclass, the @⁠TempDir "tempDir" field in the
superclass was not found because the @⁠TempDir "tempDir" field shadowed
it based solely on the field signature, ignoring the type of annotation
sought.

To address that, this commit modifies the internal search algorithms in
ReflectionUtils so that field predicates are applied while searching
the hierarchy for fields.

See #3498
Closes #3532
Closes #3533

(cherry picked from commit f30a8d5)
sbrannen added a commit to sbrannen/junit5 that referenced this issue Nov 4, 2023
This commit consistently applies method predicates in search algorithms
analogous to the application of field predicates.

See junit-team#3498
See junit-team#3532
Closes junit-team#3534
sbrannen added a commit to sbrannen/junit5 that referenced this issue Nov 4, 2023
This commit consistently applies method predicates in search algorithms
analogous to the application of field predicates.

See junit-team#3498
See junit-team#3532
Closes junit-team#3534
sbrannen added a commit that referenced this issue Nov 4, 2023
This commit consistently applies method predicates in search algorithms
analogous to the application of field predicates.

See #3498
See #3532
Closes #3534
sbrannen added a commit that referenced this issue Nov 4, 2023
This commit consistently applies method predicates in search algorithms
analogous to the application of field predicates.

See #3498
See #3532
Closes #3534

(cherry picked from commit a670d10)
@sbrannen
Copy link
Member

sbrannen commented Nov 4, 2023

This has been merged into main and backported to 5.10.x.

@sbrannen
Copy link
Member

@sbrannen
Copy link
Member

sbrannen commented Jan 7, 2024

Due to #3600, for 5.10.2 we have decided to revert the changes to our method search algorithm introduced in conjunction with this issue.

However, we will address the issue originally raised in this issue when we resolve #3553 in 5.11.

sbrannen added a commit that referenced this issue Jan 15, 2024
This commit reverts the functional changes from commit 64ed21a and
disables the associated tests for the time being.

See #3498
See #3500
See #3553
Closes #3600
sbrannen added a commit that referenced this issue Jan 15, 2024
This commit reverts the functional changes from commit 64ed21a and
disables the associated tests for the time being.

See #3498
See #3500
See #3553
Closes #3600

(cherry picked from commit c2f49f6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants