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

Add ability for InvocationInterceptor to intercept argument providers #2579

Open
stuartwdouglas opened this issue Mar 23, 2021 · 13 comments
Open

Comments

@stuartwdouglas
Copy link

stuartwdouglas commented Mar 23, 2021

Quarkus needs to run tests in its own ClassLoader, and it achieves this by intercepting all methods through InvocationInterceptor and redirecting them to a new test instance loaded from the Quarkus ClassLoader.

This works really well with the exception of @MethodSource based parameters. As there is no way to intercept that the method is invoked on the original instance, and the parameters may be loaded from the wrong ClassLoader.

It would be great if we could have some additional methods added to InvocationInterceptor to intercept this invocation as well.

Related Issues

@juliette-derancourt
Copy link
Member

Tentatively slated for 5.8-M2 solely for the purpose of team discussion

@sbrannen
Copy link
Member

One of the challenges here is that MethodArgumentsProvider (which invokes the @MethodSource factory method on the test instance) is an implementation of org.junit.jupiter.params.provider.ArgumentsProvider which is not an Extension API in junit-jupiter-api.

Whereas, InvocationInterceptor is an Extension API in junit-jupiter-api.

Thus, InvocationInterceptor cannot know about ArgumentsProvider.

@marcphilipp
Copy link
Member

marcphilipp commented Jun 6, 2021

We could add a new interceptExtensionMethod(Invocation<T> invocation, ReflectiveInvocationContext<Method> invocationContext, ExtensionContext extensionContext) method to InvocationInterceptor, and a MethodInvoker getMethodInvoker() method to ExtensionContext with interface MethodInvoker { Object invokeMethod(Method method, Object target, Object... args); }. Then, MethodArgumentsProvider could use MethodInvoker to call the configured method while InvocationInterceptor would be able to do what it needs.

@sbrannen sbrannen changed the title Add ability for InvocationInterceptor to intercept parameter suppliers Add ability for InvocationInterceptor to intercept argument providers Jun 9, 2021
@sbrannen
Copy link
Member

sbrannen commented Jun 9, 2021

@marcphilipp, your proposal is somewhat related to #2191.

If we implement a solution for #2191, that would likely result in a means for invoking any method using parameter resolvers, analogous to what org.junit.jupiter.engine.execution.ExecutableInvoker does internally.

So there may be room for synergies here.

@sbrannen
Copy link
Member

sbrannen commented Jun 9, 2021

Also related to #1139.

@stale
Copy link

stale bot commented Jul 5, 2022

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 Jul 5, 2022
@famod
Copy link

famod commented Jul 6, 2022

This is not stale. Quarkus is still waiting for this and has to apply brittle object cloning as a workaround.

@stale stale bot removed the status: stale label Jul 6, 2022
@morhei
Copy link

morhei commented Oct 20, 2022

Ran into this issue today.

@marcphilipp
Copy link
Member

I think we should address #3028 first as it might solve the problem more cleanly.

@stuartwdouglas Could you please point us to Quarkus' InvocationInterceptor implementation for reference?

@geoand
Copy link

geoand commented Oct 21, 2022

@marcphilipp you can find the main usage in https://github.com/quarkusio/quarkus/blob/2.13.3.Final/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java.

As you see in the various intercept methods use an instance of the test class created by Quarkus (which is created in interceptTestClassConstructor)

@geoand
Copy link

geoand commented Oct 21, 2022

I should also add for context that Quarkus needed to go down this route, because it applies various transformations to classes and without using a custom ClassLoader and different test instance, the transformations could fail to be applied.

@marcphilipp
Copy link
Member

@geoand Thanks!

Please feel free to also chime in on #3028.

@geoand
Copy link

geoand commented Oct 21, 2022

I'll have a look, thanks!

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

7 participants