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

Revise module declarations #2063

Closed
3 tasks done
sormuras opened this issue Oct 14, 2019 · 15 comments · Fixed by #2121
Closed
3 tasks done

Revise module declarations #2063

sormuras opened this issue Oct 14, 2019 · 15 comments · Fixed by #2121

Comments

@sormuras
Copy link
Member

sormuras commented Oct 14, 2019

Revise module declarations for wrong, unnecessary and superfluous directives and modifiers.

Related issue: #1939

Deliverables

  • Check if transitive is really needed for each requires directive that declares it
  • All internal packages are hidden, i.e. not exported
  • All modules are required explicitly when an exported package is used directly
@sormuras
Copy link
Member Author

sormuras commented Dec 3, 2019

Module org.junit.platform.engine does not export package org.junit.platform.engine.support.filter - although it contains the published ClasspathScanningSupport class. That class, deprecated, is used by 3rd-party engines. Like: https://github.com/testng-team/testng-junit5/blob/master/src/main/java/org/testng/junit5/TestNGine.java#L3

Error message reads:

Caused by: java.lang.IllegalAccessError: class org.testng.junit5.TestNGine (in module testng.junit5) cannot access class org.junit.platform.engine.support.filter.ClasspathScanningSupport (in module org.junit.platform.engine) because module org.junit.platform.engine does not export org.junit.platform.engine.support.filter to module testng.junit5
at testng.junit5/org.testng.junit5.TestNGine.discover(TestNGine.java:48)
at org.junit.platform.launcher@1.6.0-M1/org.junit.platform.launcher.core.DefaultLauncher.discoverEngineRoot(DefaultLauncher.java:181)

We should export the package in the upcoming 5.6 release.

sormuras added a commit that referenced this issue Dec 5, 2019
@sormuras
Copy link
Member Author

sormuras commented Dec 5, 2019

Is transitive needed for requires transitive org.apiguardian.api;?

Yes. E.g org.apiguardian.api.API is part of "all" JUnit 5 API and it is annotated with @Retention(RUNTIME) -- so it leaks to all downstream modules.

Is transitive needed for requires transitive org.opentest4j;?

Yes. E.g. Assertions.assertAll(...) throws org.opentest4j.MultipleFailuresError

@sormuras
Copy link
Member Author

org.junit.jupiter

org.junit.jupiter (junit-jupiter-5.6.0-M1.jar)
  [Module descriptor]
    requires mandated java.base;
    requires transitive org.junit.jupiter.api;
    requires transitive org.junit.jupiter.engine;
    requires transitive org.junit.jupiter.params;
  [Suggested module descriptor for org.junit.jupiter]
    requires mandated java.base;
  [Transitive reduced graph for org.junit.jupiter]
    requires mandated java.base;
    requires org.junit.jupiter.engine;
    requires org.junit.jupiter.params;

@sormuras
Copy link
Member Author

sormuras commented Dec 18, 2019

org.junit.jupiter.api

org.junit.jupiter.api (junit-jupiter-api-5.6.0-M1.jar)
  [Module descriptor]
    requires mandated java.base;
    requires transitive org.apiguardian.api (@1.1.0);
    requires transitive org.junit.platform.commons;
    requires transitive org.opentest4j (@1.2.0);
  [Suggested module descriptor for org.junit.jupiter.api]
    requires mandated java.base;
    requires transitive org.apiguardian.api;
    requires transitive org.junit.platform.commons;
    requires transitive org.opentest4j;
    requires transitive unnamed;
java.lang.NullPointerException
	at jdk.jdeps/com.sun.tools.jdeps.ModuleGraphBuilder.requiresTransitive(ModuleGraphBuilder.java:124)

https://github.com/sormuras/JDK-8225773/runs/354396322#step:4:166

@sormuras
Copy link
Member Author

org.junit.jupiter.engine

org.junit.jupiter.engine (junit-jupiter-engine-5.6.0-M1.jar)
  [Module descriptor]
    requires mandated java.base;
    requires transitive org.apiguardian.api (@1.1.0);
    requires transitive org.junit.jupiter.api;
    requires transitive org.junit.platform.engine;
    requires transitive org.opentest4j (@1.2.0);
  [Suggested module descriptor for org.junit.jupiter.engine]
    requires mandated java.base;
    requires org.apiguardian.api;
    requires org.junit.jupiter.api;
    requires org.junit.platform.commons;
    requires org.junit.platform.engine;
    requires org.opentest4j;
  [Transitive reduced graph for org.junit.jupiter.engine]
    requires mandated java.base;
    requires org.junit.jupiter.api;
    requires org.junit.platform.engine;

@sormuras
Copy link
Member Author

sormuras commented Dec 18, 2019

org.junit.jupiter.params

org.junit.jupiter.params (junit-jupiter-params-5.6.0-M1.jar)
  [Module descriptor]
    requires mandated java.base;
    requires transitive org.apiguardian.api (@1.1.0);
    requires transitive org.junit.jupiter.api;
  [Suggested module descriptor for org.junit.jupiter.params]
    requires mandated java.base;
    requires java.sql;
    requires transitive org.apiguardian.api;
    requires transitive org.junit.jupiter.api;
    requires transitive org.junit.platform.commons;
    requires unnamed;
java.lang.NullPointerException
	at jdk.jdeps/com.sun.tools.jdeps.ModuleGraphBuilder.requiresTransitive(ModuleGraphBuilder.java:124)

https://github.com/sormuras/JDK-8225773/runs/354396352#step:4:62

@sormuras
Copy link
Member Author

org.junit.platform.commons

org.junit.platform.commons (junit-platform-commons-1.6.0-M1.jar)
  [Module descriptor]
    requires mandated java.base;
    requires java.logging;
    requires java.management;
    requires transitive org.apiguardian.api (@1.1.0);
  [Suggested module descriptor for org.junit.platform.commons]
    requires mandated java.base;
    requires java.logging;
    requires transitive org.apiguardian.api;
  [Transitive reduced graph for org.junit.platform.commons]
    requires mandated java.base;
    requires java.logging;
    requires transitive org.apiguardian.api;
  [Unused qualified exports in org.junit.platform.commons]
    exports org.junit.platform.commons.logging to org.junit.jupiter.migrationsupport,org.junit.jupiter.params,org.junit.platform.console,org.junit.platform.reporting,org.junit.platform.runner,org.junit.platform.suite.api
    exports org.junit.platform.commons.util to org.junit.platform.suite.api

@sormuras
Copy link
Member Author

org.junit.platform.console

org.junit.platform.console (junit-platform-console-1.6.0-M1.jar)
  [Module descriptor]
    requires mandated java.base;
    requires transitive org.apiguardian.api (@1.1.0);
    requires transitive org.junit.platform.reporting;
  [Suggested module descriptor for org.junit.platform.console]
    requires mandated java.base;
    requires org.apiguardian.api;
    requires org.junit.platform.commons;
    requires org.junit.platform.engine;
    requires org.junit.platform.launcher;
    requires org.junit.platform.reporting;
  [Transitive reduced graph for org.junit.platform.console]
    requires mandated java.base;
    requires org.junit.platform.reporting;

@sormuras
Copy link
Member Author

org.junit.platform.engine

org.junit.platform.engine (junit-platform-engine-1.6.0-M1.jar)
  [Module descriptor]
    requires mandated java.base;
    requires transitive org.apiguardian.api (@1.1.0);
    requires transitive org.junit.platform.commons;
    requires transitive org.opentest4j (@1.2.0);
  [Suggested module descriptor for org.junit.platform.engine]
    requires mandated java.base;
    requires transitive org.apiguardian.api;
    requires transitive org.junit.platform.commons;
    requires transitive org.opentest4j;
  [Transitive reduced graph for org.junit.platform.engine]
    requires mandated java.base;
    requires transitive org.apiguardian.api;
    requires transitive org.junit.platform.commons;
    requires transitive org.opentest4j;

@sormuras
Copy link
Member Author

org.junit.platform.launcher

org.junit.platform.launcher (junit-platform-launcher-1.6.0-M1.jar)
  [Module descriptor]
    requires mandated java.base;
    requires java.logging;
    requires transitive org.apiguardian.api (@1.1.0);
    requires transitive org.junit.platform.engine;
  [Suggested module descriptor for org.junit.platform.launcher]
    requires mandated java.base;
    requires transitive java.logging;
    requires transitive org.apiguardian.api;
    requires transitive org.junit.platform.commons;
    requires transitive org.junit.platform.engine;
  [Transitive reduced graph for org.junit.platform.launcher]
    requires mandated java.base;
    requires transitive java.logging;
    requires transitive org.apiguardian.api;
    requires transitive org.junit.platform.commons;
    requires transitive org.junit.platform.engine;

@sormuras
Copy link
Member Author

org.junit.platform.reporting

org.junit.platform.reporting (junit-platform-reporting-1.6.0-M1.jar)
  [Module descriptor]
    requires mandated java.base;
    requires java.xml;
    requires transitive org.apiguardian.api (@1.1.0);
    requires transitive org.junit.platform.launcher;
  [Suggested module descriptor for org.junit.platform.reporting]
    requires mandated java.base;
    requires java.xml;
    requires org.apiguardian.api;
    requires org.junit.platform.commons;
    requires transitive org.junit.platform.engine;
    requires transitive org.junit.platform.launcher;
  [Transitive reduced graph for org.junit.platform.reporting]
    requires mandated java.base;
    requires java.xml;
    requires transitive org.junit.platform.engine;
    requires transitive org.junit.platform.launcher;

@sormuras
Copy link
Member Author

org.junit.platform.suite.api

org.junit.platform.suite.api (junit-platform-suite-api-1.6.0-M1.jar)
  [Module descriptor]
    requires mandated java.base;
    requires transitive org.apiguardian.api (@1.1.0);
  [Suggested module descriptor for org.junit.platform.suite.api]
    requires mandated java.base;
    requires org.apiguardian.api;
  [Transitive reduced graph for org.junit.platform.suite.api]
    requires mandated java.base;
    requires org.apiguardian.api;

@sormuras
Copy link
Member Author

org.junit.platform.testkit

org.junit.platform.testkit (junit-platform-testkit-1.6.0-M1.jar)
  [Module descriptor]
    requires mandated java.base;
    requires java.instrument;
    requires transitive org.apiguardian.api (@1.1.0);
    requires transitive org.assertj.core;
    requires transitive org.junit.platform.launcher;
    requires transitive org.opentest4j (@1.2.0);
  [Suggested module descriptor for org.junit.platform.testkit]
    requires mandated java.base;
    requires transitive org.apiguardian.api;
    requires transitive org.assertj.core;
    requires org.junit.platform.commons;
    requires transitive org.junit.platform.engine;
    requires org.junit.platform.launcher;
    requires org.opentest4j;
  [Transitive reduced graph for org.junit.platform.testkit]
    requires mandated java.base;
    requires transitive org.apiguardian.api;
    requires transitive org.assertj.core;
    requires transitive org.junit.platform.engine;
    requires org.junit.platform.launcher;

@sormuras
Copy link
Member Author

sormuras commented Dec 18, 2019

I always forget this one... now, I can't sort into the other Jupiter modules above. 😈

org.junit.jupiter.migrationsupport

org.junit.jupiter.migrationsupport (junit-jupiter-migrationsupport-5.6.0-M1.jar)
  [Module descriptor]
    requires mandated java.base;
    requires transitive junit;
    requires transitive org.apiguardian.api (@1.1.0);
    requires transitive org.junit.jupiter.api;
  [Suggested module descriptor for org.junit.jupiter.migrationsupport]
    requires mandated java.base;
    requires transitive junit;
    requires org.apiguardian.api;
    requires transitive org.junit.jupiter.api;
    requires org.junit.platform.commons;
  [Transitive reduced graph for org.junit.jupiter.migrationsupport]
    requires mandated java.base;
    requires transitive junit;
    requires transitive org.junit.jupiter.api;

@marcphilipp
Copy link
Member

"Suggested module descriptors" LGTM.

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

Successfully merging a pull request may close this issue.

2 participants