Third-party ParameterResolvers may potentially conflict with ParameterizedTestParameterResolver

[smoyer64]

Overview

In the following code snippet, the Weld CDI system provides a ParameterResolver for the third parameter in the parametersTakePriorityOverInjection() method. Since the@TestTemplate "knows" that it's producing parameters for the first two parameters via its ArgumentSource, it shouldn't call either the supportsParameter() or resolvesParameter() methods of Weld's ParameterResolver. Other registered ParameterResolvers should only be called for parameters with indexes greater than one.

@ExtendWith(WeldJunit5Extension.class)
public class ParameterizedTestCompatibilityTest {

	static final String SENTENCE = " is a greek letter.";

	@ParameterizedTest
	@CsvSource({
			"Alpha, 0",
			"Beta, 1",
			"Gamma, 2"
	})
	void parametersTakePriorityOverInjection(String letter, int ordinal, String sentence) {
		assertTrue(letter.length() >= 4);
		assertTrue(ordinal >= 0 && ordinal <= 2);
		assertEquals(SENTENCE, sentence);
	}

	@Produces
	@ApplicationScoped
	String produceSentence() {
		return " is a greek letter.";
	}

}

When run, this code currently throws a ParameterResolutionException with the message:

Discovered multiple competing ParameterResolvers for parameter [java.lang.String arg0] in executable [void org.jboss.weld.junit5.compatibility.ParameterizedTestCompatibilityTest.parametersTakePriorityOverInjection(java.lang.String,int,java.lang.String)]: org.jboss.weld.junit5.WeldJunit5Extension, org.junit.jupiter.params.ParameterizedTestParameterResolver

This issue is somewhat related to #935 but occurs when additional parameters are correctly placed after those injected by the TestTemplate.

Versions

• junit-jupiter-api: 5.0.1, 5.0.2, 5.1 M1
• junit-jupiter-params: 5.0.1, 5.0.2, 5.1 M1

Related Issues

• ParameterResolutionException with TestInfo and Parameterized Tests #935
• Create a JUnit 5 Extension to create test instances for the engine weld/weld-testing#3
• Introduce an option to make Weld injection more restrictive weld/weld-testing#13

Deliverables

• Parameterized tests should ignore other registered parameter resolvers for parameters that will be satisfied by an ArgumentSource.

[sbrannen]

@smoyer64 It appears to me that this may have been fixed in 5.0.2 in this commit: 41c76d6.

Can you please give it a try with 5.0.2?

[sbrannen]

FWIW, even if this scenario is not fixed in 5.0.2., I would argue that the WeldJunit5Extension is flawed.

Any ParameterResolver implementation (other than ParameterizedTestParameterResolver) which claims that it supports java.lang.String (or any other common Java type) without requiring the presence of a custom annotation for disambiguation is broken by design.

In other words, no extension should claim that it owns String.

Based on that principle, I therefore require users to annotate such parameters with @Autowired, @Qualifier, or @Value in the SpringExtension. 😉

[sbrannen]

@smoyer64, no need to try with 5.0.2.

I just confirmed the behavior against master with the following.

import static org.junit.jupiter.api.Assertions.assertEquals;

import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.ParameterContext;
import org.junit.jupiter.api.extension.ParameterResolver;
import org.junit.jupiter.params.provider.CsvSource;

/**
 * Tests that reproduce claims in GitHub issue #1184.
 */
@ExtendWith(EagerBeaverStringParameterResolver.class)
class Gh1184Tests {

	@ParameterizedTest
	@CsvSource("dogbert, 42")
	void test(String animal, int number, String text) {
		assertEquals("dogbert", animal);
		assertEquals(42, number);
		assertEquals("foo bar", text);
	}

}

class EagerBeaverStringParameterResolver implements ParameterResolver {

	@Override
	public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) {
		return parameterContext.getParameter().getType() == String.class;
	}

	@Override
	public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) {
		return "foo bar";
	}

}

[sbrannen]

Decision

We will see if we can avoid such collisions between such parameter resolvers for parameterized tests.

However... my previous claim that the WeldJunit5Extension is fundamentally broken still stands.

For example, even if we do address this issue for parameterized tests, the WeldJunit5Extension would still cause problems if used in conjunction with any other ParameterResolver that claims that it supports basic parameter types.

[sbrannen]

@smoyer64, is this the WeldJunit5Extension implementation you're talking about?

[smoyer64]

@sbrannen Yes, that's the class that I'm talking about. There's a whole discussion about whether that extension is "too greedy" in weld/weld-testing#3. Since Weld is a CDI implementation, it should behave much like Spring's CDI. You can control the context (which beans are managed) at a class level but I suspect I'm going to find a concrete example pretty quickly. Claiming String is a bold move!