-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Description
As it stands argument converters need to be applied (with @ConvertWith
) either to a custom annotation (at least it looks like that; couldn't make it work) or to the argument. When the former is not an option (for example because you can't make it work 😉), the latter quickly becomes repetitive.
An extension point that allows registering argument converters as the class and method level (much like parameter resolvers) would remedy this problem.
Beyond ease of use, this extension point would also enable creating more wholesome extensions which, applied to a test class, can register parameter resolvers, argument converters, post instance extensions, and more all in one annotation. It would also make it possible to do something like @ConvertByCalling("fromString")
(either on class, method, or parameter), which would try to call a static method fromString(String)
on the target type.
If you think this is a valuable feature, I'd like to take a shot at implementing it.
Activity
marcphilipp commentedon May 17, 2017
Please take a look at
JavaTimeConversionPattern
andJavaTimeArgumentConverter
for an example of a custom annotation. It should really not be that hard to make it work. 😉I'm not sure it makes sense to register
ArgumentsSources
on the class level since it would require all methods to have take the same parameter types.However, I think registering additional implicit argument converters makes sense. Can you please provide an API proposal before jumping into the actual coding?
nipafx commentedon May 21, 2017
Thanks for the hint, that worked fine. I was so stuck in the extension model thinking that I applied the custom annotation to the method, not the argument. But my point still stands, then: I have to apply that annotation to every single parameter that wants to use that converter.
Unless I'm missing something that would only be the case if the
ArgumentConverter
API was left unchanged. I wanted to change it, though (see below).Sure but I'm going to focus on my initial idea (extension point for arbitrary converters) as opposed to your suggestion to look into additional implicit argument converters. I think the former can be a superset of the latter.
First of all, as soon as converters can be registered in several places, the question of priority and collisions arises. In the end this can be designed any way you want but I think the following order makes sense:
@ConvertWith
is (meta-)present on the parameter, use that converter. This should be unsurprising to any tester as the annotation is right there.supports
, enforce unique jurisdiction).As a consequence the API for converters would be twofold
ArgumentConverter
can be used with@ConvertWith
ArgumentConverterExtension
(or whatever) has the sameconvert
method (maybe inherited?) but also a matchingsupports
and can be applied with@ExtendWith
In both cases I would consider adding the
ExtensionContext
(I was surprised that at that point we can not be sure it's aTestExtensionContext
) to the argument list to give extension more insight into their surroundings. Alternatively (and moving the needle into the other direction)supports
might only get to see the parameter's type and must decide based on that.Regarding the implementation, it looks like
ParameterizedTestParameterResolver
:57 is the place to inject the new "look for extensions" behavior.bobtiernay-okta commentedon Nov 30, 2019
A great addition would be to support the registration of implicit (a.k.a default) converters project wide (similar to extensions) to cut down on test boilerplate. Some examples of common beneficial customizations include Jackson
JsonNode
, array types,List
,Map
, as well as custom objects provided throughString
-based factories not declared on the implementing class.sbrannen commentedon Nov 30, 2019
Tentatively slated for 5.6 M2 solely for the purpose of team discussion
47 remaining items
marcphilipp commentedon May 16, 2025
Team decision: While #4219 is coming along nicely, it still requires proper review so including it in 5.13.0 (for which RC1 is scheduled for today) seems unrealistic at this point. Since it's a new API/SPI we don't want to rush things and be locked in or have to do breaking changes later. Therefore, we've decided to move this issue to 6.0.
@scordio I hope you're not discouraged by this. We're still very interested in your work!
scordio commentedon May 16, 2025
Not at all! 🙂
asarkar commentedon May 16, 2025
This ticket was opened exactly 8 years ago today; I hope those of us interested in seeing it come to fruition will have that chance before we retire 🙂
scordio commentedon May 16, 2025
@asarkar I hope it won't take me another 8 years to finish my PR 😉
Remove built-in support for `Locale` conversion in ISO 639 format
Locale
conversion in ISO 639 format #4751Remove built-in support for `Locale` conversion in ISO 639 format
Remove built-in support for `Locale` conversion in ISO 639 format
Remove built-in support for `Locale` conversion in ISO 639 format
Use IETF BCP 47 language tag format to convert from `String` to `Locale`