Apply method predicate before searching type hierarchy

documentation/src/docs/asciidoc/release-notes/release-notes-5.10.1.adoc

@@ -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.

junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java

@@ -1489,29 +1489,27 @@ public static Stream<Method> streamMethods(Class<?> clazz, Predicate<Method> 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<Method> findAllMethodsInHierarchy(Class<?> clazz, HierarchyTraversalMode traversalMode) {
+	private static List<Method> findAllMethodsInHierarchy(Class<?> clazz, Predicate<Method> predicate,
+			HierarchyTraversalMode traversalMode) {
+
 		Preconditions.notNull(clazz, "Class must not be null");
 		Preconditions.notNull(traversalMode, "HierarchyTraversalMode must not be null");
 
 		// @formatter:off
 		List<Method> localMethods = getDeclaredMethods(clazz, traversalMode).stream()
-				.filter(method -> !method.isSynthetic())
+				.filter(predicate.and(method -> !method.isSynthetic()))
 				.collect(toList());
-		List<Method> superclassMethods = getSuperclassMethods(clazz, traversalMode).stream()
+		List<Method> superclassMethods = getSuperclassMethods(clazz, predicate, traversalMode).stream()
 				.filter(method -> !isMethodShadowedByLocalMethods(method, localMethods))
 				.collect(toList());
-		List<Method> interfaceMethods = getInterfaceMethods(clazz, traversalMode).stream()
+		List<Method> 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<Method> getInterfaceMethods(Class<?> clazz, HierarchyTraversalMode traversalMode) {
+	private static List<Method> getInterfaceMethods(Class<?> clazz, Predicate<Method> predicate,
+			HierarchyTraversalMode traversalMode) {
+
 		List<Method> allInterfaceMethods = new ArrayList<>();
 		for (Class<?> ifc : clazz.getInterfaces()) {
 
 			// @formatter:off
 			List<Method> localInterfaceMethods = getMethods(ifc).stream()
-					.filter(m -> !isAbstract(m))
+					.filter(predicate.and(method -> !isAbstract(method)))
 					.collect(toList());
 
-			List<Method> superinterfaceMethods = getInterfaceMethods(ifc, traversalMode).stream()
+			List<Method> 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<Field> loc
 		return localFields.stream().anyMatch(local -> local.getName().equals(field.getName()));
 	}
 
-	private static List<Method> getSuperclassMethods(Class<?> clazz, HierarchyTraversalMode traversalMode) {
+	private static List<Method> getSuperclassMethods(Class<?> clazz, Predicate<Method> 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<Method> localMethods) {

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");

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()) {

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
+	}
+
+}

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
+	}
+
+}