Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Syntax for local @MethodSource factory method does not support canonical array names #3131

Closed
1 task done
sbrannen opened this issue Jan 22, 2023 · 3 comments
Closed
1 task done

Comments

@sbrannen
Copy link
Member

sbrannen commented Jan 22, 2023

Overview

Given a @MethodSource factory method that accepts an array -- for example, Stream<Arguments> factory(int[] quantities):

  • When using the fully qualified method name (FQMN), a user can supply either the binary or source (canonical) name for the array type.
    • @MethodSource("example.MethodSourceTests#factory([I)")
    • @MethodSource("example.MethodSourceTests#factory(int[])")
  • When using the local qualified method name (LQMN), a user should be able to supply either the binary or source (canonical) name for the array type; however, currently only the binary name is supported.
    • @MethodSource("factory([I)")
    • @MethodSource("factory(int[])")

Examples

Given the following extension:

class IntegerArrayResolver implements ParameterResolver {
	@Override
	public boolean supportsParameter(ParameterContext pc, ExtensionContext ec) {
		return pc.getParameter().getType() == int[].class;
	}
	@Override
	public Object resolveParameter(ParameterContext pc, ExtensionContext ec) {
		return new int[] { 2, 3 };
	}
}

And given the following test class:

@ExtendWith(IntegerArrayResolver.class)
class MethodSourceTests {

	@ParameterizedTest
	// @MethodSource("example.MethodSourceTests#factory([I)")
	// @MethodSource("example.MethodSourceTests#factory(int[])")
	// @MethodSource("factory([I)")
	@MethodSource("factory(int[])")
	void test(String argument) {
		assertTrue(argument.startsWith("2"));
	}

	static Stream<Arguments> factory(int quantities) {
		return Stream.of(arguments(quantities + " apples"), arguments(quantities + " lemons"));
	}

	static Stream<Arguments> factory(int[] quantities) {
		return Stream.of(arguments(quantities[0] + " apples"), arguments(quantities[0] + " lemons"));
	}

}

All @MethodSource variants that are commented out above work.

When running the test "as is", we get the following error.

org.junit.platform.commons.PreconditionViolationException: 2 factory methods named [factory(int[])] were found in class [example.MethodSourceTests]: [static java.util.stream.Stream example.MethodSourceTests.factory(int), static java.util.stream.Stream example.MethodSourceTests.factory(int[])]

Related Issues

Deliverables

  • Ensure that canonical array names are supported in the syntax for local qualified method names in @MethodSource.
@sbrannen
Copy link
Member Author

Reopening to document in release notes.

@sbrannen sbrannen reopened this Jan 23, 2023
@sbrannen sbrannen modified the milestones: 5.10.0-M1, 5.9.3 Apr 22, 2023
@sbrannen
Copy link
Member Author

Reopening to backport to 5.9.3.

@sbrannen sbrannen reopened this Apr 22, 2023
marcphilipp pushed a commit that referenced this issue Apr 22, 2023
Prior to this commit, parameters were not validated in local
@MethodSource factory methods which lead to incorrect configuration
being silently ignored or confusing error messages when the specified
factory method was overloaded. In addition, the syntax for local
@MethodSource factory methods did not support canonical array names.

This commit addresses these issues by treating local factory method
resolution the same as the existing support for resolving a fully
qualified method name for a factory method.

These changes make the recently introduced parseQualifiedMethodName()
method in ReflectionUtils obsolete. This commit therefore removes that
method as well.

Closes #3130
Closes #3131

(cherry picked from commit a0eb8e2)
@marcphilipp
Copy link
Member

Cherry-picked to the releases/5.9.x branch.

sbrannen added a commit that referenced this issue Apr 23, 2023
Prior to this commit, given overloaded @MethodSource factory methods, a
local qualified method name (LQMN) was not treated the same as a
fully-qualified method name (FQMN).

Specifically, if the user provided a FQMN without specifying the
parameter list, JUnit Jupiter would find the factory method that
accepts zero arguments. Whereas, if the user provided an LQMN without
specifying the parameter list, JUnit Jupiter would fail to find the
factory method.

This commit fixes that bug by reworking the internals of
MethodArgumentsProvider so that overloaded local and external factory
methods are looked up with the same semantics.

The commit also improves diagnostics for failure to find a factory
method specified via LQMN by falling back to the same lenient search
semantics that are used to locate a "default" local factory method.

Furthermore, this commit modifies the internals of
MethodArgumentsProvider so that it consistently throws
PreconditionViolationExceptions for user configuration errors.

This commit also introduces additional tests for error use cases.

See: #3130, #3131
Closes: #3266
sbrannen added a commit that referenced this issue Apr 23, 2023
Prior to this commit, given overloaded @MethodSource factory methods, a
local qualified method name (LQMN) was not treated the same as a
fully-qualified method name (FQMN).

Specifically, if the user provided a FQMN without specifying the
parameter list, JUnit Jupiter would find the factory method that
accepts zero arguments. Whereas, if the user provided an LQMN without
specifying the parameter list, JUnit Jupiter would fail to find the
factory method.

This commit fixes that bug by reworking the internals of
MethodArgumentsProvider so that overloaded local and external factory
methods are looked up with the same semantics.

The commit also improves diagnostics for failure to find a factory
method specified via LQMN by falling back to the same lenient search
semantics that are used to locate a "default" local factory method.

Furthermore, this commit modifies the internals of
MethodArgumentsProvider so that it consistently throws
PreconditionViolationExceptions for user configuration errors.

This commit also introduces additional tests for error use cases.

See: #3130, #3131
Closes: #3266
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants