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

Introduce mechanism to enable specific global extensions in JUnit Jupiter #3717

Closed
odenix opened this issue Mar 8, 2024 Discussed in #3703 · 12 comments · Fixed by #4120
Closed

Introduce mechanism to enable specific global extensions in JUnit Jupiter #3717

odenix opened this issue Mar 8, 2024 Discussed in #3703 · 12 comments · Fixed by #4120

Comments

@odenix
Copy link

odenix commented Mar 8, 2024

Enabling all global extensions on the class/module path with junit.jupiter.extensions.autodetection.enabled=true is asking for surprises (and potentially trouble).

Instead, I'd like to explicitly enable the global extensions that I want to use.

I imagine that I'd do this by setting junit.jupiter.extensions.autodetection.enabled=foo.bar.Baz,foo.bar.Qux or a similar property.

If the JUnit team is open to this idea, I'd be happy to discuss further and send a PR.

@sbrannen
Copy link
Member

sbrannen commented Mar 9, 2024

Thanks for the proposal, @translatenix.

We'll discuss it within the team during an upcoming team call and get back to you.

@sbrannen sbrannen changed the title Enabling specific global extensions Introduce mechanism to enable specific global extensions in JUnit Jupiter Mar 9, 2024
@marcphilipp
Copy link
Member

Team decision: Introduce junit.jupiter.extensions.autodetection.include and junit.jupiter.extensions.autodetection.exclude configuration parameters with defaults of * and empty, respectively.

@marcphilipp
Copy link
Member

Addendum: The new properties should support the same pattern matching syntax supported for other configuration parameters.

@bjmi
Copy link
Contributor

bjmi commented Mar 22, 2024

One use case would be to enable org.mockito.junit.jupiter.MockitoExtension from org.mockito:mockito-junit-jupiter globally by default. This would activate strict stubbing then. mockito/mockito#769.

@dev-jonghoonpark
Copy link

Can I Contribute?

@sbrannen
Copy link
Member

Hi @dev-jonghoonpark,

Can I Contribute?

Yes, feel free to submit a PR for review.

@dev-jonghoonpark
Copy link

dev-jonghoonpark commented May 13, 2024

@marcphilipp

Team decision: Introduce junit.jupiter.extensions.autodetection.include and junit.jupiter.extensions.autodetection.exclude configuration parameters with defaults of * and empty, respectively.

It seems like junit.jupiter.extensions.autodetection.include and junit.jupiter.extensions.autodetection.exclude should be designed to accept more than one input, is that correct?

I think it should be able to accept multiple folders and multiple classes.

@sbrannen
Copy link
Member

Yes, comma-separated entries with wildcard matching.

@dev-jonghoonpark
Copy link

hello.

I've been thinking about how to implement this feature, and I've come up with 2 approaches.

Basically, both approaches involve replacing the extension that is registered in registerAutoDetectedExtension method.


1st approach

main...dev-jonghoonpark:junit5-forked:implement-all-callbacks

Create an AutoDetectedExtension that implements all possible callbacks.

Let the AutoDetectedExtension contain the originalExtension and perform the callbacks when conditions are matched.

Disadvantages :

  • have to check for callbacks that are not actually implemented.

2nd approach

main...dev-jonghoonpark:junit5-forked:use-proxy

Use a proxy to intercept the callback function when it is executed and check the path. If it matches the conditions, perform the callback.

Disadvantages :

  • have to use a proxy (which can get complicated).

I'm going to try the first of the two approaches, what do you think?
(Note that the above code is just written to check the approach, it's not the finished code).

If you have a better way, I'd love to hear about it, even if it's a completely different approach.

Thank you.

@marcphilipp
Copy link
Member

@dev-jonghoonpark Thanks for looking into this and sorry for not getting back to you sooner!

Auto-registration of extensions is already implemented here:

if (configuration.isExtensionAutoDetectionEnabled()) {
registerAutoDetectedExtensions(extensionRegistry);
}

Therefore, all that should be needed is to implement filtering of the classes loaded via ServiceLoader based on the two new configuration parameters before calling registerAutoDetectedExtension:

private static void registerAutoDetectedExtensions(MutableExtensionRegistry extensionRegistry) {
ServiceLoader.load(Extension.class, ClassLoaderUtils.getDefaultClassLoader())//
.forEach(extensionRegistry::registerAutoDetectedExtension);
}

For the filtering, exclusions are already implemented in ClassNamePatternFilterUtils and you'll have to add support for inclusions.

Please let me know if you have any questions.

YongGoose added a commit to YongGoose/junit5 that referenced this issue Nov 10, 2024
YongGoose added a commit to YongGoose/junit5 that referenced this issue Nov 10, 2024
Issue: junit-team#3717
Signed-off-by: yongjunhong <[email protected]>
@YongGoose
Copy link
Contributor

YongGoose commented Nov 10, 2024

I've already worked on this in my personal repository, and since there’s a large amount of code changes, I decided to split it into two PRs.

I plan to proceed in the following two ways.

  1. Add an include method to ClassNamePatternFilterUtils.
  2. Implement include and exclude filtering when calling registerAutoDetectedExtensions in MutableExtensionRegistry.

The link above is the PR for the first step.

@marcphilipp
Copy link
Member

@YongGoose Thanks! I'll take a look in the next few days! 👍

marcphilipp added a commit that referenced this issue Nov 12, 2024
Issue: #3717

---------

Signed-off-by: yongjunhong <[email protected]>
Co-authored-by: Marc Philipp <[email protected]>
YongGoose added a commit to YongGoose/junit5 that referenced this issue Nov 12, 2024
YongGoose added a commit to YongGoose/junit5 that referenced this issue Nov 14, 2024
YongGoose added a commit to YongGoose/junit5 that referenced this issue Nov 14, 2024
YongGoose added a commit to YongGoose/junit5 that referenced this issue Nov 15, 2024
YongGoose added a commit to YongGoose/junit5 that referenced this issue Nov 16, 2024
YongGoose added a commit to YongGoose/junit5 that referenced this issue Nov 18, 2024
Issue: junit-team#3717
Signed-off-by: yongjunhong <[email protected]>
YongGoose added a commit to YongGoose/junit5 that referenced this issue Nov 19, 2024
YongGoose added a commit to YongGoose/junit5 that referenced this issue Nov 19, 2024
YongGoose added a commit to YongGoose/junit5 that referenced this issue Nov 21, 2024
YongGoose added a commit to YongGoose/junit5 that referenced this issue Nov 27, 2024
Issue: junit-team#3717
Signed-off-by: yongjunhong <[email protected]>
YongGoose added a commit to YongGoose/junit5 that referenced this issue Nov 27, 2024
Issue: junit-team#3717
Signed-off-by: yongjunhong <[email protected]>
YongGoose added a commit to YongGoose/junit5 that referenced this issue Nov 27, 2024
Issue: junit-team#3717
Signed-off-by: yongjunhong <[email protected]>
YongGoose added a commit to YongGoose/junit5 that referenced this issue Nov 27, 2024
YongGoose added a commit to YongGoose/junit5 that referenced this issue Nov 30, 2024
Issue: junit-team#3717
Signed-off-by: yongjunhong <[email protected]>
YongGoose added a commit to YongGoose/junit5 that referenced this issue Dec 5, 2024
Issue: junit-team#3717
Signed-off-by: yongjunhong <[email protected]>
YongGoose added a commit to YongGoose/junit5 that referenced this issue Dec 5, 2024
Issue: junit-team#3717
Signed-off-by: yongjunhong <[email protected]>
YongGoose added a commit to YongGoose/junit5 that referenced this issue Dec 14, 2024
Issue: junit-team#3717
Signed-off-by: yongjunhong <[email protected]>
@marcphilipp marcphilipp added this to the 5.12 M1 milestone Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment