junit5 icon indicating copy to clipboard operation
junit5 copied to clipboard

Introduce mechanism for programmatic extension management

Open sbrannen opened this issue 9 years ago • 23 comments

Background

This issue addresses some of the topics discussed in #112.

Status Quo

It is currently possible to register extensions declaratively via @ExtendWith on types and methods, and #497 will support programmatic extension registration via fields. However...

  • Developers cannot alter the order of registered extensions.
  • Developers cannot remove a registered extension.
  • Developers cannot insert an extension into the list of registered extensions -- for example, at the front or somewhere in the middle.

Rationale

In order to avoid the introduction of an overly complex declarative model for sorting, appending, prepending, inserting, and removing extensions, we have opted for a programmatic means to achieve all of the above. Providing a programmatic means to achieve these goals frees developers to manage registered extensions as they see fit without any unnecessary restrictions imposed on them by the framework itself.

Considerations

Such a mechanism could itself be an Extension registered declaratively via @ExtendWith; however, the current thinking in the team is that there should be one and only one such component registered at any given time. Since this is such a special case which affects all extensions which have been registered declaratively, it is therefore considered best to introduce a new declarative mechanism for registering this single component. Similarly, the API for such a component should therefore not extend Extension since doing so would allow users to incorrectly register it via @ExtendWith (in which case it would simply be silently ignored).

Proposal

  • Introduce an ExtensionManager API that receives a list of all registered extensions as input and returns a list of extensions.
  • Introduce a @ManageExtensionsWith annotation that allows the user to declare a single class reference for an implementation of ExtensionManager.

Related Issues

  • #13
  • #416
  • #497
  • #864
  • #1707

Deliverables

  • [ ] Introduce a mechanism for programmatic extension management.

sbrannen avatar Sep 10 '16 15:09 sbrannen

Gist from https://stackoverflow.com/questions/47633819/injecting-test-classes-created-by-platform-launcher ... allow extension instances be part of a LauncherDiscoveryRequest:

    LauncherDiscoveryRequest request = LauncherDiscoveryRequestBuilder
            .request()
            .selectors(...)
            .extendWith(TestInstancePostProcessor.class, new MyExtension("local value", what, ever))
            .build();

sormuras avatar Dec 04 '17 19:12 sormuras

@sormuras, the proposed extendWith() method in the launcher API does not address the issue of extension management. Rather, it simply provides another means for programmatic extension registration.

So, unless I'm missing something, the proposal is not relevant to this GitHub issue.

Furthermore, I am not a fan of registering extensions as mere objects at the level of the launcher API: I think extension registration should be left to the individual test engine implementations as it is now. In any case, that is a topic best suited for a different GitHub issue.

sbrannen avatar Dec 07 '17 00:12 sbrannen

I'd just like to add that we have existing JUnit 4 tests which used RuleChain and it would be very useful for us to programatically declare extensions in a certain order because there are dependencies between the extensions and therefore the order in which their callbacks run is important.

I can see that if I change the name of my extensions their callbacks will run in the desired order, but this is clearly not sustainable as this behaviour is likely to be JVM-dependent.

My guess is that somewhere behind the scenes, these extensions are being fetched with Class. getFields() where "The elements in the returned array are not sorted and are not in any particular order" (from the javadoc).

tomblench avatar Feb 26 '18 16:02 tomblench

In the meantime I've written a custom extension which runs the callbacks in the order the extensions were passed in to the constructor: https://gist.github.com/tomblench/999634f126e215132dcad2b0eccec870

tomblench avatar Feb 27 '18 08:02 tomblench

Looks like a plain and simple solution. Do you register an instance of MultiExtension via 5.1's @RegisterExtension in your test class?

sormuras avatar Feb 27 '18 09:02 sormuras

@sormuras that's correct

tomblench avatar Feb 27 '18 12:02 tomblench

I am not sure that I completely understood the proposal. From what I can see this would mean that users will have to register the ordering explicitly. However, as a library writer I would like to have control on the way my extensions are ordered, maybe even control how they are ordered in relation to other libraries (SpringExtension).

Proposal

Why not allow Extension to have a single default method getOrder() with some default value 0. This would allow new extensions to be written and have the order implemented to provide custom order and go in front or after others.

For example we have a base extension that provides the basic support for what we need. Additionally we want to offer support for Spring, i.e. get the required beans from the ApplicationContext and provide them to our extension.

filiphr avatar Aug 15 '18 15:08 filiphr

@filiphr, thanks for the feedback.

The getOrder() method you are proposing is actually what I did for extensions in Spring. AbstractTestExecutionListener implements Spring's Ordered interface which defines a getOrder() method.

That seems to work OK for the Spring ecosystem, since most of the key events are defined/supported by TestExecutionListener implementations from Core Spring.

We discussed such an approach for JUnit Jupiter but in the end decided against it... since most extensions used with JUnit Jupiter will in fact not come from the JUnit Team.

In other words, it would be difficult for various, competing third-party extensions to decide on a common number scheme to use for values returned from getOrder().

Of course, we are open to ideas and brainstorming on how to solve such a problem in the greater JUnit ecosystem.

sbrannen avatar Aug 15 '18 16:08 sbrannen

For example we have a base extension that provides the basic support for what we need. Additionally we want to offer support for Spring, i.e. get the required beans from the ApplicationContext and provide them to our extension

There's likely no need to "order" your extension relative to the SpringExtension.

If you need beans from the Spring ApplicationContext, you can simply delegate to the SpringExtension via the static getApplicationContext(ExtensionContext) method. 😉

sbrannen avatar Aug 15 '18 16:08 sbrannen

@tomblench, that's a pretty nice workaround for the time being.

However, such an approach has a few drawbacks.

  • it does not have proper exception handling and/or guarantees about which callbacks are invoked in case of failure.
  • it invokes "after" callbacks in the order in which they are registered instead of in reverse order.
    • in other words, proper "wrapping" is not enforced.

sbrannen avatar Aug 15 '18 16:08 sbrannen

That's a good point on the "wrapping", I guess I didn't notice because for my limited use case it didn't matter.

tomblench avatar Aug 15 '18 16:08 tomblench

Yeah, we take great care to ensure proper wrapping.

Note the use of registry.getReversedExtensions(AfterAllCallback.class) below and in other such cases for extensions that are invoked "after" an extension point.

https://github.com/junit-team/junit5/blob/e2a95a0d19f80e0f90e06e1c760f765e2465b0c8/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassTestDescriptor.java#L397-L404

sbrannen avatar Aug 15 '18 16:08 sbrannen

There's likely no need to "order" your extension relative to the SpringExtension.

Yes the SpringExtension was a bad example (you've done a good job on it for the order not to matter).

In other words, it would be difficult for various, competing third-party extensions to decide on a common number scheme to use for values returned from getOrder().

I don't see why it would be difficult. Me as an extension writer will write an extension that has the default order, i.e. I don't care that much about it. Then I can have another extension that actually requires a different 3rd party extension that should run before my first one and after the 3rd party. Knowing this I can define a public static int field in my extension that would have it's order, this way me as an extension provider have control over my own and support for other extensions. I don't see a case where there would be cyclic dependencies between extensions, if there is really such need then those 2 teams would need to work together to come up with a better solution 😄.

filiphr avatar Aug 16 '18 08:08 filiphr

We're not concerned about cyclic dependencies.

Rather, we are concerned about extension authors knowing what a valid/reasonable "order value" would be for a particular type of extension.

The problem for the community is that such order values would typically be static in nature (e.g., hard coded as you suggested by public static int). An extension author could of course programmatically/dynamically decide on what the "order value" should be right now, but for that to work, extensions would have to provided information regarding all currently registered extensions -- in order to make an informed decision.

But even in the case of the latter, how "informed" can an extension be?

It obviously cannot know about every extension that has ever been developed or will be developed.

sbrannen avatar Aug 16 '18 14:08 sbrannen

In the end, even if JUnit Jupiter provides a mechanism for extensions to decide on their own "order value", there will be scenarios that require user intervention in order to override the extension-supplied order value due to incompatibilities with competing third-party extensions that know nothing of each other.

sbrannen avatar Aug 16 '18 14:08 sbrannen

Update: I've added a Deliverable for consideration of support for @Order in conjunction with @RegisterExtension.

sbrannen avatar Oct 26 '18 11:10 sbrannen

Being able to have JUnit show the "effective extensions" (which are applied, and what order) would be a good preliminary step in helping to work around some of these concerns.

sebersole avatar Dec 21 '18 15:12 sebersole

Another quick thought that would work (at least for my use cases, not sure about generally)... How about the ability for an extension to register that it depends on another extension? E.g.

@ExtendWith( ShouldBeSecond.class, dependsOn={ShouldBeFirst.class} )
@ExtendWith( ShouldBeFirst.class )
class MyTest ...

I know in this trivial example I can just re-order the @ExtendWith declarations. In my real use-case however, the extensions are registered across different "meta annotations" which I have found to be very difficult to get the ordering just right.

sebersole avatar Dec 21 '18 15:12 sebersole

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 May 13 '21 19:05 stale[bot]

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.

stale[bot] avatar Jun 03 '21 20:06 stale[bot]

  • Reopening in light of #2651.

sbrannen avatar Aug 03 '21 16:08 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 Aug 03 '22 23:08 stale[bot]

This issue still seems useful to have around.

jbduncan avatar Aug 05 '22 12:08 jbduncan