diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-RC1.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-RC1.adoc index 1b3a5f0a8fd7..3d53298ff989 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-RC1.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-RC1.adoc @@ -27,6 +27,10 @@ repository on GitHub. ==== New Features and Improvements * Prune stack traces up to test or lifecycle method +* Convention-based conversion in `ConversionSupport` now supports factory methods and + factory constructors that accept a single `CharSequence` argument in addition to the + existing support for factories that accept a single `String` argument. + [[release-notes-6.0.0-RC1-junit-jupiter]] === JUnit Jupiter @@ -52,6 +56,10 @@ repository on GitHub. In addition, special characters are escaped within quoted text. Please refer to the <<../user-guide/index.adoc#writing-tests-parameterized-tests-display-names-quoted-text, User Guide>> for details. +* <<../user-guide/index.adoc#writing-tests-parameterized-tests-argument-conversion-implicit-fallback, + Fallback String-to-Object Conversion>> for parameterized tests now supports factory + methods and factory constructors that accept a single `CharSequence` argument in + addition to the existing support for factories that accept a single `String` argument. [[release-notes-6.0.0-RC1-junit-vintage]] diff --git a/documentation/src/docs/asciidoc/user-guide/writing-tests.adoc b/documentation/src/docs/asciidoc/user-guide/writing-tests.adoc index d0d514e7678e..6252c77d010b 100644 --- a/documentation/src/docs/asciidoc/user-guide/writing-tests.adoc +++ b/documentation/src/docs/asciidoc/user-guide/writing-tests.adoc @@ -2519,11 +2519,12 @@ table, JUnit Jupiter also provides a fallback mechanism for automatic conversion method_ or a _factory constructor_ as defined below. - __factory method__: a non-private, `static` method declared in the target type that - accepts a single `String` argument and returns an instance of the target type. The name - of the method can be arbitrary and need not follow any particular convention. + accepts either a single `String` argument or a single `CharSequence` argument and + returns an instance of the target type. The name of the method can be arbitrary and need + not follow any particular convention. - __factory constructor__: a non-private constructor in the target type that accepts a - single `String` argument. Note that the target type must be declared as either a - top-level class or as a `static` nested class. + either a single `String` argument or a single `CharSequence` argument. Note that the + target type must be declared as either a top-level class or as a `static` nested class. NOTE: If multiple _factory methods_ are discovered, they will be ignored. If a _factory method_ and a _factory constructor_ are discovered, the factory method will be used diff --git a/junit-platform-commons/src/main/java/org/junit/platform/commons/support/conversion/ConversionSupport.java b/junit-platform-commons/src/main/java/org/junit/platform/commons/support/conversion/ConversionSupport.java index 34ed6ef458f4..ade3e248ca67 100644 --- a/junit-platform-commons/src/main/java/org/junit/platform/commons/support/conversion/ConversionSupport.java +++ b/junit-platform-commons/src/main/java/org/junit/platform/commons/support/conversion/ConversionSupport.java @@ -75,15 +75,20 @@ private ConversionSupport() { * *
    *
  1. Search for a single, non-private static factory method in the target - * type that converts from a String to the target type. Use the factory method - * if present.
    2. + * type that converts from a {@link String} to the target type. Use the + * factory method if present. *
  2. Search for a single, non-private constructor in the target type that - * accepts a String. Use the constructor if present.
    3. + * accepts a {@link String}. Use the constructor if present. + *
  3. Search for a single, non-private static factory method in the target + * type that converts from a {@link CharSequence} to the target type. Use the + * factory method if present.
    4. + *
  4. Search for a single, non-private constructor in the target type that + * accepts a {@link CharSequence}. Use the constructor if present.
    5. *
* - *

If multiple suitable factory methods are discovered they will be ignored. - * If neither a single factory method nor a single constructor is found, the - * convention-based conversion strategy will not apply. + *

If multiple suitable factory methods or constructors are discovered they + * will be ignored. If neither a single factory method nor a single constructor + * is found, the convention-based conversion strategy will not apply. * * @param source the source {@code String} to convert; may be {@code null} * but only if the target type is a reference type diff --git a/junit-platform-commons/src/main/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverter.java b/junit-platform-commons/src/main/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverter.java index 916406e3fbcb..80be72128211 100644 --- a/junit-platform-commons/src/main/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverter.java +++ b/junit-platform-commons/src/main/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverter.java @@ -38,16 +38,21 @@ *

Search Algorithm

* *
    - *
  1. Search for a single, non-private static factory method in the target - * type that converts from a String to the target type. Use the factory method + *
  2. Search for a single, non-private static factory method in the target type + * that converts from a {@link String} to the target type. Use the factory method * if present.
    3. - *
  3. Search for a single, non-private constructor in the target type that - * accepts a String. Use the constructor if present.
    4. + *
  4. Search for a single, non-private constructor in the target type that accepts + * a {@link String}. Use the constructor if present.
    5. + *
  5. Search for a single, non-private static factory method in the target type + * that converts from a {@link CharSequence} to the target type. Use the factory + * method if present.
    6. + *
  6. Search for a single, non-private constructor in the target type that accepts + * a {@link CharSequence}. Use the constructor if present.
    7. *
* - *

If multiple suitable factory methods are discovered they will be ignored. - * If neither a single factory method nor a single constructor is found, this - * converter acts as a no-op. + *

If multiple suitable factory methods or constructors are discovered they + * will be ignored. If neither a single factory method nor a single constructor + * is found, this converter acts as a no-op. * * @since 1.11 * @see ConversionSupport @@ -86,28 +91,47 @@ public boolean canConvertTo(Class targetType) { private static Function findFactoryExecutable(Class targetType) { return factoryExecutableCache.computeIfAbsent(targetType, type -> { - Method factoryMethod = findFactoryMethod(type); - if (factoryMethod != null) { - return source -> invokeMethod(factoryMethod, null, source); + // First, search for exact String argument matches. + Function factory = findFactoryExecutable(type, String.class); + if (factory != null) { + return factory; } - Constructor constructor = findFactoryConstructor(type); - if (constructor != null) { - return source -> newInstance(constructor, source); + // Second, fall back to CharSequence argument matches. + factory = findFactoryExecutable(type, CharSequence.class); + if (factory != null) { + return factory; } + // Else, nothing found. return NULL_EXECUTABLE; }); } - private static @Nullable Method findFactoryMethod(Class targetType) { - List factoryMethods = findMethods(targetType, new IsFactoryMethod(targetType), BOTTOM_UP); + private static @Nullable Function findFactoryExecutable(Class targetType, + Class parameterType) { + + Method factoryMethod = findFactoryMethod(targetType, parameterType); + if (factoryMethod != null) { + return source -> invokeMethod(factoryMethod, null, source); + } + Constructor constructor = findFactoryConstructor(targetType, parameterType); + if (constructor != null) { + return source -> newInstance(constructor, source); + } + return null; + } + + private static @Nullable Method findFactoryMethod(Class targetType, Class parameterType) { + List factoryMethods = findMethods(targetType, new IsFactoryMethod(targetType, parameterType), + BOTTOM_UP); if (factoryMethods.size() == 1) { return factoryMethods.get(0); } return null; } - private static @Nullable Constructor findFactoryConstructor(Class targetType) { - List> constructors = findConstructors(targetType, new IsFactoryConstructor(targetType)); + private static @Nullable Constructor findFactoryConstructor(Class targetType, Class parameterType) { + List> constructors = findConstructors(targetType, + new IsFactoryConstructor(targetType, parameterType)); if (constructors.size() == 1) { return constructors.get(0); } @@ -117,15 +141,9 @@ public boolean canConvertTo(Class targetType) { /** * {@link Predicate} that determines if the {@link Method} supplied to * {@link #test(Method)} is a non-private static factory method for the - * supplied {@link #targetType}. + * supplied {@link #targetType} and {@link #parameterType}. */ - static class IsFactoryMethod implements Predicate { - - private final Class targetType; - - IsFactoryMethod(Class targetType) { - this.targetType = targetType; - } + record IsFactoryMethod(Class targetType, Class parameterType) implements Predicate { @Override public boolean test(Method method) { @@ -136,23 +154,16 @@ public boolean test(Method method) { if (isNotStatic(method)) { return false; } - return isNotPrivateAndAcceptsSingleStringArgument(method); + return isFactoryCandidate(method, this.parameterType); } - } /** * {@link Predicate} that determines if the {@link Constructor} supplied to * {@link #test(Constructor)} is a non-private factory constructor for the - * supplied {@link #targetType}. + * supplied {@link #targetType} and {@link #parameterType}. */ - static class IsFactoryConstructor implements Predicate> { - - private final Class targetType; - - IsFactoryConstructor(Class targetType) { - this.targetType = targetType; - } + record IsFactoryConstructor(Class targetType, Class parameterType) implements Predicate> { @Override public boolean test(Constructor constructor) { @@ -160,15 +171,18 @@ public boolean test(Constructor constructor) { if (!constructor.getDeclaringClass().equals(this.targetType)) { return false; } - return isNotPrivateAndAcceptsSingleStringArgument(constructor); + return isFactoryCandidate(constructor, this.parameterType); } - } - private static boolean isNotPrivateAndAcceptsSingleStringArgument(Executable executable) { + /** + * Determine if the supplied {@link Executable} is not private and accepts a + * single argument of the supplied parameter type. + */ + private static boolean isFactoryCandidate(Executable executable, Class parameterType) { return isNotPrivate(executable) // && (executable.getParameterCount() == 1) // - && (executable.getParameterTypes()[0] == String.class); + && (executable.getParameterTypes()[0] == parameterType); } } diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/params/ParameterizedTestIntegrationTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/params/ParameterizedTestIntegrationTests.java index 6eea54d48e21..cb7fda26c9ff 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/params/ParameterizedTestIntegrationTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/params/ParameterizedTestIntegrationTests.java @@ -311,6 +311,17 @@ void executesWithImplicitGenericConverter() { event(test(), displayName("[2] book = book 2"), finishedWithFailure(message("book 2")))); } + /** + * @since 6.0 + */ + @Test + void executesWithImplicitGenericConverterWithCharSequenceConstructor() { + var results = execute("testWithImplicitGenericConverterWithCharSequenceConstructor", Record.class); + results.testEvents().assertThatEvents() // + .haveExactly(1, event(displayName("\"record 1\""), finishedWithFailure(message("record 1")))) // + .haveExactly(1, event(displayName("\"record 2\""), finishedWithFailure(message("record 2")))); + } + @Test void legacyReportingNames() { var results = execute("testWithCustomName", String.class, int.class); @@ -1460,6 +1471,12 @@ void testWithImplicitGenericConverter(Book book) { fail(book.title); } + @ParameterizedTest(name = "{0}") + @ValueSource(strings = { "record 1", "record 2" }) + void testWithImplicitGenericConverterWithCharSequenceConstructor(Record record) { + fail(record.title.toString()); + } + @ParameterizedTest(quoteTextArguments = false) @ValueSource(strings = { "O", "XXX" }) void testWithExplicitConverter(@ConvertWith(StringLengthConverter.class) int length) { @@ -2673,6 +2690,9 @@ static Book factory(String title) { } } + record Record(CharSequence title) { + } + static class FailureInBeforeEachTestCase { @BeforeEach diff --git a/platform-tests/src/test/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverterTests.java b/platform-tests/src/test/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverterTests.java index 5bfc428ec8d7..64b028cb6808 100644 --- a/platform-tests/src/test/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverterTests.java +++ b/platform-tests/src/test/java/org/junit/platform/commons/support/conversion/FallbackStringToObjectConverterTests.java @@ -12,6 +12,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.platform.commons.support.ReflectionSupport.findMethod; +import static org.junit.platform.commons.util.ReflectionUtils.getDeclaredConstructor; import java.lang.reflect.Constructor; import java.lang.reflect.Method; @@ -31,13 +32,16 @@ */ class FallbackStringToObjectConverterTests { - private static final IsFactoryMethod isBookFactoryMethod = new IsFactoryMethod(Book.class); + private static final IsFactoryMethod isBookFactoryMethod = new IsFactoryMethod(Book.class, String.class); private static final FallbackStringToObjectConverter converter = new FallbackStringToObjectConverter(); @Test void isNotFactoryMethodForWrongParameterType() { assertThat(isBookFactoryMethod).rejects(bookMethod("factory", Object.class)); + assertThat(isBookFactoryMethod).rejects(bookMethod("factory", Number.class)); + assertThat(isBookFactoryMethod).rejects(bookMethod("factory", StringBuilder.class)); + assertThat(new IsFactoryMethod(Record2.class, String.class)).rejects(record2Method("from")); } @Test @@ -52,26 +56,53 @@ void isNotFactoryMethodForNonStaticMethod() { @Test void isFactoryMethodForValidMethods() { - assertThat(isBookFactoryMethod).accepts(bookMethod("factory")); - assertThat(new IsFactoryMethod(Newspaper.class)).accepts(newspaperMethod("from"), newspaperMethod("of")); - assertThat(new IsFactoryMethod(Magazine.class)).accepts(magazineMethod("from"), magazineMethod("of")); + assertThat(new IsFactoryMethod(Book.class, String.class))// + .accepts(bookMethod("factory", String.class)); + assertThat(new IsFactoryMethod(Book.class, CharSequence.class))// + .accepts(bookMethod("factory", CharSequence.class)); + assertThat(new IsFactoryMethod(Newspaper.class, String.class))// + .accepts(newspaperMethod("from"), newspaperMethod("of")); + assertThat(new IsFactoryMethod(Magazine.class, String.class))// + .accepts(magazineMethod("from"), magazineMethod("of")); + assertThat(new IsFactoryMethod(Record2.class, CharSequence.class))// + .accepts(record2Method("from")); } @Test void isNotFactoryConstructorForPrivateConstructor() { - assertThat(new IsFactoryConstructor(Magazine.class)).rejects(constructor(Magazine.class)); + assertThat(new IsFactoryConstructor(Magazine.class, String.class)).rejects(constructor(Magazine.class)); + } + + @Test + void isNotFactoryConstructorForWrongParameterType() { + assertThat(new IsFactoryConstructor(Record1.class, String.class))// + .rejects(getDeclaredConstructor(Record1.class)); + assertThat(new IsFactoryConstructor(Record2.class, String.class))// + .rejects(getDeclaredConstructor(Record2.class)); } @Test void isFactoryConstructorForValidConstructors() { - assertThat(new IsFactoryConstructor(Book.class)).accepts(constructor(Book.class)); - assertThat(new IsFactoryConstructor(Journal.class)).accepts(constructor(Journal.class)); - assertThat(new IsFactoryConstructor(Newspaper.class)).accepts(constructor(Newspaper.class)); + assertThat(new IsFactoryConstructor(Book.class, String.class))// + .accepts(constructor(Book.class)); + assertThat(new IsFactoryConstructor(Journal.class, String.class))// + .accepts(constructor(Journal.class)); + assertThat(new IsFactoryConstructor(Newspaper.class, String.class))// + .accepts(constructor(Newspaper.class)); + assertThat(new IsFactoryConstructor(Record1.class, CharSequence.class))// + .accepts(getDeclaredConstructor(Record1.class)); + assertThat(new IsFactoryConstructor(Record2.class, CharSequence.class))// + .accepts(getDeclaredConstructor(Record2.class)); } @Test void convertsStringToBookViaStaticFactoryMethod() throws Exception { - assertConverts("enigma", Book.class, Book.factory("enigma")); + assertConverts("enigma", Book.class, new Book("factory(String): enigma")); + } + + @Test + void convertsStringToRecord2ViaStaticFactoryMethodAcceptingCharSequence() throws Exception { + assertConvertsRecord2("enigma", Record2.from(new StringBuffer("enigma"))); } @Test @@ -79,6 +110,11 @@ void convertsStringToJournalViaFactoryConstructor() throws Exception { assertConverts("enigma", Journal.class, new Journal("enigma")); } + @Test + void convertsStringToRecord1ViaFactoryConstructorAcceptingCharSequence() throws Exception { + assertConvertsRecord1("enigma", new Record1(new StringBuffer("enigma"))); + } + @Test void convertsStringToNewspaperViaConstructorIgnoringMultipleFactoryMethods() throws Exception { assertConverts("enigma", Newspaper.class, new Newspaper("enigma")); @@ -119,16 +155,43 @@ private static Method magazineMethod(String methodName) { return findMethod(Magazine.class, methodName, String.class).orElseThrow(); } + private static Method record2Method(String methodName) { + return findMethod(Record2.class, methodName, CharSequence.class).orElseThrow(); + } + private static void assertConverts(String input, Class targetType, Object expectedOutput) throws Exception { - assertThat(converter.canConvertTo(targetType)).isTrue(); + assertCanConvertTo(targetType); var result = converter.convert(input, targetType); assertThat(result) // - .describedAs(input + " --(" + targetType.getName() + ")--> " + expectedOutput) // + .as(input + " (" + targetType.getSimpleName() + ") --> " + expectedOutput) // .isEqualTo(expectedOutput); } + private static void assertConvertsRecord1(String input, Record1 expected) throws Exception { + Class targetType = Record1.class; + assertCanConvertTo(targetType); + + Record1 result = (Record1) converter.convert(input, targetType); + + assertThat(result).isNotNull(); + assertThat(result.title.toString()).isEqualTo(expected.title.toString()); + } + + private static void assertConvertsRecord2(String input, Record2 expected) throws Exception { + Class targetType = Record2.class; + assertCanConvertTo(targetType); + + var result = converter.convert(input, targetType); + + assertThat(result).isEqualTo(expected); + } + + private static void assertCanConvertTo(Class targetType) { + assertThat(converter.canConvertTo(targetType)).as("canConvertTo(%s)", targetType.getSimpleName()).isTrue(); + } + static class Book { private final String title; @@ -139,12 +202,35 @@ static class Book { // static and non-private static Book factory(String title) { - return new Book(title); + return new Book("factory(String): " + title); + } + + /** + * Static and non-private, but intentionally overloads {@link #factory(String)} + * with a {@link CharSequence} argument to ensure that we don't introduce a + * regression in 6.0, since the String-based factory method should take + * precedence over a CharSequence-based factory method. + */ + static Book factory(CharSequence title) { + return new Book("factory(CharSequence): " + title); } // wrong parameter type static Book factory(Object obj) { - return new Book(String.valueOf(obj)); + throw new UnsupportedOperationException(); + } + + // wrong parameter type + static Book factory(Number number) { + throw new UnsupportedOperationException(); + } + + /** + * Wrong parameter type, intentionally a subtype of {@link CharSequence} + * other than {@link String}. + */ + static Book factory(StringBuilder builder) { + throw new UnsupportedOperationException(); } @SuppressWarnings("unused") @@ -166,6 +252,10 @@ public int hashCode() { return Objects.hash(title); } + @Override + public String toString() { + return "Book [title=" + this.title + "]"; + } } static class Journal { @@ -176,6 +266,16 @@ static class Journal { this.title = title; } + /** + * Intentionally overloads {@link #Journal(String)} with a {@link CharSequence} + * argument to ensure that we don't introduce a regression in 6.0, since the + * String-based constructor should take precedence over a CharSequence-based + * constructor. + */ + Journal(CharSequence title) { + this("Journal(CharSequence): " + title); + } + @Override public boolean equals(Object obj) { return (this == obj) || (obj instanceof Journal that && Objects.equals(this.title, that.title)); @@ -186,6 +286,10 @@ public int hashCode() { return Objects.hash(title); } + @Override + public String toString() { + return "Journal [title=" + this.title + "]"; + } } static class Newspaper { @@ -214,6 +318,10 @@ public int hashCode() { return Objects.hash(title); } + @Override + public String toString() { + return "Newspaper [title=" + this.title + "]"; + } } static class Magazine { @@ -231,6 +339,16 @@ static Magazine of(String title) { } + record Record1(CharSequence title) { + } + + record Record2(CharSequence title) { + + static Record2 from(CharSequence title) { + return new Record2("Record2(CharSequence): " + title); + } + } + static class Diary { }