Skip to content

Commit a0eb8e2

Browse files
committed
Fix issues with @MethodSource local factory method lookups
Prior to this commit, parameters were not validated in local @MethodSource factory methods which lead to incorrect configuration being silently ignored or confusing error messages when the specified factory method was overloaded. In addition, the syntax for local @MethodSource factory methods did not support canonical array names. This commit addresses these issues by treating local factory method resolution the same as the existing support for resolving a fully qualified method name for a factory method. These changes make the recently introduced parseQualifiedMethodName() method in ReflectionUtils obsolete. This commit therefore removes that method as well. Closes #3130 Closes #3131
1 parent 30cfc3b commit a0eb8e2

File tree

4 files changed

+242
-149
lines changed

4 files changed

+242
-149
lines changed

junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodArgumentsProvider.java

+37-51
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212

1313
import static java.lang.String.format;
1414
import static java.util.Arrays.stream;
15-
import static java.util.stream.Collectors.joining;
1615
import static java.util.stream.Collectors.toList;
1716
import static org.junit.jupiter.params.provider.Arguments.arguments;
1817
import static org.junit.platform.commons.util.AnnotationUtils.isAnnotated;
@@ -48,25 +47,33 @@ public void accept(MethodSource annotation) {
4847

4948
@Override
5049
public Stream<Arguments> provideArguments(ExtensionContext context) {
50+
Class<?> testClass = context.getRequiredTestClass();
51+
Method testMethod = context.getRequiredTestMethod();
5152
Object testInstance = context.getTestInstance().orElse(null);
5253
// @formatter:off
5354
return stream(this.methodNames)
54-
.map(factoryMethodName -> getFactoryMethod(context, factoryMethodName))
55+
.map(factoryMethodName -> getFactoryMethod(testClass, testMethod, factoryMethodName))
5556
.map(factoryMethod -> context.getExecutableInvoker().invoke(factoryMethod, testInstance))
5657
.flatMap(CollectionUtils::toStream)
5758
.map(MethodArgumentsProvider::toArguments);
5859
// @formatter:on
5960
}
6061

61-
private Method getFactoryMethod(ExtensionContext context, String factoryMethodName) {
62-
Method testMethod = context.getRequiredTestMethod();
63-
if (StringUtils.isBlank(factoryMethodName)) {
64-
factoryMethodName = testMethod.getName();
62+
private Method getFactoryMethod(Class<?> testClass, Method testMethod, String factoryMethodName) {
63+
if (!StringUtils.isBlank(factoryMethodName)) {
64+
if (looksLikeAFullyQualifiedMethodName(factoryMethodName)) {
65+
return getFactoryMethodByFullyQualifiedName(factoryMethodName);
66+
}
67+
else if (looksLikeALocalQualifiedMethodName(factoryMethodName)) {
68+
return getFactoryMethodByFullyQualifiedName(testClass.getName() + "#" + factoryMethodName);
69+
}
6570
}
66-
if (looksLikeAFullyQualifiedMethodName(factoryMethodName)) {
67-
return getFactoryMethodByFullyQualifiedName(factoryMethodName);
71+
else {
72+
// User did not provide a factory method name, so we search for a
73+
// factory method with the same name as the parameterized test method.
74+
factoryMethodName = testMethod.getName();
6875
}
69-
return getFactoryMethodBySimpleOrQualifiedName(context.getRequiredTestClass(), testMethod, factoryMethodName);
76+
return findFactoryMethodBySimpleName(testClass, testMethod, factoryMethodName);
7077
}
7178

7279
private static boolean looksLikeAFullyQualifiedMethodName(String factoryMethodName) {
@@ -89,6 +96,18 @@ private static boolean looksLikeAFullyQualifiedMethodName(String factoryMethodNa
8996
return true;
9097
}
9198

99+
private static boolean looksLikeALocalQualifiedMethodName(String factoryMethodName) {
100+
// This method is intended to be called after looksLikeAFullyQualifiedMethodName()
101+
// and therefore does not check for the absence of '#' and does not reason about
102+
// the presence or absence of a fully qualified class name.
103+
if (factoryMethodName.endsWith("()")) {
104+
return true;
105+
}
106+
int indexOfLastOpeningParenthesis = factoryMethodName.lastIndexOf('(');
107+
return (indexOfLastOpeningParenthesis > 0)
108+
&& (indexOfLastOpeningParenthesis < factoryMethodName.lastIndexOf(')'));
109+
}
110+
92111
private Method getFactoryMethodByFullyQualifiedName(String fullyQualifiedMethodName) {
93112
String[] methodParts = ReflectionUtils.parseFullyQualifiedMethodName(fullyQualifiedMethodName);
94113
String className = methodParts[0];
@@ -100,33 +119,17 @@ private Method getFactoryMethodByFullyQualifiedName(String fullyQualifiedMethodN
100119
methodParameters, className)));
101120
}
102121

103-
private Method getFactoryMethodBySimpleOrQualifiedName(Class<?> testClass, Method testMethod,
104-
String simpleOrQualifiedMethodName) {
105-
String[] methodParts = ReflectionUtils.parseQualifiedMethodName(simpleOrQualifiedMethodName);
106-
String methodSimpleName = methodParts[0];
107-
String methodParameters = methodParts[1];
108-
109-
List<Method> factoryMethods = findFactoryMethodsBySimpleName(testClass, testMethod, methodSimpleName);
110-
if (factoryMethods.size() == 1) {
111-
return factoryMethods.get(0);
112-
}
113-
114-
List<Method> exactMatches = filterFactoryMethodsWithMatchingParameters(factoryMethods,
115-
simpleOrQualifiedMethodName, methodParameters);
116-
Preconditions.condition(exactMatches.size() == 1,
117-
() -> format("%d factory methods named [%s] were found in class [%s]: %s", factoryMethods.size(),
118-
simpleOrQualifiedMethodName, testClass.getName(), factoryMethods));
119-
return exactMatches.get(0);
120-
}
121-
122122
/**
123123
* Find all methods in the given {@code testClass} with the desired {@code factoryMethodName}
124124
* which have return types that can be converted to a {@link Stream}, ignoring the
125125
* {@code testMethod} itself as well as any {@code @Test}, {@code @TestTemplate},
126126
* or {@code @TestFactory} methods with the same name.
127+
* @return the factory method, if found
128+
* @throws org.junit.platform.commons.PreconditionViolationException if the
129+
* factory method was not found or if multiple competing factory methods with
130+
* the same name were found
127131
*/
128-
private List<Method> findFactoryMethodsBySimpleName(Class<?> testClass, Method testMethod,
129-
String factoryMethodName) {
132+
private Method findFactoryMethodBySimpleName(Class<?> testClass, Method testMethod, String factoryMethodName) {
130133
Predicate<Method> isCandidate = candidate -> factoryMethodName.equals(candidate.getName())
131134
&& !testMethod.equals(candidate);
132135
List<Method> candidates = ReflectionUtils.findMethods(testClass, isCandidate);
@@ -147,27 +150,10 @@ private List<Method> findFactoryMethodsBySimpleName(Class<?> testClass, Method t
147150
// Otherwise, report that we didn't find anything.
148151
return format("Could not find factory method [%s] in class [%s]", factoryMethodName, testClass.getName());
149152
});
150-
return factoryMethods;
151-
}
152-
153-
private static List<Method> filterFactoryMethodsWithMatchingParameters(List<Method> factoryMethods,
154-
String factoryMethodName, String factoryMethodParameters) {
155-
156-
if (!factoryMethodName.endsWith(")")) {
157-
// If parameters are not specified, nothing is filtered.
158-
return factoryMethods;
159-
}
160-
161-
// Compare against canonical parameter list, ignoring whitespace.
162-
String parameterList = factoryMethodParameters.replaceAll("\\s+", "");
163-
Predicate<Method> hasRequiredParameters = method -> {
164-
if (parameterList.isEmpty()) {
165-
return method.getParameterCount() == 0;
166-
}
167-
return parameterList.equals(stream(method.getParameterTypes()).map(Class::getName).collect(joining(",")));
168-
};
169-
170-
return factoryMethods.stream().filter(hasRequiredParameters).collect(toList());
153+
Preconditions.condition(factoryMethods.size() == 1,
154+
() -> format("%d factory methods named [%s] were found in class [%s]: %s", factoryMethods.size(),
155+
factoryMethodName, testClass.getName(), factoryMethods));
156+
return factoryMethods.get(0);
171157
}
172158

173159
private boolean isTestMethod(Method candidate) {

0 commit comments

Comments
 (0)