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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should ExecutableInvoker#invoke(Method, Object) run all extensions ? #3014

Closed
ledoyen opened this issue Aug 25, 2022 · 5 comments
Closed

Should ExecutableInvoker#invoke(Method, Object) run all extensions ? #3014

ledoyen opened this issue Aug 25, 2022 · 5 comments

Comments

@ledoyen
Copy link
Contributor

ledoyen commented Aug 25, 2022

Hello 馃憢

Toying around with the new extension point ExecutableInvoker, I figured that ExecutableInvoker#invoke(Method, Object) does only run ParameterResolvers.

Should it not run all extensions applicable ?
My reasoning is the following:

  • Any extension can have side effects (such as adding a closable resource into the extension context)
  • If ParameterResolver extensions are executed, but the others are not, may the test not be in an inconsistent state ?

I can provide more utilization context if needed.

@sbrannen
Copy link
Member

Should it not run all extensions applicable ?

I'm not quite sure what you mean by "applicable".

So, yes, please expound with concrete use cases.

@ledoyen
Copy link
Contributor Author

ledoyen commented Aug 26, 2022

By applicable, I meant extensions which makes sense in the context of a test method execution (mainly BeforeEach & AfterEach ones).

The same way BeforeAll extensions may be expected to be executed by ExecutableInvoker#invoke(Constructor)

My case is to be able to re-run all extensions with a different classpath.
So far it works well, except for composition with extensions (others than ParameterResolver ones).

The code of my experimentations can be found here if needed https://github.com/fridujo/classpath-junit-extension/blob/feature/extension_composition/src/main/java/com/github/fridujo/classpath/junit/extension/jupiter/AbstractClasspathExtension.java#L89-L102

As I see it, to go further down the path of supplying bits of the execution part of the framework, either

  • ExecutableInvoker can provide methods which are bound to execution lifecycle (and at least another one such as #tearDown(Object instance) will be needed)
  • or ExtensionRegistry can be provided as public API

I am not sure it makes sense TBH.

@marcphilipp
Copy link
Member

Team Decision: ExecutableInvoker was introduced as a means to provide support for executing custom methods while reusing Jupiter's parameter resolution support. It's not meant to provide a way to run a test class or method with the full lifecycle.

We think the use case you outline in your extension is very interesting and think we should provide dedicated extension APIs to replace the classloader for a certain test classes or methods. The long-standing issue #201 is probably too abstract for that. Could you please raise a new dedicated feature request for your use case?

@marcphilipp marcphilipp closed this as not planned Won't fix, can't repro, duplicate, stale Sep 9, 2022
@sbrannen
Copy link
Member

sbrannen commented Sep 9, 2022

We think the use case you outline in your extension is very interesting and think we should provide dedicated extension APIs to replace the classloader for a certain test classes or methods.

Providing dedicated ClassLoader APIs in Jupiter might also allow the Spring Framework to stop launching the entire JUnit Platform within an intercepted test method as can be see in the @CompileWithTargetClassAccess support in this package.

@ledoyen
Copy link
Contributor Author

ledoyen commented Sep 9, 2022

@marcphilipp , I tried to summarize the idea in #3028

Do not hesitate to modify the content if you feel that is necessary.

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

3 participants