From 82f1f4be2ff43b8d445f970ae8de659cab2022d5 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sat, 22 Apr 2023 12:20:31 +0200 Subject: [PATCH] Treat overloaded local & external @MethodSource factory methods equally Prior to this commit, given overloaded @MethodSource factory methods, a local qualified method name (LQMN) was not treated the same as a fully-qualified method name (FQMN). Specifically, if the user provided a FQMN without specifying the parameter list, JUnit Jupiter would find the factory method that accepts zero arguments. Whereas, if the user provided an LQMN without specifying the parameter list, JUnit Jupiter would fail to find the factory method. This commit fixes that bug by reworking the internals of MethodArgumentsProvider so that overloaded local and external factory methods are looked up with the same semantics. The commit also improves diagnostics for failure to find a factory method specified via LQMN by falling back to the same lenient search semantics that are used to locate a "default" local factory method. Furthermore, this commit modifies the internals of MethodArgumentsProvider so that it consistently throws PreconditionViolationExceptions for user configuration errors. This commit also introduces additional tests for error use cases. See: #3130, #3131 Closes: #3266 --- .../release-notes/release-notes-5.9.3.adoc | 8 +- .../provider/MethodArgumentsProvider.java | 113 ++++--- .../jupiter/params/provider/MethodSource.java | 16 +- .../MethodArgumentsProviderTests.java | 288 +++++++++++++----- 4 files changed, 300 insertions(+), 125 deletions(-) diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-5.9.3.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-5.9.3.adoc index 8e4f71799f9b..cb4b0f1ec7a2 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-5.9.3.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-5.9.3.adoc @@ -40,6 +40,9 @@ JUnit repository on GitHub. `@MethodSource("myFactory([I)"` (which was already supported) and `@MethodSource("myFactory(java.lang.String[])` instead of `@MethodSource("myFactory([Ljava.lang.String;)`. +* The search algorithm used to find `@MethodSource` factory methods now applies consistent + semantics for _local_ qualified method names and fully-qualified method names for + overloaded factory methods. * Exceptions thrown for files that cannot be deleted when cleaning up a temporary directory created via `@TempDir` now include the root cause. * Lifecycle methods are allowed to be declared as `private` again for backwards @@ -52,7 +55,10 @@ JUnit repository on GitHub. ==== New Features and Improvements -* ❓ +* The search algorithm used to find `@MethodSource` factory methods now falls back to + lenient search semantics when a factory method cannot be found by qualified name + (without a parameter list) and also provides better diagnostics when a unique factory + method cannot be found. [[release-notes-5.9.3-junit-vintage]] diff --git a/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodArgumentsProvider.java b/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodArgumentsProvider.java index 638f33b19bb5..d9fdffab336f 100644 --- a/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodArgumentsProvider.java +++ b/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodArgumentsProvider.java @@ -28,6 +28,7 @@ import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.params.support.AnnotationConsumer; import org.junit.platform.commons.JUnitException; +import org.junit.platform.commons.PreconditionViolationException; import org.junit.platform.commons.util.CollectionUtils; import org.junit.platform.commons.util.Preconditions; import org.junit.platform.commons.util.ReflectionUtils; @@ -40,6 +41,9 @@ class MethodArgumentsProvider implements ArgumentsProvider, AnnotationConsumer isFactoryMethod = // + method -> isConvertibleToStream(method.getReturnType()) && !isTestMethod(method); + @Override public void accept(MethodSource annotation) { this.methodNames = annotation.value(); @@ -52,28 +56,37 @@ public Stream provideArguments(ExtensionContext context) { Object testInstance = context.getTestInstance().orElse(null); // @formatter:off return stream(this.methodNames) - .map(factoryMethodName -> getFactoryMethod(testClass, testMethod, factoryMethodName)) + .map(factoryMethodName -> findFactoryMethod(testClass, testMethod, factoryMethodName)) .map(factoryMethod -> context.getExecutableInvoker().invoke(factoryMethod, testInstance)) .flatMap(CollectionUtils::toStream) .map(MethodArgumentsProvider::toArguments); // @formatter:on } - private Method getFactoryMethod(Class testClass, Method testMethod, String factoryMethodName) { - if (!StringUtils.isBlank(factoryMethodName)) { - if (looksLikeAFullyQualifiedMethodName(factoryMethodName)) { - return getFactoryMethodByFullyQualifiedName(factoryMethodName); - } - else if (looksLikeALocalQualifiedMethodName(factoryMethodName)) { - return getFactoryMethodByFullyQualifiedName(testClass.getName() + "#" + factoryMethodName); - } - } - else { - // User did not provide a factory method name, so we search for a - // factory method with the same name as the parameterized test method. + private static Method findFactoryMethod(Class testClass, Method testMethod, String factoryMethodName) { + String originalFactoryMethodName = factoryMethodName; + + // If the user did not provide a factory method name, find a "default" local + // factory method with the same name as the parameterized test method. + if (StringUtils.isBlank(factoryMethodName)) { factoryMethodName = testMethod.getName(); + return findFactoryMethodBySimpleName(testClass, testMethod, factoryMethodName); } - return findFactoryMethodBySimpleName(testClass, testMethod, factoryMethodName); + + // Convert local factory method name to fully-qualified method name. + if (!looksLikeAFullyQualifiedMethodName(factoryMethodName)) { + factoryMethodName = testClass.getName() + "#" + factoryMethodName; + } + + // Find factory method using fully-qualified name. + Method factoryMethod = findFactoryMethodByFullyQualifiedName(testMethod, factoryMethodName); + + // Ensure factory method has a valid return type and is not a test method. + Preconditions.condition(isFactoryMethod.test(factoryMethod), () -> format( + "Could not find valid factory method [%s] for test class [%s] but found the following invalid candidate: %s", + originalFactoryMethodName, testClass.getName(), factoryMethod)); + + return factoryMethod; } private static boolean looksLikeAFullyQualifiedMethodName(String factoryMethodName) { @@ -90,52 +103,54 @@ private static boolean looksLikeAFullyQualifiedMethodName(String factoryMethodNa return indexOfDot < indexOfOpeningParenthesis; } // If we get this far, we conclude the supplied factory method name "looks" - // like it was intended to be a fully qualified method name, even if the + // like it was intended to be a fully-qualified method name, even if the // syntax is invalid. We do this in order to provide better diagnostics for - // the user when a fully qualified method name is in fact invalid. + // the user when a fully-qualified method name is in fact invalid. return true; } - private static boolean looksLikeALocalQualifiedMethodName(String factoryMethodName) { - // This method is intended to be called after looksLikeAFullyQualifiedMethodName() - // and therefore does not check for the absence of '#' and does not reason about - // the presence or absence of a fully qualified class name. - if (factoryMethodName.endsWith("()")) { - return true; - } - int indexOfLastOpeningParenthesis = factoryMethodName.lastIndexOf('('); - return (indexOfLastOpeningParenthesis > 0) - && (indexOfLastOpeningParenthesis < factoryMethodName.lastIndexOf(')')); - } - - private Method getFactoryMethodByFullyQualifiedName(String fullyQualifiedMethodName) { + private static Method findFactoryMethodByFullyQualifiedName(Method testMethod, String fullyQualifiedMethodName) { String[] methodParts = ReflectionUtils.parseFullyQualifiedMethodName(fullyQualifiedMethodName); String className = methodParts[0]; String methodName = methodParts[1]; String methodParameters = methodParts[2]; + Class clazz = loadRequiredClass(className); + + // Attempt to find an exact match first. + Method factoryMethod = ReflectionUtils.findMethod(clazz, methodName, methodParameters).orElse(null); + if (factoryMethod != null) { + return factoryMethod; + } + + boolean explicitParameterListSpecified = // + StringUtils.isNotBlank(methodParameters) || fullyQualifiedMethodName.endsWith("()"); + + // If we didn't find an exact match but an explicit parameter list was specified, + // that's a user configuration error. + Preconditions.condition(!explicitParameterListSpecified, + () -> format("Could not find factory method [%s(%s)] in class [%s]", methodName, methodParameters, + className)); - return ReflectionUtils.findMethod(loadRequiredClass(className), methodName, methodParameters).orElseThrow( - () -> new JUnitException(format("Could not find factory method [%s(%s)] in class [%s]", methodName, - methodParameters, className))); + // Otherwise, fall back to the same lenient search semantics that are used + // to locate a "default" local factory method. + return findFactoryMethodBySimpleName(clazz, testMethod, methodName); } /** - * Find all methods in the given {@code testClass} with the desired {@code factoryMethodName} - * which have return types that can be converted to a {@link Stream}, ignoring the - * {@code testMethod} itself as well as any {@code @Test}, {@code @TestTemplate}, - * or {@code @TestFactory} methods with the same name. - * @return the factory method, if found - * @throws org.junit.platform.commons.PreconditionViolationException if the - * factory method was not found or if multiple competing factory methods with - * the same name were found + * Find the factory method by searching for all methods in the given {@code clazz} + * with the desired {@code factoryMethodName} which have return types that can be + * converted to a {@link Stream}, ignoring the {@code testMethod} itself as well + * as any {@code @Test}, {@code @TestTemplate}, or {@code @TestFactory} methods + * with the same name. + * @return the single factory method matching the search criteria + * @throws PreconditionViolationException if the factory method was not found or + * multiple competing factory methods with the same name were found */ - private Method findFactoryMethodBySimpleName(Class testClass, Method testMethod, String factoryMethodName) { + private static Method findFactoryMethodBySimpleName(Class clazz, Method testMethod, String factoryMethodName) { Predicate isCandidate = candidate -> factoryMethodName.equals(candidate.getName()) && !testMethod.equals(candidate); - List candidates = ReflectionUtils.findMethods(testClass, isCandidate); + List candidates = ReflectionUtils.findMethods(clazz, isCandidate); - Predicate isFactoryMethod = method -> isConvertibleToStream(method.getReturnType()) - && !isTestMethod(method); List factoryMethods = candidates.stream().filter(isFactoryMethod).collect(toList()); Preconditions.condition(factoryMethods.size() > 0, () -> { @@ -145,23 +160,23 @@ private Method findFactoryMethodBySimpleName(Class testClass, Method testMeth if (candidates.size() > 0) { return format( "Could not find valid factory method [%s] in class [%s] but found the following invalid candidates: %s", - factoryMethodName, testClass.getName(), candidates); + factoryMethodName, clazz.getName(), candidates); } // Otherwise, report that we didn't find anything. - return format("Could not find factory method [%s] in class [%s]", factoryMethodName, testClass.getName()); + return format("Could not find factory method [%s] in class [%s]", factoryMethodName, clazz.getName()); }); Preconditions.condition(factoryMethods.size() == 1, () -> format("%d factory methods named [%s] were found in class [%s]: %s", factoryMethods.size(), - factoryMethodName, testClass.getName(), factoryMethods)); + factoryMethodName, clazz.getName(), factoryMethods)); return factoryMethods.get(0); } - private boolean isTestMethod(Method candidate) { + private static boolean isTestMethod(Method candidate) { return isAnnotated(candidate, Test.class) || isAnnotated(candidate, TestTemplate.class) || isAnnotated(candidate, TestFactory.class); } - private Class loadRequiredClass(String className) { + private static Class loadRequiredClass(String className) { return ReflectionUtils.tryToLoadClass(className).getOrThrow( cause -> new JUnitException(format("Could not load class [%s]", className), cause)); } diff --git a/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodSource.java b/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodSource.java index 2d0ffe9eb8a5..94f6225fd53f 100644 --- a/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodSource.java +++ b/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodSource.java @@ -110,12 +110,20 @@ * The names of factory methods within the test class or in external classes * to use as sources for arguments. * - *

Factory methods in external classes must be referenced by fully - * qualified method name — for example, - * {@code com.example.StringsProviders#blankStrings} or - * {@code com.example.TopLevelClass$NestedClass#classMethod} for a factory + *

Factory methods in external classes must be referenced by + * fully-qualified method name — for example, + * {@code "com.example.StringsProviders#blankStrings"} or + * {@code "com.example.TopLevelClass$NestedClass#classMethod"} for a factory * method in a static nested class. * + *

If a factory method accepts arguments that are provided by a + * {@link org.junit.jupiter.api.extension.ParameterResolver ParameterResolver}, + * you can supply the formal parameter list in the qualified method name to + * disambiguate between overloaded variants of the factory method. For example, + * {@code "blankStrings(int)"} for a local qualified method name or + * {@code "com.example.StringsProviders#blankStrings(int)"} for a fully-qualified + * method name. + * *

If no factory method names are declared, a method within the test class * that has the same name as the test method will be used as the factory * method by default. diff --git a/junit-jupiter-params/src/test/java/org/junit/jupiter/params/provider/MethodArgumentsProviderTests.java b/junit-jupiter-params/src/test/java/org/junit/jupiter/params/provider/MethodArgumentsProviderTests.java index 281670a4435f..1bd49479bb06 100644 --- a/junit-jupiter-params/src/test/java/org/junit/jupiter/params/provider/MethodArgumentsProviderTests.java +++ b/junit-jupiter-params/src/test/java/org/junit/jupiter/params/provider/MethodArgumentsProviderTests.java @@ -13,7 +13,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.engine.extension.MutableExtensionRegistry.createRegistryWithDefaultExtensions; -import static org.junit.jupiter.params.provider.MethodArgumentsProviderTests.DefaultFactoryMethodNameTestCase.TEST_METHOD; import static org.junit.platform.commons.util.ReflectionUtils.findMethod; import static org.mockito.Mockito.doCallRealMethod; import static org.mockito.Mockito.mock; @@ -50,26 +49,6 @@ class MethodArgumentsProviderTests { private MutableExtensionRegistry extensionRegistry; - @Test - void throwsExceptionWhenFactoryMethodDoesNotExist() { - var exception = assertThrows(JUnitException.class, () -> provideArguments("unknownMethod").toArray()); - - assertThat(exception.getMessage()).isEqualTo( - "Could not find factory method [unknownMethod] in class [" + TestCase.class.getName() + "]"); - } - - @Test - void throwsExceptionForIllegalReturnType() { - var exception = assertThrows(PreconditionViolationException.class, - () -> provideArguments("providerWithIllegalReturnType").toArray()); - - assertThat(exception.getMessage())// - .containsSubsequence("Could not find valid factory method [providerWithIllegalReturnType] in class [", - TestCase.class.getName() + "]", // - "but found the following invalid candidates: ", // - "[static java.lang.Object org.junit.jupiter.params.provider.MethodArgumentsProviderTests$TestCase.providerWithIllegalReturnType()]"); - } - @Test void providesArgumentsUsingStream() { var arguments = provideArguments("stringStreamProvider"); @@ -195,7 +174,7 @@ void providesArgumentsUsingListOfObjectArrays() { @Test void throwsExceptionWhenNonStaticFactoryMethodIsReferencedAndStaticIsRequired() { - var exception = assertThrows(JUnitException.class, + var exception = assertThrows(PreconditionViolationException.class, () -> provideArguments(NonStaticTestCase.class, null, false, "nonStaticStringStreamProvider").toArray()); assertThat(exception).hasMessageContaining("Cannot invoke non-static method"); @@ -210,8 +189,10 @@ void providesArgumentsFromNonStaticFactoryMethodWhenStaticIsNotRequired() { @Test void providesArgumentsUsingDefaultFactoryMethodName() { - Class testClass = DefaultFactoryMethodNameTestCase.class; - var testMethod = findMethod(testClass, TEST_METHOD, String.class).get(); + var testClass = DefaultFactoryMethodNameTestCase.class; + var methodName = "testDefaultFactoryMethodName"; + var testMethod = findMethod(testClass, methodName, String.class).get(); + var arguments = provideArguments(testClass, testMethod, false, ""); assertThat(arguments).containsExactly(array("foo"), array("bar")); @@ -246,35 +227,6 @@ void providesArgumentsUsingExternalAndInternalFactoryMethodsCombined() { assertThat(arguments).containsExactly(array("foo"), array("bar"), array("string1"), array("string2")); } - @Test - void throwsExceptionWhenClassForExternalFactoryMethodCannotBeLoaded() { - var exception = assertThrows(JUnitException.class, - () -> provideArguments("com.example.NonExistentExternalFactoryMethods#stringsProvider").toArray()); - - assertThat(exception.getMessage()).isEqualTo( - "Could not load class [com.example.NonExistentExternalFactoryMethods]"); - } - - @Test - void throwsExceptionWhenExternalFactoryMethodCannotBeFound() { - var exception = assertThrows(JUnitException.class, - () -> provideArguments(ExternalFactoryMethods.class.getName() + "#nonExistentMethod").toArray()); - - assertThat(exception.getMessage()).isEqualTo("Could not find factory method [nonExistentMethod()] in class [" - + ExternalFactoryMethods.class.getName() + "]"); - } - - @Test - void throwsExceptionWhenFullyQualifiedMethodNameIsInvalid() { - var exception = assertThrows(JUnitException.class, - () -> provideArguments(ExternalFactoryMethods.class.getName() + ".wrongSyntax").toArray()); - - assertThat(exception.getMessage()).isEqualTo( - "[" + ExternalFactoryMethods.class.getName() + ".wrongSyntax] is not a valid fully qualified method name: " - + "it must start with a fully qualified class name followed by a '#' and then the method name, " - + "optionally followed by a parameter list enclosed in parentheses."); - } - @Nested class PrimitiveArrays { @@ -391,6 +343,13 @@ void providesArgumentsUsingSimpleNameForFactoryMethodThatAcceptsArgumentWithoutS assertThat(arguments).containsExactly(array("foo!"), array("bar!")); } + @Test + void providesArgumentsUsingFullyQualifiedNameForFactoryMethodThatAcceptsArgumentWithoutSpecifyingParameterList() { + var arguments = provideArguments(TestCase.class.getName() + "#stringStreamProviderWithParameter"); + + assertThat(arguments).containsExactly(array("foo!"), array("bar!")); + } + @Test void providesArgumentsUsingFullyQualifiedNameSpecifyingParameter() { var arguments = provideArguments( @@ -461,7 +420,8 @@ in class [org.junit.jupiter.params.provider.MethodArgumentsProviderTests$TestCas @Test void failsToProvideArgumentsUsingFullyQualifiedNameSpecifyingIncorrectParameterType() { String method = TestCase.class.getName() + "#stringStreamProviderWithParameter(java.lang.Integer)"; - var exception = assertThrows(JUnitException.class, () -> provideArguments(method).toArray()); + var exception = assertThrows(PreconditionViolationException.class, + () -> provideArguments(method).toArray()); assertThat(exception).hasMessage(""" Could not find factory method [stringStreamProviderWithParameter(java.lang.Integer)] in \ @@ -471,7 +431,7 @@ class [org.junit.jupiter.params.provider.MethodArgumentsProviderTests$TestCase]" @Test void failsToProvideArgumentsUsingLocalQualifiedNameSpecifyingIncorrectParameterType() { var method = "stringStreamProviderWithParameter(java.lang.Integer)"; - var exception = assertThrows(JUnitException.class, + var exception = assertThrows(PreconditionViolationException.class, () -> provideArguments(this.testMethod, method).toArray()); assertThat(exception).hasMessage(""" @@ -539,11 +499,6 @@ void providesArgumentsUsingLocalQualifiedNameSpecifyingMultipleParameters(String assertThat(arguments).containsExactly(array("foo!!"), array("bar!!")); } - /** - * In contrast to {@link #failsToProvideArgumentsUsingLocalQualifiedNameForOverloadedFactoryMethodWhenParameterListIsNotSpecified()}, - * using the fully qualified method name without specifying the parameter list "selects" - * the overloaded method that accepts zero arguments. - */ @Test void providesArgumentsUsingFullyQualifiedNameForOverloadedFactoryMethodWhenParameterListIsNotSpecified() { var arguments = provideArguments(TestCase.class.getName() + "#stringStreamProviderWithOrWithoutParameter"); @@ -552,16 +507,160 @@ void providesArgumentsUsingFullyQualifiedNameForOverloadedFactoryMethodWhenParam } @Test - void failsToProvideArgumentsUsingLocalQualifiedNameForOverloadedFactoryMethodWhenParameterListIsNotSpecified() { + void providesArgumentsUsingLocalQualifiedNameForOverloadedFactoryMethodWhenParameterListIsNotSpecified() { + var arguments = provideArguments("stringStreamProviderWithOrWithoutParameter").toArray(); + + assertThat(arguments).containsExactly(array("foo"), array("bar")); + } + + } + + @Nested + class ErrorCases { + + @Test + void throwsExceptionWhenFullyQualifiedMethodNameSyntaxIsInvalid() { + var exception = assertThrows(PreconditionViolationException.class, + () -> provideArguments("org.example.wrongSyntax").toArray()); + + assertThat(exception.getMessage()).isEqualTo( + "[org.example.wrongSyntax] is not a valid fully qualified method name: " + + "it must start with a fully qualified class name followed by a '#' and then the method name, " + + "optionally followed by a parameter list enclosed in parentheses."); + } + + @Test + void throwsExceptionWhenClassForExternalFactoryMethodCannotBeLoaded() { + var exception = assertThrows(JUnitException.class, + () -> provideArguments("com.example.NonExistentClass#stringsProvider").toArray()); + + assertThat(exception.getMessage()).isEqualTo("Could not load class [com.example.NonExistentClass]"); + } + + @Test + void throwsExceptionWhenExternalFactoryMethodDoesNotExist() { + String factoryClass = ExternalFactoryMethods.class.getName(); + + var exception = assertThrows(PreconditionViolationException.class, + () -> provideArguments(factoryClass + "#nonExistentMethod").toArray()); + + assertThat(exception.getMessage()).isEqualTo( + "Could not find factory method [nonExistentMethod] in class [%s]", factoryClass); + } + + @Test + void throwsExceptionWhenLocalFactoryMethodDoesNotExist() { + var exception = assertThrows(PreconditionViolationException.class, + () -> provideArguments("nonExistentMethod").toArray()); + + assertThat(exception.getMessage()).isEqualTo( + "Could not find factory method [nonExistentMethod] in class [%s]", TestCase.class.getName()); + } + + @Test + void throwsExceptionWhenExternalFactoryMethodAcceptingSingleArgumentDoesNotExist() { + String factoryClass = ExternalFactoryMethods.class.getName(); + + var exception = assertThrows(PreconditionViolationException.class, + () -> provideArguments(factoryClass + "#nonExistentMethod(int)").toArray()); + + assertThat(exception.getMessage()).isEqualTo( + "Could not find factory method [nonExistentMethod(int)] in class [%s]", factoryClass); + } + + @Test + void throwsExceptionWhenLocalFactoryMethodAcceptingSingleArgumentDoesNotExist() { + var exception = assertThrows(PreconditionViolationException.class, + () -> provideArguments("nonExistentMethod(int)").toArray()); + + assertThat(exception.getMessage()).isEqualTo( + "Could not find factory method [nonExistentMethod(int)] in class [%s]", TestCase.class.getName()); + } + + @Test + void throwsExceptionWhenExternalFactoryMethodAcceptingMultipleArgumentsDoesNotExist() { + String factoryClass = ExternalFactoryMethods.class.getName(); + + var exception = assertThrows(PreconditionViolationException.class, + () -> provideArguments(factoryClass + "#nonExistentMethod(int, java.lang.String)").toArray()); + + assertThat(exception.getMessage()).isEqualTo( + "Could not find factory method [nonExistentMethod(int, java.lang.String)] in class [%s]", factoryClass); + } + + @Test + void throwsExceptionWhenLocalFactoryMethodAcceptingMultipleArgumentsDoesNotExist() { var exception = assertThrows(PreconditionViolationException.class, - () -> provideArguments("stringStreamProviderWithOrWithoutParameter").toArray()); + () -> provideArguments("nonExistentMethod(java.lang.String,int)").toArray()); + + assertThat(exception.getMessage()).isEqualTo( + "Could not find factory method [nonExistentMethod(java.lang.String,int)] in class [%s]", + TestCase.class.getName()); + } + + @Test + void throwsExceptionWhenExternalFactoryMethodHasInvalidReturnType() { + String testClass = TestCase.class.getName(); + String factoryClass = ExternalFactoryMethods.class.getName(); + String factoryMethod = factoryClass + "#factoryWithInvalidReturnType"; + + var exception = assertThrows(PreconditionViolationException.class, + () -> provideArguments(TestCase.class, null, false, factoryMethod).toArray()); + + assertThat(exception.getMessage())// + .containsSubsequence("Could not find valid factory method [" + factoryMethod + "] for test class [", + testClass + "]", // + "but found the following invalid candidate: ", + "static java.lang.Object " + factoryClass + ".factoryWithInvalidReturnType()"); + } + + @Test + void throwsExceptionWhenLocalFactoryMethodHasInvalidReturnType() { + String testClass = TestCase.class.getName(); + String factoryClass = testClass; + String factoryMethod = "factoryWithInvalidReturnType"; + + var exception = assertThrows(PreconditionViolationException.class, + () -> provideArguments(factoryMethod).toArray()); assertThat(exception.getMessage())// - .startsWith("3 factory methods named [stringStreamProviderWithOrWithoutParameter] were found in " - + "class [org.junit.jupiter.params.provider.MethodArgumentsProviderTests$TestCase]: ")// - .contains("stringStreamProviderWithOrWithoutParameter()", - "stringStreamProviderWithOrWithoutParameter(java.lang.String)", - "stringStreamProviderWithOrWithoutParameter(java.lang.String,java.lang.String)"); + .containsSubsequence("Could not find valid factory method [" + factoryMethod + "] for test class [", + factoryClass + "]", // + "but found the following invalid candidate: ", // + "static java.lang.Object " + factoryClass + ".factoryWithInvalidReturnType()"); + } + + @Test + void throwsExceptionWhenMultipleDefaultFactoryMethodCandidatesExist() { + var testClass = MultipleDefaultFactoriesTestCase.class; + var methodName = "test"; + var testMethod = findMethod(testClass, methodName, String.class).get(); + + var exception = assertThrows(PreconditionViolationException.class, + () -> provideArguments(testClass, testMethod, false, "").toArray()); + + assertThat(exception.getMessage()).contains(// + "2 factory methods named [test] were found in class [", testClass.getName() + "]: ", // + "$MultipleDefaultFactoriesTestCase.test()", // + "$MultipleDefaultFactoriesTestCase.test(int)"// + ); + } + + @Test + void throwsExceptionWhenMultipleInvalidDefaultFactoryMethodCandidatesExist() { + var testClass = MultipleInvalidDefaultFactoriesTestCase.class; + var methodName = "test"; + var testMethod = findMethod(testClass, methodName, String.class).get(); + + var exception = assertThrows(PreconditionViolationException.class, + () -> provideArguments(testClass, testMethod, false, "").toArray()); + + assertThat(exception.getMessage()).contains(// + "Could not find valid factory method [test] in class [", testClass.getName() + "]", // + "but found the following invalid candidates: ", // + "$MultipleInvalidDefaultFactoriesTestCase.test()", // + "$MultipleInvalidDefaultFactoriesTestCase.test(int)"// + ); } } @@ -585,8 +684,13 @@ private Stream provideArguments(Class testClass, Method testMethod, if (testMethod == null) { try { + class DummyClass { + @SuppressWarnings("unused") + public void dummyMethod() { + }; + } // ensure we have a non-null method, even if it's not a real test method. - testMethod = getClass().getMethod("toString"); + testMethod = DummyClass.class.getMethod("dummyMethod"); } catch (Exception ex) { throw new RuntimeException(ex); @@ -618,13 +722,47 @@ private Stream provideArguments(Class testClass, Method testMethod, static class DefaultFactoryMethodNameTestCase { - static final String TEST_METHOD = "testDefaultFactoryMethodName"; + // Test + void testDefaultFactoryMethodName(String param) { + } + // Factory static Stream testDefaultFactoryMethodName() { return Stream.of("foo", "bar"); } + } + + static class MultipleDefaultFactoriesTestCase { + + // Test + void test(String param) { + } - void testDefaultFactoryMethodName(String param) { + // Factory + static Stream test() { + return Stream.of(); + } + + // Another Factory + static Stream test(int num) { + return Stream.of(); + } + } + + static class MultipleInvalidDefaultFactoriesTestCase { + + // Test + void test(String param) { + } + + // NOT a Factory + static String test() { + return null; + } + + // Also NOT a Factory + static Object test(int num) { + return null; } } @@ -635,7 +773,7 @@ void test() { // --- Invalid --------------------------------------------------------- - static Object providerWithIllegalReturnType() { + static Object factoryWithInvalidReturnType() { return -1; } @@ -671,6 +809,10 @@ static Stream stringStreamProviderWithOrWithoutParameter(String paramete return stringStreamProviderWithParameter(parameter1 + parameter2); } + // Overloaded method, but not a valid return type for a factory method + static void stringStreamProviderWithOrWithoutParameter(String parameter1, int parameter2) { + } + // @ParameterizedTest // @MethodSource // use default, inferred factory method void overloadedStringStreamProvider(Object parameter) { @@ -798,6 +940,10 @@ Stream nonStaticStringStreamProvider() { static class ExternalFactoryMethods { + static Object factoryWithInvalidReturnType() { + return -1; + } + static Stream stringsProvider() { return Stream.of("string1", "string2"); }