Apply method predicate before searching type hierarchy

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

Author: sbrannen

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.

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