Pass test Method to TestInstanceFactory when using Lifecycle.PER_METHOD

[seanf]

I'm trying to update CDI-Unit to support JUnit 5, specifically CDI-Unit's ProducerConfig feature for JUnit 4 which uses annotations on the test method to help configure the deployment payload for Weld. Weld is then used to construct test instances. This can almost be replicated in JUnit 5, by implementing a TestInstanceFactory. (Obviously this won't be compatible with Lifecycle.PER_CLASS, because the instances must allow for per-method configuration.)

The problem is, when TestInstanceFactory.createTestInstance() is called, extensionContext.getTestMethod() always returns Optional.empty(), even when using Lifecycle.PER_METHOD.

If the TestInstanceFactory were to receive the test Method, when using Lifecycle.PER_METHOD, it would be possible to use annotations from the test method to control test instance creation, thus allowing features like ProducerConfig to be created.

[seanf]

Related question on StackOverflow: https://stackoverflow.com/q/52108869/14379

[sbrannen]

The problem is, when TestInstanceFactory.createTestInstance() is called, extensionContext.getTestMethod() always returns Optional.empty(), even when using Lifecycle.PER_METHOD.

That's to be expected. As stated in the class-level JavaDoc for TestInstanceFactory:

Extensions that implement TestInstanceFactory must be registered at the class level.

That means that the underlying ExtensionContext is scoped at the class level. Specifically, it is a ClassExtensionContext which has no access to the Method.

The MethodExtensionContext has access to the Method, but the ClassExtensionContext does not have any means to access the MethodExtensionContext, since extension contexts only know about their parents.

In light of that, I don't see any way to provide the test method via the ExtensionContext supplied to a TestInstanceFactory.

Furthermore, doing so would not align with the design of JUnit Jupiter, namely that class-level extensions should not do anything particular to specific methods.

[sbrannen]

FWIW, you may be interested in #878.

[sbrannen]

In any case, what I stated above is naturally based on the status quo.

It may be technically possible to redesign the internals such that TestMethodTestDescriptor.prepare(JupiterEngineExecutionContext) passes the current test Method to TestInstanceProvider.getTestInstance(Optional<ExtensionRegistry>) to allow our internal implementations to create a one-off, modified ExtensionContext that contains the "method about to be executed".

maybe...

[seanf]

That means that the underlying ExtensionContext is scoped at the class level. Specifically, it is a ClassExtensionContext which has no access to the Method.

Right, I see.

Furthermore, doing so would not align with the design of JUnit Jupiter, namely that class-level extensions should not do anything particular to specific methods.

I'm not sure I understand what you mean by class-level extensions.

Perhaps the title of this feature request is too specific in its details (ie TestInstanceFactory), but it would still be useful IMHO, when using PER_METHOD Lifecycle, to be able construct test instances based on method attributes.

[sbrannen]

I'm not sure I understand what you mean by class-level extensions.

Extensions fall into certain categories depending on where they are applied / when they are invoked.

For example, BeforeEachCallback is always a method-level extension; whereas, BeforeAllCallback is always a container-level extension.

Specifically, BeforeAllCallback is invoked as a class-level extension if the test instance lifecycle is per-method, and as an instance-level extension if the lifecycle is per-class. (at least, I hope I'm explaining that right).

So, yeah, I admit: the terminology we use can be rather confusing.

In summary, I suppose we could say that extensions are invoked at one of the following levels, where class level and instance level can be somewhat generically grouped under the container level.

1. class level
2. instance level
3. method level

What I was trying convey earlier is that we have traditionally not considered that a container-level invocation of an extension would ever have knowledge of anything at the method level (like the test Method).

[seanf]

Thanks. Given that an instance may be constructed for every method (if PER_METHOD), the levels do seem a bit mixed already. From the outside view, anyway.

[PaulSh84]

@sbrannen
Can you please show at least one simple implementation of

TestInstanceFactory.createTestInstance()

I am looking for ways how to create additional test class instances and add different parameter dependencies in runtime.

Lifecycle.PER_CLASS

[sbrannen]

@PaulSh84, please ask your question on Gitter or Stack Overflow.

[PaulSh84]

@sbrannen
I did, please take a look here

[marcphilipp]

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

[marcphilipp]

@seanf Is this use case still relevant for you?

[stale]

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]

This issue has been automatically closed due to inactivity. If you have a good use case for this feature, please feel free to reopen the issue.

[michaeldimchuk]

@marcphilipp I don't know if the team is open to re-evaluating this feature request, but here's an example scenario where something like this is useful: quarkusio/quarkus#38336.

In a scenario where all fields of a test class are final and are injected through the constructor with a PER_METHOD lifecycle, a CDI container has to be launched before the test instance is created. However at times the annotations applied to a method may be needed to configure the CDI container itself before actually launching it.

In the case of Quarkus, they allow @TestConfigProperty annotations to be applied at either the class or method level, to override configuration properties defined for a test case, and allow for the container to be tested with those properties.

Since neither the TestInstancePreConstructCallback or TestInstanceFactory have access to the test method to be eventually invoked, something like that wouldn't be possible, and injection in that scenario has to be done after the test instance is created, so would only be applicable to non-final fields.

Being able to access the test method to be executed during test instance creation would allow for something like that to be done, while relying purely on constructor injection and not leaving mutable fields around, which works a bit better when using Lombok to apply private final to all non-static fields across a project.

[JojOatXGME]

FYI, I created a PR at #4032. The PR was written for #3445, but it would resolve this issue as well. However, since the change of the PR could potentially break existing extensions, more work may be required to provide proper legacy.

Extensions that implement TestInstanceFactory must be registered at the class level.

That means that the underlying ExtensionContext is scoped at the class level. Specifically, it is a ClassExtensionContext which has no access to the Method.

The Javadoc only declares where the extension has to be registered, it doesn't say anything about the ExtensionContext which is provided.

Furthermore, doing so would not align with the design of JUnit Jupiter, namely that class-level extensions should not do anything particular to specific methods.

I am not sure about that. Extension callbacks are usually executed with the ExtensionContext matching the current state of the execution. For example, ParameterResolver will receive different ExtensionContext implementations depending on when a parameter needs to be resolved. As far as I know, the instance creation (with all the related extensions callbacks) is the only place which breaks this rule. I would therefore argue that the current behavior of TestInstanceFactory does not align with the design of JUnit Jupiter, as it behaves inconsistently to other extension callbacks. This difference also causes issues with CloseableResource in these extension methods, which is the topic of #3445.

Anyway, that is my perspective on this topic. Feel free to review my pull request. I would say my PR actually reduces the complexity of the production code a bit. However, there is still the problem that it might break existing extensions.

[marcphilipp]

Please see #3445 (comment).

[marcphilipp]

Duplicate of #3445