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

LauncherInterceptor is missing from the org.junit.platform.launcher module's uses declaration #3561

Closed
shartte opened this issue Nov 19, 2023 · 9 comments · Fixed by #3562
Closed

Comments

@shartte
Copy link
Contributor

shartte commented Nov 19, 2023

Steps to reproduce

  • Set junit.platform.launcher.interceptors.enabled to true
  • Try running JUnit tests as modules
  • Exception: Caused by: java.util.ServiceConfigurationError: org.junit.platform.launcher.LauncherInterceptor: module org.junit.platform.launcher does not declare uses

Context

  • Used versions (Jupiter/Vintage/Platform): Platform 1.10.1
  • Build Tool/IDE: Gradle

Probably an oversight since JUnit platform consumed as a module is an uncommon configuration.
The new Interceptor probably just needs to be added here:
https://github.com/junit-team/junit5/blob/main/junit-platform-launcher/src/module/org.junit.platform.launcher/module-info.java#L37

@sbrannen sbrannen changed the title LauncherInterceptor is missing from org.junit.platform.launcher uses declaration LauncherInterceptor is missing from the org.junit.platform.launcher module's uses declaration Nov 19, 2023
@sbrannen sbrannen added this to the 5.11 M1 milestone Nov 19, 2023
@sormuras
Copy link
Member

Good catch!

I wonder why our modular compilation tests didn't catch that missing uses directive. 🤔

@shartte
Copy link
Contributor Author

shartte commented Nov 19, 2023

You can compile without it. It'll only throw at runtime and only if you enable the launcher interceptor configuration:

(From LauncherFactory)

if (configurationParameters.getBoolean(ENABLE_LAUNCHER_INTERCEPTORS).orElse(false)) {
	List<LauncherInterceptor> interceptors = new ArrayList<>();
	ServiceLoaderRegistry.load(LauncherInterceptor.class).forEach(interceptors::add);
	return interceptors;
}

@sormuras
Copy link
Member

I guess javac doesn't perform the "static" check due to that we no longer use ServiceLoader.load(...) method directly:

Iterable<T> listeners = ServiceLoader.load(serviceProviderClass, ClassLoaderUtils.getDefaultClassLoader());

@sormuras
Copy link
Member

You can compile without it.

Sure. Yet, a warning should be emitted when running javac in modular mode and there's a ServiceLoader.load(LauncherInterceptor.class[, ...]) statement and no uses [...]LauncherInterceptor in the associated module declaration. And as we usually compile with "treat all warnings as errors" flag, it should not compile (in modular mode).

@shartte
Copy link
Contributor Author

shartte commented Nov 19, 2023

Ah, I was unaware of the javac warning 🤔

@sormuras
Copy link
Member

At least, it is an IntelliJ IDEA code inspection: https://www.jetbrains.com/help/idea/list-of-java-inspections.html#visibility "Usage of service not declared in module-info"

@sormuras
Copy link
Member

Seems like I there's no such lint warning from javac - neither javac -Xlint:module ... nor javac -Xlint:all ... emits the warning that IDEA generates. Perhaps, we can teach javac to perform a similar check. But that all can be done in parallel with your PR here.

@marcphilipp
Copy link
Member

Reopening to backport it to 5.10.2

@marcphilipp marcphilipp reopened this Jan 12, 2024
@sormuras
Copy link
Member

Backported via ec8d428

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment