Skip to content

Commit 125e10b

Browse files
marcphilippmarcphilipp
committed
Allow ArgumentConverter implementations to use constructor injection
Issue: #4018
1 parent ef63ac9 commit 125e10b

File tree

8 files changed

+109
-53
lines changed

8 files changed

+109
-53
lines changed

‎junit-jupiter-params/src/jmh/java/org/junit/jupiter/params/ParameterizedTestNameFormatterBenchmarks.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ public void formatTestNames(Blackhole blackhole) throws Exception {
4747
var formatter = new ParameterizedTestNameFormatter(
4848
ParameterizedTest.DISPLAY_NAME_PLACEHOLDER + " " + ParameterizedTest.DEFAULT_DISPLAY_NAME + " ({0})",
4949
"displayName",
50-
new ParameterizedTestMethodContext(TestCase.class.getDeclaredMethod("parameterizedTest", int.class)), 512);
50+
new ParameterizedTestMethodContext(TestCase.class.getDeclaredMethod("parameterizedTest", int.class), null),
51+
512);
5152
for (int i = 0; i < argumentsList.size(); i++) {
5253
Arguments arguments = argumentsList.get(i);
5354
blackhole.consume(formatter.format(i, arguments, arguments.get()));

‎junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTestExtension.java

+2-38
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,8 @@
1313
import static org.junit.platform.commons.support.AnnotationSupport.findAnnotation;
1414
import static org.junit.platform.commons.support.AnnotationSupport.findRepeatableAnnotations;
1515
import static org.junit.platform.commons.support.AnnotationSupport.isAnnotated;
16-
import static org.junit.platform.commons.util.CollectionUtils.getFirstElement;
1716

18-
import java.lang.reflect.Constructor;
1917
import java.lang.reflect.Method;
20-
import java.util.Optional;
2118
import java.util.concurrent.atomic.AtomicLong;
2219
import java.util.stream.Stream;
2320

@@ -29,12 +26,8 @@
2926
import org.junit.jupiter.params.provider.ArgumentsProvider;
3027
import org.junit.jupiter.params.provider.ArgumentsSource;
3128
import org.junit.jupiter.params.support.AnnotationConsumerInitializer;
32-
import org.junit.platform.commons.JUnitException;
33-
import org.junit.platform.commons.PreconditionViolationException;
34-
import org.junit.platform.commons.support.ModifierSupport;
3529
import org.junit.platform.commons.util.ExceptionUtils;
3630
import org.junit.platform.commons.util.Preconditions;
37-
import org.junit.platform.commons.util.ReflectionUtils;
3831

3932
/**
4033
* @since 5.0
@@ -57,7 +50,7 @@ public boolean supportsTestTemplate(ExtensionContext context) {
5750
return false;
5851
}
5952

60-
ParameterizedTestMethodContext methodContext = new ParameterizedTestMethodContext(testMethod);
53+
ParameterizedTestMethodContext methodContext = new ParameterizedTestMethodContext(testMethod, context);
6154

6255
Preconditions.condition(methodContext.hasPotentiallyValidSignature(),
6356
() -> String.format(
@@ -89,7 +82,7 @@ public Stream<TestTemplateInvocationContext> provideTestTemplateInvocationContex
8982
return findRepeatableAnnotations(templateMethod, ArgumentsSource.class)
9083
.stream()
9184
.map(ArgumentsSource::value)
92-
.map(clazz -> instantiateArgumentsProvider(clazz, extensionContext))
85+
.map(clazz -> ParameterizedTestSpiInstantiator.instantiate(ArgumentsProvider.class, clazz, extensionContext))
9386
.map(provider -> AnnotationConsumerInitializer.initialize(templateMethod, provider))
9487
.flatMap(provider -> arguments(provider, extensionContext))
9588
.map(arguments -> {
@@ -102,35 +95,6 @@ public Stream<TestTemplateInvocationContext> provideTestTemplateInvocationContex
10295
// @formatter:on
10396
}
10497

105-
private ArgumentsProvider instantiateArgumentsProvider(Class<? extends ArgumentsProvider> clazz,
106-
ExtensionContext extensionContext) {
107-
return extensionContext.getExecutableInvoker().invoke(findConstructor(ArgumentsProvider.class, clazz));
108-
}
109-
110-
@SuppressWarnings("unchecked")
111-
private static <T> Constructor<? extends T> findConstructor(Class<T> spiClass, Class<? extends T> clazz) {
112-
Optional<Constructor<?>> defaultConstructor = getFirstElement(
113-
ReflectionUtils.findConstructors(clazz, it -> it.getParameterCount() == 0));
114-
if (defaultConstructor.isPresent()) {
115-
return (Constructor<? extends T>) defaultConstructor.get();
116-
}
117-
if (ModifierSupport.isNotStatic(clazz)) {
118-
String message = String.format("The %s [%s] must be either a top-level class or a static nested class",
119-
spiClass.getSimpleName(), clazz.getName());
120-
throw new JUnitException(message);
121-
}
122-
try {
123-
return ReflectionUtils.getDeclaredConstructor(clazz);
124-
}
125-
catch (PreconditionViolationException ex) {
126-
String message = String.format(
127-
"Failed to find constructor for %s [%s]. "
128-
+ "Please ensure that a no-argument or a single constructor exists.",
129-
spiClass.getSimpleName(), clazz.getName());
130-
throw new JUnitException(message);
131-
}
132-
}
133-
13498
private ExtensionContext.Store getStore(ExtensionContext context) {
13599
return context.getStore(Namespace.create(ParameterizedTestExtension.class, context.getRequiredTestMethod()));
136100
}

‎junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTestMethodContext.java

+9-6
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.List;
2121
import java.util.Optional;
2222

23+
import org.junit.jupiter.api.extension.ExtensionContext;
2324
import org.junit.jupiter.api.extension.ParameterContext;
2425
import org.junit.jupiter.api.extension.ParameterResolutionException;
2526
import org.junit.jupiter.params.aggregator.AggregateWith;
@@ -43,16 +44,18 @@
4344
class ParameterizedTestMethodContext {
4445

4546
private final Parameter[] parameters;
47+
private final ExtensionContext extensionContext;
4648
private final Resolver[] resolvers;
4749
private final List<ResolverType> resolverTypes;
4850

49-
ParameterizedTestMethodContext(Method testMethod) {
51+
ParameterizedTestMethodContext(Method testMethod, ExtensionContext extensionContext) {
5052
this.parameters = testMethod.getParameters();
5153
this.resolvers = new Resolver[this.parameters.length];
5254
this.resolverTypes = new ArrayList<>(this.parameters.length);
5355
for (Parameter parameter : this.parameters) {
5456
this.resolverTypes.add(isAggregator(parameter) ? AGGREGATOR : CONVERTER);
5557
}
58+
this.extensionContext = extensionContext;
5659
}
5760

5861
/**
@@ -167,7 +170,7 @@ Object resolve(ParameterContext parameterContext, Object[] arguments, int invoca
167170
private Resolver getResolver(ParameterContext parameterContext) {
168171
int index = parameterContext.getIndex();
169172
if (resolvers[index] == null) {
170-
resolvers[index] = resolverTypes.get(index).createResolver(parameterContext);
173+
resolvers[index] = resolverTypes.get(index).createResolver(parameterContext, extensionContext);
171174
}
172175
return resolvers[index];
173176
}
@@ -176,11 +179,11 @@ enum ResolverType {
176179

177180
CONVERTER {
178181
@Override
179-
Resolver createResolver(ParameterContext parameterContext) {
182+
Resolver createResolver(ParameterContext parameterContext, ExtensionContext extensionContext) {
180183
try { // @formatter:off
181184
return AnnotationSupport.findAnnotation(parameterContext.getParameter(), ConvertWith.class)
182185
.map(ConvertWith::value)
183-
.map(clazz -> (ArgumentConverter) ReflectionSupport.newInstance(clazz))
186+
.map(clazz -> ParameterizedTestSpiInstantiator.instantiate(ArgumentConverter.class, clazz, extensionContext))
184187
.map(converter -> AnnotationConsumerInitializer.initialize(parameterContext.getParameter(), converter))
185188
.map(Converter::new)
186189
.orElse(Converter.DEFAULT);
@@ -193,7 +196,7 @@ Resolver createResolver(ParameterContext parameterContext) {
193196

194197
AGGREGATOR {
195198
@Override
196-
Resolver createResolver(ParameterContext parameterContext) {
199+
Resolver createResolver(ParameterContext parameterContext, ExtensionContext extensionContext) {
197200
try { // @formatter:off
198201
return AnnotationSupport.findAnnotation(parameterContext.getParameter(), AggregateWith.class)
199202
.map(AggregateWith::value)
@@ -207,7 +210,7 @@ Resolver createResolver(ParameterContext parameterContext) {
207210
}
208211
};
209212

210-
abstract Resolver createResolver(ParameterContext parameterContext);
213+
abstract Resolver createResolver(ParameterContext parameterContext, ExtensionContext extensionContext);
211214

212215
}
213216

‎junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTestSpiInstantiator.java

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/*
2+
* Copyright 2015-2024 the original author or authors.
3+
*
4+
* All rights reserved. This program and the accompanying materials are
5+
* made available under the terms of the Eclipse Public License v2.0 which
6+
* accompanies this distribution and is available at
7+
*
8+
* https://www.eclipse.org/legal/epl-v20.html
9+
*/
10+
11+
package org.junit.jupiter.params;
12+
13+
import static org.junit.platform.commons.util.CollectionUtils.getFirstElement;
14+
15+
import java.lang.reflect.Constructor;
16+
import java.util.Optional;
17+
18+
import org.junit.jupiter.api.extension.ExtensionContext;
19+
import org.junit.platform.commons.JUnitException;
20+
import org.junit.platform.commons.PreconditionViolationException;
21+
import org.junit.platform.commons.support.ModifierSupport;
22+
import org.junit.platform.commons.util.ReflectionUtils;
23+
24+
/**
25+
* @since 5.12
26+
*/
27+
class ParameterizedTestSpiInstantiator {
28+
29+
static <T> T instantiate(Class<T> spiClass, Class<? extends T> clazz, ExtensionContext extensionContext) {
30+
return extensionContext.getExecutableInvoker().invoke(findConstructor(spiClass, clazz));
31+
}
32+
33+
/**
34+
* Find the "best" constructor for the supplied class.
35+
*
36+
* <p>For backward compatibility, it first checks for a default constructor
37+
* which takes precedence over any other constructor. If no default
38+
* constructor is found, it checks for a single constructor and returns it.
39+
*/
40+
@SuppressWarnings("unchecked")
41+
private static <T> Constructor<? extends T> findConstructor(Class<T> spiClass, Class<? extends T> clazz) {
42+
Optional<Constructor<?>> defaultConstructor = getFirstElement(
43+
ReflectionUtils.findConstructors(clazz, it -> it.getParameterCount() == 0));
44+
if (defaultConstructor.isPresent()) {
45+
return (Constructor<? extends T>) defaultConstructor.get();
46+
}
47+
if (ModifierSupport.isNotStatic(clazz)) {
48+
String message = String.format("The %s [%s] must be either a top-level class or a static nested class",
49+
spiClass.getSimpleName(), clazz.getName());
50+
throw new JUnitException(message);
51+
}
52+
try {
53+
return ReflectionUtils.getDeclaredConstructor(clazz);
54+
}
55+
catch (PreconditionViolationException ex) {
56+
String message = String.format(
57+
"Failed to find constructor for %s [%s]. "
58+
+ "Please ensure that a no-argument or a single constructor exists.",
59+
spiClass.getSimpleName(), clazz.getName());
60+
throw new JUnitException(message);
61+
}
62+
}
63+
}

‎junit-jupiter-params/src/main/java/org/junit/jupiter/params/converter/ArgumentConverter.java

+6-4
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import org.apiguardian.api.API;
1616
import org.junit.jupiter.api.extension.ParameterContext;
17+
import org.junit.jupiter.api.extension.ParameterResolver;
1718

1819
/**
1920
* {@code ArgumentConverter} is an abstraction that allows an input object to
@@ -24,10 +25,11 @@
2425
* method with the help of a
2526
* {@link org.junit.jupiter.params.converter.ConvertWith @ConvertWith} annotation.
2627
*
27-
* <p>Implementations must provide a no-args constructor and should not make any
28-
* assumptions regarding when they are instantiated or how often they are called.
29-
* Since instances may potentially be cached and called from different threads,
30-
* they should be thread-safe and designed to be used as singletons.
28+
* <p>Implementations must provide a no-args constructor or a single unambiguous
29+
* constructor to use {@linkplain ParameterResolver parameter resolution}. They
30+
* should not make any assumptions regarding when they are instantiated or how
31+
* often they are called. Since instances may potentially be cached and called
32+
* from different threads, they should be thread-safe.
3133
*
3234
* <p>Extend {@link SimpleArgumentConverter} if your implementation only needs
3335
* to know the target type and does not need access to the {@link ParameterContext}

‎jupiter-tests/src/test/java/org/junit/jupiter/params/ParameterizedTestIntegrationTests.java

+22
Original file line numberDiff line numberDiff line change
@@ -1218,6 +1218,13 @@ void injectsParametersIntoArgumentsProviderConstructor() {
12181218
.testEvents() //
12191219
.assertStatistics(it -> it.succeeded(1));
12201220
}
1221+
1222+
@Test
1223+
void injectsParametersIntoArgumentConverterConstructor() {
1224+
execute(SpiParameterInjectionTestCase.class, "argumentConverterWithConstructorParameter", String.class) //
1225+
.testEvents() //
1226+
.assertStatistics(it -> it.succeeded(1));
1227+
}
12211228
}
12221229

12231230
// -------------------------------------------------------------------------
@@ -2162,13 +2169,28 @@ void argumentsProviderWithConstructorParameter(String argument) {
21622169
assertEquals("resolved value", argument);
21632170
}
21642171

2172+
@ParameterizedTest
2173+
@ValueSource(strings = "value")
2174+
void argumentConverterWithConstructorParameter(
2175+
@ConvertWith(ArgumentConverterWithConstructorParameter.class) String argument) {
2176+
assertEquals("resolved value", argument);
2177+
}
2178+
21652179
record ArgumentsProviderWithConstructorParameter(String value) implements ArgumentsProvider {
21662180

21672181
@Override
21682182
public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
21692183
return Stream.of(arguments(value));
21702184
}
21712185
}
2186+
2187+
record ArgumentConverterWithConstructorParameter(String value) implements ArgumentConverter {
2188+
2189+
@Override
2190+
public Object convert(Object source, ParameterContext context) throws ArgumentConversionException {
2191+
return value;
2192+
}
2193+
}
21722194
}
21732195

21742196
private static class TwoSingleStringArgumentsProvider implements ArgumentsProvider {

‎jupiter-tests/src/test/java/org/junit/jupiter/params/ParameterizedTestMethodContextTests.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import static org.junit.jupiter.api.Assertions.assertFalse;
1414
import static org.junit.jupiter.api.Assertions.assertTrue;
15+
import static org.mockito.Mockito.mock;
1516

1617
import java.lang.reflect.Method;
1718
import java.util.Arrays;
@@ -33,13 +34,13 @@ class ParameterizedTestMethodContextTests {
3334
@ValueSource(strings = { "onePrimitive", "twoPrimitives", "twoAggregators", "twoAggregatorsWithTestInfoAtTheEnd",
3435
"mixedMode" })
3536
void validSignatures(String name) {
36-
assertTrue(new ParameterizedTestMethodContext(method(name)).hasPotentiallyValidSignature());
37+
assertTrue(new ParameterizedTestMethodContext(method(name), mock()).hasPotentiallyValidSignature());
3738
}
3839

3940
@ParameterizedTest
4041
@ValueSource(strings = { "twoAggregatorsWithPrimitiveInTheMiddle", "twoAggregatorsWithTestInfoInTheMiddle" })
4142
void invalidSignatures(String name) {
42-
assertFalse(new ParameterizedTestMethodContext(method(name)).hasPotentiallyValidSignature());
43+
assertFalse(new ParameterizedTestMethodContext(method(name), mock()).hasPotentiallyValidSignature());
4344
}
4445

4546
private Method method(String name) {

‎jupiter-tests/src/test/java/org/junit/jupiter/params/ParameterizedTestNameFormatterTests.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -330,8 +330,8 @@ private static ParameterizedTestNameFormatter formatter(String pattern, String d
330330
}
331331

332332
private static ParameterizedTestNameFormatter formatter(String pattern, String displayName, Method method) {
333-
return new ParameterizedTestNameFormatter(pattern, displayName, new ParameterizedTestMethodContext(method),
334-
512);
333+
return new ParameterizedTestNameFormatter(pattern, displayName,
334+
new ParameterizedTestMethodContext(method, mock()), 512);
335335
}
336336

337337
private static String format(ParameterizedTestNameFormatter formatter, int invocationIndex, Arguments arguments) {

0 commit comments

Comments
 (0)