junit5 icon indicating copy to clipboard operation
junit5 copied to clipboard

Add ability for InvocationInterceptor to intercept argument providers

Open stuartwdouglas opened this issue 3 years ago • 7 comments

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

  • #1139
  • #2191

stuartwdouglas avatar Mar 23 '21 06:03 stuartwdouglas

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

juliette-derancourt avatar May 22 '21 11:05 juliette-derancourt

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.

sbrannen avatar May 23 '21 16:05 sbrannen

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.

marcphilipp avatar Jun 06 '21 07:06 marcphilipp

@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 avatar Jun 09 '21 11:06 sbrannen

Also related to #1139.

sbrannen avatar Jun 09 '21 11:06 sbrannen

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[bot] avatar Jul 05 '22 22:07 stale[bot]

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

famod avatar Jul 06 '22 20:07 famod

Ran into this issue today.

morhei avatar Oct 20 '22 14:10 morhei

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?

marcphilipp avatar Oct 21 '22 10:10 marcphilipp

@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 avatar Oct 21 '22 11:10 geoand

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.

geoand avatar Oct 21 '22 11:10 geoand

@geoand Thanks!

Please feel free to also chime in on #3028.

marcphilipp avatar Oct 21 '22 11:10 marcphilipp

I'll have a look, thanks!

geoand avatar Oct 21 '22 12:10 geoand