@ParameterizedTest fails to convert String to Class with custom ClassLoader

[sbrannen]

Overview

This is a follow up to #3279.

See also: #3279 (comment)

The assumption is that the following test case fails when running within OSGi.

package example;

import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

class StringToClassConversionTests {

	@ParameterizedTest
	@ValueSource(strings = "example.ClassFromBundle")
	void test(Class<?> clazz) {
		assertThat(clazz).isNotNull();
	}
}

class ClassFromBundle {
}

[sbrannen]

@kriegfrj and @chrisr3,

Can you please confirm that the above test case is problematic within OSGi?

If so, I think we should also address this in 5.10.

I already have ideas about how to fix it, so I'd probably be able push a fix in time for 5.10 M1, depending on your feedback.

[sbrannen]

FYI: the following is the code in question.

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

Lines 286 to 293 in ce8dcbb

	private static Class<?> toClass(String type) {
	//@formatter:off
	return ReflectionUtils
	.tryToLoadClass(type)
	.getOrThrow(cause -> new ArgumentConversionException(
	"Failed to convert String \"" + type + "\" to type " + Class.class.getName(), cause));
	//@formatter:on
	}

To fix it, I'd use the ClassLoader of the declaring class of the @ParameterizedTest method to load the Class.

[kriegfrj]

To fix it, I'd use the ClassLoader of the declaring class of the @ParameterizedTest method to load the Class.

Good catch!

Yes, that would be the best solution. Though having glanced at the code I realised that this is probably not trivial because DefaultArgumentConverter is currently stateless - at least to the extent that it does not provide the reference of said declaring class. I have put together a proof-of-concept to work around these issues for discussion.

[sbrannen]

I have put together a proof-of-concept to work around these issues for discussion.

To be honest, I was not requesting that someone else submit a PR for this. We would prefer that we collectively complete the other open issues before spending much time on this one.

I already had coded a 90% solution in my mind and was planning to go with that.

However, now that you've taken the time to submit a PR, I'll see if I can take a look at it, but that will potentially take more of my time than implementing the fix myself.

[kriegfrj]

Sorry @sbrannen, the goal was to make your life easier! If it's easier for you to go with the solution you've implemented, I won't be offended in the least if you don't use my PR.

[sbrannen]

Hi @kriegfrj,

Sorry @sbrannen, the goal was to make your life easier!

Yes, of course. I totally understand that.

And I assume that you also enjoyed drafting a solution, or I at least hope you enjoyed it. 😉

If it's easier for you to go with the solution you've implemented, I won't be offended in the least if you don't use my PR.

Thank you for your understanding. Very much appreciated! 👍

If you'd like to view what I had in mind (and completed), see commit f6e73ac.