Junit5: Add module descriptors to all JUnit 5 modules

[sormuras]

Overview

Conceal internal packages by moving from Level 1 "Automatic-Module-Name" to Level 2 "Publish modularized JAR": https://vimeo.com/289846017#t=1298s 21:38

Draft Pull Requests

• #1593 Modules via using Java 9 as release target _outdated_
• #1844 Modules via extra compilation task
• #1846 Modules via @moditect
• #1847 Modules via module source set
• #1848 Modules via src/modules directory using javac
• #1857 Modules via extra compilation and jar tasks 💯

Deliverables

• [x] Create many drafts
• [x] Select one draft and make it happen

1. org.junit.platform.commons
2. org.junit.platform.console
3. org.junit.platform.engine
4. org.junit.platform.launcher
5. org.junit.platform.reporting
6. org.junit.platform.runner
7. org.junit.platform.suite.api
8. org.junit.platform.testkit
9. org.junit.jupiter
10. org.junit.jupiter.api
11. org.junit.jupiter.engine
12. org.junit.jupiter.migrationsupport
13. org.junit.jupiter.params
14. org.junit.vintage.engine

• [x] Include module-info.class in https://github.com/ota4j-team/opentest4j/pull/51
• [x] Include module-info.class in https://github.com/apiguardian-team/apiguardian

[sormuras]

Added "exports internal packages to friend modules" sections in description above.

[marcphilipp]

@sormuras This should be done in 5.1, right?

[sormuras]

It depends. Category: nice-to-have.

As we already support and provide JDK-9 features in this artifact, a module descriptor could be introduced here as well. With it, we only export the packages to other modules we want to be used/read by external parties.

But from a higher point of view, we could post-pone this feature until will are able to add module descriptors to all of our artifacts. Perhaps without adding a new JDK-9 project for each subproject we maintain.

[sbrannen]

What about the following artifacts?

• junit-platform-runner
• junit-platform-surefire-provider
• junit-jupiter-params
• junit-jupiter-migrationsupport
• junit-jupiter-api

All of the above consume packages from commons.util and potentially from commons.logging as well.

[sormuras]

True. All of our modules that read types from concealed commons packages need to be listed here. My local test branch, which doesn't use all of the modules you listed, worked with the module descriptor shown above.

Another quirk/flaw is the requires java.compiler; statement needed due to the usage of javax.lang.model.SourceVersion in PackageUtils. I tend to replace that usage with a similar function and drop the java.compiler dependency.

[sormuras]

If we ship our jars as modular jars, we should consider to offer a ~classis~ classic variant of the jars, that don't contain the module-info.class. Just like http://www.joda.org/joda-convert/ does -- see section "Releases".

[sbrannen]

I'm assuming you meant "classic". 😉

[sormuras]

[sbrannen]

ha ha ha 😛

[sormuras]

Simply adding a module-info.java to junit-platform-commons fails with the expected error message:

.../junit-platform-commons/src/main/java/module-info.java:10:
  error: modules are not supported in -source 8
module org.junit.platform.commons {
^
  (use -source 9 or higher to enable modules)
1 error

Is there a groovy/gradle way to only compile module descriptors with --release 9, @marcphilipp @melix @eriwen ... and others?

[sormuras]

@zyxist 's chainsaw plugin could help and/or change the source tree layout as outlined in #1216

...as we won't drop support for Java 8 soon, do we? @junit-team/junit-lambda ;-)

[sbrannen]

...as we won't drop support for Java 8 soon, do we? @junit-team/junit-lambda ;-)

No, we are not dropping base support for Java 8 any time in the near future.

[tlinkowski]

Is there a groovy/gradle way to only compile module descriptors with --release 9, @marcphilipp @melix @eriwen ... and others?

Hi, I don't know if you're still interested in this (module-info.class + JDK 8 baseline), but it can be done manually as shown in my Stack Overflow answer.

I also provided a PR with this feature for https://github.com/java9-modularity/gradle-modules-plugin/pull/73.

Finally, you can find an overview of this in my recent blog post.

Hope it helps 🙂

[sormuras]

Started to work on this issue, again. Running into https://github.com/johnrengelman/shadow/issues/425 now.

[sormuras]

Current jar state:

jar --file junit-platform-commons-1.5.0-SNAPSHOT.jar --describe-module

org.junit.platform.commons jar:file:///junit-platform-commons-1.5.0-SNAPSHOT.jar/!module-info.class

exports org.junit.platform.commons
exports org.junit.platform.commons.annotation
exports org.junit.platform.commons.support

requires java.base mandated
requires java.compiler
requires java.logging
requires org.apiguardian.api static

qualified exports org.junit.platform.commons.function to org.junit.jupiter.engine org.junit.platform.console org.junit.platform.engine org.junit.platform.launcher org.junit.vintage.engine
qualified exports org.junit.platform.commons.logging to org.junit.jupiter.engine org.junit.platform.console org.junit.platform.engine org.junit.platform.launcher org.junit.vintage.engine
qualified exports org.junit.platform.commons.util to org.junit.jupiter.api org.junit.jupiter.engine org.junit.platform.console org.junit.platform.engine org.junit.platform.launcher org.junit.vintage.engine

[sormuras]

Working-in-progress branch: https://github.com/junit-team/junit5/compare/issues/1091-explicit-java-module-descriptors -- will create a draft PR soon.

[sormuras]

> Task :junit-jupiter-api:compileModuleJava
junit5\junit-platform-commons\src\module\org.junit.platform.commons\module-info.java:14: warning: requires directive for an automatic module
    requires static org.apiguardian.api;
                                   ^
junit5\junit-jupiter-api\src\module\org.junit.jupiter.api\module-info.java:12: warning: requires directive for an automatic module
    requires org.apiguardian.api;
                            ^
junit5\junit-jupiter-api\src\module\org.junit.jupiter.api\module-info.java:14: warning: requires directive for an automatic module
    requires org.opentest4j;
                ^
3 warnings

We need to publish apiguardian and opentest4j as explicit modules as well, methinks.

Or drop -Xlint from the compileModuleJava task...

[sormuras]

Another solution would be @moditect -- see https://github.com/junit-team/junit5/compare/issues/1091-moditect for a spike.

Running into the following errors, though. After invoking gradlew build _n_ times, it sometimes works. @gunnarmorling @siordache any idea what's going wrong here? (Already tried to set requires="" to avoid the getAutoModuleNameFromInputJar call, to no avail.)

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':junit-platform-engine:generateModuleInfo'.
> java.util.NoSuchElementException (no error message)

Caused by: java.util.NoSuchElementException
    at org.moditect.model.DependencyDescriptor.getAutoModuleNameFromInputJar(DependencyDescriptor.java:52)
    at org.moditect.gradleplugin.ModitectExtension$_retrieveArtifactsInfo_closure3.doCall(ModitectExtension.groovy:105)
    at org.moditect.gradleplugin.ModitectExtension$_retrieveArtifactsInfo_closure3.call(ModitectExtension.groovy)
    at org.moditect.gradleplugin.ModitectExtension.retrieveArtifactsInfo(ModitectExtension.groovy:104)
    at org.moditect.gradleplugin.ModitectExtension.access$0(ModitectExtension.groovy)
    at org.moditect.gradleplugin.ModitectExtension$_getArtifactsInfo_closure6.doCall(ModitectExtension.groovy:52)
    at org.moditect.gradleplugin.ModitectExtension$_getArtifactsInfo_closure6.call(ModitectExtension.groovy)
    at org.moditect.gradleplugin.ModitectExtension.getArtifactsInfo(ModitectExtension.groovy:52)
    at org.moditect.gradleplugin.ModitectExtension_Decorated.getArtifactsInfo(Unknown Source)
    at org.moditect.gradleplugin.generate.GenerateModuleInfoTask.generateModuleInfo(GenerateModuleInfoTask.groovy:106)

[tlinkowski]

@sormuras I don't want to intrude here (or waste your time), but I had a look at java-library-conventions.gradle.kts, and I wonder:

Is there a reason for creating an extra source set (module) instead of just creating an extra JavaCompile task (and setting its destinationDir to compileJava.destinationDir)?

It's just that a source set introduces unnecessary configurations and tasks. On the other hand, maybe you feel that a source set better represents what module-info.java is (I certainly like the idea of keeping it in a directory named after the module).

If there's a reason, but it's not easy to explain, just forget this comment :wink:

[sormuras]

You're referring to https://github.com/junit-team/junit5/compare/issues/1091-explicit-java-module-descriptors -- right?

We tried to create the extra source set to be aligned with "common" Gradle-style and also have better IDE support allowing us to set the Java --release version to 9 ... and also being able to use --module-source-path from javac.

Edit: Additionally, using the destinationDir to compileJava.destinationDir trick messes with the cache-ability of Gradle tasks and also, you have to ignore/exclude all module-info.java files from IDE(A) compiliation, as our base is Java 8.

[tlinkowski]

You're referring to https://github.com/junit-team/junit5/compare/issues/1091-explicit-java-module-descriptors -- right?

Yes.

We tried to create the extra source set to be aligned with "common" Gradle-style and also have better IDE support allowing us to set the Java --release version to 9 ... and also being able to use --module-source-path from javac.

Thanks! The IDE support is a very good point, indeed.

Note that by an extra source set I didn't mean an extra directory but an extra Gradle SourceSet. So you could set --release and --module-source-path without such a SourceSet, too (these options are set for a JavaCompile task, not for a SourceSet per se).

I don't know if you managed to get it working or not. If you did, that's great! 👍

If you didn't, I can hint that, in my small project (just a couple of modules, no checkstyle, javadocs for Java 11), I managed to get this working with full IntelliJ support (i.e. it reports missing requires, wrong exports, etc.). However, I kept module-info.java in src/main/java, and I didn't use sourceCompatibility = 8 (only --release 8), so IntelliJ "thought" it's a Java 11 project. Haven't pushed that configuration yet, though (still figuring out module names :wink:).

Additionally, using the destinationDir to compileJava.destinationDir trick messes with the cache-ability of Gradle tasks and also, you have to ignore/exclude all module-info.java files from IDE(A) compiliation, as our base is Java 8.

Good points!

• I forgot to consider how this could affect caching! OTOH, there will never be any overwriting (module-info.java and the rest of Java classes are "disjoint"), so perhaps it won't be such a problem? I'll have to consider it closely.
• As to IDE compilation, I use "Delegate IDE build/run actions to gradle". But you're right, you'd need to exclude it in case someone doesn't use this option or uses a different IDE.

[siordache]

Execution failed for task ':junit-platform-engine:generateModuleInfo'.
java.util.NoSuchElementException (no error message)

@sormuras It's probably a bug in the moditect-gradle-plugin. The error is caused by the tasks not being executed in the correct order. For example, :junit-platform-engine:generateModuleInfo must run after :junit-platform-commons:jar, but currently this is not the case.

I will provide a fix soon.

[sormuras]

@siordache Great news. Looking forward to include it in the issues/1091-moditect branch.

[sormuras]

Meanwhile, I produced another spike using @tlinkowski 's approach. See PR #1844 -- which works _ok-ish_ for the single org.junit.platform.commons module.

Let's see which way will prevail... 🥇 🥈 🥉

[siordache]

@sormuras I just released moditect-gradle-plugin 1.0.0-beta2, which fixes the task execution order. Please give it a try.

[sormuras]

Looks better. Now, I'm running into:

> Task :junit-platform-engine:addMainModuleInfo FAILED
Error: junit-platform-commons-1.5.0-SNAPSHOT.jar is a multi-release jar file but --multi-release option is not set

https://github.com/moditect/moditect-gradle-plugin/issues/9, methinks.

[siordache]

Yes. And the root cause is moditect/moditect#70.

[sormuras]

Mh, I could defer the MR-JAR creation to a very late ... task. When all moditect tasks are finish. Would that help?

[siordache]

It may help.

But I'm not able to reproduce the MR-JAR error on my machine. I get no error when running gradlew build on the 1091-moditect branch. How do you run gradle?

[sormuras]

gradlew from IDEA using clean spotlessApply build and --scan --stacktrace

Edit: now, it seem to run fine...

[sormuras]

Branch issues/1091-moditect builds locally: https://scans.gradle.com/s/ktuvor6v2xcvs/

Creating a draft PR for it: https://github.com/junit-team/junit5/pull/1846

Thanks @siordache for the quick help!

[sormuras]

Updated the initial issue description to include all available drafts: four as of now.

Please direct specific comments to appropriate PRs. They are:

• #1593 Modules via using Java 9 as release target (_outdated_)
• #1844 Modules via extra compilation task
• #1846 Modules via @moditect
• #1847 Modules via module source set

[tlinkowski]

Updated the initial issue description to include all available drafts: four as of now.

Now you only need the fifth one: "Modules via @java9-modularity (Gradle Modules Plugin)" 🙂

My bet would be on this one, provided you configure your tests to run:

• on classpath only (easier to set up):
  groovy test.moduleOptions.runOnClasspath = true
• on both classpath and module-path (harder to set up, but testing is more versatile):
  groovy test.moduleOptions.runOnClasspath = hasProperty('runOnClasspath')
  and then run Gradle twice:
  
  
  
  1. ./gradlew test (for module-path testing)
  
  
  2. ./gradlew test -PrunOnClasspath (for classpath testing)
  
  

[sormuras]

Now you only need the fifth one: "Modules via @java9-modularity (Gradle Modules Plugin)" 🙂

You just volunteered to provide no. 5, didn't you? 😄

Our module-path integration tests are stored in the platform-tooling-support-tests subproject. So, running all "standard/unit" tests on the class-path is fine.

[tlinkowski]

You just volunteered to provide no. 5, didn't you? 😄

I'd actually love to try this, if I only had more time! 🙂

However, it should be rather straightforward, based on no. 2:

1. remove exclude("module-info.java") from compileJava
2. remove
   groovy compileJava { sourceCompatibility = extension.mainJavaVersion.majorVersion targetCompatibility = extension.mainJavaVersion.majorVersion // needed by asm // --release release // Compiles against the public, supported and documented API for a specific VM version. // Supported release targets are 6, 7, 8, 9, 10, and 11. // Note that if --release is added then -target and -source are ignored. options.compilerArgs.addAll(listOf("--release", extension.mainJavaVersion.majorVersion)) }
   (hmm, this "needed by asm" might be a problem here - if you set compileJava.targetCompatibility explicitly, Gradle Modules Plugin will complain :/ )
3. remove compileModuleInfoJava task definition
4. apply org.javamodularity.moduleplugin
5. call modularity.mixedJavaRelease extension.mainJavaVersion.majorVersion

---

Our module-path integration tests are stored in the platform-tooling-support-tests subproject. So, running all "standard/unit" tests on the class-path is fine.

I see. Sorry, I think my remark about parametrizing these tests was wrong because I confused:

• black-box tests: platform-tooling-support-tests in your case
• white-box tests: "standard/unit" in your case

So it seems it shouldn't matter if you set test.moduleOptions.runOnClasspath = true or not for your white-box tests. As @pbakker writes here:

Either option is fine. By default, the plugin will automatically setup the compiler and test runtime to run on the module path, and patch the module to avoid split packages.

[sormuras]

Added "_#1848 Modules via src/modules directory using javac_" to the initial issue description.

No. 5 is alive! And it is my absolute favorite draft. It only needs some Gradle-integration love...

[sormuras]

• [x] Include module-info.class in https://github.com/ota4j-team/opentest4j/pull/51
• [x] Include module-info.class in https://github.com/apiguardian-team/apiguardian