Revise module declaration #1711

sormuras · 2019-11-27T07:17:33Z

Relates to #1656 -- here are the changes that made AssertJ Core compile with Java 9+, including the module descriptor.

Added the "base" source root /src/main/java to the compilation set of the java9+ profile.
Encountered a lot of "invisible package" errors...
...and added a bunch of requires ...; statements to fix those. Left some comments in the module-info.java file.

Open ends:

Delete/ignore all Java-9+ compiled classes, except for module-info.class
java.instrument is a special snow flake, it's not required directly to compile AssertJ Core, but ByteBuddy seems to need it. @raphw ... any new input here? (removed in 116798b)
Module java.sql is only(?) needed due to a link reference in the javadoc of Assertions.java (not needed since 5c136d6)
Module name junit is file named based -- JUnit 4.13 will fix this, though. (fixed in b0385fc)
Module name jsr305 is file name based -- don't see a fix in the near future, do you? (not needed since 24176f5)

TODO:

Provide integration test that uses module org.assertj.core on the module path.
Run jdeps --check ..., similar to Run jdeps --check on all modules junit-team/junit-framework#1939

sormuras · 2019-11-27T07:31:00Z

The jdeps --check run yields:

jdeps --module-path target\assertj-core-3.14.1-SNAPSHOT.jar --multi-release BASE --check org.assertj.core
org.assertj.core (file:///D:/dev/github/sormuras/assertj-core/target/assertj-core-3.14.1-SNAPSHOT.jar)
  [Module descriptor]
    requires mandated java.base;
    requires static java.management;
    requires static java.sql;
    requires static java.xml;
    requires static jsr305;
    requires static junit;
    requires static net.bytebuddy;
    requires static org.hamcrest;
    requires static org.junit.jupiter.api (@5.5.2);
    requires static org.opentest4j (@1.2.0);
  [Suggested module descriptor for org.assertj.core]
    requires mandated java.base;
    requires java.instrument;
    requires java.logging;
    requires java.management;
    requires java.xml;
    requires transitive unnamed;
Exception in thread "main" java.lang.NullPointerException
        at jdk.jdeps/com.sun.tools.jdeps.ModuleGraphBuilder.requiresTransitive(ModuleGraphBuilder.java:124)
        at jdk.jdeps/com.sun.tools.jdeps.ModuleGraphBuilder.buildGraph(ModuleGraphBuilder.java:110)
        at jdk.jdeps/com.sun.tools.jdeps.ModuleGraphBuilder.buildGraph(ModuleGraphBuilder.java:80)
        at jdk.jdeps/com.sun.tools.jdeps.ModuleAnalyzer$ModuleDeps.buildReducedGraph(ModuleAnalyzer.java:182)
        at jdk.jdeps/com.sun.tools.jdeps.ModuleAnalyzer$ModuleDeps.reduced(ModuleAnalyzer.java:195)
        at jdk.jdeps/com.sun.tools.jdeps.ModuleAnalyzer$ModuleDeps.analyzeDeps(ModuleAnalyzer.java:215)
        at jdk.jdeps/com.sun.tools.jdeps.ModuleAnalyzer.lambda$run$3(ModuleAnalyzer.java:92)
        at java.base/java.util.HashMap$Values.forEach(HashMap.java:976)
        at jdk.jdeps/com.sun.tools.jdeps.ModuleAnalyzer.run(ModuleAnalyzer.java:88)
        at jdk.jdeps/com.sun.tools.jdeps.JdepsTask$CheckModuleDeps.run(JdepsTask.java:968)
        at jdk.jdeps/com.sun.tools.jdeps.JdepsTask.run(JdepsTask.java:560)
        at jdk.jdeps/com.sun.tools.jdeps.JdepsTask.run(JdepsTask.java:519)
        at jdk.jdeps/com.sun.tools.jdeps.Main.main(Main.java:49)

Hit by https://bugs.openjdk.java.net/browse/JDK-8225773 again.

joel-costigliola · 2019-11-27T08:36:46Z

src/main/java9/module-info.java


-  requires java.instrument;
+
+  requires static java.sql; // import java.sql.Timestamp; in Assertions.java for Javadoc-only


In order to remove this line, https://github.com/joel-costigliola/assertj-core/blob/master/src/main/java/org/assertj/core/api/Assertions.java#L2552 can be replaced by

* <li><code>yyyy-MM-dd HH:mm:ss.SSS</code> (for {@code java.sql.Timestamp} String representation support)</li>

Not needed anymore since 5c136d6.

scordio · 2019-11-27T08:46:42Z

Provide integration test that uses module org.assertj.core on the module path.

@sormuras I already started something about this topic, following Testing In The Modular World. I am happy to continue on it if you like (also happy to have any review and hints from you once I am ready)

joel-costigliola · 2019-11-27T08:48:52Z

That would be great @scordio, I'm a bit busy these days.

sormuras · 2019-11-27T08:59:37Z

Yes, please proceed, @scordio -- mainrunner is my external integration test project, that also uses Bach.java as its build tool. Thus, there're at least two moving parts involved.

raphw · 2019-11-27T20:52:54Z

Byte Buddy needs java.instrument statically. It is fully optional. I don't think assertj needs it in a direct way.

sormuras · 2019-12-02T07:30:54Z

Thanks for the clarification, Rafael.

Partial changes applied from #1711.

scordio · 2019-12-29T15:15:03Z

Side question about:
https://github.com/joel-costigliola/assertj-core/blob/88a22d3a237e3400ed0fd4143b81bd2ccb6c8f10/src/main/java9/module-info.java#L31
why is the internal package exported?

@joel-costigliola would you expect something in there which should be directly usable by consumers (i.e., end users or third party libraries)? If so, we should probably relocate them.

joel-costigliola · 2019-12-29T22:46:19Z

Ideally it should not be exported

This "it" project runs tests on the module path, using AssertJ's SoftAssertions utility. As no module declares a read edge to module "java.instrument", the build of this "it" project fails. Issue: assertj#1711

sormuras · 2020-02-18T09:07:51Z

Superseded.

Revise module description

bdb4aef

sormuras changed the title ~~Revise module description~~ Revise module declaration Nov 27, 2019

joel-costigliola reviewed Nov 27, 2019

View reviewed changes

scordio mentioned this pull request Dec 1, 2019

Add modular integration test #1717

Closed

3 tasks

scordio mentioned this pull request Dec 26, 2019

org.assertj.core.api.junit.jupiter not exported in module-info #1735

Closed

scordio pushed a commit that referenced this pull request Dec 26, 2019

Add missing module declarations

116798b

Partial changes applied from #1711.

sormuras mentioned this pull request Feb 1, 2020

Upgrade to AssertJ 3.15.0 junit-team/junit-framework#2170

Closed

7 tasks

sormuras closed this Feb 18, 2020

sormuras deleted the module-info branch February 18, 2020 09:08


		requires java.instrument;

		requires static java.sql; // import java.sql.Timestamp; in Assertions.java for Javadoc-only

Uh oh!

Revise module declaration #1711

Revise module declaration #1711

Uh oh!

Conversation

sormuras commented Nov 27, 2019 • edited by scordio Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sormuras commented Nov 27, 2019

Uh oh!

joel-costigliola Nov 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sormuras Dec 2, 2019

Choose a reason for hiding this comment

Uh oh!

scordio Dec 26, 2019

Choose a reason for hiding this comment

Uh oh!

scordio commented Nov 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joel-costigliola commented Nov 27, 2019

Uh oh!

sormuras commented Nov 27, 2019

Uh oh!

raphw commented Nov 27, 2019

Uh oh!

sormuras commented Dec 2, 2019

Uh oh!

scordio commented Dec 29, 2019

Uh oh!

joel-costigliola commented Dec 29, 2019

Uh oh!

sormuras commented Feb 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sormuras commented Nov 27, 2019 •

edited by scordio

Loading

joel-costigliola Nov 27, 2019 •

edited

Loading

scordio commented Nov 27, 2019 •

edited

Loading