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

Configure types referenced via @ExtendWith, @ArgumentsSource, etc. for reflection #54

Closed
2 tasks
sbrannen opened this issue Jun 16, 2021 · 5 comments · Fixed by #59
Closed
2 tasks

Configure types referenced via @ExtendWith, @ArgumentsSource, etc. for reflection #54

sbrannen opened this issue Jun 16, 2021 · 5 comments · Fixed by #59
Labels
enhancement New feature or request junit-support Related to JUnit Support project
Milestone

Comments

@sbrannen
Copy link
Collaborator

sbrannen commented Jun 16, 2021

Overview

As discussed in #50 (comment), the JUnitPlatformFeature currently has hard-coded reflection config for JUnit Jupiter's CsvArgumentsProvider, but as we see from #50 and #51 the JUnitPlatformFeature would at the very least need to configure all junit-jupiter-params ArgumentProvider implementations for reflective instantiation.

That is possible, but it would not help with ArgumentProvider implementations from users or third party libraries that are loaded by JUnit Jupiter via @ArgumentSource declarations (such as the @ArgumentsSource(MethodArgumentsProvider.class) declaration on @MethodSource).

Details

The broader topic that needs to be addressed is how to automatically register test-related types for reflection.

Within JUnit Jupiter, the following annotations are used to refer to types or methods that JUnit accesses via reflection: @ExtendWith, @ArgumentsSource, @TestMethodOrder, @DisplayNameGeneration, @IndicativeSentencesGeneration, @ConvertWith, @AggregateWith, @EnabledIf, @DisabledIf, @MethodSource.

These fall into two categories.

Type References

@ExtendWith, @ArgumentsSource, @TestMethodOrder, @DisplayNameGeneration, @IndicativeSentencesGeneration, @ConvertWith, and @AggregateWith are used to reference a type that will be instantiated via reflection.

Method References

@MethodSource, @EnabledIf, and @DisabledIf are used to reference methods that will be invoked via reflection.

If the method that is referenced is local to the test class -- for example, @MethodSource("localMethod") -- there should not be any need for additional reflection configuration, since all methods of the test class should have already been registered for reflection. However, there may potentially be an issue if the referenced method is declared in a super type.

On the contrary, if one of these three annotations is used to reference a static method in a different class (i.e., not within the test class itself) -- for example, @MethodSource("org.example.MyTestUtils#externalMethod") -- the declared methods in the external class will need to be opened up for reflection.

Deliverables

  • Determine how to automatically configure types and methods referenced via @ExtendWith, @ArgumentsSource, @TestMethodOrder, @DisplayNameGeneration, @IndicativeSentencesGeneration, @ConvertWith, @AggregateWith, @EnabledIf, @DisabledIf, @MethodSource for reflection.
  • If appropriate, remove all manual reflection configuration from the JUnitPlatformFeature for types that will be picked up automatically by changes made in conjunction with this issue.
@sbrannen
Copy link
Collaborator Author

I took another look at candidate annotations for this support and discovered I'd missed the following: @TestMethodOrder, @DisplayNameGeneration, @IndicativeSentencesGeneration, @ConvertWith, @AggregateWith.

So I'll update this issue's description accordingly.

@gradinac
Copy link
Contributor

One approach we could take to help support these features is to introduce a new service (ExtensionConfigProvider). We would get all available implementations of this service via SPI and call into each implementation at a different point. We could start with two basic callbacks:

    default void onLoad(ConfigRegistry registry) {
    }

    default void onTestClassRegistered(Class<?> testClass, ConfigRegistry registry) {
    }

where ConfigRegistry would just be an abstraction over RuntimeReflection to decouple these from native-image. We could then write these config providers for features like parameterized tests. If a user has a custom extension/annotation that requires special handling, they could provide the appropriate config service while not pulling in anything native-image related - making their extension work out of the box in the native mode. What do you think?

@sbrannen
Copy link
Collaborator Author

sbrannen commented Jun 17, 2021

I've been saying for about 6 months now that we will eventually need an extension/plugin mechanism for TestEngine implementers so that they can provide their own configuration (or so that a third-party can implement the extension/plugin for them).

So, I am glad you came to the same conclusion! 😄

As for what the extension/plugin SPI is called, let's hold off on deciding on a concrete name until we have worked out the actual API.

So why don't you make a concrete proposal (perhaps simply continuing with your ExtensionConfigProvider/ConfigRegistry ideas above) , and let's take it from there.

@sbrannen
Copy link
Collaborator Author

I've been saying for about 6 months now that we will eventually need an extension/plugin mechanism for TestEngine implementers so that they can provide their own configuration (or so that a third-party can implement the extension/plugin for them).

For the time being, the native-build-tools project will play the role of such a "third-party". This will allow us to experiment with the SPI, use it ourselves, and improve it before potentially asking TestEngine implementers to take over the plugins/extensions.

Implementing this mechanism will also allow us to greatly simplify the JUnitPlatformFeature. Ideally, the feature's duringSetup and beforeAnalysis methods should simply delegate to all registered plugins/extensions.

This means, as a proof of concept, we should aim to extract the configuration in the JUnitPlatformFeature into three separate plugins -- JUnit Platform, JUnit Jupiter, JUnit Vintage.

@gradinac
Copy link
Contributor

It really seems like the best tool for this purpose :)
I completely agree about the naming - I just picked these as an example - we will be able to change them later on.

As soon as I get a chance, I'll put together an initial implementation on which we can discuss more - I also agree that something like this should at first be used only internally by us and potentially open it up in the future as an API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request junit-support Related to JUnit Support project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants