Non-adjacent repeatable annotations do not work with @CartesianProductTest

[scordio]

While working on #414, I realized there is a nasty issue when repeatable annotations have other annotations (repeatable or not) in between.

Take this example:

@CartesianProductTest
@IntRangeSource(from = 0, to = 4)
@CartesianValueSource(longs = { 2, 4 })
@IntRangeSource(from = 0, to = 4)
void test(int i1, long l, int i2) {
}

The test execution will fail with:

org.junit.jupiter.api.extension.ParameterResolutionException: No ParameterResolver registered for parameter [long arg1] in method [void org.example.MyTest.test(int,long,int)].
	at org.junit.jupiter.engine.execution.ExecutableInvoker.resolveParameter(ExecutableInvoker.java:200)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.resolveParameters(ExecutableInvoker.java:183)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.resolveParameters(ExecutableInvoker.java:144)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:96)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:202)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:198)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:135)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:69)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:135)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:171)
	at java.base/java.util.concurrent.RecursiveAction.exec(RecursiveAction.java:189)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1016)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1665)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1598)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)

This is due to the fact that the absolute declaration order of repeatable annotations is not kept when accessing them via reflection, as each group is encapsulated in the container annotation (still keeping the relative order, though).

So, when the extension invokes the corresponding argument providers, the order of:

@IntRangeSource(from = 0, to = 4)
@CartesianValueSource(longs = { 2, 4 })
@IntRangeSource(from = 0, to = 4)

is actually:

@IntRangeSources({
    @IntRangeSource(from = 0, to = 4)
    @IntRangeSource(from = 0, to = 4)
})
@CartesianValueSource(longs = { 2, 4 })

and an int value is proposed to the second parameter, which is a long, hence the exception.

At the moment I don't see any way to solve this, but I'm far from being an expert on this topic.

What I see is that the error message does not help an end-user who is not fully familiar with how repeatable annotation ordering works under the hood (like me 🙂). He will not understand that reordering the annotations, and the corresponding parameters, is needed to make the test work.

I think the docs should be enhanced to mention this limitation. Changing annotations and parameters order should not be an issue for the users.

Another option could be to make the extension able to detect those cases and fail with a better message, but I'm not sure it is possible.

Any thoughts?

[Michael1993]

Give me a bit of time to ponder the problem.

[Michael1993]

I've been thinking about this. The way I see it, we have 3 options: the two @scordio already listed and to try and make the extension work with the annotation mismatch.

The documentation update should be fairly straightforward.

I have a hunch how the extension could detect such a scenario, but it would require more reflection and evaluation (increased complexity), which could make the extension both slower and harder to maintain, which could bring unintended downsides with it.

Right now, I'm leaning towards updating the documentation and opening an issue which people can "upvote" if they want to see this feature implemented... Getting some more feedback would be great.

[Bukama]

I'm also for updating the documentation

[mjsztainbok]

I just hit this issue myself. I had the following code:

            @CartesianProductTest
            @CartesianValueSource(booleans = {true, false})
            @CartesianEnumSource(value = ReadOnlyConnectionSupport.class)
            @CartesianValueSource(booleans = {true, false})
            void whenTheUserHasASystemSession(boolean hasExistingConnectionMgr, ReadOnlyConnectionSupport readOnlyConnectionSupport, boolean enableForOrgByConfigSetting) throws Exception {

and it was failing as it thought that the 2 boolean values were first and the ReadOnlyConnectionSupport was the last parameter.

The ordering is important to me as these tests are incremental with an extra parameter so I don't want to have to make the tests more difficult to read because of ordering

[Michael1993]

We can't really do mix-and-match with the annotations because the annotation order is simply not preserved at all.

For example:

@CartesianProductTest
@CartesianValueSource(ints = {1,2,3})
@IntRangeSource(from = 0, to = 5)
@CartesianValueSource(ints = {1,2,3})
void testInts(int i1, int i2, int i3) {

}

becomes

@CartesianProductTest
@CartesianValueSources({
    @CartesianValueSource(ints = {1,2,3})
    @CartesianValueSource(ints = {1,2,3})
})
@IntRangeSource(from = 1, to = 5)
void testInts(int arg0, int arg1, int arg2) {
}

Which is not correct, but works. I'm fairly certain it's not possible to determine the original order of the annotations, therefore we can't "fix" this error.

I recommend you do either of these:

• reorder your parameters
• use a factory method for your test input

[Michael1993]

Investigated this issue a bit further - we could accommodate special edge cases. It would require careful checking of parameter types and arguments provided from the annotations. The risk of introducing unwanted side-effects seem high.

I'm kind of at my wits end. If someone else has a good solution for this, feel free to open a PR or propose a solution,

EDIT: Okay, I've asked for advice and this can be easily solved if we just add index() default -1 to all annotations the extension uses. It's backward-compatible and people don't have to use it if they are not encountering this issue. I'll update the documentation and implement this Soon™.

[nipafx]

I thought about improving the error message. Does the extension work if m out of n parameters (m < n) use this extension, but it's not the first m? If not, then:

• that should probably be mentioned somewhere
• when there are m Cartesian annotations and we're at an index < m, but the parameter doesn't match, we can throw an error or log a message that while an annotation was found it doesn't have the right type
• in that case, we can furthermore check whether there are repeatable annotations in play and then add a guess to the message that this could be the cause

Sounds like a lot of work for an error case, though.

[Michael1993]

The new recommendation for anyone encountering this issue is: try CartesianTest instead of CartesianProductTest!

(...and if you still have trouble, feel free to open a new issue.)