diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-5.10.1.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-5.10.1.adoc index cb575e6f4dc3..abd61752cc23 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-5.10.1.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-5.10.1.adoc @@ -15,7 +15,10 @@ 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]] @@ -23,6 +26,11 @@ JUnit repository on GitHub. ==== 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. diff --git a/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java b/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java index 816b7fadd104..927b97451e0a 100644 --- a/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java +++ b/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java @@ -1489,29 +1489,27 @@ public static Stream streamMethods(Class clazz, Predicate pre Preconditions.notNull(predicate, "Predicate must not be null"); Preconditions.notNull(traversalMode, "HierarchyTraversalMode must not be null"); - // @formatter:off - return findAllMethodsInHierarchy(clazz, traversalMode).stream() - .filter(predicate) - .distinct(); - // @formatter:on + return findAllMethodsInHierarchy(clazz, predicate, traversalMode).stream().distinct(); } /** * Find all non-synthetic methods in the superclass and interface hierarchy, - * excluding Object. + * excluding Object, that match the specified {@code predicate}. */ - private static List findAllMethodsInHierarchy(Class clazz, HierarchyTraversalMode traversalMode) { + private static List findAllMethodsInHierarchy(Class clazz, Predicate predicate, + HierarchyTraversalMode traversalMode) { + Preconditions.notNull(clazz, "Class must not be null"); Preconditions.notNull(traversalMode, "HierarchyTraversalMode must not be null"); // @formatter:off List localMethods = getDeclaredMethods(clazz, traversalMode).stream() - .filter(method -> !method.isSynthetic()) + .filter(predicate.and(method -> !method.isSynthetic())) .collect(toList()); - List superclassMethods = getSuperclassMethods(clazz, traversalMode).stream() + List superclassMethods = getSuperclassMethods(clazz, predicate, traversalMode).stream() .filter(method -> !isMethodShadowedByLocalMethods(method, localMethods)) .collect(toList()); - List interfaceMethods = getInterfaceMethods(clazz, traversalMode).stream() + List interfaceMethods = getInterfaceMethods(clazz, predicate, traversalMode).stream() .filter(method -> !isMethodShadowedByLocalMethods(method, localMethods)) .collect(toList()); // @formatter:on @@ -1647,16 +1645,18 @@ private static int defaultMethodSorter(Method method1, Method method2) { return comparison; } - private static List getInterfaceMethods(Class clazz, HierarchyTraversalMode traversalMode) { + private static List getInterfaceMethods(Class clazz, Predicate predicate, + HierarchyTraversalMode traversalMode) { + List allInterfaceMethods = new ArrayList<>(); for (Class ifc : clazz.getInterfaces()) { // @formatter:off List localInterfaceMethods = getMethods(ifc).stream() - .filter(m -> !isAbstract(m)) + .filter(predicate.and(method -> !isAbstract(method))) .collect(toList()); - List superinterfaceMethods = getInterfaceMethods(ifc, traversalMode).stream() + List superinterfaceMethods = getInterfaceMethods(ifc, predicate, traversalMode).stream() .filter(method -> !isMethodShadowedByLocalMethods(method, localInterfaceMethods)) .collect(toList()); // @formatter:on @@ -1706,12 +1706,14 @@ private static boolean isFieldShadowedByLocalFields(Field field, List loc return localFields.stream().anyMatch(local -> local.getName().equals(field.getName())); } - private static List getSuperclassMethods(Class clazz, HierarchyTraversalMode traversalMode) { + private static List getSuperclassMethods(Class clazz, Predicate predicate, + HierarchyTraversalMode traversalMode) { + Class superclass = clazz.getSuperclass(); if (!isSearchable(superclass)) { return Collections.emptyList(); } - return findAllMethodsInHierarchy(superclass, traversalMode); + return findAllMethodsInHierarchy(superclass, predicate, traversalMode); } private static boolean isMethodShadowedByLocalMethods(Method method, List localMethods) { diff --git a/platform-tests/src/test/java/org/junit/platform/commons/util/AnnotationUtilsTests.java b/platform-tests/src/test/java/org/junit/platform/commons/util/AnnotationUtilsTests.java index 478491014be1..f26f5ab755d3 100644 --- a/platform-tests/src/test/java/org/junit/platform/commons/util/AnnotationUtilsTests.java +++ b/platform-tests/src/test/java/org/junit/platform/commons/util/AnnotationUtilsTests.java @@ -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}. @@ -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"); diff --git a/platform-tests/src/test/java/org/junit/platform/commons/util/ReflectionUtilsTests.java b/platform-tests/src/test/java/org/junit/platform/commons/util/ReflectionUtilsTests.java index 56ed88a8ddcc..30a403fe00c6 100644 --- a/platform-tests/src/test/java/org/junit/platform/commons/util/ReflectionUtilsTests.java +++ b/platform-tests/src/test/java/org/junit/platform/commons/util/ReflectionUtilsTests.java @@ -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}. @@ -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()) { diff --git a/platform-tests/src/test/java/org/junit/platform/commons/util/pkg1/SuperclassWithStaticPackagePrivateBeforeMethod.java b/platform-tests/src/test/java/org/junit/platform/commons/util/pkg1/SuperclassWithStaticPackagePrivateBeforeMethod.java new file mode 100644 index 000000000000..2895f2b4980d --- /dev/null +++ b/platform-tests/src/test/java/org/junit/platform/commons/util/pkg1/SuperclassWithStaticPackagePrivateBeforeMethod.java @@ -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 + } + +} diff --git a/platform-tests/src/test/java/org/junit/platform/commons/util/pkg1/subpkg/SubclassWithNonStaticPackagePrivateBeforeMethod.java b/platform-tests/src/test/java/org/junit/platform/commons/util/pkg1/subpkg/SubclassWithNonStaticPackagePrivateBeforeMethod.java new file mode 100644 index 000000000000..c7c6d1e0ac22 --- /dev/null +++ b/platform-tests/src/test/java/org/junit/platform/commons/util/pkg1/subpkg/SubclassWithNonStaticPackagePrivateBeforeMethod.java @@ -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 + } + +}