Convert strings to Locale using the IETF BCP 47 format by default

Strings parameters of parameterized tests are now converted using 
`Locale.forLanguageTag` by default. The
`junit.jupiter.params.arguments.conversion.locale.format` configuration 
parameter allows to revert to the old behavior.

Resolves junit-team#3141.

Author: scordio

documentation/src/docs/asciidoc/release-notes/release-notes-5.13.0-M3.adoc

@@ -98,7 +98,10 @@ to start reporting discovery issues.
     - Blank `@SentenceFragment` declarations
     - `@BeforeParameterizedClassInvocation` and `@AfterParameterizedClassInvocation`
       methods declared in non-parameterized test classes
-
+* `java.util.Locale` arguments are now converted according to the IETF BCP 47 language tag
+  format. See the
+  <<../user-guide/index.adoc#writing-tests-parameterized-tests-argument-conversion-implicit, User Guide>>
+  for details.
 
 [[release-notes-5.13.0-M3-junit-vintage]]
 === JUnit Vintage

documentation/src/docs/asciidoc/user-guide/writing-tests.adoc

@@ -2483,10 +2483,16 @@ integral types: `byte`, `short`, `int`, `long`, and their boxed counterparts.
 | `java.time.ZoneId`         | `"Europe/Berlin"`                        -> `ZoneId.of("Europe/Berlin")`
 | `java.time.ZoneOffset`     | `"+02:30"`                               -> `ZoneOffset.ofHoursMinutes(2, 30)`
 | `java.util.Currency`       | `"JPY"`                                  -> `Currency.getInstance("JPY")`
-| `java.util.Locale`         | `"en"`                                   -> `new Locale("en")`
+| `java.util.Locale`         | `"en-US"`                                -> `Locale.forLanguageTag("en-US")`
 | `java.util.UUID`           | `"d043e930-7b3b-48e3-bdbe-5a3ccfb833db"` -> `UUID.fromString("d043e930-7b3b-48e3-bdbe-5a3ccfb833db")`
 |===
 
+WARNING: To revert to the old `java.util.Locale` conversion behavior of version 5.12 and
+earlier (which called the deprecated `Locale(String)` constructor), you can set the
+`junit.jupiter.params.arguments.conversion.locale.format`
+<<running-tests-config-params, configuration parameter>> to `iso_639`. However, please
+note that this parameter is deprecated and will be removed in a future release.
+
 [[writing-tests-parameterized-tests-argument-conversion-implicit-fallback]]
 ====== Fallback String-to-Object Conversion
 
@@ -2523,7 +2529,7 @@ include::{testDir}/example/ParameterizedTestDemo.java[tags=implicit_fallback_con
 [[writing-tests-parameterized-tests-argument-conversion-explicit]]
 ===== Explicit Conversion
 
-Instead of relying on implicit argument conversion you may explicitly specify an
+Instead of relying on implicit argument conversion, you may explicitly specify an
 `ArgumentConverter` to use for a certain parameter using the `@ConvertWith` annotation
 like in the following example. Note that an implementation of `ArgumentConverter` must be
 declared as either a top-level class or as a `static` nested class.

junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedInvocationContext.java

@@ -72,7 +72,7 @@ private void storeParameterInfo(ExtensionContext context) {
 		ParameterDeclarations declarations = this.declarationContext.getResolverFacade().getIndexedParameterDeclarations();
 		ClassLoader classLoader = getClassLoader(this.declarationContext.getTestClass());
 		Object[] arguments = this.arguments.getConsumedPayloads();
-		ArgumentsAccessor accessor = DefaultArgumentsAccessor.create(invocationIndex, classLoader, arguments);
+		ArgumentsAccessor accessor = DefaultArgumentsAccessor.create(context, invocationIndex, classLoader, arguments);
 		new DefaultParameterInfo(declarations, accessor).store(context);
 	}
 

junit-jupiter-params/src/main/java/org/junit/jupiter/params/ResolverFacade.java

@@ -425,7 +425,7 @@ private static Converter createConverter(ParameterDeclaration declaration, Exten
 					.map(clazz -> ParameterizedTestSpiInstantiator.instantiate(ArgumentConverter.class, clazz, extensionContext))
 					.map(converter -> AnnotationConsumerInitializer.initialize(declaration.getAnnotatedElement(), converter))
 					.map(Converter::new)
-					.orElse(Converter.DEFAULT);
+					.orElseGet(() -> Converter.createDefault(extensionContext));
 		} // @formatter:on
 		catch (Exception ex) {
 			throw parameterResolutionException("Error creating ArgumentConverter", ex, declaration.getParameterIndex());
@@ -467,10 +467,12 @@ Object resolve(FieldContext fieldContext, ExtensionContext extensionContext, Eva
 
 	private static class Converter implements Resolver {
 
-		private static final Converter DEFAULT = new Converter(DefaultArgumentConverter.INSTANCE);
-
 		private final ArgumentConverter argumentConverter;
 
+		private static Converter createDefault(ExtensionContext context) {
+			return new Converter(new DefaultArgumentConverter(context));
+		}
+
 		Converter(ArgumentConverter argumentConverter) {
 			this.argumentConverter = argumentConverter;
 		}

junit-jupiter-params/src/main/java/org/junit/jupiter/params/aggregator/DefaultArgumentsAccessor.java

@@ -19,6 +19,7 @@
 import java.util.function.BiFunction;
 
 import org.apiguardian.api.API;
+import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.jupiter.params.converter.DefaultArgumentConverter;
 import org.junit.platform.commons.util.ClassUtils;
 import org.junit.platform.commons.util.Preconditions;
@@ -40,10 +41,11 @@ public class DefaultArgumentsAccessor implements ArgumentsAccessor {
 	private final Object[] arguments;
 	private final BiFunction<Object, Class<?>, Object> converter;
 
-	public static DefaultArgumentsAccessor create(int invocationIndex, ClassLoader classLoader, Object[] arguments) {
+	public static DefaultArgumentsAccessor create(ExtensionContext context, int invocationIndex,
+			ClassLoader classLoader, Object[] arguments) {
 		Preconditions.notNull(classLoader, "ClassLoader must not be null");
 
-		BiFunction<Object, Class<?>, Object> converter = (source, targetType) -> DefaultArgumentConverter.INSTANCE //
+		BiFunction<Object, Class<?>, Object> converter = (source, targetType) -> new DefaultArgumentConverter(context) //
 				.convert(source, targetType, classLoader);
 		return new DefaultArgumentsAccessor(converter, invocationIndex, arguments);
 	}

junit-jupiter-params/src/main/java/org/junit/jupiter/params/converter/DefaultArgumentConverter.java

@@ -21,8 +21,10 @@
 import java.util.Currency;
 import java.util.Locale;
 import java.util.UUID;
+import java.util.function.Function;
 
 import org.apiguardian.api.API;
+import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.jupiter.api.extension.ParameterContext;
 import org.junit.jupiter.params.support.FieldContext;
 import org.junit.platform.commons.support.conversion.ConversionException;
@@ -50,10 +52,31 @@
 @API(status = INTERNAL, since = "5.0")
 public class DefaultArgumentConverter implements ArgumentConverter {
 
-	public static final DefaultArgumentConverter INSTANCE = new DefaultArgumentConverter();
+	/**
+	 * Property name used to set the format for the conversion of {@link Locale}
+	 * arguments: {@value}
+	 *
+	 * <h4>Supported Values</h4>
+	 * <ul>
+	 * <li>{@code bcp_47}: uses the IETF BCP 47 language tag format, delegating
+	 * the conversion to {@link Locale#forLanguageTag(String)}</li>
+	 * <li>{@code iso_639}: uses the ISO 639 alpha-2 or alpha-3 language code
+	 * format, delegating the conversion to {@link Locale#Locale(String)}</li>
+	 * </ul>
+	 *
+	 * <p>If not specified, the default is {@code bcp_47}.
+	 *
+	 * @since 5.13
+	 */
+	public static final String DEFAULT_LOCALE_CONVERSION_FORMAT_PROPERTY_NAME = "junit.jupiter.params.arguments.conversion.locale.format";
 
-	private DefaultArgumentConverter() {
-		// nothing to initialize
+	private static final Function<String, LocaleConversionFormat> TRANSFORMER = value -> LocaleConversionFormat.valueOf(
+		value.trim().toUpperCase(Locale.ROOT));
+
+	private final ExtensionContext context;
+
+	public DefaultArgumentConverter(ExtensionContext context) {
+		this.context = context;
 	}
 
 	@Override
@@ -84,6 +107,10 @@ public final Object convert(Object source, Class<?> targetType, ClassLoader clas
 		}
 
 		if (source instanceof String) {
+			if (targetType == Locale.class && getLocaleConversionFormat() == LocaleConversionFormat.BCP_47) {
+				return Locale.forLanguageTag((String) source);
+			}
+
 			try {
 				return convert((String) source, targetType, classLoader);
 			}
@@ -97,8 +124,21 @@ public final Object convert(Object source, Class<?> targetType, ClassLoader clas
 				source.getClass().getTypeName(), targetType.getTypeName()));
 	}
 
+	private LocaleConversionFormat getLocaleConversionFormat() {
+		return context.getConfigurationParameter(DEFAULT_LOCALE_CONVERSION_FORMAT_PROPERTY_NAME, TRANSFORMER) //
+				.orElse(LocaleConversionFormat.BCP_47);
+	}
+
 	Object convert(String source, Class<?> targetType, ClassLoader classLoader) {
 		return ConversionSupport.convert(source, targetType, classLoader);
 	}
 
+	enum LocaleConversionFormat {
+
+		BCP_47,
+
+		ISO_639
+
+	}
+
 }

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

@@ -22,6 +22,7 @@
 import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;
 import static org.junit.jupiter.engine.discovery.JupiterUniqueIdBuilder.appendTestTemplateInvocationSegment;
 import static org.junit.jupiter.engine.discovery.JupiterUniqueIdBuilder.uniqueIdForTestTemplateMethod;
+import static org.junit.jupiter.params.converter.DefaultArgumentConverter.DEFAULT_LOCALE_CONVERSION_FORMAT_PROPERTY_NAME;
 import static org.junit.jupiter.params.provider.Arguments.arguments;
 import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
 import static org.junit.platform.engine.discovery.DiscoverySelectors.selectIteration;
@@ -476,6 +477,29 @@ void failsWhenNoArgumentsSourceIsDeclared() {
 						"Configuration error: You must configure at least one arguments source for this @ParameterizedTest"))));
 	}
 
+	@Test
+	void executesWithDefaultLocaleConversionFormat() {
+		var results = execute(LocaleConversionTestCase.class, "testWithBcp47", Locale.class);
+
+		results.allEvents().assertStatistics(stats -> stats.started(4).succeeded(4));
+	}
+
+	@Test
+	void executesWithBcp47LocaleConversionFormat() {
+		var results = execute(Map.of(DEFAULT_LOCALE_CONVERSION_FORMAT_PROPERTY_NAME, "bcp_47"),
+			LocaleConversionTestCase.class, "testWithBcp47", Locale.class);
+
+		results.allEvents().assertStatistics(stats -> stats.started(4).succeeded(4));
+	}
+
+	@Test
+	void executesWithIso639LocaleConversionFormat() {
+		var results = execute(Map.of(DEFAULT_LOCALE_CONVERSION_FORMAT_PROPERTY_NAME, "iso_639"),
+			LocaleConversionTestCase.class, "testWithIso639", Locale.class);
+
+		results.allEvents().assertStatistics(stats -> stats.started(4).succeeded(4));
+	}
+
 	private EngineExecutionResults execute(DiscoverySelector... selectors) {
 		return EngineTestKit.engine(new JupiterTestEngine()).selectors(selectors).execute();
 	}
@@ -484,6 +508,14 @@ private EngineExecutionResults execute(Class<?> testClass, String methodName, Cl
 		return execute(selectMethod(testClass, methodName, ClassUtils.nullSafeToString(methodParameterTypes)));
 	}
 
+	private EngineExecutionResults execute(Map<String, String> configurationParameters, Class<?> testClass,
+			String methodName, Class<?>... methodParameterTypes) {
+		return EngineTestKit.engine(new JupiterTestEngine()) //
+				.selectors(selectMethod(testClass, methodName, ClassUtils.nullSafeToString(methodParameterTypes))) //
+				.configurationParameters(configurationParameters) //
+				.execute();
+	}
+
 	private EngineExecutionResults execute(String methodName, Class<?>... methodParameterTypes) {
 		return execute(TestCase.class, methodName, methodParameterTypes);
 	}
@@ -2508,6 +2540,24 @@ public static Stream<Arguments> zeroArgumentsProvider() {
 		}
 	}
 
+	static class LocaleConversionTestCase {
+
+		@ParameterizedTest
+		@ValueSource(strings = "en-US")
+		void testWithBcp47(Locale locale) {
+			assertEquals("en", locale.getLanguage());
+			assertEquals("US", locale.getCountry());
+		}
+
+		@ParameterizedTest
+		@ValueSource(strings = "en-US")
+		void testWithIso639(Locale locale) {
+			assertEquals("en-us", locale.getLanguage());
+			assertEquals("", locale.getCountry());
+		}
+
+	}
+
 	private static class TwoSingleStringArgumentsProvider implements ArgumentsProvider {
 
 		@Override

jupiter-tests/src/test/java/org/junit/jupiter/params/aggregator/DefaultArgumentsAccessorTests.java

@@ -16,10 +16,12 @@
 import static org.junit.jupiter.api.Assertions.assertIterableEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.Mockito.mock;
 
 import java.util.Arrays;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.platform.commons.PreconditionViolationException;
 
 /**
@@ -164,8 +166,9 @@ void size() {
 	}
 
 	private static DefaultArgumentsAccessor defaultArgumentsAccessor(int invocationIndex, Object... arguments) {
+		var context = mock(ExtensionContext.class);
 		var classLoader = DefaultArgumentsAccessorTests.class.getClassLoader();
-		return DefaultArgumentsAccessor.create(invocationIndex, classLoader, arguments);
+		return DefaultArgumentsAccessor.create(context, invocationIndex, classLoader, arguments);
 	}
 
 	@SuppressWarnings("unused")

jupiter-tests/src/test/java/org/junit/jupiter/params/converter/DefaultArgumentConverterTests.java

@@ -12,15 +12,25 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.junit.jupiter.params.converter.DefaultArgumentConverter.DEFAULT_LOCALE_CONVERSION_FORMAT_PROPERTY_NAME;
+import static org.junit.jupiter.params.converter.DefaultArgumentConverter.LocaleConversionFormat.BCP_47;
+import static org.junit.jupiter.params.converter.DefaultArgumentConverter.LocaleConversionFormat.ISO_639;
 import static org.junit.platform.commons.util.ClassLoaderUtils.getClassLoader;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Locale;
+import java.util.Optional;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
 import org.junit.platform.commons.support.ReflectionSupport;
@@ -35,7 +45,8 @@
  */
 class DefaultArgumentConverterTests {
 
-	private final DefaultArgumentConverter underTest = spy(DefaultArgumentConverter.INSTANCE);
+	private final ExtensionContext context = mock();
+	private final DefaultArgumentConverter underTest = spy(new DefaultArgumentConverter(context));
 
 	@Test
 	void isAwareOfNull() {
@@ -100,6 +111,36 @@ void delegatesStringsConversion() {
 		verify(underTest).convert("value", int.class, getClassLoader(DefaultArgumentConverterTests.class));
 	}
 
+	@Test
+	void convertsLocaleWithDefaultFormat() {
+		when(context.getConfigurationParameter(eq(DEFAULT_LOCALE_CONVERSION_FORMAT_PROPERTY_NAME), any())) //
+				.thenReturn(Optional.empty());
+
+		assertConverts("en", Locale.class, Locale.ENGLISH);
+		assertConverts("en-US", Locale.class, Locale.US);
+	}
+
+	@Test
+	void convertsLocaleWithExplicitBcp47Format() {
+		when(context.getConfigurationParameter(eq(DEFAULT_LOCALE_CONVERSION_FORMAT_PROPERTY_NAME), any())) //
+				.thenReturn(Optional.of(BCP_47));
+
+		assertConverts("en", Locale.class, Locale.ENGLISH);
+		assertConverts("en-US", Locale.class, Locale.US);
+	}
+
+	@Test
+	void delegatesLocaleConversionWithExplicitIso639Format() {
+		when(context.getConfigurationParameter(eq(DEFAULT_LOCALE_CONVERSION_FORMAT_PROPERTY_NAME), any())) //
+				.thenReturn(Optional.of(ISO_639));
+
+		doReturn(null).when(underTest).convert(any(), any(), any(ClassLoader.class));
+
+		convert("en", Locale.class);
+
+		verify(underTest).convert("en", Locale.class, getClassLoader(DefaultArgumentConverterTests.class));
+	}
+
 	@Test
 	void throwsExceptionForDelegatedConversionFailure() {
 		ConversionException exception = new ConversionException("fail");

jupiter-tests/src/test/kotlin/org/junit/jupiter/params/aggregator/ArgumentsAccessorKotlinTests.kt

@@ -13,6 +13,8 @@ import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertThrows
+import org.junit.jupiter.api.extension.ExtensionContext
+import org.mockito.Mockito.mock
 
 /**
  * Unit tests for using [ArgumentsAccessor] from Kotlin.
@@ -52,8 +54,9 @@ class ArgumentsAccessorKotlinTests {
         invocationIndex: Int,
         vararg arguments: Any
     ): DefaultArgumentsAccessor {
+        val context = mock(ExtensionContext::class.java)
         val classLoader = ArgumentsAccessorKotlinTests::class.java.classLoader
-        return DefaultArgumentsAccessor.create(invocationIndex, classLoader, arguments)
+        return DefaultArgumentsAccessor.create(context, invocationIndex, classLoader, arguments)
     }
 
     fun foo() {