From 98c7154651d19048b8dbeca432f4673d28c058c3 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Mon, 27 Jan 2025 09:38:59 +0100 Subject: [PATCH 1/3] Support `@TempDir` constructor injection for Java record classes The `@TempDir` annotation on a constructor parameter is copied to the generated `final` instance field. This caused it top be picked up by `TempDirectory` for instance field injection which then failed because the field was `final`. This change now checks if a test class is an instance of a record type and skips instance field injection. Since records cannot have any instance fields besides the ones generated from their constructor, there's no need to inject values into any of them. --- .../engine/extension/TempDirectory.java | 5 ++++- .../platform/commons/util/ReflectionUtils.java | 18 ++++++++++++++++++ .../TempDirectoryPerDeclarationTests.java | 14 ++++++++++++++ .../commons/util/ReflectionUtilsTests.java | 15 +++++++++++++++ 4 files changed, 51 insertions(+), 1 deletion(-) diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/TempDirectory.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/TempDirectory.java index b35dc58c20d8..357edb731c6e 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/TempDirectory.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/TempDirectory.java @@ -19,6 +19,7 @@ import static org.junit.platform.commons.support.AnnotationSupport.findAnnotatedFields; import static org.junit.platform.commons.support.AnnotationSupport.findAnnotation; import static org.junit.platform.commons.support.ReflectionSupport.makeAccessible; +import static org.junit.platform.commons.util.ReflectionUtils.isRecordObject; import java.io.File; import java.io.IOException; @@ -135,7 +136,9 @@ private void injectStaticFields(ExtensionContext context, Class testClass) { } private void injectInstanceFields(ExtensionContext context, Object instance) { - injectFields(context, instance, instance.getClass(), ModifierSupport::isNotStatic); + if (!isRecordObject(instance)) { + injectFields(context, instance, instance.getClass(), ModifierSupport::isNotStatic); + } } private void injectFields(ExtensionContext context, Object testInstance, Class testClass, 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 d0c9921d92e3..a413d11cc509 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 @@ -1993,6 +1993,24 @@ public static Set> getAllAssignmentCompatibleClasses(Class clazz) { return result; } + /** + * {@return whether the supplied {@code object} is an instance of a record class} + * @since 1.12 + */ + @API(status = INTERNAL, since = "1.12") + public static boolean isRecordObject(Object object) { + return object != null && isRecordClass(object.getClass()); + } + + /** + * {@return whether the supplied {@code clazz} is a record class} + * @since 1.12 + */ + @API(status = INTERNAL, since = "1.12") + public static boolean isRecordClass(Class clazz) { + return "java.lang.Record".equals(clazz.getSuperclass().getName()); + } + private static void getAllAssignmentCompatibleClasses(Class clazz, Set> result) { for (Class current = clazz; current != null; current = current.getSuperclass()) { result.add(current); diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryPerDeclarationTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryPerDeclarationTests.java index 2deb303768b9..5eddab66bcad 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryPerDeclarationTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryPerDeclarationTests.java @@ -164,6 +164,12 @@ void resolvesSeparateTempDirsForEachAnnotationDeclaration(TestInstance.Lifecycle assertThat(testATempDirs).doesNotContainEntry("afterEach2", testBTempDirs.get("afterEach2")); } + @Test + void supportsConstructorInjectionOnRecords() { + executeTestsForClass(TempDirRecordTestCase.class).testEvents()// + .assertStatistics(stats -> stats.started(1).succeeded(1)); + } + @Test @DisplayName("does not prevent constructor parameter resolution") void tempDirectoryDoesNotPreventConstructorParameterResolution() { @@ -1527,4 +1533,12 @@ void test(@SuppressWarnings("unused") @TempDir Path tempDir) { } + @SuppressWarnings("JUnitMalformedDeclaration") + record TempDirRecordTestCase(@TempDir Path tempDir) { + @Test + void shouldExists() { + assertTrue(Files.exists(tempDir)); + } + } + } 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 fb45c4835725..c88c91daca85 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 @@ -285,6 +285,21 @@ void getInterfaceMethodIfPossible() throws Exception { assertThat(interfaceMethod.getDeclaringClass()).isEqualTo(Closeable.class); } + @Test + void isRecordObject() { + assertTrue(ReflectionUtils.isRecordObject(new SomeRecord(1))); + assertFalse(ReflectionUtils.isRecordObject(new ClassWithOneCustomConstructor(""))); + } + + @Test + void isRecordClass() { + assertTrue(ReflectionUtils.isRecordClass(SomeRecord.class)); + assertFalse(ReflectionUtils.isRecordClass(ClassWithOneCustomConstructor.class)); + } + + record SomeRecord(int n) { + } + static class ClassWithVoidAndNonVoidMethods { void voidMethod() { From 6c020d50294c98fdee506eb768f5765761c9052e Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Mon, 27 Jan 2025 10:50:45 +0100 Subject: [PATCH 2/3] Move methods --- .../commons/util/ReflectionUtils.java | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) 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 a413d11cc509..a0b1c466c76f 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 @@ -365,6 +365,24 @@ public static boolean isInnerClass(Class clazz) { return !isStatic(clazz) && clazz.isMemberClass(); } + /** + * {@return whether the supplied {@code object} is an instance of a record class} + * @since 1.12 + */ + @API(status = INTERNAL, since = "1.12") + public static boolean isRecordObject(Object object) { + return object != null && isRecordClass(object.getClass()); + } + + /** + * {@return whether the supplied {@code clazz} is a record class} + * @since 1.12 + */ + @API(status = INTERNAL, since = "1.12") + public static boolean isRecordClass(Class clazz) { + return "java.lang.Record".equals(clazz.getSuperclass().getName()); + } + /** * Determine if the return type of the supplied method is primitive {@code void}. * @@ -1993,24 +2011,6 @@ public static Set> getAllAssignmentCompatibleClasses(Class clazz) { return result; } - /** - * {@return whether the supplied {@code object} is an instance of a record class} - * @since 1.12 - */ - @API(status = INTERNAL, since = "1.12") - public static boolean isRecordObject(Object object) { - return object != null && isRecordClass(object.getClass()); - } - - /** - * {@return whether the supplied {@code clazz} is a record class} - * @since 1.12 - */ - @API(status = INTERNAL, since = "1.12") - public static boolean isRecordClass(Class clazz) { - return "java.lang.Record".equals(clazz.getSuperclass().getName()); - } - private static void getAllAssignmentCompatibleClasses(Class clazz, Set> result) { for (Class current = clazz; current != null; current = current.getSuperclass()) { result.add(current); From 58cb54143017afb945b4b6e67f5f069f34a1d735 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Mon, 27 Jan 2025 10:53:01 +0100 Subject: [PATCH 3/3] Fix potential NPE --- .../java/org/junit/platform/commons/util/ReflectionUtils.java | 3 ++- .../org/junit/platform/commons/util/ReflectionUtilsTests.java | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) 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 a0b1c466c76f..01f0e89d0fd2 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 @@ -380,7 +380,8 @@ public static boolean isRecordObject(Object object) { */ @API(status = INTERNAL, since = "1.12") public static boolean isRecordClass(Class clazz) { - return "java.lang.Record".equals(clazz.getSuperclass().getName()); + Class superclass = clazz.getSuperclass(); + return superclass != null && "java.lang.Record".equals(superclass.getName()); } /** 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 c88c91daca85..6b89d7aaeb1d 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 @@ -289,12 +289,14 @@ void getInterfaceMethodIfPossible() throws Exception { void isRecordObject() { assertTrue(ReflectionUtils.isRecordObject(new SomeRecord(1))); assertFalse(ReflectionUtils.isRecordObject(new ClassWithOneCustomConstructor(""))); + assertFalse(ReflectionUtils.isRecordObject(null)); } @Test void isRecordClass() { assertTrue(ReflectionUtils.isRecordClass(SomeRecord.class)); assertFalse(ReflectionUtils.isRecordClass(ClassWithOneCustomConstructor.class)); + assertFalse(ReflectionUtils.isRecordClass(Object.class)); } record SomeRecord(int n) {