Introduce EnableTestScopedConstructorContext annotation for extensions

[JojOatXGME]

Overview

Resolve #1568.
Resolve #2970.
Resolve #3445.

This PR moves the instance creation into the execution context of the test method. This affects the following callbacks:

• TestInstancePreConstructCallback
• TestInstanceFactory
• ParameterResolver (when resolving parameters for the constructor)
• TestInstancePostProcessor

Assuming the test is using TestInstance.Lifecycle.PER_METHOD (the default), these callbacks will now receive the ExtensionContext for the currently executed test method. This has the following effects:

• The callbacks can now access ExtensionContext.getTestMethod()
• ExtensionContext.getTestClass() and TestInstanceFactoryContext.getTestClass() are no-longer interchangeable
• ExtensionContext.getTestClass() now returns the (nested) class of the currently executed test
• Instances of CloseableResource which are added to the store will now be closed at the end of the test execution

If the test is using TestInstance.Lifecycle.PER_CLASS, the behavior is unaltered.

Not sure if the change is considered API breaking. The previous behavior was rather unintuitive and not explicitly documented. However, the example at TestInfoDemo is no-longer valid and has been changed as part of the PR. I also think there is a good chance that there are a few extensions in the wild which rely on the previous behavior, since the behavior was in place for multiple years. I would therefore like to know how I should proceed with the PR.

---

I hereby agree to the terms of the JUnit Contributor License Agreement.

---

Definition of Done

• There are no TODOs left in the code
• Method preconditions are checked and documented in the method's Javadoc
• Coding conventions (e.g. for logging) have been followed
• Change is covered by automated tests including corner cases, errors, and exception handling
• Public API has Javadoc and @API annotations
• Change is documented in the User Guide and Release Notes

[marcphilipp]

see #3445 (comment)

[JojOatXGME]

I introduced the annotation @EnableTestScopedConstructorContext to enable the new behavior for TestInstancePreConstructCallback, TestInstancePostProcessor and TestInstanceFactory. I have few remaining questions, which I have added as review comments.

[JojOatXGME]

Feel free to suggest other names. This is the best name that came to mind. I think the name is good enough, at least I like it much more than my other ideas (e.g. NewTestInstanceConstructionContext).

[JojOatXGME]

I just realized that you have already suggested @MethodLevelExtensionContextAware. I don't have a strong preference. Your suggestion is a bit shorter.

[marcphilipp]

Let's stick with your name for now. I'll brainstorm with the team in our next team call.

[JojOatXGME]

I just wanted to note, that after the idea of a common method has lingered in my head over the last day, I have started to prefer it. The idea would be to add a default method to the empty Extension interface. At the end, I don't think any of my reasons is that significant, and I am totally happy with the current solution as well. Anyway, here are the reasons why I started to like the idea of a default method, although it is more clunky to use.

• Improved discoverability as the method is shown by the IDE when you look for methods to override (e.g. Ctrl+I in IntelliJ). However, the Javadoc also makes it rather easy to find already.
• Extensions can use arbitrary logic for the flag. For example, a wrapper or proxy which delegates all method calls to another extension would be able to preserve the flag from the delegate. However, it is probably a very special use case and not that relevant.
• Less likely to impact the performance without having to introduce any cache.
• Maybe more consistent, as I think there are currently no other annotations used in the extension logic.
• A default method could technically be removed again without breaking binary compatibility (ABI). However, I assume removing it from the API again is not really an option anyway, so it is not really relevant.

Feel free to pick whatever you prefer. I just wanted to communicate my thoughts.

[marcphilipp]

Those are good points! To get a better feel for it, I've given it a try in #4062. I may be subject to the IKEA effect now, but it like the solution better for the reasons you've given above.

[marcphilipp]

Let's stick with your name for now. I'll brainstorm with the team in our next team call.

[marcphilipp]

I'm going ahead with merging this PR. I will address the remaining work in #4057.

[marcphilipp]

Thanks a lot, @JojOatXGME! 👍

[JojOatXGME]

Also thanks from my site. I am happy to remove my workaround in my projects once it is released. 😄 Feel free to reach out if you have any questions, otherwise I will just leave it to you now.