Skip to content

Commit

Permalink
Fix bug and polish contribution
Browse files Browse the repository at this point in the history
This commit fixes a bug regarding whitespace in parameter lists for
local factory methods specified via `@MethodSource`. Specifically,
whitespace is now supported.

For example, @MethodSource("data(int, java.lang.String)") is now
supported (note the space after the comma).

As a side note, whitespace in parameter lists was already supported for
a fully qualified method name.

See #3080, #3101
  • Loading branch information
sbrannen committed Jan 8, 2023
1 parent 825ea38 commit a9a3cf5
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ JUnit repository on GitHub.

==== Bug Fixes

* Introduce new `@MethodSource` syntax to add the possibility to explicitly select
an overloaded local factory method without specifying its fully qualified name
* New `@MethodSource` syntax for explicitly selecting an overloaded local factory method
without specifying its fully qualified name.

==== Deprecations and Breaking Changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@
package org.junit.jupiter.params.provider;

import static java.lang.String.format;
import static java.util.Arrays.stream;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;
import static org.junit.jupiter.params.provider.Arguments.arguments;
import static org.junit.platform.commons.util.AnnotationUtils.isAnnotated;
import static org.junit.platform.commons.util.CollectionUtils.isConvertibleToStream;

import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.List;
import java.util.function.Predicate;
import java.util.stream.Stream;
Expand All @@ -28,7 +29,6 @@
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.util.ClassUtils;
import org.junit.platform.commons.util.CollectionUtils;
import org.junit.platform.commons.util.Preconditions;
import org.junit.platform.commons.util.ReflectionUtils;
Expand All @@ -50,7 +50,7 @@ public void accept(MethodSource annotation) {
public Stream<Arguments> provideArguments(ExtensionContext context) {
Object testInstance = context.getTestInstance().orElse(null);
// @formatter:off
return Arrays.stream(this.methodNames)
return stream(this.methodNames)
.map(factoryMethodName -> getFactoryMethod(context, factoryMethodName))
.map(factoryMethod -> context.getExecutableInvoker().invoke(factoryMethod, testInstance))
.flatMap(CollectionUtils::toStream)
Expand All @@ -73,11 +73,20 @@ private static boolean looksLikeAFullyQualifiedMethodName(String factoryMethodNa
if (factoryMethodName.contains("#")) {
return true;
}
if (factoryMethodName.contains(".") && factoryMethodName.contains("(")) {
// Excluding cases of simple method names with parameters
return factoryMethodName.indexOf(".") < factoryMethodName.indexOf("(");
int indexOfDot = factoryMethodName.indexOf(".");
if (indexOfDot == -1) {
return false;
}
return factoryMethodName.contains(".");
int indexOfOpeningParenthesis = factoryMethodName.indexOf("(");
if (indexOfOpeningParenthesis > 0) {
// Exclude simple method names with parameters
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
// 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.
return true;
}

private Method getFactoryMethodByFullyQualifiedName(String fullyQualifiedMethodName) {
Expand Down Expand Up @@ -143,12 +152,21 @@ private List<Method> findFactoryMethodsBySimpleName(Class<?> testClass, Method t

private static List<Method> filterFactoryMethodsWithMatchingParameters(List<Method> factoryMethods,
String factoryMethodName, String factoryMethodParameters) {

if (!factoryMethodName.endsWith(")")) {
// If parameters are not specified, no choice is made
// If parameters are not specified, nothing is filtered.
return factoryMethods;
}
Predicate<Method> hasRequiredParameters = method -> factoryMethodParameters.equals(
ClassUtils.nullSafeToString(method.getParameterTypes()));

// Compare against canonical parameter list, ignoring whitespace.
String parameterList = factoryMethodParameters.replaceAll("\\s+", "");
Predicate<Method> hasRequiredParameters = method -> {
if (parameterList.isEmpty()) {
return method.getParameterCount() == 0;
}
return parameterList.equals(stream(method.getParameterTypes()).map(Class::getName).collect(joining(",")));
};

return factoryMethods.stream().filter(hasRequiredParameters).collect(toList());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.junit.jupiter.engine.config.JupiterConfiguration;
import org.junit.jupiter.engine.execution.DefaultExecutableInvoker;
import org.junit.jupiter.engine.extension.MutableExtensionRegistry;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.platform.commons.JUnitException;
import org.junit.platform.commons.PreconditionViolationException;
import org.junit.platform.commons.util.ReflectionUtils;
Expand Down Expand Up @@ -408,16 +409,25 @@ void providesArgumentsUsingSimpleNameWithParameter() {
assertThat(arguments).containsExactly(array("foo!"), array("bar!"));
}

@ParameterizedTest
@ValueSource(strings = { "java.lang.String,java.lang.String", "java.lang.String, java.lang.String",
"java.lang.String, java.lang.String" })
void providesArgumentsUsingSimpleNameWithMultipleParameters(String params) {
var arguments = provideArguments("stringStreamProviderWithOrWithoutParameter(" + params + ")");
assertThat(arguments).containsExactly(array("foo!!"), array("bar!!"));
}

@Test
void throwsExceptionWhenSeveralFactoryMethodsWithSameNameAreAvailable() {
var exception = assertThrows(PreconditionViolationException.class,
() -> provideArguments("stringStreamProviderWithOrWithoutParameter").toArray());

assertThat(exception.getMessage())//
.startsWith("2 factory methods named [stringStreamProviderWithOrWithoutParameter] were found in "
.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)",
"stringStreamProviderWithOrWithoutParameter(java.lang.String,java.lang.String)");
}

@Test
Expand All @@ -428,6 +438,16 @@ void providesArgumentsUsingFactoryMethodSelectedViaFullyQualifiedNameWithParamet
assertThat(arguments).containsExactly(array("foo!"), array("bar!"));
}

@ParameterizedTest
@ValueSource(strings = { "java.lang.String,java.lang.String", "java.lang.String, java.lang.String",
"java.lang.String, java.lang.String" })
void providesArgumentsUsingFactoryMethodSelectedViaFullyQualifiedNameWithMultipleParameters(String params) {
var arguments = provideArguments(
TestCase.class.getName() + "#stringStreamProviderWithOrWithoutParameter(" + params + ")");

assertThat(arguments).containsExactly(array("foo!!"), array("bar!!"));
}

@Test
void providesArgumentsUsingFactoryMethodSelectedViaFullyQualifiedNameWithoutParameter() {
var arguments = provideArguments(
Expand Down Expand Up @@ -522,6 +542,10 @@ static Stream<String> stringStreamProviderWithOrWithoutParameter(String paramete
return stringStreamProviderWithParameter(parameter);
}

static Stream<String> stringStreamProviderWithOrWithoutParameter(String parameter1, String parameter2) {
return stringStreamProviderWithParameter(parameter1 + parameter2);
}

// @ParameterizedTest
// @MethodSource // use default, inferred factory method
void overloadedStringStreamProvider(Object parameter) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,7 @@ public static String[] parseFullyQualifiedMethodName(String fullyQualifiedMethod
* @param qualifiedMethodName a qualified method name, never {@code null} or blank
* @return a 2-element array of strings containing the parsed values
*/
@API(status = INTERNAL, since = "1.9")
@API(status = INTERNAL, since = "1.9.2")
public static String[] parseQualifiedMethodName(String qualifiedMethodName) {
String methodName = qualifiedMethodName;
String methodParameters = "";
Expand Down

0 comments on commit a9a3cf5

Please sign in to comment.