From 85ec2fccb32a51fbe6ff966e25726c94a67dd418 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Mon, 15 Jan 2024 16:42:02 +0100 Subject: [PATCH] Revert "Apply field predicate before searching type hierarchy" This commit reverts the functional changes from commit f30a8d5517 and disables the associated tests for the time being. See #3532 See #3553 Closes #3638 (cherry picked from commit 7789d32d80ccd4449844e4466b1fd974a8353e2d) --- .../release-notes/release-notes-5.10.2.adoc | 32 +++++++++++---- .../commons/util/ReflectionUtils.java | 41 ++++++++----------- .../commons/util/AnnotationUtilsTests.java | 6 ++- .../commons/util/ReflectionUtilsTests.java | 6 ++- ...sWithStaticPackagePrivateTempDirField.java | 2 +- ...thNonStaticPackagePrivateTempDirField.java | 2 +- 6 files changed, 52 insertions(+), 37 deletions(-) diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-5.10.2.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-5.10.2.adoc index 14949dd01d38..183431a5a83d 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-5.10.2.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-5.10.2.adoc @@ -3,11 +3,11 @@ *Date of Release:* ❓ -*Scope:* minor bug fixes since 5.10.1. +*Scope:* minor bug fixes and changes since 5.10.1. For a complete list of all _closed_ issues and pull requests for this release, consult the -link:{junit5-repo}+/milestone/73?closed=1+[5.10.2] milestone page in the -JUnit repository on GitHub. +link:{junit5-repo}+/milestone/73?closed=1+[5.10.2] milestone page in the JUnit repository +on GitHub. [[release-notes-5.10.2-junit-platform]] @@ -15,9 +15,18 @@ JUnit repository on GitHub. ==== Bug Fixes -* Allow `junit-platform-launcher` to be used as a Java module when +* The `junit-platform-launcher` may now be used as a Java module when `junit.platform.launcher.interceptors.enabled` is set to `true`. - - See link:https://github.com/junit-team/junit5/issues/3561[issue 3561] for details. + - See issue link:https://github.com/junit-team/junit5/issues/3561[#3561] for details. + +==== Deprecations and Breaking Changes + +* Field predicates are no longer applied eagerly while searching the type hierarchy. This + reverts changes made in 5.10.1 that affected `findFields(...)` and `streamFields(...)` + in `ReflectionSupport` as well as `findAnnotatedFields(...)` and + `findAnnotatedFieldValues(...)` in `AnnotationSupport`. + - See issues link:https://github.com/junit-team/junit5/issues/3638[#3638] and + link:https://github.com/junit-team/junit5/issues/3553[#3553] for details. [[release-notes-5.10.2-junit-jupiter]] @@ -25,7 +34,16 @@ JUnit repository on GitHub. ==== Bug Fixes -* _none so far_ +* ❓ + +==== Deprecations and Breaking Changes + +* A package-private static field annotated with `@TempDir` is once again _shadowed_ by a + non-static field annotated with `@TempDir` when the non-static field resides in a + different package and has the same name as the static field. This reverts changes made + in 5.10.1. + - See issues link:https://github.com/junit-team/junit5/issues/3638[#3638] and + link:https://github.com/junit-team/junit5/issues/3553[#3553] for details. [[release-notes-5.10.2-junit-vintage]] @@ -33,4 +51,4 @@ JUnit repository on GitHub. ==== Bug Fixes -* _none so far_ +* ❓ 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 478b3372add0..b83ec42f91b0 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 @@ -1253,23 +1253,21 @@ public static Stream streamFields(Class clazz, Predicate predic Preconditions.notNull(predicate, "Predicate must not be null"); Preconditions.notNull(traversalMode, "HierarchyTraversalMode must not be null"); - return findAllFieldsInHierarchy(clazz, predicate, traversalMode).stream(); + return findAllFieldsInHierarchy(clazz, traversalMode).stream().filter(predicate); } - private static List findAllFieldsInHierarchy(Class clazz, Predicate predicate, - HierarchyTraversalMode traversalMode) { - + private static List findAllFieldsInHierarchy(Class clazz, HierarchyTraversalMode traversalMode) { Preconditions.notNull(clazz, "Class must not be null"); Preconditions.notNull(traversalMode, "HierarchyTraversalMode must not be null"); // @formatter:off - List localFields = getDeclaredFields(clazz, predicate).stream() + List localFields = getDeclaredFields(clazz).stream() .filter(field -> !field.isSynthetic()) .collect(toList()); - List superclassFields = getSuperclassFields(clazz, predicate, traversalMode).stream() + List superclassFields = getSuperclassFields(clazz, traversalMode).stream() .filter(field -> !isFieldShadowedByLocalFields(field, localFields)) .collect(toList()); - List interfaceFields = getInterfaceFields(clazz, predicate, traversalMode).stream() + List interfaceFields = getInterfaceFields(clazz, traversalMode).stream() .filter(field -> !isFieldShadowedByLocalFields(field, localFields)) .collect(toList()); // @formatter:on @@ -1532,18 +1530,18 @@ private static List findAllMethodsInHierarchy(Class clazz, Predicate< /** * Custom alternative to {@link Class#getFields()} that sorts the fields - * which match the supplied predicate and converts them to a mutable list. + * and converts them to a mutable list. */ - private static List getFields(Class clazz, Predicate predicate) { - return toSortedMutableList(clazz.getFields(), predicate); + private static List getFields(Class clazz) { + return toSortedMutableList(clazz.getFields()); } /** * Custom alternative to {@link Class#getDeclaredFields()} that sorts the - * fields which match the supplied predicate and converts them to a mutable list. + * fields and converts them to a mutable list. */ - private static List getDeclaredFields(Class clazz, Predicate predicate) { - return toSortedMutableList(clazz.getDeclaredFields(), predicate); + private static List getDeclaredFields(Class clazz) { + return toSortedMutableList(clazz.getDeclaredFields()); } /** @@ -1607,10 +1605,9 @@ private static List getDefaultMethods(Class clazz, Predicate // @formatter:on } - private static List toSortedMutableList(Field[] fields, Predicate predicate) { + private static List toSortedMutableList(Field[] fields) { // @formatter:off return Arrays.stream(fields) - .filter(predicate) .sorted(ReflectionUtils::defaultFieldSorter) // Use toCollection() instead of toList() to ensure list is mutable. .collect(toCollection(ArrayList::new)); @@ -1679,15 +1676,13 @@ private static List getInterfaceMethods(Class clazz, Predicate getInterfaceFields(Class clazz, Predicate predicate, - HierarchyTraversalMode traversalMode) { - + private static List getInterfaceFields(Class clazz, HierarchyTraversalMode traversalMode) { List allInterfaceFields = new ArrayList<>(); for (Class ifc : clazz.getInterfaces()) { - List localInterfaceFields = getFields(ifc, predicate); + List localInterfaceFields = getFields(ifc); // @formatter:off - List superinterfaceFields = getInterfaceFields(ifc, predicate, traversalMode).stream() + List superinterfaceFields = getInterfaceFields(ifc, traversalMode).stream() .filter(field -> !isFieldShadowedByLocalFields(field, localInterfaceFields)) .collect(toList()); // @formatter:on @@ -1703,14 +1698,12 @@ private static List getInterfaceFields(Class clazz, Predicate p return allInterfaceFields; } - private static List getSuperclassFields(Class clazz, Predicate predicate, - HierarchyTraversalMode traversalMode) { - + private static List getSuperclassFields(Class clazz, HierarchyTraversalMode traversalMode) { Class superclass = clazz.getSuperclass(); if (!isSearchable(superclass)) { return Collections.emptyList(); } - return findAllFieldsInHierarchy(superclass, predicate, traversalMode); + return findAllFieldsInHierarchy(superclass, traversalMode); } private static boolean isFieldShadowedByLocalFields(Field field, List localFields) { 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 5dde2c34cbd9..d3f94b7b752e 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 @@ -44,6 +44,7 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.platform.commons.PreconditionViolationException; import org.junit.platform.commons.util.pkg1.ClassLevelDir; @@ -509,10 +510,11 @@ private List findShadowingAnnotatedFields(Class ann } /** - * @see https://github.com/junit-team/junit5/issues/3532 + * @see https://github.com/junit-team/junit5/issues/3553 */ + @Disabled("Until #3553 is resolved") @Test - void findAnnotatedFieldsAppliesPredicateBeforeSearchingTypeHierarchy() throws Exception { + void findAnnotatedFieldsDoesNotAllowInstanceFieldToHideStaticField() throws Exception { final String TEMP_DIR = "tempDir"; Class superclass = SuperclassWithStaticPackagePrivateTempDirField.class; Field staticField = superclass.getDeclaredField(TEMP_DIR); 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 7cccbe13a70c..b1fc048cec9a 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 @@ -52,6 +52,7 @@ import java.util.stream.IntStream; import java.util.stream.Stream; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.fixtures.TrackLogRecords; @@ -1383,10 +1384,11 @@ void isGeneric() { } /** - * @see https://github.com/junit-team/junit5/issues/3532 + * @see https://github.com/junit-team/junit5/issues/3553 */ + @Disabled("Until #3553 is resolved") @Test - void findFieldsAppliesPredicateBeforeSearchingTypeHierarchy() throws Exception { + void findFieldsDoesNotAllowInstanceFieldToHideStaticField() throws Exception { final String TEMP_DIR = "tempDir"; Class superclass = SuperclassWithStaticPackagePrivateTempDirField.class; Field staticField = superclass.getDeclaredField(TEMP_DIR); diff --git a/platform-tests/src/test/java/org/junit/platform/commons/util/pkg1/SuperclassWithStaticPackagePrivateTempDirField.java b/platform-tests/src/test/java/org/junit/platform/commons/util/pkg1/SuperclassWithStaticPackagePrivateTempDirField.java index 4e2bbe7ec696..f091bddc35a5 100644 --- a/platform-tests/src/test/java/org/junit/platform/commons/util/pkg1/SuperclassWithStaticPackagePrivateTempDirField.java +++ b/platform-tests/src/test/java/org/junit/platform/commons/util/pkg1/SuperclassWithStaticPackagePrivateTempDirField.java @@ -13,7 +13,7 @@ import java.nio.file.Path; /** - * @see https://github.com/junit-team/junit5/issues/3532 + * @see https://github.com/junit-team/junit5/issues/3553 */ public class SuperclassWithStaticPackagePrivateTempDirField { diff --git a/platform-tests/src/test/java/org/junit/platform/commons/util/pkg1/subpkg/SubclassWithNonStaticPackagePrivateTempDirField.java b/platform-tests/src/test/java/org/junit/platform/commons/util/pkg1/subpkg/SubclassWithNonStaticPackagePrivateTempDirField.java index d7eb33f6a326..83504ecab639 100644 --- a/platform-tests/src/test/java/org/junit/platform/commons/util/pkg1/subpkg/SubclassWithNonStaticPackagePrivateTempDirField.java +++ b/platform-tests/src/test/java/org/junit/platform/commons/util/pkg1/subpkg/SubclassWithNonStaticPackagePrivateTempDirField.java @@ -16,7 +16,7 @@ import org.junit.platform.commons.util.pkg1.SuperclassWithStaticPackagePrivateTempDirField; /** - * @see https://github.com/junit-team/junit5/issues/3532 + * @see https://github.com/junit-team/junit5/issues/3553 */ public class SubclassWithNonStaticPackagePrivateTempDirField extends SuperclassWithStaticPackagePrivateTempDirField {