[#3345/#3393] Support Maven 4 module source hierarchy (single + multiple modules per POM)

[ascheman]

Fixes #3345. Fixes #3393.

Note

On @desruisseaux's request (via Slack) this PR now also contains the multi-module support that previously lived in the stacked draft PR #3394 — the split created review noise without value. The commit history keeps the two steps separate (Support Maven 4 module source hierarchy / Support multiple modules per POM).

Maven 4 with maven-compiler-plugin 4.x compiles module-source-hierarchy projects
(<build><sources> with one or more <module> entries, POM model 4.1.0) to a nested
output layout — target/classes/<module>/ and target/test-classes/<module>/ — and
writes a runtime handoff file target/test-classes/META-INF/maven/module-info-patch.args
derived from module-info-patch.maven (the non-deprecated replacement for a second
module-info.java in the test tree).

Surefire previously handled none of this, so modular whitebox tests silently fell back to
the classpath (module detection returned null) or failed test discovery
(DirectoryScanner interpreted the module directory as a package prefix, producing
doubled FQCNs such as com.example.com.example.CalculatorTest). The only workaround was
a per-module classesDirectory/testClassesDirectory/useModulePath/argLine
configuration hardcoding a single module name.

Changes — single module (#3345)

• AbstractSurefireMojo
  • findModuleDescriptor(...) now also looks for module-info.class in immediate
    subdirectories of the build output (target/classes/<module>/).
  • scanForTestClasses() scans target/test-classes/<module>/ when the nested layout
    is detected, instead of treating <module> as a package segment.
  • The --patch-module source is the nested target/test-classes/<module> directory
    when present.
• ModularClasspathForkConfiguration
  • createArgsFile(...) reads the (single, per-project) compiler-generated
    module-info-patch.args and passes its directives (e.g. --add-exports) through to
    the forked JVM argfile.
  • --add-reads/--add-modules lines from the handoff file are skipped for now (they
    may reference named modules that surefire still places on the classpath); surefire
    keeps generating --add-reads <module>=ALL-UNNAMED, --add-modules ALL-MODULE-PATH
    and --add-opens for test packages itself. Removing this bridge is [SUREFIRE-1755] Put dependencies from module-info in test sources on module path #3090
    (implementation in progress).
• surefire-its fixture: MavenLauncher defaults to forked JVM when the Maven under
  test is 4.x (Embedded3xLauncher only supports the Maven 3 entry point).

Changes — multiple modules per POM (#3393)

• findModuleDescriptor(...) resolves ALL nested module descriptors; the primary module
  (driving scanning and --patch-module) is the first one with a nested test output
  directory, siblings travel as additional results in ResolvePathResultWrapper.
• scanDirectories() unions the test-class scan over every nested
  target/test-classes/<module>/ directory.
• newStartupConfigWithModularPath(...): the classpath/module-path split is computed per
  module descriptor and unioned — an element required on the module path by ANY module
  ends up there (the fork has a single boot layer); the primary module only opens its own
  test packages; each sibling module with tests gets --patch-module, --add-reads and
  per-package --add-opens, passed through the existing
  StartupConfiguration#getJpmsArguments() channel (no surefire-booter API change).

Tests

• Unit tests for nested-descriptor detection (flat layout unaffected, module-name/package
  collision, multi-module ordering), handoff-file pass-through and fallback.
• New ITs:
  • ModulePathWhiteboxIT — classic single-module layout, runs under Maven 3 and 4.
  • Surefire3345ModuleSourceHierarchyIT — model-4.1.0 module source hierarchy with
    module-info-patch.maven; skips itself under Maven 3.
  • Surefire3393MultiModuleSourceHierarchyIT — two modules in one POM
    (com.example.extra requires com.example.core, core requires transitive jakarta.json), whitebox tests in both modules, 5 tests in ONE execution; skips
    itself under Maven 3.

Verification (local, JDK 17.0.18-tem, macOS)

Maven	classic IT	single-module MSH IT	multi-module MSH IT
3.9.16	pass	skip	skip
3.10.0-rc-1	pass	skip	skip
4.0.0-rc-5	pass	pass	pass (5 tests, one execution)
4.0.x-SNAPSHOT (rc-6 candidate)	pass	pass	pass

Full maven-surefire-common unit suite green. Real-world acceptance: a 3-module
jakarta.json/jsonb component (602 tests) runs zero-config in ONE surefire execution
(previously: silent 0-test run, or hand-written per-module executions).

Scope notes

• Test-scope dependencies still run from the classpath; honoring the handoff file
  verbatim (deps on the module path, no auto --add-reads/--add-modules) is [SUREFIRE-1755] Put dependencies from module-info in test sources on module path #3090,
  in progress on top of this PR.
• Known adjacent issues intentionally not addressed here: [SUREFIRE-1765] target/test-classes should not be added to classpath when tests run on modulepath using patch-module #3088, [SUREFIRE-1811] Add resources to JPMS test module #3040.
• The MSH ITs are skipped on the current Maven 3 CI matrix; they ran for real locally
  against 4.0.0-rc-5 and the current 4.0.x snapshot. Once Enable Maven 4 build #3352 enables a Maven 4 CI
  job, they will run there too.

---

Following this checklist to help us incorporate your contribution quickly and easily:

• Each commit in the pull request should have a meaningful subject line and body.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Run mvn clean install to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
• You have run the integration tests successfully (the new ITs plus the matrix above; the full suite runs in CI).
• I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004

[Copilot]

Pull request overview

This pull request updates Surefire’s JPMS handling to work with Maven 4’s module source hierarchy output layout (target/{classes,test-classes}/<module>/) and to consume the compiler-generated module-info-patch.args so modular whitebox tests can run on the module path without per-module Surefire configuration.

Changes:

• Extend module descriptor detection and test class scanning to support nested build output layouts produced by Maven 4.
• Enhance forked JPMS argfile generation to read module-info-patch.args and forward its directives while still letting Surefire manage --add-reads/--add-modules.
• Add new IT fixtures/tests covering classic module whitebox testing and Maven 4 module-source-hierarchy + module-info-patch.maven behavior; default IT launcher to fork for Maven 4+.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java	Detect nested module descriptor layout, scan tests from nested test output, and choose correct patch directory for --patch-module.
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java	Read compiler-generated module-info-patch.args and incorporate supported JPMS directives into the fork argfile.
maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java	Add unit tests for nested module descriptor/layout detection edge cases.
maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfigurationTest.java	Add unit tests validating consumption/skipping behavior for module-info-patch.args.
surefire-its/src/test/java/org/apache/maven/surefire/its/fixture/MavenLauncher.java	Default to forked launcher for Maven 4+ since embedded launcher is Maven 3-specific.
surefire-its/src/test/java/org/apache/maven/surefire/its/ModulePathWhiteboxIT.java	New IT asserting classic single-module whitebox testing works via --patch-module.
surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire3345ModuleSourceHierarchyIT.java	New Maven 4-only IT asserting module-source-hierarchy + patch args support.
surefire-its/src/test/resources/modulepath-whitebox/pom.xml	IT fixture POM for classic module-path whitebox testing.
surefire-its/src/test/resources/modulepath-whitebox/src/main/java/module-info.java	IT fixture module descriptor for classic layout.
surefire-its/src/test/resources/modulepath-whitebox/src/main/java/com/example/Calculator.java	IT fixture main class for classic layout.
surefire-its/src/test/resources/modulepath-whitebox/src/main/java/com/example/internal/MathHelper.java	IT fixture internal class used for whitebox access checks.
surefire-its/src/test/resources/modulepath-whitebox/src/test/java/com/example/CalculatorTest.java	IT fixture test verifying exported API behavior.
surefire-its/src/test/resources/modulepath-whitebox/src/test/java/com/example/internal/MathHelperWhiteboxTest.java	IT fixture whitebox test verifying non-exported internal access works.
surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/pom.xml	New Maven 4 model-4.1.0 module-source-hierarchy fixture POM.
surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/main/java/module-info.java	Fixture module descriptor for Maven 4 module source hierarchy layout.
surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/main/java/com/example/Calculator.java	Fixture main class for module-source-hierarchy project.
surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/main/java/com/example/internal/MathHelper.java	Fixture internal class for module-source-hierarchy project.
surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/test/java/module-info-patch.maven	Fixture patch description used by compiler to generate module-info-patch.args.
surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/test/java/com/example/CalculatorTest.java	Fixture test in nested test output layout.
surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/test/java/com/example/internal/MathHelperWhiteboxTest.java	Fixture whitebox test in nested test output layout.

[desruisseaux]

Thanks for doing that! I will look closely at the pull request. In the meantime, below is an observation based on the description only. It does not necessarily need an action in this pull request, maybe it is okay to process in steps and address the following point in future pull requests.

--add-reads / --add-modules lines from the handoff file are intentionally skipped (both one-line and two-line argfile form): they may reference named modules that surefire places on the classpath rather than the module path; surefire keeps generating --add-reads <module>=ALL-UNNAMED and --add-modules ALL-MODULE-PATH itself, plus --add-opens for test packages

I think that it would be nice if we could remove all --add-reads and --add-modules options added by Surefire when a module-info-patch.args file is present (these options would still be added automatically as a fallback when module-info-patch.args is not present). The intend is:

• give more control to the developer (via module-info-patch.maven),
• ensure that the tests are executed with the same options as the compilation,
• make debugging easier by giving the guarantee to the developers that what they see in module-info-patch.args is what has been used.

[ascheman]

@desruisseaux

I think that it would be nice if we could remove all --add-reads and --add-modules options added by Surefire when a module-info-patch.args file is present (these options would still be added automatically as a fallback when module-info-patch.args is not present).

Agreed on the direction — and you anticipated the reason for the current compromise: today surefire places test-scope dependencies (JUnit engine etc.) on the classpath, so honoring the file's --add-reads <m>=org.junit.jupiter.api / --add-modules verbatim would reference modules that are not in the fork's boot layer, and the JVM fails with FindException. That is why #3392 keeps surefire's own ALL-UNNAMED/ALL-MODULE-PATH handling as a bridge.

Fully honoring module-info-patch.args is, at its core, SUREFIRE-1755 (#3090): put the test-scope dependencies referenced by add-modules TEST-MODULE-PATH on the module path. Once that is in place, the natural completion is exactly what you describe — pass the file's directives through verbatim and only auto-generate --add-reads/--add-opens/--add-modules as a fallback when no module-info-patch.args exists.

I'm starting on that right away on a branch stacked on top of #3392/#3394 (the descriptor-driven path split from #3393 is the natural foundation), tracked under #3090 — the PR will arrive as a draft and become mergeable once these two have landed.

[desruisseaux]

I suggest to disable the auto-generation of --add-opens by default. I suggest that the default behaviour should be to use module-info-patch.maven exactly as-is. We may provide an option for enabling auto-generation of --add-opens if this is really what the user wants, but I would rather encourage developers to declare public the methods invoked by JUnit. It is not clear to me why some developers want to keep those methods private, as src/test/java classes are not distributed anyway and public makes clear that those methods are intended to be invoked from outside their package (from this point of view, declaring test methods as private or package-private seems misleading to me). Since --add-opens gives access to all private methods, not only test methods, an encapsulation argument in favour of this approach is questionable. I do not object if some developers really want to keep test methods private and rely on --add-opens, but I think that it should be opt-in rather than the default. A little bit in the spirit of "integrity by default" that Oracle is promoting.

[desruisseaux]

Correction: Surefire would still need to generate --add-exports for each non-exported package to JUnit. These --add-exports are not present in the module-info-patch.args file because they were not needed for compilation, but they are needed for execution.

If the test methods are public, --add-exports are sufficient. If the test methods are private, --add-exports need to be replaced by --add-opens (I'm not sure that we need both, let's try if we can provide only one of these options). The use of --add-exports versus --add-opens could be controlled by the configuration option suggested in above comment.

[ascheman]

Agreed with the direction, one empirical addition: the constraint is not only on the test methods but already on the test classes. The common JUnit 5 idiom uses package-private test classes, and --add-exports only grants access to public types — the engine then needs setAccessible (i.e. --add-opens) even for public methods in such classes, so exports-only as default would break that idiom.

This becomes actionable in the #3090 implementation (in progress, stacked on this PR and #3394), where the --add-opens generation already moved to the mojo side: I would add a configuration option along the lines of testModuleAccess = opens | exports | none. Given the JUnit 5 idiom I lean towards opens as the default — happy to follow your preference there.

[ascheman]

Yes — exports vs. opens becomes exactly that configuration choice in #3090, and surefire keeps generating one of the two for the test packages (they are not in module-info-patch.args since compilation does not need them, as you say).