Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create extension point for ArgumentConverter #853

Open
nipafx opened this issue May 16, 2017 · 8 comments
Open

Create extension point for ArgumentConverter #853

nipafx opened this issue May 16, 2017 · 8 comments

Comments

@nipafx
Copy link
Contributor

nipafx commented May 16, 2017

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.

@marcphilipp
Copy link
Member

Please take a look at JavaTimeConversionPattern and JavaTimeArgumentConverter 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?

@marcphilipp marcphilipp added this to the 5.1 Backlog milestone May 18, 2017
@nipafx
Copy link
Contributor Author

nipafx commented 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.

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.

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).

However, I think registering additional implicit argument converters makes sense. Can you please provide an API proposal before jumping into the actual coding?

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:

  • If @ConvertWith is (meta-)present on the parameter, use that converter. This should be unsurprising to any tester as the annotation is right there.
  • Otherwise look into converter extensions that were registered on the method or class level and give them an API that allows a similar technique to parameter resolvers (call supports, enforce unique jurisdiction).
  • Otherwise use default extensions that Jupiter provides.

As a consequence the API for converters would be twofold

  • the current ArgumentConverter can be used with @ConvertWith
  • a new ArgumentConverterExtension (or whatever) has the same convert method (maybe inherited?) but also a matching supports 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 a TestExtensionContext) 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
Copy link

bobtiernay-okta commented 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 through String-based factories not declared on the implementing class.

@sbrannen
Copy link
Member

Tentatively slated for 5.6 M2 solely for the purpose of team discussion

@marcphilipp
Copy link
Member

Team decision: Waiting for additional interest from the community.

@stale
Copy link

stale bot commented May 13, 2021

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the status: stale label May 13, 2021
@stale
Copy link

stale bot commented Jun 3, 2021

This issue has been automatically closed due to inactivity. If you have a good use case for this feature, please feel free to reopen the issue.

@bjmi
Copy link
Contributor

bjmi commented Apr 4, 2024

I'm also interested in a solution to register certain ArgumentConverters project wide.
What occurs very often is

  • Class to Instance conversion via default constructor e.g.
@ValueSource(classes = { ProductRequest.class, ArticleRequest.class, StockRequest.class, PriceRequest.class })
@ParameterizedTest
void sendAndReceive(@ToInstance Request<Payload> request) { ... }
  • Parsing dates with custom DateTimeFormatters
@CsvSource({ "2021-11-22 11:31",
             "2024-12-24 00:00" ...)
@ParameterizedTest
void deliveryDay(@Timing Instant deliveryDay) { ... }
  • Support of built-in Kotlin types e.g.
@ValueSource(classes = [ Int::class, Long::class, Double::class ])
@ParameterizedTest
fun withNumbers(type: KClass<out Number>) { ... }

It would be great if repetitive argument converters could be used implicitly without polluting the code with annotations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants