Implement include and exclude filtering when calling registerAutoDetectedExtensions in MutableExtensionRegistry

[YongGoose]

fix #3717

Overview

This PR enables the activation of specific global extensions in JUnit Jupiter.

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.

YongGoose@a2ef07f
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.

---

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

[YongGoose]

@marcphilipp

There are a few points we need to discuss further.

1. When an extension is both included and excluded, exclude takes priority. WDYT?
2. Should the log display the extensions that were either included or excluded?

[marcphilipp]

We're getting there but I found a few more things. 😅

[marcphilipp]

When an extension is both included and excluded, exclude takes priority. WDYT?

Yes, I think that's how it generally works.

Should the log display the extensions that were either included or excluded?

Yes, I think that's a good idea. We should use Logger.config to log the classes that don't make it through the combined predicate.

[marcphilipp]

@YongGoose I sent a PR to your PR branch: YongGoose#1

Initially, I wanted to reuse the new ServiceLoaderUtils in LauncherFactory. However, the tests made me realize we can't call ServiceLoader.load from junit-platform-commons because it reflectively validates its caller. See YongGoose/junit5@d5a3511 (#1) for details.

Please merge my PR into your branch so we can incorporate its commits in this PR. I didn't want to push to your PR branch directly to avoid conflicts.

[YongGoose]

@YongGoose I sent a PR to your PR branch: YongGoose#1

Initially, I wanted to reuse the new ServiceLoaderUtils in LauncherFactory. However, the tests made me realize we can't call ServiceLoader.load from junit-platform-commons because it reflectively validates its caller. See YongGoose/junit5@d5a3511 (#1) for details.

Please merge my PR into your branch so we can incorporate its commits in this PR. I didn't want to push to your PR branch directly to avoid conflicts.

@marcphilipp
Thank you for your detailed explanation and for sending the PR. 👍

I understand the issue with ServiceLoader.load's caller validation and the resulting limitations in junit-platform-commons. I've reviewed the solution you proposed (YongGoose/junit5@d5a3511), and it seems like a good approach.

I'll merge the PR into my branch as requested.

[marcphilipp]

@YongGoose Please let me know when you're done with your changes and I should re-review.

[marcphilipp]

We should instead just link to the running-tests-config-params-deactivation-pattern.

[YongGoose]

I'm not very skilled at documentation work yet 😅

[[automatic-extension-registration-pattern-matching]]
===== Pattern Matching Syntax

Refer to <<running-tests-config-params-deactivation-pattern>> for details.

Would this change be fine?

[marcphilipp]

I haven't checked the history but I think the main reason this indirection exists in the other places is because we didn't want to break existing links. Since we're introducing new content here, I think we can simplify and link to the target section directly, i.e. without introducing another subsection.

[YongGoose]

I made this change in 13c1b19

[YongGoose]

@marcphilipp

Thank you very much for correcting my mistakes. 6e5b161
For the documentation work in other PRs, I will take more time to analyze and improve my approach to ensure better quality.

[marcphilipp]

Please also add integration tests for this to ExtensionRegistryTests. It currently verifies that ServiceLoaderExtension gets registered. If you add another one, you should be able to configure the two new config params in jupiter-tests/src/test/resources/junit-platform.properties so that one gets included and the other excluded.

[marcphilipp]

It seems weird to me to introduce a constant for an empty string here. Let's keep it internal and remove it from here, ok?

[marcphilipp]

For the documentation work in other PRs, I will take more time to analyze and improve my approach to ensure better quality.

No worries, that always requires a lot of context. I took a stab at it and pushed a commit to your branch.

[YongGoose]

Please also add integration tests for this to ExtensionRegistryTests

I couldn't work on this because I had an interview this week. I apologize for the delay.

Can I proceed with the integration test after mocking ConfigurationParameters as shown in the code below?

@Test
void registryIncludesAndExcludesAutoDetectedExtensionsBasedOnConfiguration() {
	ConfigurationParameters configurationParameters = mock(ConfigurationParameters.class);
	when(configurationParameters.get("junit.jupiter.extensions.autodetection.enabled"))
			.thenReturn(Optional.of("true"));
	when(configurationParameters.get("junit.jupiter.extensions.autodetection.include"))
			.thenReturn(Optional.of("org.junit.jupiter.engine.extension.AutoCloseExtension"));
	when(configurationParameters.get("junit.jupiter.extensions.autodetection.exclude"))
			.thenReturn(Optional.of("org.junit.jupiter.engine.extension.DisabledCondition"));

	JupiterConfiguration configuration = new DefaultJupiterConfiguration(configurationParameters, null);

	registry = createRegistryWithDefaultExtensions(configuration);
	assertExtensionRegistered(registry, AutoCloseExtension.class);
}

I tried implementing it using junit-platform.properties, but I couldn't figure out how to do it because JupiterConfiguration was mocked.

[marcphilipp]

I couldn't work on this because I had an interview this week. I apologize for the delay.

No worries! I hope the interview went well!

Can I proceed with the integration test after mocking ConfigurationParameters as shown in the code below?

LGTM 👍

[YongGoose]

After much consideration, I’ve concluded that mocking the getFilterForAutoDetectedExtensions method in the configuration is the correct approach. 😅

However, the getFilterForAutoDetectedExtensions method returns a Predicate<Class<? extends Extension>>.
Could you help me a bit with writing the thenReturn part for this?

@Test
void registryIncludesAndExcludesAutoDetectedExtensionsBasedOnConfiguration() {
	when(configuration.isExtensionAutoDetectionEnabled()).thenReturn(true);
	when(configuration.getFilterForAutoDetectedExtensions()).thenReturn(/* need help..  */);
	registry = createRegistryWithDefaultExtensions(configuration);

	assertExtensionRegistered(registry, ServiceLoaderExtension.class);
}

---

Additionally, through debugging, I confirmed that the intended Extension is applied based on the value of the property.

• junit-platform.properties

junit.jupiter.extensions.autodetection.enabled=true
junit.jupiter.extensions.autodetection.include=org.junit.jupiter.engine.extension.ServiceLoaderExtension
junit.jupiter.extensions.autodetection.exclude=org.junit.jupiter.engine.extension.ConfigLoaderExtension

• org.junit.jupiter.api.extension.Extension

org.junit.jupiter.engine.extension.ServiceLoaderExtension
org.junit.jupiter.engine.extension.ConfigLoaderExtension

[YongGoose]

No worries! I hope the interview went well!

Thank you as always.
I will do my best for the JUnit project❕

[YongGoose]

After much consideration, I’ve concluded that mocking the getFilterForAutoDetectedExtensions method in the configuration is the correct approach. 😅

However, the getFilterForAutoDetectedExtensions method returns a Predicate<Class<? extends Extension>>. Could you help me a bit with writing the thenReturn part for this?

I made this change in ec13b6d !

[marcphilipp]

@YongGoose Thank you for your contribution! I improved a few mostly minor things in the last few commits.