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

Apply method predicate before searching type hierarchy #3500

Conversation

sbrannen
Copy link
Member

@sbrannen sbrannen commented Oct 9, 2023

@pixeebot

This comment was marked as off-topic.

@sbrannen

This comment was marked as outdated.

@nahsra

This comment was marked as outdated.

Copy link

@hjohn hjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@marcphilipp

This comment was marked as off-topic.

sbrannen added a commit to sbrannen/junit5 that referenced this pull request 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
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 sbrannen force-pushed the issues/3498-apply-predicate-before-searching-hierarchy branch from 4732d12 to 64ed21a Compare October 30, 2023 18:22
@sbrannen sbrannen marked this pull request as ready for review October 30, 2023 18:23
@sbrannen sbrannen merged commit 64ed21a into junit-team:main Oct 30, 2023
9 checks passed
sbrannen added a commit that referenced this pull request 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 that referenced this pull request 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 pull request 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 this pull request may close these issues.

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