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

Implement include and exclude filtering when calling registerAutoDetectedExtensions in MutableExtensionRegistry #4120

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

YongGoose
Copy link
Contributor

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

Issue: junit-team#3717
Signed-off-by: yongjunhong <kevin0928@naver.com>
@YongGoose
Copy link
Contributor Author

@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?

Comment on lines +113 to +114
return ClassNamePatternFilterUtils.includeMatchingClassNames(include).and(
ClassNamePatternFilterUtils.excludeMatchingClassNames(exclude));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return ClassNamePatternFilterUtils.includeMatchingClassNames(include).and(
ClassNamePatternFilterUtils.excludeMatchingClassNames(exclude));
return ClassNamePatternFilterUtils.includeMatchingClassNames(include) //
.and(ClassNamePatternFilterUtils.excludeMatchingClassNames(exclude));

ServiceLoader.load(Extension.class, ClassLoaderUtils.getDefaultClassLoader())//
.forEach(extensionRegistry::registerAutoDetectedExtension);
.forEach(extension -> {
if (filter.test(extension.getClass().getName())) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether it would be worth creating a Java 9+ variant that avoids instantiating extension that don't pass the filter. We could do this via a new ServiceLoaderUtils class in junit-platform-commons that has a Java 9+ implementation in src/main/java9. The signature should look sth. like

Stream<T> load(
	Class<T> service,
	Predicate<? super Class<? extends T>> providerPredicate,
	ClassLoader loader
);

with the Java 8 implementation being

StreamSupport.stream(ServiceLoader.load(service, loader).spliterator(), false)
	.filter(it -> {
		@SuppressWarnings("unchecked") Class<? extends T> type = (Class<? extends T>) it.getClass();
		return providerPredicate.test(type);
	});

and Java 9+

ServiceLoader.load(service, loader).stream()
	.filter(provider -> providerPredicate.test(provider.type()))
	.map(ServiceLoader.Provider::get);

WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce mechanism to enable specific global extensions in JUnit Jupiter
2 participants