Introduce ConversionService in junit-platform-commons

[scordio]

Overview

• Resolves Create extension point for ArgumentConverter #853
• Resolves Locale argument conversion not setting language and country properly #3141
• Relates to Implicit conversion from integral numbers to byte array #2702

---

I hereby agree to the terms of the JUnit Contributor License Agreement.

---

Definition of Done

• There are no TODOs left in the code
• Method preconditions are checked and documented in the method's Javadoc
• Coding conventions (e.g. for logging) have been followed
• Change is covered by automated tests including corner cases, errors, and exception handling
• Public API has Javadoc and @API annotations
• Change is documented in the User Guide and Release Notes

[scordio]

There is plenty of work to do 🙃

The current highlights:

• New ConversionService interface, with ClassLoader param instead of ConversionContext (I will come back to it)
• New TypedConversionService base class
• The implementation of the existing ConversionSupport.convert(String, Class, ClassLoader) is now part of the new DefaultConversionService
• A test setup to demonstrate how to solve Locale argument conversion not setting language and country properly #3141 (I'll shape it better as an integration test)

Any feedback would be highly appreciated!

[marcphilipp]

Thanks for the draft! 👍

The tests are failing due to:

org.junit.platform.commons.support.conversion.ConversionService: module org.junit.platform.commons does not declare uses

That's because junit-platform-commons/src/module/org.junit.platform.commons/module-info.java is missing

uses org.junit.platform.commons.support.conversion.ConversionService;

[marcphilipp]

Looks very promising! 👍

[marcphilipp]

I was also wondering about that but am not sure. I guess we should only move it if it makes things simpler or helps us produce a better error message.

[scordio]

I think I found a good reason to move the logic to canConvert, I'll dig a bit more later.

[marcphilipp]

This would make a good test case, though. We have existing tests that register services for tests using an extra class loader:

junit5/platform-tests/src/test/java/org/junit/platform/launcher/core/LauncherFactoryTests.java

Lines 353 to 366 in 16c6f72

	private static void withTestServices(Runnable runnable) {
	var current = Thread.currentThread().getContextClassLoader();
	var url = LauncherFactoryTests.class.getClassLoader().getResource("testservices/");
	try (var classLoader = new URLClassLoader(new URL[] { url }, current)) {
	Thread.currentThread().setContextClassLoader(classLoader);
	runnable.run();
	}
	catch (IOException e) {
	throw new UncheckedIOException(e);
	}
	finally {
	Thread.currentThread().setContextClassLoader(current);
	}
	}

We could generalize and move that method to a test utility class (e.g. in junit-jupiter-api/src/testFixtures) so it can be reused here.

[marcphilipp]

That's because junit-platform-commons/src/module/org.junit.platform.commons/module-info.java is missing

uses org.junit.platform.commons.support.conversion.ConversionService;

When you add that, you'll also have to add it to platform-tooling-support-tests/projects/jar-describe-module/junit-platform-commons.expected.txt to adjust the integration test.

[scordio]

I've been lagging behind with this one but I should be able to spend time on it in the upcoming weekend.