Skip to content

Commit

Permalink
Remove Arguments from CartesianArgumentsProvider API (#523 / #528)
Browse files Browse the repository at this point in the history
Jupiter's `Arguments` interface is used for parameterized tests, where each
`Arguments` instance corresponds to one test execution and thus carries an
`Object[]` with ine argument per parameter. A `CartesianArgumentsProvider`,
on the other hand, provides a set of arguments for a single parameter and
thus there's no need to wrap them in a container. They can simply return the
arguments directly.

As a welcome side effect, this change now allows passing a arrays to a
parameter without accidentally flattening it, which is what #523 was mainly
concerned with.

Closes: #523
PR: #528
  • Loading branch information
filiphr committed Nov 14, 2021
1 parent a7efc1c commit 97376ba
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 50 deletions.
9 changes: 4 additions & 5 deletions docs/cartesian-product.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -346,9 +346,9 @@ import org.junitpioneer.jupiter.cartesian.CartesianArgumentsProvider;
class IntArgumentsProvider implements CartesianArgumentsProvider {
@Override
public Stream<? extends Arguments> provideArguments(ExtensionContext context, Parameter parameter) {
public Stream<Integer> provideArguments(ExtensionContext context, Parameter parameter) {
Ints source = Objects.requireNonNull(parameter.getAnnotation(Ints.class));
return Arrays.stream(source.value()).map(Arguments::of);
return Arrays.stream(source.value());
}
}
Expand Down Expand Up @@ -386,11 +386,10 @@ import org.junitpioneer.jupiter.cartesian.CartesianArgumentsProvider;
class PeopleProvider implements CartesianArgumentsProvider {
@Override
public Stream<? extends Arguments> provideArguments(ExtensionContext context, Parameter parameter) {
public Stream<Person> provideArguments(ExtensionContext context, Parameter parameter) {
People source = Objects.requireNonNull(parameter.getAnnotation(People.class));
return IntStream.range(0, source.names().length)
.mapToObj(i -> new Person(source.names()[i], source.ages()[i]))
.map(Arguments::of);
.mapToObj(i -> new Person(source.names()[i], source.ages()[i]));
}
}
----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,27 @@
import java.util.stream.Stream;

import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.params.provider.Arguments;

/**
* If you are implementing an {@link org.junit.jupiter.params.provider.ArgumentsProvider ArgumentsProvider}
* for {@link CartesianTest}, it has to implement this interface <b>as well</b> to know which parameter it provides
* arguments to. For more information, see
* <a href="https://junit-pioneer.org/docs/cartesian-product/" target="_top">the Cartesian product documentation</a>.
*
* @param <T> type of arguments this provider returns
*
* @see org.junit.jupiter.params.provider.ArgumentsProvider
* @see CartesianTestExtension
*/
public interface CartesianArgumentsProvider {
public interface CartesianArgumentsProvider<T> {

/**
* Provider a {@link Stream} of {@link Arguments} that needs to be used for the {@code @CartesianTest}.
* Provider a {@link Stream} of values that needs to be used for a single parameter in {@code @CartesianTest}.
*
* @param context the current extension context; never {@code null}
* @param parameter the parameter for which the arguments needs to be provided
* @return a stream of arguments; never {@code null}
* @return a stream of values; never {@code null}
*/
Stream<? extends Arguments> provideArguments(ExtensionContext context, Parameter parameter) throws Exception;
Stream<T> provideArguments(ExtensionContext context, Parameter parameter) throws Exception;

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import java.util.stream.Stream;

import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.platform.commons.PreconditionViolationException;
import org.junit.platform.commons.support.AnnotationSupport;

Expand All @@ -31,10 +30,10 @@
* @implNote This class does not implement {@code ArgumentsProvider} since the Jupiter's {@code EnumSource}
* should be used for that.
*/
class CartesianEnumArgumentsProvider implements CartesianArgumentsProvider {
class CartesianEnumArgumentsProvider<E extends Enum<E>> implements CartesianArgumentsProvider<E> {

@Override
public Stream<? extends Arguments> provideArguments(ExtensionContext context, Parameter parameter) {
public Stream<E> provideArguments(ExtensionContext context, Parameter parameter) {
Class<?> parameterType = parameter.getType();
if (!Enum.class.isAssignableFrom(parameterType))
throw new PreconditionViolationException(String
Expand All @@ -46,7 +45,7 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context, Pa
.orElseThrow(() -> new PreconditionViolationException(
"Parameter has to be annotated with " + CartesianTest.Enum.class.getName()));

Set<? extends Enum<?>> constants = getEnumConstants(enumSource, parameterType);
Set<E> constants = getEnumConstants(enumSource, parameterType);
CartesianTest.Enum.Mode mode = enumSource.mode();
String[] declaredConstantNames = enumSource.names();
if (declaredConstantNames.length > 0) {
Expand All @@ -57,17 +56,16 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context, Pa
mode.validate(enumSource, constants, uniqueNames);
constants.removeIf(constant -> !mode.select(constant, uniqueNames));
}
return constants.stream().map(Arguments::of);
return constants.stream();
}

private <E extends Enum<E>> Set<? extends E> getEnumConstants(CartesianTest.Enum enumSource,
Class<?> parameterType) {
private Set<E> getEnumConstants(CartesianTest.Enum enumSource, Class<?> parameterType) {
Class<E> enumClass = determineEnumClass(enumSource, parameterType);
return EnumSet.allOf(enumClass);
}

@SuppressWarnings({ "unchecked", "rawtypes" })
private <E extends Enum<E>> Class<E> determineEnumClass(CartesianTest.Enum enumSource, Class<?> parameterType) {
private Class<E> determineEnumClass(CartesianTest.Enum enumSource, Class<?> parameterType) {
Class enumClass = enumSource.value();
if (enumClass.equals(NullEnum.class)) {
enumClass = parameterType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.TestTemplateInvocationContext;
import org.junit.jupiter.api.extension.TestTemplateInvocationContextProvider;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.ArgumentsProvider;
import org.junit.jupiter.params.provider.ArgumentsSource;
import org.junit.jupiter.params.support.AnnotationConsumerInitializer;
Expand Down Expand Up @@ -89,17 +88,17 @@ private List<List<?>> getSetsFromArgumentsSources(List<? extends Annotation> arg
return sets;
}

private List<Object> getSetFromAnnotation(ExtensionContext context, Annotation source, Parameter parameter) {
private List<?> getSetFromAnnotation(ExtensionContext context, Annotation source, Parameter parameter) {
try {
CartesianArgumentsProvider provider = initializeArgumentsProvider(source, parameter);
CartesianArgumentsProvider<?> provider = initializeArgumentsProvider(source, parameter);
return provideArguments(context, parameter, provider);
}
catch (Exception ex) {
throw new ExtensionConfigurationException("Could not provide arguments because of exception.", ex);
}
}

private CartesianArgumentsProvider initializeArgumentsProvider(Annotation source, Parameter parameter) {
private CartesianArgumentsProvider<?> initializeArgumentsProvider(Annotation source, Parameter parameter) {
Optional<CartesianArgumentsSource> cartesianProviderAnnotation = AnnotationSupport
.findAnnotation(parameter, CartesianArgumentsSource.class);

Expand All @@ -116,18 +115,16 @@ private CartesianArgumentsProvider initializeArgumentsProvider(Annotation source
source.annotationType())));
ArgumentsProvider provider = ReflectionSupport.newInstance(providerAnnotation.value());
if (provider instanceof CartesianArgumentsProvider)
return AnnotationConsumerInitializer.initialize(parameter, (CartesianArgumentsProvider) provider);
return AnnotationConsumerInitializer.initialize(parameter, (CartesianArgumentsProvider<?>) provider);
else
throw new PreconditionViolationException(
format("%s does not implement the CartesianArgumentsProvider interface.", provider.getClass()));
}

private List<Object> provideArguments(ExtensionContext context, Parameter source,
CartesianArgumentsProvider provider) throws Exception {
private List<?> provideArguments(ExtensionContext context, Parameter source, CartesianArgumentsProvider<?> provider)
throws Exception {
return provider
.provideArguments(context, source)
.map(Arguments::get)
.flatMap(Arrays::stream)
.distinct()
// We like to keep arguments in the order in which they were listed
// in the annotation. Could use a set with defined iteration, but
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.util.stream.Stream;

import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.support.AnnotationConsumer;
import org.junit.platform.commons.PreconditionViolationException;

Expand All @@ -32,7 +31,8 @@
* @implNote This class does not implement {@code ArgumentsProvider} since the Jupiter's {@code ValueSource}
* should be used for that.
*/
class CartesianValueArgumentsProvider implements CartesianArgumentsProvider, AnnotationConsumer<CartesianTest.Values> {
class CartesianValueArgumentsProvider
implements CartesianArgumentsProvider<Object>, AnnotationConsumer<CartesianTest.Values> {

private Object[] arguments;

Expand Down Expand Up @@ -69,8 +69,8 @@ Stream.<Object> of(
}

@Override
public Stream<? extends Arguments> provideArguments(ExtensionContext context, Parameter parameter) {
return Arrays.stream(arguments).map(Arguments::of);
public Stream<Object> provideArguments(ExtensionContext context, Parameter parameter) {
return Arrays.stream(arguments);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,16 @@
* @see DoubleRangeSource
* @see FloatRangeSource
*/
class RangeSourceArgumentsProvider
implements ArgumentsProvider, CartesianAnnotationConsumer<Annotation>, CartesianArgumentsProvider { //NOSONAR deprecated interface use will be removed in later release
class RangeSourceArgumentsProvider<N extends Number & Comparable<N>>
implements ArgumentsProvider, CartesianAnnotationConsumer<Annotation>, CartesianArgumentsProvider<N> { //NOSONAR deprecated interface use will be removed in later release

// Once the CartesianAnnotationConsumer is removed we can make this provider stateless.
private Annotation argumentsSource;

@Override
public Stream<? extends Arguments> provideArguments(ExtensionContext context, Parameter parameter)
throws Exception {
public Stream<N> provideArguments(ExtensionContext context, Parameter parameter) throws Exception {
initArgumentsSource(parameter);
return provideArguments(context, argumentsSource);
return provideArguments(argumentsSource);
}

@Override
Expand All @@ -64,17 +63,17 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) th
// since it's a method annotation, the element will always be present
initArgumentsSource(context.getRequiredTestMethod());

return provideArguments(context, argumentsSource);
return provideArguments(argumentsSource).map(Arguments::of);
}

private Stream<? extends Arguments> provideArguments(ExtensionContext context, Annotation argumentsSource)
throws Exception {
@SuppressWarnings({ "unchecked", "rawtypes" })
private Stream<N> provideArguments(Annotation argumentsSource) throws Exception {
Class<? extends Annotation> argumentsSourceClass = argumentsSource.annotationType();
Class<? extends Range> rangeClass = argumentsSourceClass.getAnnotation(RangeClass.class).value();

Range<?> range = (Range<?>) rangeClass.getConstructors()[0].newInstance(argumentsSource);
Range<N> range = (Range<N>) rangeClass.getConstructors()[0].newInstance(argumentsSource);
range.validate();
return asStream(range).map(Arguments::of);
return asStream(range);
}

private void initArgumentsSource(AnnotatedElement element) {
Expand All @@ -91,8 +90,8 @@ private void initArgumentsSource(AnnotatedElement element) {
argumentsSource = argumentsSources.get(0);
}

private Stream<?> asStream(Range<?> r) {
return StreamSupport.stream(Spliterators.spliteratorUnknownSize(r, Spliterator.ORDERED), false);
private Stream<N> asStream(Range<N> range) {
return StreamSupport.stream(Spliterators.spliteratorUnknownSize(range, Spliterator.ORDERED), false);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.ParameterResolutionException;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.platform.commons.PreconditionViolationException;
import org.junitpioneer.jupiter.ReportEntry;
import org.junitpioneer.jupiter.cartesian.CartesianTest.Enum.Mode;
Expand Down Expand Up @@ -375,6 +374,16 @@ void usesCustomCartesianArgumentsProviderOnParameters() {
.withValues("first(1)", "first(2)", "second(1)", "second(2)", "third(1)", "third(2)");
}

@Test
@DisplayName("when configured with array parameters")
void usesCustomCartesianArgumentsProviderWithArrayArgumentOnParameters() {
ExecutionResults results = PioneerTestKit
.executeTestMethodWithParameterTypes(CustomCartesianArgumentsProviderTestCases.class,
"singleArrayArgument", String[].class);

assertThat(results).hasNumberOfDynamicallyRegisteredTests(2).hasNumberOfSucceededTests(2);
}

}

}
Expand Down Expand Up @@ -921,6 +930,11 @@ void twoCustomCartesianArgumentProviders(

}

@CartesianTest
void singleArrayArgument(@CartesianArgumentsSource(StringArrayArgumentsProvider.class) String[] source) {
assertThat(source).hasSize(2);
}

}

private enum TestEnum {
Expand All @@ -931,20 +945,29 @@ private enum AnotherTestEnum {
ALPHA, BETA, GAMMA
}

static class FirstCustomCartesianArgumentsProvider implements CartesianArgumentsProvider {
static class FirstCustomCartesianArgumentsProvider implements CartesianArgumentsProvider<String> {

@Override
public Stream<String> provideArguments(ExtensionContext context, Parameter parameter) {
return Stream.of("first", "second", "third");
}

}

static class SecondCustomCartesianArgumentsProvider implements CartesianArgumentsProvider<Integer> {

@Override
public Stream<? extends Arguments> provideArguments(ExtensionContext context, Parameter parameter) {
return Stream.of("first", "second", "third").map(Arguments::of);
public Stream<Integer> provideArguments(ExtensionContext context, Parameter parameter) {
return Stream.of(1, 2);
}

}

static class SecondCustomCartesianArgumentsProvider implements CartesianArgumentsProvider {
static class StringArrayArgumentsProvider implements CartesianArgumentsProvider<String[]> {

@Override
public Stream<? extends Arguments> provideArguments(ExtensionContext context, Parameter parameter) {
return Stream.of(1, 2).map(Arguments::of);
public Stream<String[]> provideArguments(ExtensionContext context, Parameter parameter) {
return Stream.of(new String[] { "1", "2" }, new String[] { "3", "4" });
}

}
Expand Down

0 comments on commit 97376ba

Please sign in to comment.