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

Allow extensions to leverage registered ParameterResolvers for method and constructor invocations #2393

Closed
sebersole opened this issue Aug 25, 2020 · 20 comments · Fixed by #2786

Comments

@sebersole
Copy link

sebersole commented Aug 25, 2020

I am working on an extension for tests to be able to react to failed tests or tests that end in exception by including a method annotated with @OnFailure.

My specific high-level "requirement" is to be able to rebuild a Hibernate SessionFactory in these cases. That method needs access to the SessionFactory to be rebuilt. Actually it is a SessionFactoryScope class that manages a SessionFactory and exposes it via a ParameterResolver. E.g.

@SessionFactory
public class MyTest {
    @Test
    public void theTest(SessionFactoryScope scope) {
        // ...
    }
}

If this test fails, I'd like to be able to access the SessionFactoryScope to trigger it to release its SessionFactory and build a new one. To do that it needs access to the SessionFactoryScope. I.e.:

    @OnFailure
    public void rebuild(SessionFactoryScope scope) {
        // ...
    }

Currently, JUnit Jupiter does not allow access to registered ParameterResolvers nor any way to invoke such "extension methods" with resolved parameter arguments.

Seems like a generally useful feature.

@sormuras
Copy link
Member

Tentatively slated for 5.8 for team discussion

@sebersole
Copy link
Author

Any word on this?

I looked at the "related issues", though to my noob eyes I was not really seeing the relationship aside from them also asking about ParameterResolvers

  • the first 2 specifically related to parameterized tests
  • the 3rd may relate, but I really did not understand fully what was being asked there

@marcphilipp
Copy link
Member

Tentatively slated for 5.8 M1 solely for the purpose of team discussion.

@marcphilipp marcphilipp modified the milestones: 5.8 Backlog, 5.8 M1 Dec 27, 2020
@marcphilipp
Copy link
Member

marcphilipp commented Jan 22, 2021

Team brainstorming

One option would be to introduce a method directly on ExtensionContext: Object invokeWithParameterResolution(Object target, Executable executable).

An intermediate interface, e.g. ParameterResolution, that could be accessed via ExtensionContext would allow us to later support additional use cases.

@marcphilipp
Copy link
Member

Team decision: Introduce ExtensionContext.getParameterResolution() and ParameterResolution.invoke(Object target, Executable executable) methods. Investigate how other use cases (see #2393 (comment)) would be addressed in this new API.

@marcphilipp marcphilipp modified the milestones: 5.8 M1, 5.8 M2/RC1 Feb 7, 2021
@sebersole
Copy link
Author

sebersole commented May 21, 2021

I just updated to 5.8.0.M1 and there is neither a method ExtensionContext#getParameterResolution() nor a method ParameterResolution#invoke (nor even an accessible class named ParameterResolution).

Did I misunderstand the associated milestones here?

@sormuras
Copy link
Member

Yes.

Those two methods are targeted to be included in milestone 5.8 RC1 - which follows after not-yet released milestone 5.8 M2 - which in turn was predated by 5.8 M1 in February 2021.

@sebersole
Copy link
Author

So any idea when 5.9 will be released now?

@marcphilipp
Copy link
Member

@sebersole This change has not yet been made. Would you be interested and have time to work on it?

@sbrannen
Copy link
Member

sbrannen commented Dec 4, 2021

@juliette-derancourt is currently working on a prototype for this feature.

@sbrannen sbrannen changed the title Ability to define extensions that leverage registered ParameterResolvers Allow extensions to leverage registered ParameterResolvers for method and constructor invocations Dec 4, 2021
@sebersole
Copy link
Author

Sorry @marcphilipp I was on vacation when you sent that and it got lost in the weeds.

I'd be willing to help if I can. @juliette-derancourt - let me know if there is something I can do

@juliette-derancourt
Copy link
Member

@sebersole Thanks for the offer!
I just opened a draft PR, you're welcome to review it if you want to 🙂 (The documentation is missing for now)
Please tell me if this is what you had in mind when opening this issue!

@sebersole
Copy link
Author

@juliette-derancourt I took a look at the PR, but have to admit that most of it is beyond my knowledge of JUnit internals.

But I think I got the gist... It revolves around ExecutableInvoker which can now be obtained from ExtensionContext#getExecutableInvoker. So in my extension, I'd invoke the method I mentioned using context.getExecutableInvoker().invoke( onFailureMethod )? If so, looks perfect.

@juliette-derancourt
Copy link
Member

So in my extension, I'd invoke the method I mentioned using context.getExecutableInvoker().invoke( onFailureMethod )?

@sebersole Yes, that would work like this 👍

@sebersole
Copy link
Author

Looks beautiful to me :)

juliette-derancourt added a commit that referenced this issue Feb 4, 2022
… and constructor invocations (#2786)

This commit introduces a new `ExecutableInvoker` API in Jupiter, available in the `ExtensionContext`.
This invoker allows extensions to leverage registered `ParameterResolvers` for method and constructor invocations.

Resolves #2393.
@ledoyen
Copy link
Contributor

ledoyen commented Feb 4, 2022

If I may, can we imagine this as a building block for the classloader extension discussed in #201 ?

The next move could be to build at start up (and run with it) a class loader such as: boot -> platform -> junit-related urlCL -> what's left urlCL

This later would allow an extension point to let users decide how to build their custom classloader as long as the junit-related urlCL is in the parent chain.

WDYT ?

@sbrannen
Copy link
Member

sbrannen commented Feb 4, 2022

@ledoyen, I don't see how this new Jupiter feature is related to the Platform-level ClassLoader issue.

This particular new feature doesn't have anything to do with class loading.

@ledoyen
Copy link
Contributor

ledoyen commented Feb 4, 2022

@sbrannen, sorry I rushed into this a little fast, I was thinking of my current implementation of an extension using custom classloaders which lacks compatibility with other extensions and will benefit from this new feature.

However, from the JUnit framework perspective, this is no different than the actual test method invocation which already exists and indeed brings nothing to the classloader topic.

Sorry for that 😞

@sbrannen
Copy link
Member

sbrannen commented Feb 4, 2022

No worries, @ledoyen 👍

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