junit-team
ParameterizedTest with varargs
I'm using parameterized tests for quite a while now and for the most part they work really intuitive
.
However today I stumbled upon one thing that felt like it should work, but to my initial surprise didn't.
Basically I expected this code to work (in my case it was a @CsvFileSource, but doesn't matter):
@ParameterizedTest
@CsvSource({"1,a", "1,a,b,c"})
void testAbc(int arg1, String... elements) {
System.out.println(Arrays.toString(elements));
}
I had a quick look at the implementation of the source suppliers and it looks like this ultimately boils down on how an Argument instance is "spread" to the individual test method arguments.
And from my testing it seems like any additional arguments are just cut off.
I'd really like to see sort of smart behaviour here: If the last argument of a method is an array (aka varargs) it should try to stuff any trailing arguments into this parameter instead of treating it as a normal object.
I must admit that I don't really know what the implications of such a change are. I can imagine that this might break an extension or two that rely on the current behaviour, but overall I think this could be a quality of life improvement, especially if the varargs argument has a different type than String, like java.time or some other more complex type so the dev doesn't have to manually implement a conversion method.
Alternative
EDIT: I just learned about argument aggregators, which is more or less the thing I'm describing below (pleasant surpsrise tbh 😅), but I still couldn't find a built-in way of splitting a string and potentially auto-converting it to something else.
I came up with a different solution for my particular problem that goes a little bit further, but allows for consistent behaviour and even more complex argument structures.
Think of a fixed example of my previous code:
@ParameterizedTest
@CsvSource({"1,a", "1,a:b:c"})
void testAbc(int arg1, String elements) {
System.out.println(Arrays.toString(elements.split(":")));
}
Basically I ended up splitting the variable arguments on my own, but what if there was an annotation similar to @ConvertWith, let's call it @Split that accepts an optional separator to further subdivides any argument passed to it? So my example would look like this:
@ParameterizedTest
@CsvSource({"1,a", "1,a:b:c"})
void testAbc(int arg1, @Split(':') String[] elements) {
System.out.println(Arrays.toString(elements));
}
The benefit of making such a step explicit is that it allows to use this behaviour more than once inside a method. If we go one step further in the example we could even allow for nesting to get n dimensional structures:
@ParameterizedTest
@CsvSource({"1,a", "1,a.0:b.1:c.3"})
void testAbc(int arg1, @Split(value = ':', sub= @Split('.')) String[][] elements) {
System.out.println(Arrays.toString(elements));
}
Note that in all cases @Split is just a special @ConvertWith annotation, so they should be interchangeable.
Conclusion
I ended up writing a custom ArgumentConverter that accepts a string and splits it accordingly, but it would still be nice if JUnit-Params offered such a behaviour out-of-the-box
This works:
@ParameterizedTest
@CsvSource({"1,a", "1,a,b,c"})
void testAbc(int arg1, @AggregateWith(VarargsAggregator.class) String... elements) {
System.out.println(Arrays.toString(elements));
}
static class VarargsAggregator implements ArgumentsAggregator {
@Override
public Object aggregateArguments(ArgumentsAccessor accessor, ParameterContext context) throws ArgumentsAggregationException {
return accessor.toList().stream()
.skip(context.getIndex())
.map(String::valueOf)
.toArray(String[]::new);
}
}
Here's a version that works for any non-primitive component type:
@ParameterizedTest
@CsvSource({"1,a", "1,a,b,c"})
void testAbc(int arg1, @AggregateWith(VarargsAggregator.class) String... elements) {
System.out.println(Arrays.toString(elements));
}
static class VarargsAggregator implements ArgumentsAggregator {
@Override
public Object aggregateArguments(ArgumentsAccessor accessor, ParameterContext context) throws ArgumentsAggregationException {
Class<?> parameterType = context.getParameter().getType();
Preconditions.condition(parameterType.isArray(), () -> "must be an array type, but was " + parameterType);
Class<?> componentType = parameterType.getComponentType();
return IntStream.range(context.getIndex(), accessor.size())
.mapToObj(index -> accessor.get(index, componentType))
.toArray(size -> (Object[]) Array.newInstance(componentType, size));
}
}
Interesting solution: VarargsAggregator implements ArgumentsAggregator
Wonder, whether the mapping and array-aggregation can be made so adoptable, that the VarargsAggregator could become a standard feature of the org.junit.jupiter.params module.
Tentatively slated for 5.7 M2 solely for the purpose of team discussion regarding the possible introduction of a built-in VarargsAggregator that handles primitive and non-primitive array component types.
Team decision: Investigate whether supporting varargs parameters by default would break any existing use case. If not, do it.
I think it's also worth considering whether you should implement a placeholder to represent a range of arguments for the format string of the display name to be used for individual invocations of the parameterized test.
For example, for the use case stated in this issue, I would like to format the name of each invocation like this:
@ParameterizedTest(name = "[{index}] arg1: {0}, elements: {args...}") @CsvSource({"1,a", "1,a,b,c"}) void testAbc(int arg1, String... elements) { ... }
testAbc(int, String[]) ✔
├─ [1] arg1: 1, elements: a ✔
└─ [2] arg1: 1, elements: a, b, c ✔
@marcphilipp I would suggest adding a case for when the parameter list is empty, to avoid getting an array with a single null entry. Perhaps there's a cleaner way, but I used:
if (result.length == 1 && result[0] == null) {
return Array.newInstance(componentType, 0);
}
return result;
@marcphilipp I would suggest adding a case for when the parameter list is empty, to avoid getting an array with a single null entry.
@joshgold22 Sometimes, null entries are useful, so loosing the ability to distinguish between the two cases seems like a loss to me.
FYI, you can still express an empty varargs list by just omitting the leading comma in a @CsvSource
@CsvSource({
"1, a" // No varargs
"1, a, vararg1, vararg2"
"1, a, vararg1, , vararg3" // null for vararg2
"1, a, " // null for vararg1
})
And just in case someone else finds this helpful, here's my own VarargsAggregator:
public class VarargsAggregator implements ArgumentsAggregator {
@Override
public Object aggregateArguments(ArgumentsAccessor accessor, ParameterContext context)
throws ArgumentsAggregationException {
Preconditions.condition(context.getParameter().isVarArgs(),
"VarargsAggregator is meant to be applied to varargs arguments only");
Class<?> componentType = context.getParameter().getType().getComponentType();
Object[] varargs = accessor.toList().stream().skip(context.getIndex()).toArray();
return Arrays.stream(varargs)
.map(value -> DefaultArgumentConverter.INSTANCE.convert(value, componentType))
.toArray(size -> (Object[]) Array.newInstance(componentType, size));
}
}
@sewe, thanks for pointing out that leaving off the leading comma before the vararg creates an empty array in @marcphilipp 's code (or yours) without my tweak.
It still feels a unintuitive to me personally that the trailing comma leads to a singleton null rather than an empty varag, but for that I would just ask to make the distinction between the behaviors with/without the comma very explicit both in javadoc and user documentation.
Also, I like the use of DefaultArgumentConverter.INSTANCE in your version @sewe, and wondered if it might be possible to support a stacked @ConvertWith to specify an another converter for the components (if that's not already supported.) I'm picturing something akin to ParameterizedTestMethodContext.ResolverType#createResolver. (This can't be used in a user-level library because of permissions, but something like it could be used in an internal implementation.)