Skip to content

Commit

Permalink
Apply method predicate before searching type hierarchy
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sbrannen committed Oct 30, 2023
1 parent 1b05c88 commit 4732d12
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,22 @@ JUnit repository on GitHub.

==== Bug Fixes

* ❓
* Method predicates are now applied while searching the type hierarchy. This fixes bugs
in `findMethods(...)` and `streamMethods(...)` in `ReflectionSupport` as well as
`findAnnotatedMethods(...)` in `AnnotationSupport`.
- See link:https://github.com/junit-team/junit5/issues/3498[issue 3498] for details.


[[release-notes-5.10.1-junit-jupiter]]
=== JUnit Jupiter

==== Bug Fixes

* A package-private class-level lifecycle method annotated with `@BeforeAll` or
`@AfterAll` is no longer _shadowed_ by a method-level lifecycle method annotated with
`@BeforeEach` or `@AfterEach` when the method-level lifecycle method resides in a
different package and has the same name as the class-level lifecycle method.
- See link:https://github.com/junit-team/junit5/issues/3498[issue 3498] for details.
* The `RandomNumberExtension` example in the
<<../user-guide/index.adoc#extensions-RandomNumberExtension, User Guide>> has been
updated to properly support `Integer` types as well as non-static field injection.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,18 @@
import java.lang.annotation.Target;
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.math.BigDecimal;
import java.util.List;
import java.util.Optional;
import java.util.function.Predicate;

import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.platform.commons.PreconditionViolationException;
import org.junit.platform.commons.util.pkg1.SuperclassWithStaticPackagePrivateBeforeMethod;
import org.junit.platform.commons.util.pkg1.subpkg.SubclassWithNonStaticPackagePrivateBeforeMethod;

/**
* Unit tests for {@link AnnotationUtils}.
Expand Down Expand Up @@ -380,6 +385,28 @@ void findAnnotatedMethodsForAnnotationUsedInClassAndSuperclassHierarchyDown() th
assertThat(methods.subList(1, 3)).containsOnly(method1, method3);
}

/**
* @see https://github.com/junit-team/junit5/issues/3498
*/
@Test
void findAnnotatedMethodsAppliesPredicateBeforeSearchingTypeHierarchy() throws Exception {
final String BEFORE = "before";
Class<?> superclass = SuperclassWithStaticPackagePrivateBeforeMethod.class;
Method beforeAllMethod = superclass.getDeclaredMethod(BEFORE);
Class<?> subclass = SubclassWithNonStaticPackagePrivateBeforeMethod.class;
Method beforeEachMethod = subclass.getDeclaredMethod(BEFORE);

// Prerequisite
var methods = findAnnotatedMethods(superclass, BeforeAll.class, TOP_DOWN);
assertThat(methods).containsExactly(beforeAllMethod);

// Actual use cases for this test
methods = findAnnotatedMethods(subclass, BeforeAll.class, TOP_DOWN);
assertThat(methods).containsExactly(beforeAllMethod);
methods = findAnnotatedMethods(subclass, BeforeEach.class, TOP_DOWN);
assertThat(methods).containsExactly(beforeEachMethod);
}

@Test
void findAnnotatedMethodsForAnnotationUsedInInterface() throws Exception {
var interfaceMethod = InterfaceWithAnnotatedDefaultMethod.class.getDeclaredMethod("interfaceMethod");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@
import org.junit.platform.commons.util.ReflectionUtilsTests.OuterClass.StaticNestedSiblingClass;
import org.junit.platform.commons.util.ReflectionUtilsTests.OuterClassImplementingInterface.InnerClassImplementingInterface;
import org.junit.platform.commons.util.classes.CustomType;
import org.junit.platform.commons.util.pkg1.SuperclassWithStaticPackagePrivateBeforeMethod;
import org.junit.platform.commons.util.pkg1.subpkg.SubclassWithNonStaticPackagePrivateBeforeMethod;

/**
* Unit tests for {@link ReflectionUtils}.
Expand Down Expand Up @@ -1344,6 +1346,28 @@ void findMethodsIgnoresBridgeMethods() throws Exception {
assertEquals(0, methods.stream().filter(Method::isBridge).count());
}

/**
* @see https://github.com/junit-team/junit5/issues/3498
*/
@Test
void findMethodsAppliesPredicateBeforeSearchingTypeHierarchy() throws Exception {
final String BEFORE = "before";
Class<?> superclass = SuperclassWithStaticPackagePrivateBeforeMethod.class;
Method staticMethod = superclass.getDeclaredMethod(BEFORE);
Class<?> subclass = SubclassWithNonStaticPackagePrivateBeforeMethod.class;
Method nonStaticMethod = subclass.getDeclaredMethod(BEFORE);

// Prerequisite
var methods = findMethods(superclass, ReflectionUtils::isStatic);
assertThat(methods).containsExactly(staticMethod);

// Actual use cases for this test
methods = findMethods(subclass, ReflectionUtils::isStatic);
assertThat(methods).containsExactly(staticMethod);
methods = findMethods(subclass, ReflectionUtils::isNotStatic);
assertThat(methods).containsExactly(nonStaticMethod);
}

@Test
void isGeneric() {
for (var method : Generic.class.getMethods()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright 2015-2023 the original author or authors.
*
* All rights reserved. This program and the accompanying materials are
* made available under the terms of the Eclipse Public License v2.0 which
* accompanies this distribution and is available at
*
* https://www.eclipse.org/legal/epl-v20.html
*/

package org.junit.platform.commons.util.pkg1;

import org.junit.jupiter.api.BeforeAll;

/**
* @see https://github.com/junit-team/junit5/issues/3498
*/
public class SuperclassWithStaticPackagePrivateBeforeMethod {

@BeforeAll
static void before() {
// no-op
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright 2015-2023 the original author or authors.
*
* All rights reserved. This program and the accompanying materials are
* made available under the terms of the Eclipse Public License v2.0 which
* accompanies this distribution and is available at
*
* https://www.eclipse.org/legal/epl-v20.html
*/

package org.junit.platform.commons.util.pkg1.subpkg;

import org.junit.jupiter.api.BeforeEach;
import org.junit.platform.commons.util.pkg1.SuperclassWithStaticPackagePrivateBeforeMethod;

/**
* @see https://github.com/junit-team/junit5/issues/3498
*/
public class SubclassWithNonStaticPackagePrivateBeforeMethod extends SuperclassWithStaticPackagePrivateBeforeMethod {

@BeforeEach
void before() {
// no-op
}

}

0 comments on commit 4732d12

Please sign in to comment.