Improve AutoExtensionRegistration behavior with filtering

[amishra-u]

When using AutoExtensionRegistration together with the include and exclude properties, the ServiceLoader should not throw an exception for extensions that would be filtered out anyway.

Currently, this causes issues when a /META-INF/services/org.junit.jupiter.api.extension.Extension file exists in a third-party dependency and references an extension class that is unavailable at runtime. Even though this extension would be excluded by the filtering configuration, the test execution fails to start due to the loading failure.

Let me know if this enhancements make sense, I will raise the PR for the same.

[marcphilipp]

Which version of Java are you using to run your tests?

[amishra-u]

java-17

[marcphilipp]

We're already applying the filter before instantiating the registered class on Java 9+: https://github.com/junit-team/junit5/blob/main/junit-platform-commons/src/main/java9/org/junit/platform/commons/util/ServiceLoaderUtils.java

It seems that's not enough? Could you please share a reproducer?

[amishra-u]

This override implementation should work, I am suspecting jvm is picking standard implementation instead of override implementation.

We use bazel, so might be a setup issue.

[marcphilipp]

Could you please share a stacktrace?

[amishra-u]

Verified that both versions fail. Instead of using a stream, we likely need to switch to a safe iterator with a try-catch around hasNext.

java.util.ServiceConfigurationError: org.junit.jupiter.api.extension.Extension: Provider org.apache.flink.util.TestLoggerExtension not found
        at java.base/java.util.ServiceLoader.fail(ServiceLoader.java:593)
        at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.nextProviderClass(ServiceLoader.java:1219)
        at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.hasNextService(ServiceLoader.java:1228)
        at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.hasNext(ServiceLoader.java:1273)
        at java.base/java.util.ServiceLoader$2.hasNext(ServiceLoader.java:1309)
        at java.base/java.util.ServiceLoader$ProviderSpliterator.tryAdvance(ServiceLoader.java:1491)
        at java.base/java.util.Spliterator.forEachRemaining(Spliterator.java:332)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
        at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)

[sormuras]

Seems like already the initialization of the provider class fails - perhaps due to some static code reaching out to in this setup non-existing classes. I see two options here:

• filter earlier by name, before calling provider.type() (if possible)
• add the missing dependencies to the setup

[sormuras]

https://github.com/apache/flink/blob/c151909d5a0d3568178c1a984da76b39d7e94dee/flink-test-utils-parent/flink-test-utils-junit/src/main/java/org/apache/flink/util/TestLogger.java#L38-L40

    static {
        TestSignalHandler.register();
    }

This should be moved into a non-static context.

[sormuras]

[...] switch to a safe iterator with a try-catch around hasNext.

In order to catch ServiceConfigurationError (as described in ServiceLoader API documentation) and ignore them based on String-based filter configuration?

An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch. Most such errors are abnormal conditions.

I think that JUnit should not try to mend such configuration errors. Either:

• add missing artifacts to your setup
• fix the service implementation to not fail in static code
• wait until the SericeLoader API offers a String-based filter feature.

In the light of the above, I tend to close the issue as "won't fix" or "not planned".