From 09f0e5b0bbd875282533e7cb09adcee6005063fb Mon Sep 17 00:00:00 2001 From: Gerd Aschemann Date: Sat, 18 Jul 2026 14:00:36 +0200 Subject: [PATCH 01/10] Use forked JVM mode for Maven 4 in ITs The IT fixture defaults to embedded launching via Embedded3xLauncher (maven-shared-verifier), which looks up org.apache.maven.cli.MavenCli. Maven 4 renamed the entry point to org.apache.maven.cling.MavenCling with a different signature, so every embedded launch against Maven 4 aborts with NoSuchMethodException before any test runs. Detect Maven 4 by the presence of maven-api-core in ${maven.home}/lib and default forkJvm to true in that case; Maven 3 keeps embedded mode. Co-Authored-By: Claude Fable 5 --- .../surefire/its/fixture/MavenLauncher.java | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/surefire-its/src/test/java/org/apache/maven/surefire/its/fixture/MavenLauncher.java b/surefire-its/src/test/java/org/apache/maven/surefire/its/fixture/MavenLauncher.java index 463d8b46e7..117529c750 100755 --- a/surefire-its/src/test/java/org/apache/maven/surefire/its/fixture/MavenLauncher.java +++ b/surefire-its/src/test/java/org/apache/maven/surefire/its/fixture/MavenLauncher.java @@ -76,8 +76,8 @@ public final class MavenLauncher { this.resourceName = resourceName; this.suffix = suffix != null ? suffix : ""; this.cli = cli == null ? null : cli.clone(); - // by default use embedded mode - this.forkJvm = false; + // Use forked mode for Maven 4+ (Embedded3xLauncher only supports Maven 3.x) + this.forkJvm = isMaven4Plus(); resetGoals(); resetCliOptions(); } @@ -416,4 +416,18 @@ private static File settingsXmlPath() { throw new IllegalStateException(e.getLocalizedMessage(), e); } } + + private static boolean isMaven4Plus() { + String mavenHome = System.getProperty("maven.home"); + if (mavenHome == null) { + return false; + } + File mavenLib = new File(mavenHome, "lib"); + if (!mavenLib.isDirectory()) { + return false; + } + // Maven 4 ships maven-api-core; Maven 3 does not + File[] files = mavenLib.listFiles((dir, name) -> name.startsWith("maven-api-core-") && name.endsWith(".jar")); + return files != null && files.length > 0; + } } From 2fc6f9d5ff7ec5a74ee64a0303eb43659c04a3f3 Mon Sep 17 00:00:00 2001 From: Gerd Aschemann Date: Sat, 18 Jul 2026 15:50:58 +0200 Subject: [PATCH 02/10] Support Maven 4 module source hierarchy (#3345) Maven 4 with maven-compiler-plugin 4.x compiles module source hierarchy projects to a nested layout (target/classes//, target/test-classes//) and emits a runtime handoff file META-INF/maven/module-info-patch.args derived from module-info-patch.maven. Surefire handled neither: - findModuleDescriptor() only looked for module-info.class at the build output root, so detection failed and execution silently fell back to the classpath. - DirectoryScanner read the module directory as a package prefix, producing doubled FQCNs and "Tests run: 0" or "Unable to create test class" failures. - The compiler-generated module-info-patch.args was ignored, dropping the --add-exports directives declared in module-info-patch.maven. Detect the nested module directory, scan its test classes, patch the module with target/test-classes/, and merge the handoff file into the fork argfile. --add-reads/--add-modules lines from the file are skipped (one-line and two-line argfile forms): they may reference named modules that surefire places on the classpath; surefire keeps generating --add-reads =ALL-UNNAMED, --add-opens for test packages and --add-modules ALL-MODULE-PATH itself. The nested layout is decided by the MAIN build output only (no root module-info.class plus a module subdirectory containing one): a classic modular project whose module is named after its root package (module "it", package "it") has target/test-classes/it/ as a package directory, which must not be mistaken for a nested test output and patched into the module (would break class loading of every test). Covered by unit tests and two ITs: ModulePathWhiteboxIT (classic layout, runs on Maven 3+4) and Surefire3345ModuleSourceHierarchyIT (nested layout, skips itself below Maven 4). Co-Authored-By: Claude Fable 5 --- .../plugin/surefire/AbstractSurefireMojo.java | 78 ++++++- .../ModularClasspathForkConfiguration.java | 95 +++++++- .../AbstractSurefireMojoJava7PlusTest.java | 91 ++++++++ ...ModularClasspathForkConfigurationTest.java | 214 ++++++++++++++++++ .../surefire/its/ModulePathWhiteboxIT.java | 43 ++++ .../Surefire3345ModuleSourceHierarchyIT.java | 67 ++++++ .../resources/modulepath-whitebox/pom.xml | 40 ++++ .../src/main/java/com/example/Calculator.java | 9 + .../java/com/example/internal/MathHelper.java | 11 + .../src/main/java/module-info.java | 3 + .../test/java/com/example/CalculatorTest.java | 12 + .../internal/MathHelperWhiteboxTest.java | 22 ++ .../pom.xml | 57 +++++ .../main/java/com/example/Calculator.java | 9 + .../java/com/example/internal/MathHelper.java | 11 + .../com.example/main/java/module-info.java | 3 + .../test/java/com/example/CalculatorTest.java | 12 + .../internal/MathHelperWhiteboxTest.java | 22 ++ .../test/java/module-info-patch.maven | 9 + 19 files changed, 796 insertions(+), 12 deletions(-) create mode 100644 surefire-its/src/test/java/org/apache/maven/surefire/its/ModulePathWhiteboxIT.java create mode 100644 surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire3345ModuleSourceHierarchyIT.java create mode 100644 surefire-its/src/test/resources/modulepath-whitebox/pom.xml create mode 100644 surefire-its/src/test/resources/modulepath-whitebox/src/main/java/com/example/Calculator.java create mode 100644 surefire-its/src/test/resources/modulepath-whitebox/src/main/java/com/example/internal/MathHelper.java create mode 100644 surefire-its/src/test/resources/modulepath-whitebox/src/main/java/module-info.java create mode 100644 surefire-its/src/test/resources/modulepath-whitebox/src/test/java/com/example/CalculatorTest.java create mode 100644 surefire-its/src/test/resources/modulepath-whitebox/src/test/java/com/example/internal/MathHelperWhiteboxTest.java create mode 100644 surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/pom.xml create mode 100644 surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/main/java/com/example/Calculator.java create mode 100644 surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/main/java/com/example/internal/MathHelper.java create mode 100644 surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/main/java/module-info.java create mode 100644 surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/test/java/com/example/CalculatorTest.java create mode 100644 surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/test/java/com/example/internal/MathHelperWhiteboxTest.java create mode 100644 surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/test/java/module-info-patch.maven diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java index 676e9cdc6c..6ee49a06de 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java @@ -1064,7 +1064,18 @@ private DefaultScanResult scanForTestClasses() throws MojoFailureException { } private DefaultScanResult scanDirectories() throws MojoFailureException { - DirectoryScanner scanner = new DirectoryScanner(getTestClassesDirectory(), getIncludedAndExcludedTests()); + File scanDir = getTestClassesDirectory(); + // Maven 4 Module Source Hierarchy: test classes may be nested under / + if (scanDir != null && isNestedModuleLayout(getMainBuildPath())) { + File nestedModule = findNestedModuleDescriptor(getMainBuildPath()); + if (nestedModule != null) { + File nestedTestDir = new File(scanDir, nestedModule.getName()); + if (nestedTestDir.isDirectory()) { + scanDir = nestedTestDir; + } + } + } + DirectoryScanner scanner = new DirectoryScanner(scanDir, getIncludedAndExcludedTests()); return scanner.scan(); } @@ -1443,6 +1454,14 @@ private ResolvePathResultWrapper findModuleDescriptor(File jdkHome, File buildPa boolean isJpmsModule = buildPath.isDirectory() ? new File(buildPath, "module-info.class").exists() : isModule(buildPath); + // Maven 4 Module Source Hierarchy: classes may be in target/classes// + if (!isJpmsModule && buildPath.isDirectory()) { + File nestedModuleDir = findNestedModuleDescriptor(buildPath); + if (nestedModuleDir != null) { + return findModuleDescriptor(jdkHome, nestedModuleDir, isMainDescriptor); + } + } + if (!isJpmsModule) { return new ResolvePathResultWrapper(null, isMainDescriptor); } @@ -1457,6 +1476,43 @@ private ResolvePathResultWrapper findModuleDescriptor(File jdkHome, File buildPa } } + /** + * Whether the main build output uses the Maven 4 Module Source Hierarchy layout: + * no module-info.class at the root of the build output directory, but at least one + * immediate subdirectory containing one ({@code target/classes//}). + * A classic modular build (module-info.class at the root) is never nested, even if + * a subdirectory happens to share the module's name. + * + * @param buildPath the build output directory (e.g., target/classes) + * @return {@code true} for the nested module source hierarchy layout + */ + private static boolean isNestedModuleLayout(File buildPath) { + return buildPath != null + && buildPath.isDirectory() + && !new File(buildPath, "module-info.class").exists() + && findNestedModuleDescriptor(buildPath) != null; + } + + /** + * Searches for a module-info.class in immediate subdirectories of the given directory. + * This supports Maven 4 Module Source Hierarchy where compiled classes are placed + * in {@code target/classes//} instead of directly in {@code target/classes/}. + * + * @param buildPath the build output directory (e.g., target/classes) + * @return the subdirectory containing module-info.class, or null if not found + */ + private static File findNestedModuleDescriptor(File buildPath) { + File[] subdirs = buildPath.listFiles(File::isDirectory); + if (subdirs != null) { + for (File subdir : subdirs) { + if (new File(subdir, "module-info.class").exists()) { + return subdir; + } + } + } + return null; + } + private static boolean isModule(File jar) { try (ZipFile zip = new ZipFile(jar)) { return zip.getEntry("module-info.class") != null; @@ -2025,12 +2081,22 @@ private StartupConfiguration newStartupConfigWithModularPath( getConsoleLogger().debug("main module descriptor name: " + javaModuleDescriptor.name()); + File patchFile = null; + if (isMainDescriptor) { + File testDir = getTestClassesDirectory(); + if (testDir != null && testDir.isDirectory() && isNestedModuleLayout(getMainBuildPath())) { + // Maven 4 Module Source Hierarchy: test classes nested under /. + // Only the main output layout decides — a test-classes subdirectory merely + // sharing the module name (module named after its root package) must not. + File nestedTestDir = new File(testDir, javaModuleDescriptor.name()); + patchFile = nestedTestDir.isDirectory() ? nestedTestDir : testDir; + } else { + patchFile = testDir; + } + } + ModularClasspath modularClasspath = new ModularClasspath( - javaModuleDescriptor.name(), - testModulepath.getClassPath(), - packages, - isMainDescriptor ? getTestClassesDirectory() : null, - isMainDescriptor); + javaModuleDescriptor.name(), testModulepath.getClassPath(), packages, patchFile, isMainDescriptor); Artifact[] additionalInProcArtifacts = { getCommonArtifact(), diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java index ab85aca420..824103a588 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java @@ -22,7 +22,9 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import java.io.BufferedReader; import java.io.File; +import java.io.FileReader; import java.io.FileWriter; import java.io.IOException; import java.util.Collection; @@ -184,6 +186,15 @@ File createArgsFile( .append('"') .append(NL); + // Check for module-info-patch.args generated by maven-compiler-plugin 4.x + File patchArgs = findModuleInfoPatchArgs(patchFile); + if (patchArgs != null) { + // Use --add-exports directives from module-info-patch.args + appendModuleInfoPatchArgs(args, patchArgs, moduleName); + } + + // Always auto-generate --add-opens for test packages (JUnit needs reflection access). + // module-info-patch.maven cannot use ALL-UNNAMED in add-opens, so surefire handles this. for (String pkg : packages) { args.append("--add-opens") .append(NL) @@ -195,12 +206,15 @@ File createArgsFile( .append(NL); } - args.append("--add-reads") - .append(NL) - .append(moduleName) - .append('=') - .append("ALL-UNNAMED") - .append(NL); + if (patchArgs == null) { + // Without module-info-patch.args, also auto-generate --add-reads + args.append("--add-reads") + .append(NL) + .append(moduleName) + .append('=') + .append("ALL-UNNAMED") + .append(NL); + } } args.append("--add-modules").append(NL).append("ALL-MODULE-PATH").append(NL); @@ -224,4 +238,73 @@ File createArgsFile( return surefireArgs; } } + + /** + * Searches for module-info-patch.args generated by maven-compiler-plugin 4.x. + * The file is expected at {@code target/test-classes/META-INF/maven/module-info-patch.args} + * or within a module subdirectory at + * {@code target/test-classes//META-INF/maven/module-info-patch.args}. + * + * @param patchFile the test classes directory (may be the module subdirectory) + * @return the module-info-patch.args file, or null if not found + */ + @Nullable + private static File findModuleInfoPatchArgs(File patchFile) { + // Direct location: target/test-classes//META-INF/maven/module-info-patch.args + File argsFile = new File(patchFile, "META-INF/maven/module-info-patch.args"); + if (argsFile.isFile()) { + return argsFile; + } + // Parent location: target/test-classes/META-INF/maven/module-info-patch.args + File parent = patchFile.getParentFile(); + if (parent != null) { + argsFile = new File(parent, "META-INF/maven/module-info-patch.args"); + if (argsFile.isFile()) { + return argsFile; + } + } + return null; + } + + /** + * Reads the module-info-patch.args file and appends its --add-exports and --add-opens + * directives to the args builder. The --add-reads directive is handled by surefire itself + * (always adds ALL-UNNAMED) to avoid referencing modules that may be on the classpath + * rather than the module-path. + * + * @param args the args builder to append to + * @param patchArgs the module-info-patch.args file + * @param moduleName the module name for the fallback --add-reads + * @throws IOException if the file cannot be read + */ + private static void appendModuleInfoPatchArgs(StringBuilder args, File patchArgs, String moduleName) + throws IOException { + try (BufferedReader reader = new BufferedReader(new FileReader(patchArgs))) { + String line; + while ((line = reader.readLine()) != null) { + line = line.trim(); + if (line.isEmpty() || line.startsWith("#")) { + continue; + } + // Skip --add-reads and --add-modules from the file — surefire manages these itself. + // The compiler-generated args may reference named modules that surefire places on + // the classpath rather than the module-path, causing boot layer errors. + if (line.startsWith("--add-reads") || line.startsWith("--add-modules")) { + if (line.equals("--add-reads") || line.equals("--add-modules")) { + // two-line form: the value is on the following line + reader.readLine(); + } + continue; + } + args.append(line).append(NL); + } + } + // Surefire always needs --add-reads =ALL-UNNAMED for its classpath-based runner + args.append("--add-reads") + .append(NL) + .append(moduleName) + .append('=') + .append("ALL-UNNAMED") + .append(NL); + } } diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java index e0c7c689e9..5b7fbb23ea 100644 --- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java +++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java @@ -20,6 +20,7 @@ import java.io.File; import java.lang.reflect.Method; +import java.nio.file.Files; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; @@ -486,6 +487,96 @@ public void shouldJoinStrings() throws Exception { assertThat(result).isEmpty(); } + @Test + public void shouldFindNestedModuleDescriptor() throws Exception { + // Create a temp directory structure: target/classes/com.example/module-info.class + File tempDir = Files.createTempDirectory("surefire-test-nested-module").toFile(); + try { + File moduleDir = new File(tempDir, "com.example"); + moduleDir.mkdirs(); + new File(moduleDir, "module-info.class").createNewFile(); + + File result = invokeMethod(AbstractSurefireMojo.class, "findNestedModuleDescriptor", tempDir); + assertThat(result).isNotNull(); + assertThat(result.getName()).isEqualTo("com.example"); + } finally { + // Cleanup + new File(new File(tempDir, "com.example"), "module-info.class").delete(); + new File(tempDir, "com.example").delete(); + tempDir.delete(); + } + } + + @Test + public void shouldNotFindNestedModuleDescriptorInFlatLayout() throws Exception { + // Create a temp directory structure without nested module-info.class + File tempDir = Files.createTempDirectory("surefire-test-flat").toFile(); + try { + File pkgDir = new File(tempDir, "com/example"); + pkgDir.mkdirs(); + new File(pkgDir, "Foo.class").createNewFile(); + + File result = invokeMethod(AbstractSurefireMojo.class, "findNestedModuleDescriptor", tempDir); + assertThat(result).isNull(); + } finally { + new File(new File(tempDir, "com/example"), "Foo.class").delete(); + new File(tempDir, "com/example").delete(); + new File(tempDir, "com").delete(); + tempDir.delete(); + } + } + + @Test + public void shouldReturnNullForEmptyDirectory() throws Exception { + File tempDir = Files.createTempDirectory("surefire-test-empty").toFile(); + try { + File result = invokeMethod(AbstractSurefireMojo.class, "findNestedModuleDescriptor", tempDir); + assertThat(result).isNull(); + } finally { + tempDir.delete(); + } + } + + @Test + public void shouldNotTreatClassicModularLayoutAsNested() throws Exception { + // Classic layout with a root module descriptor: module "it" with root package "it" + // produces target/classes/module-info.class and target/classes/it/ — the package + // directory sharing the module name must NOT switch surefire to the nested layout. + File tempDir = + Files.createTempDirectory("surefire-test-classic-modular").toFile(); + try { + new File(tempDir, "module-info.class").createNewFile(); + File pkgDir = new File(tempDir, "it"); + pkgDir.mkdirs(); + new File(pkgDir, "Main.class").createNewFile(); + + boolean result = invokeMethod(AbstractSurefireMojo.class, "isNestedModuleLayout", tempDir); + assertThat(result).isFalse(); + } finally { + new File(new File(tempDir, "it"), "Main.class").delete(); + new File(tempDir, "it").delete(); + new File(tempDir, "module-info.class").delete(); + tempDir.delete(); + } + } + + @Test + public void shouldTreatModuleSourceHierarchyLayoutAsNested() throws Exception { + File tempDir = Files.createTempDirectory("surefire-test-nested-layout").toFile(); + try { + File moduleDir = new File(tempDir, "com.example"); + moduleDir.mkdirs(); + new File(moduleDir, "module-info.class").createNewFile(); + + boolean result = invokeMethod(AbstractSurefireMojo.class, "isNestedModuleLayout", tempDir); + assertThat(result).isTrue(); + } finally { + new File(new File(tempDir, "com.example"), "module-info.class").delete(); + new File(tempDir, "com.example").delete(); + tempDir.delete(); + } + } + private static File mockFile(String absolutePath) { File f = mock(File.class); when(f.getAbsolutePath()).thenReturn(absolutePath); diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfigurationTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfigurationTest.java index d3fe5d32a6..bc8ba14f2f 100644 --- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfigurationTest.java +++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfigurationTest.java @@ -19,6 +19,8 @@ package org.apache.maven.plugin.surefire.booterclient; import java.io.File; +import java.io.FileWriter; +import java.nio.file.Files; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -165,4 +167,216 @@ public void shouldCreateModularArgsFile() throws Exception { assertThat(line).isEqualTo(argsFileLines.get(i)); } } + + @Test + @SuppressWarnings("ResultOfMethodCallIgnored") + public void shouldUseModuleInfoPatchArgsWhenPresent() throws Exception { + Classpath booter = new Classpath(asList("booter.jar", "non-modular.jar")); + File target = new File("target").getCanonicalFile(); + File tmp = new File(target, "surefire"); + tmp.mkdirs(); + File pwd = new File(".").getCanonicalFile(); + + ModularClasspathForkConfiguration config = new ModularClasspathForkConfiguration( + booter, + tmp, + "", + pwd, + new Properties(), + "", + Collections.emptyMap(), + new String[0], + true, + 1, + true, + new Platform(), + new NullConsoleLogger(), + mock(ForkNodeFactory.class)); + + // Create a patchFile directory with module-info-patch.args + File patchFile = Files.createTempDirectory("surefire-test-patch").toFile(); + try { + File metaInf = new File(patchFile, "META-INF/maven"); + metaInf.mkdirs(); + File patchArgs = new File(metaInf, "module-info-patch.args"); + try (FileWriter fw = new FileWriter(patchArgs)) { + fw.write("--add-modules junit,org.junit.jupiter.api\n"); + fw.write("--add-reads mymod=junit,org.junit.jupiter.api,ALL-UNNAMED\n"); + fw.write("--add-exports mymod/com.example.internal=ALL-UNNAMED\n"); + } + + List modulePath = asList("modular.jar", "target" + separatorChar + "classes"); + List classPath = asList("booter.jar", "non-modular.jar", patchFile.getPath()); + Collection packages = singleton("com.example.test"); + String startClassName = ForkedBooter.class.getName(); + + File jigsawArgsFile = config.createArgsFile( + "mymod", + modulePath, + classPath, + packages, + patchFile, + startClassName, + true, + Collections.emptyList()); + + assertThat(jigsawArgsFile).isNotNull(); + List lines = readAllLines(jigsawArgsFile.toPath(), UTF_8); + + // Should contain --patch-module (always generated by surefire) + assertThat(lines).anyMatch(l -> l.equals("--patch-module")); + + // Should contain --add-exports from module-info-patch.args (not auto-generated) + assertThat(lines).anyMatch(l -> l.equals("--add-exports mymod/com.example.internal=ALL-UNNAMED")); + + // Should NOT contain --add-modules from module-info-patch.args (surefire skips these) + assertThat(lines) + .noneMatch(l -> l.contains("junit,org.junit.jupiter.api") && !l.contains("ALL-MODULE-PATH")); + + // Should contain --add-reads with ALL-UNNAMED (surefire's own, not from file) + assertThat(lines).anyMatch(l -> l.equals("mymod=ALL-UNNAMED")); + + // Should STILL contain auto-generated --add-opens for test packages + // (surefire always generates these for JUnit reflection access) + assertThat(lines).anyMatch(l -> l.equals("mymod/com.example.test=ALL-UNNAMED")); + + // Should still have --add-modules ALL-MODULE-PATH + assertThat(lines).anyMatch(l -> l.equals("ALL-MODULE-PATH")); + } finally { + // Cleanup + new File(new File(patchFile, "META-INF/maven"), "module-info-patch.args").delete(); + new File(patchFile, "META-INF/maven").delete(); + new File(patchFile, "META-INF").delete(); + patchFile.delete(); + } + } + + @Test + @SuppressWarnings("ResultOfMethodCallIgnored") + public void shouldFallbackWithoutModuleInfoPatchArgs() throws Exception { + Classpath booter = new Classpath(asList("booter.jar", "non-modular.jar")); + File target = new File("target").getCanonicalFile(); + File tmp = new File(target, "surefire"); + tmp.mkdirs(); + File pwd = new File(".").getCanonicalFile(); + + ModularClasspathForkConfiguration config = new ModularClasspathForkConfiguration( + booter, + tmp, + "", + pwd, + new Properties(), + "", + Collections.emptyMap(), + new String[0], + true, + 1, + true, + new Platform(), + new NullConsoleLogger(), + mock(ForkNodeFactory.class)); + + // Create a patchFile directory WITHOUT module-info-patch.args + File patchFile = Files.createTempDirectory("surefire-test-nopatch").toFile(); + try { + List modulePath = asList("modular.jar", "target" + separatorChar + "classes"); + List classPath = asList("booter.jar", "non-modular.jar", patchFile.getPath()); + Collection packages = singleton("com.example.test"); + String startClassName = ForkedBooter.class.getName(); + + File jigsawArgsFile = config.createArgsFile( + "mymod", + modulePath, + classPath, + packages, + patchFile, + startClassName, + true, + Collections.emptyList()); + + assertThat(jigsawArgsFile).isNotNull(); + List lines = readAllLines(jigsawArgsFile.toPath(), UTF_8); + + // Should contain auto-generated --add-opens for the test package + assertThat(lines).anyMatch(l -> l.equals("mymod/com.example.test=ALL-UNNAMED")); + + // Should contain auto-generated --add-reads + assertThat(lines).anyMatch(l -> l.equals("mymod=ALL-UNNAMED")); + + // Should contain --patch-module + assertThat(lines).anyMatch(l -> l.equals("--patch-module")); + } finally { + patchFile.delete(); + } + } + + @Test + @SuppressWarnings("ResultOfMethodCallIgnored") + public void shouldNotSwallowDirectiveAfterSameLineSkippedOption() throws Exception { + Classpath booter = new Classpath(asList("booter.jar", "non-modular.jar")); + File target = new File("target").getCanonicalFile(); + File tmp = new File(target, "surefire"); + tmp.mkdirs(); + File pwd = new File(".").getCanonicalFile(); + + ModularClasspathForkConfiguration config = new ModularClasspathForkConfiguration( + booter, + tmp, + "", + pwd, + new Properties(), + "", + Collections.emptyMap(), + new String[0], + true, + 1, + true, + new Platform(), + new NullConsoleLogger(), + mock(ForkNodeFactory.class)); + + // maven-compiler-plugin 4.x writes option and value on the SAME line. + // A directive directly after a skipped option must not be swallowed. + File patchFile = Files.createTempDirectory("surefire-test-patch-order").toFile(); + try { + File metaInf = new File(patchFile, "META-INF/maven"); + metaInf.mkdirs(); + File patchArgs = new File(metaInf, "module-info-patch.args"); + try (FileWriter fw = new FileWriter(patchArgs)) { + fw.write("--add-modules junit,org.junit.jupiter.api\n"); + fw.write("--add-exports mymod/com.example.internal=ALL-UNNAMED\n"); + fw.write("--add-reads mymod=junit,org.junit.jupiter.api,ALL-UNNAMED\n"); + } + + List modulePath = asList("modular.jar", "target" + separatorChar + "classes"); + List classPath = asList("booter.jar", "non-modular.jar", patchFile.getPath()); + Collection packages = singleton("com.example.test"); + String startClassName = ForkedBooter.class.getName(); + + File jigsawArgsFile = config.createArgsFile( + "mymod", + modulePath, + classPath, + packages, + patchFile, + startClassName, + true, + Collections.emptyList()); + + assertThat(jigsawArgsFile).isNotNull(); + List lines = readAllLines(jigsawArgsFile.toPath(), UTF_8); + + // The --add-exports following the skipped --add-modules must survive + assertThat(lines).anyMatch(l -> l.equals("--add-exports mymod/com.example.internal=ALL-UNNAMED")); + + // The skipped options must not leak through from the file + assertThat(lines) + .noneMatch(l -> l.contains("junit,org.junit.jupiter.api") && !l.contains("ALL-MODULE-PATH")); + } finally { + new File(new File(patchFile, "META-INF/maven"), "module-info-patch.args").delete(); + new File(patchFile, "META-INF/maven").delete(); + new File(patchFile, "META-INF").delete(); + patchFile.delete(); + } + } } diff --git a/surefire-its/src/test/java/org/apache/maven/surefire/its/ModulePathWhiteboxIT.java b/surefire-its/src/test/java/org/apache/maven/surefire/its/ModulePathWhiteboxIT.java new file mode 100644 index 0000000000..3d15c93914 --- /dev/null +++ b/surefire-its/src/test/java/org/apache/maven/surefire/its/ModulePathWhiteboxIT.java @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.surefire.its; + +import org.apache.maven.surefire.its.fixture.AbstractJava9PlusIT; +import org.junit.jupiter.api.Test; + +/** + * Integration test for whitebox testing of Java modules. + * Tests that surefire correctly patches test classes into the module + * via --patch-module, allowing access to non-exported internal packages. + *

+ * This test works with Maven 3 and the standard source layout. + */ +class ModulePathWhiteboxIT extends AbstractJava9PlusIT { + + @Test + void testWhiteboxModulePath() { + // 3 tests: CalculatorTest.testAdd + MathHelperWhiteboxTest.testAdd + testMultiply + assumeJava9().debugLogging().executeTest().verifyErrorFreeLog().assertTestSuiteResults(3); + } + + @Override + protected String getProjectDirectoryName() { + return "modulepath-whitebox"; + } +} diff --git a/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire3345ModuleSourceHierarchyIT.java b/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire3345ModuleSourceHierarchyIT.java new file mode 100644 index 0000000000..98abed3aa0 --- /dev/null +++ b/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire3345ModuleSourceHierarchyIT.java @@ -0,0 +1,67 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.surefire.its.jiras; + +import java.io.File; + +import org.apache.maven.surefire.its.fixture.AbstractJava9PlusIT; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +/** + * Integration test for whitebox testing of Java modules using Maven 4 + * Module Source Hierarchy and module-info-patch.maven. + *

+ * This test requires Maven 4 and is skipped when running with Maven 3. + * It verifies that surefire correctly: + *

    + *
  • Detects module-info.class in nested target/classes/<module>/ directories
    • + *
  • Scans test classes from nested target/test-classes/<module>/ directories
    • + *
  • Reads module-info-patch.args generated by maven-compiler-plugin 4.x
    • + *
+ */ +class Surefire3345ModuleSourceHierarchyIT extends AbstractJava9PlusIT { + + @Test + void testWhiteboxWithModuleSourceHierarchy() { + assumeTrue(isMaven4Plus(), "This test requires Maven 4."); + // 3 tests: CalculatorTest.testAdd + MathHelperWhiteboxTest.testAdd + testMultiply + assumeJava9().debugLogging().executeTest().verifyErrorFreeLog().assertTestSuiteResults(3); + } + + @Override + protected String getProjectDirectoryName() { + return "surefire-3345-module-source-hierarchy"; + } + + private static boolean isMaven4Plus() { + String mavenHome = System.getProperty("maven.home"); + if (mavenHome == null) { + return false; + } + File mavenLib = new File(mavenHome, "lib"); + if (!mavenLib.isDirectory()) { + return false; + } + // Maven 4 ships maven-api-core; Maven 3 does not + File[] files = mavenLib.listFiles((dir, name) -> name.startsWith("maven-api-core-") && name.endsWith(".jar")); + return files != null && files.length > 0; + } +} diff --git a/surefire-its/src/test/resources/modulepath-whitebox/pom.xml b/surefire-its/src/test/resources/modulepath-whitebox/pom.xml new file mode 100644 index 0000000000..0c33beff2a --- /dev/null +++ b/surefire-its/src/test/resources/modulepath-whitebox/pom.xml @@ -0,0 +1,40 @@ + + + 4.0.0 + + org.apache.maven.surefire + it-parent + 1.0 + ../pom.xml + + com.example + modulepath-whitebox + 1.0.0-SNAPSHOT + + ${java.specification.version} + + + + org.junit.jupiter + junit-jupiter-engine + 5.9.1 + test + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.10.0 + + + org.apache.maven.plugins + maven-surefire-plugin + ${surefire.version} + + + + diff --git a/surefire-its/src/test/resources/modulepath-whitebox/src/main/java/com/example/Calculator.java b/surefire-its/src/test/resources/modulepath-whitebox/src/main/java/com/example/Calculator.java new file mode 100644 index 0000000000..42827959c8 --- /dev/null +++ b/surefire-its/src/test/resources/modulepath-whitebox/src/main/java/com/example/Calculator.java @@ -0,0 +1,9 @@ +package com.example; + +import com.example.internal.MathHelper; + +public class Calculator { + public long add(long a, long b) { + return MathHelper.add(a, b); + } +} diff --git a/surefire-its/src/test/resources/modulepath-whitebox/src/main/java/com/example/internal/MathHelper.java b/surefire-its/src/test/resources/modulepath-whitebox/src/main/java/com/example/internal/MathHelper.java new file mode 100644 index 0000000000..90c0416310 --- /dev/null +++ b/surefire-its/src/test/resources/modulepath-whitebox/src/main/java/com/example/internal/MathHelper.java @@ -0,0 +1,11 @@ +package com.example.internal; + +public class MathHelper { + public static long add(long x, long y) { + return x + y; + } + + public static long multiply(long x, long y) { + return x * y; + } +} diff --git a/surefire-its/src/test/resources/modulepath-whitebox/src/main/java/module-info.java b/surefire-its/src/test/resources/modulepath-whitebox/src/main/java/module-info.java new file mode 100644 index 0000000000..ee83e429b9 --- /dev/null +++ b/surefire-its/src/test/resources/modulepath-whitebox/src/main/java/module-info.java @@ -0,0 +1,3 @@ +module com.example { + exports com.example; +} diff --git a/surefire-its/src/test/resources/modulepath-whitebox/src/test/java/com/example/CalculatorTest.java b/surefire-its/src/test/resources/modulepath-whitebox/src/test/java/com/example/CalculatorTest.java new file mode 100644 index 0000000000..34952a8bed --- /dev/null +++ b/surefire-its/src/test/resources/modulepath-whitebox/src/test/java/com/example/CalculatorTest.java @@ -0,0 +1,12 @@ +package com.example; + +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; + +class CalculatorTest { + @Test + void testAdd() { + Calculator calc = new Calculator(); + assertEquals(42L, calc.add(21L, 21L)); + } +} diff --git a/surefire-its/src/test/resources/modulepath-whitebox/src/test/java/com/example/internal/MathHelperWhiteboxTest.java b/surefire-its/src/test/resources/modulepath-whitebox/src/test/java/com/example/internal/MathHelperWhiteboxTest.java new file mode 100644 index 0000000000..69b5ea32a1 --- /dev/null +++ b/surefire-its/src/test/resources/modulepath-whitebox/src/test/java/com/example/internal/MathHelperWhiteboxTest.java @@ -0,0 +1,22 @@ +package com.example.internal; + +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Whitebox test that accesses the non-exported internal package. + * This works because surefire patches test classes into the module via --patch-module. + */ +class MathHelperWhiteboxTest { + @Test + void testAdd() { + assertEquals(0L, MathHelper.add(0L, 0L)); + assertEquals(42L, MathHelper.add(21L, 21L)); + } + + @Test + void testMultiply() { + assertEquals(0L, MathHelper.multiply(0L, 1L)); + assertEquals(42L, MathHelper.multiply(2L, 21L)); + } +} diff --git a/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/pom.xml b/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/pom.xml new file mode 100644 index 0000000000..287f002b6f --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/pom.xml @@ -0,0 +1,57 @@ + + + 4.1.0 + + com.example + modulepath-whitebox-msh + 1.0.0-SNAPSHOT + + + ${java.specification.version} + UTF-8 + + + + + org.junit.jupiter + junit-jupiter-api + 5.9.1 + test + + + org.junit.jupiter + junit-jupiter-engine + 5.9.1 + test + + + + + + + com.example + + + com.example + test + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 4.0.0-beta-4 + + + org.apache.maven.plugins + maven-surefire-plugin + ${surefire.version} + + + + diff --git a/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/main/java/com/example/Calculator.java b/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/main/java/com/example/Calculator.java new file mode 100644 index 0000000000..42827959c8 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/main/java/com/example/Calculator.java @@ -0,0 +1,9 @@ +package com.example; + +import com.example.internal.MathHelper; + +public class Calculator { + public long add(long a, long b) { + return MathHelper.add(a, b); + } +} diff --git a/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/main/java/com/example/internal/MathHelper.java b/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/main/java/com/example/internal/MathHelper.java new file mode 100644 index 0000000000..90c0416310 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/main/java/com/example/internal/MathHelper.java @@ -0,0 +1,11 @@ +package com.example.internal; + +public class MathHelper { + public static long add(long x, long y) { + return x + y; + } + + public static long multiply(long x, long y) { + return x * y; + } +} diff --git a/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/main/java/module-info.java b/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/main/java/module-info.java new file mode 100644 index 0000000000..ee83e429b9 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/main/java/module-info.java @@ -0,0 +1,3 @@ +module com.example { + exports com.example; +} diff --git a/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/test/java/com/example/CalculatorTest.java b/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/test/java/com/example/CalculatorTest.java new file mode 100644 index 0000000000..34952a8bed --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/test/java/com/example/CalculatorTest.java @@ -0,0 +1,12 @@ +package com.example; + +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; + +class CalculatorTest { + @Test + void testAdd() { + Calculator calc = new Calculator(); + assertEquals(42L, calc.add(21L, 21L)); + } +} diff --git a/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/test/java/com/example/internal/MathHelperWhiteboxTest.java b/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/test/java/com/example/internal/MathHelperWhiteboxTest.java new file mode 100644 index 0000000000..62a9c9b471 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/test/java/com/example/internal/MathHelperWhiteboxTest.java @@ -0,0 +1,22 @@ +package com.example.internal; + +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Whitebox test accessing the non-exported internal package. + * Works via module-info-patch.maven and Maven 4 Module Source Hierarchy. + */ +class MathHelperWhiteboxTest { + @Test + void testAdd() { + assertEquals(0L, MathHelper.add(0L, 0L)); + assertEquals(42L, MathHelper.add(21L, 21L)); + } + + @Test + void testMultiply() { + assertEquals(0L, MathHelper.multiply(0L, 1L)); + assertEquals(42L, MathHelper.multiply(2L, 21L)); + } +} diff --git a/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/test/java/module-info-patch.maven b/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/test/java/module-info-patch.maven new file mode 100644 index 0000000000..5ab8ba0ffa --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3345-module-source-hierarchy/src/com.example/test/java/module-info-patch.maven @@ -0,0 +1,9 @@ +/* + * Whitebox test patch for com.example module. + * Allows test code to access the non-exported com.example.internal package. + */ +patch-module com.example { + add-modules TEST-MODULE-PATH; + add-reads TEST-MODULE-PATH; + add-exports com.example.internal to ALL-UNNAMED; +} From 7b78650584217e023b72c823abe955e0a6ea3ba3 Mon Sep 17 00:00:00 2001 From: Gerd Aschemann Date: Sun, 19 Jul 2026 09:29:15 +0200 Subject: [PATCH 03/10] Clarify patch-args handling, sort module dirs Address Copilot review feedback on #3392: - Reword the inline comment and the appendModuleInfoPatchArgs Javadoc: the method forwards ALL directives from module-info-patch.args except --add-reads/--add-modules, not only --add-exports. - Sort the nested module directory candidates so the picked module no longer depends on filesystem iteration order. Co-Authored-By: Claude Fable 5 --- .../plugin/surefire/AbstractSurefireMojo.java | 3 +++ .../ModularClasspathForkConfiguration.java | 15 +++++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java index 6ee49a06de..d49c2f89ab 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java @@ -28,6 +28,7 @@ import java.nio.file.Files; import java.text.ChoiceFormat; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Enumeration; @@ -1504,6 +1505,8 @@ private static boolean isNestedModuleLayout(File buildPath) { private static File findNestedModuleDescriptor(File buildPath) { File[] subdirs = buildPath.listFiles(File::isDirectory); if (subdirs != null) { + // deterministic pick independent of filesystem iteration order + Arrays.sort(subdirs); for (File subdir : subdirs) { if (new File(subdir, "module-info.class").exists()) { return subdir; diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java index 824103a588..8b8000cae3 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java @@ -189,7 +189,8 @@ File createArgsFile( // Check for module-info-patch.args generated by maven-compiler-plugin 4.x File patchArgs = findModuleInfoPatchArgs(patchFile); if (patchArgs != null) { - // Use --add-exports directives from module-info-patch.args + // Merge the file's directives, except --add-reads/--add-modules + // which surefire manages itself (see appendModuleInfoPatchArgs) appendModuleInfoPatchArgs(args, patchArgs, moduleName); } @@ -267,14 +268,16 @@ private static File findModuleInfoPatchArgs(File patchFile) { } /** - * Reads the module-info-patch.args file and appends its --add-exports and --add-opens - * directives to the args builder. The --add-reads directive is handled by surefire itself - * (always adds ALL-UNNAMED) to avoid referencing modules that may be on the classpath - * rather than the module-path. + * Reads the module-info-patch.args file and appends its directives (e.g. --add-exports, + * --add-opens) to the args builder. Exceptions are --add-reads and --add-modules, which + * are skipped: they may reference named modules that surefire places on the classpath + * rather than the module path. Surefire appends its own + * {@code --add-reads =ALL-UNNAMED} instead, and {@code --add-modules + * ALL-MODULE-PATH} is generated by the caller. * * @param args the args builder to append to * @param patchArgs the module-info-patch.args file - * @param moduleName the module name for the fallback --add-reads + * @param moduleName the module name for surefire's own --add-reads * @throws IOException if the file cannot be read */ private static void appendModuleInfoPatchArgs(StringBuilder args, File patchArgs, String moduleName) From 8dd1792bfe69535c5ec72cba945e706aa2250564 Mon Sep 17 00:00:00 2001 From: Gerd Aschemann Date: Sun, 19 Jul 2026 11:06:39 +0200 Subject: [PATCH 04/10] Read module-info-patch.args as UTF-8 The compiler-generated handoff file is UTF-8; FileReader uses the platform default charset and could misparse non-ASCII module or package names. Addresses Copilot review feedback on #3394. Co-Authored-By: Claude Fable 5 --- .../booterclient/ModularClasspathForkConfiguration.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java index 8b8000cae3..30a6f29a34 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java @@ -24,9 +24,10 @@ import java.io.BufferedReader; import java.io.File; -import java.io.FileReader; import java.io.FileWriter; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.util.Collection; import java.util.Iterator; import java.util.List; @@ -282,7 +283,8 @@ private static File findModuleInfoPatchArgs(File patchFile) { */ private static void appendModuleInfoPatchArgs(StringBuilder args, File patchArgs, String moduleName) throws IOException { - try (BufferedReader reader = new BufferedReader(new FileReader(patchArgs))) { + // explicit charset: the compiler writes the args file as UTF-8, the platform default may differ + try (BufferedReader reader = Files.newBufferedReader(patchArgs.toPath(), StandardCharsets.UTF_8)) { String line; while ((line = reader.readLine()) != null) { line = line.trim(); From 9c51a44de060604ce1b8a14ff686d894c83b1a68 Mon Sep 17 00:00:00 2001 From: Gerd Aschemann Date: Mon, 20 Jul 2026 13:11:22 +0200 Subject: [PATCH 05/10] Polish patch-args handling after review Address desruisseaux's review comments on #3392: - Javadoc for createArgsFile, documenting in particular the patchFile parameter (test output directory, or its per-module subdirectory for a Maven 4 module source hierarchy build). - Correct the findModuleInfoPatchArgs documentation: there is exactly one module-info-patch.args per project, at target/test-classes/META-INF/maven/; the parent lookup exists because the patch directory is the nested per-module directory in a module source hierarchy build. - Drop the explicit charset: Files.newBufferedReader(Path) already reads UTF-8. Co-Authored-By: Claude Fable 5 --- .../ModularClasspathForkConfiguration.java | 38 ++++++++++++++----- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java index 30a6f29a34..45429339c8 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java @@ -26,7 +26,6 @@ import java.io.File; import java.io.FileWriter; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.util.Collection; import java.util.Iterator; @@ -130,6 +129,23 @@ protected void resolveClasspath( } } + /** + * Writes the Java argument file for the forked JVM: module path, classpath, + * {@code --patch-module} for the module under test and the related module options. + * + * @param moduleName the name of the module under test (from the main module descriptor) + * @param modulePath the module path elements + * @param classPath the classpath elements + * @param packages the test packages to open for reflective access + * @param patchFile the directory with the compiled test classes patched into the module + * under test: the test output directory itself, or one of its per-module + * subdirectories for a Maven 4 module source hierarchy build + * @param startClassName the fork's main class + * @param isMainDescriptor whether the module descriptor stems from the main build output + * @param providerJpmsArguments additional Java Modules arguments required by the provider + * @return the created argument file + * @throws IOException if the file cannot be written + */ @Nonnull File createArgsFile( @Nonnull String moduleName, @@ -242,22 +258,24 @@ File createArgsFile( } /** - * Searches for module-info-patch.args generated by maven-compiler-plugin 4.x. - * The file is expected at {@code target/test-classes/META-INF/maven/module-info-patch.args} - * or within a module subdirectory at - * {@code target/test-classes//META-INF/maven/module-info-patch.args}. + * Locates the module-info-patch.args file generated by maven-compiler-plugin 4.x. + * There is exactly one such file per project, at + * {@code target/test-classes/META-INF/maven/module-info-patch.args}. The patch + * directory passed in is either the test output directory itself (classic layout) or + * one of its per-module subdirectories (Maven 4 module source hierarchy), which is why + * the file is looked up in the given directory and in its parent. * - * @param patchFile the test classes directory (may be the module subdirectory) + * @param patchFile the patch directory: the test output directory or a nested module directory * @return the module-info-patch.args file, or null if not found */ @Nullable private static File findModuleInfoPatchArgs(File patchFile) { - // Direct location: target/test-classes//META-INF/maven/module-info-patch.args + // classic layout: patchFile == target/test-classes File argsFile = new File(patchFile, "META-INF/maven/module-info-patch.args"); if (argsFile.isFile()) { return argsFile; } - // Parent location: target/test-classes/META-INF/maven/module-info-patch.args + // module source hierarchy: patchFile == target/test-classes/ File parent = patchFile.getParentFile(); if (parent != null) { argsFile = new File(parent, "META-INF/maven/module-info-patch.args"); @@ -283,8 +301,8 @@ private static File findModuleInfoPatchArgs(File patchFile) { */ private static void appendModuleInfoPatchArgs(StringBuilder args, File patchArgs, String moduleName) throws IOException { - // explicit charset: the compiler writes the args file as UTF-8, the platform default may differ - try (BufferedReader reader = Files.newBufferedReader(patchArgs.toPath(), StandardCharsets.UTF_8)) { + // Files.newBufferedReader(Path) reads UTF-8, matching the compiler-written file + try (BufferedReader reader = Files.newBufferedReader(patchArgs.toPath())) { String line; while ((line = reader.readLine()) != null) { line = line.trim(); From f9d59d5a16ac8c349535398b96ffe191852f710f Mon Sep 17 00:00:00 2001 From: Gerd Aschemann Date: Sat, 18 Jul 2026 15:15:09 +0200 Subject: [PATCH 06/10] Support multiple modules per POM (#3393) A Maven 4 module source hierarchy build may declare several Java modules in one POM; the single-module support from #3345 detected only the first nested module, scanned only its tests, and decided classpath-vs-module-path dependency placement with one descriptor, so the fork died at the Java Modules boot layer when a sibling module required a dependency that stayed on the classpath. - 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 scan over every nested target/test-classes// directory. - The classpath/module-path split is the union over all module descriptors: an element required on the module path by ANY module goes there, since 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 change to the surefire-booter API or its serialization. Covered by a unit test for the multi-descriptor detection and the IT Surefire3393MultiModuleSourceHierarchyIT: two modules in one POM (com.example.extra requires com.example.core, core requires transitive jakarta.json to cover the boot-layer failure), whitebox tests in both modules, five tests in one execution. Co-Authored-By: Claude Fable 5 --- .../plugin/surefire/AbstractSurefireMojo.java | 261 +++++++++++++++--- .../surefire/ResolvePathResultWrapper.java | 25 ++ .../AbstractSurefireMojoJava7PlusTest.java | 28 ++ ...efire3393MultiModuleSourceHierarchyIT.java | 70 +++++ .../pom.xml | 69 +++++ .../java/com/example/core/Calculator.java | 14 + .../com/example/core/internal/MathHelper.java | 9 + .../main/java/module-info.java | 5 + .../java/com/example/core/CalculatorTest.java | 19 ++ .../core/internal/MathHelperWhiteboxTest.java | 16 ++ .../test/java/module-info-patch.maven | 9 + .../main/java/com/example/extra/Doubler.java | 11 + .../example/extra/internal/TwiceHelper.java | 9 + .../main/java/module-info.java | 5 + .../java/com/example/extra/DoublerTest.java | 12 + .../internal/TwiceHelperWhiteboxTest.java | 16 ++ .../test/java/module-info-patch.maven | 9 + 17 files changed, 556 insertions(+), 31 deletions(-) create mode 100644 surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire3393MultiModuleSourceHierarchyIT.java create mode 100644 surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/pom.xml create mode 100644 surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/main/java/com/example/core/Calculator.java create mode 100644 surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/main/java/com/example/core/internal/MathHelper.java create mode 100644 surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/main/java/module-info.java create mode 100644 surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/test/java/com/example/core/CalculatorTest.java create mode 100644 surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/test/java/com/example/core/internal/MathHelperWhiteboxTest.java create mode 100644 surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/test/java/module-info-patch.maven create mode 100644 surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/main/java/com/example/extra/Doubler.java create mode 100644 surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/main/java/com/example/extra/internal/TwiceHelper.java create mode 100644 surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/main/java/module-info.java create mode 100644 surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/test/java/com/example/extra/DoublerTest.java create mode 100644 surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/test/java/com/example/extra/internal/TwiceHelperWhiteboxTest.java create mode 100644 surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/test/java/module-info-patch.maven diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java index d49c2f89ab..609068b2e2 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java @@ -1068,18 +1068,35 @@ private DefaultScanResult scanDirectories() throws MojoFailureException { File scanDir = getTestClassesDirectory(); // Maven 4 Module Source Hierarchy: test classes may be nested under / if (scanDir != null && isNestedModuleLayout(getMainBuildPath())) { - File nestedModule = findNestedModuleDescriptor(getMainBuildPath()); - if (nestedModule != null) { - File nestedTestDir = new File(scanDir, nestedModule.getName()); - if (nestedTestDir.isDirectory()) { - scanDir = nestedTestDir; - } + DefaultScanResult nestedResult = scanNestedModuleTestDirectories(scanDir); + if (nestedResult != null) { + return nestedResult; } } DirectoryScanner scanner = new DirectoryScanner(scanDir, getIncludedAndExcludedTests()); return scanner.scan(); } + /** + * Scans each nested {@code target/test-classes//} directory of a Maven 4 + * module source hierarchy build and unions the results. + * + * @param scanDir the test classes directory + * @return the union of all nested scans, or null if no nested test directory exists + * @throws MojoFailureException if the include/exclude filter is malformed + */ + private DefaultScanResult scanNestedModuleTestDirectories(File scanDir) throws MojoFailureException { + DefaultScanResult nestedResult = null; + for (File nestedModule : findNestedModuleDescriptors(getMainBuildPath())) { + File nestedTestDir = new File(scanDir, nestedModule.getName()); + if (nestedTestDir.isDirectory()) { + DefaultScanResult scan = new DirectoryScanner(nestedTestDir, getIncludedAndExcludedTests()).scan(); + nestedResult = nestedResult == null ? scan : nestedResult.append(scan); + } + } + return nestedResult; + } + List getProjectTestArtifacts() { return project.getTestArtifacts(); } @@ -1457,9 +1474,9 @@ private ResolvePathResultWrapper findModuleDescriptor(File jdkHome, File buildPa // Maven 4 Module Source Hierarchy: classes may be in target/classes// if (!isJpmsModule && buildPath.isDirectory()) { - File nestedModuleDir = findNestedModuleDescriptor(buildPath); - if (nestedModuleDir != null) { - return findModuleDescriptor(jdkHome, nestedModuleDir, isMainDescriptor); + List nestedModuleDirs = findNestedModuleDescriptors(buildPath); + if (!nestedModuleDirs.isEmpty()) { + return resolveNestedModuleDescriptors(jdkHome, nestedModuleDirs, isMainDescriptor); } } @@ -1467,16 +1484,67 @@ private ResolvePathResultWrapper findModuleDescriptor(File jdkHome, File buildPa return new ResolvePathResultWrapper(null, isMainDescriptor); } + ResolvePathResult result = resolveModuleDescriptor(jdkHome, buildPath); + return new ResolvePathResultWrapper(result, isMainDescriptor); + } + + /** + * Resolves the descriptors of all modules of a Maven 4 module source hierarchy build. + * The primary module drives test scanning and {@code --patch-module}, so a module + * with a nested test output directory is preferred; the remaining modules travel as + * additional results in the wrapper. + * + * @param jdkHome the JDK to parse module descriptors with + * @param nestedModuleDirs the nested module directories, each containing a module-info.class + * @param isMainDescriptor whether the descriptors stem from the main build output + * @return wrapper with the primary descriptor and all sibling descriptors + */ + private ResolvePathResultWrapper resolveNestedModuleDescriptors( + File jdkHome, List nestedModuleDirs, boolean isMainDescriptor) { + ResolvePathResult primary = null; + List additional = new ArrayList<>(); + for (File moduleDir : orderModulesWithTestsFirst(nestedModuleDirs)) { + ResolvePathResult result = resolveModuleDescriptor(jdkHome, moduleDir); + if (result != null) { + if (primary == null) { + primary = result; + } else { + additional.add(result); + } + } + } + return new ResolvePathResultWrapper(primary, isMainDescriptor, additional); + } + + private List orderModulesWithTestsFirst(List moduleDirs) { + List ordered = new ArrayList<>(moduleDirs.size()); + List withoutTests = new ArrayList<>(); + for (File moduleDir : moduleDirs) { + if (hasNestedTestDirectory(moduleDir)) { + ordered.add(moduleDir); + } else { + withoutTests.add(moduleDir); + } + } + ordered.addAll(withoutTests); + return ordered; + } + + private ResolvePathResult resolveModuleDescriptor(File jdkHome, File buildPath) { try { ResolvePathRequest request = ResolvePathRequest.ofFile(buildPath).setJdkHome(jdkHome); ResolvePathResult result = getLocationManager().resolvePath(request); - boolean isEmpty = result.getModuleNameSource() == null; - return new ResolvePathResultWrapper(isEmpty ? null : result, isMainDescriptor); + return result.getModuleNameSource() == null ? null : result; } catch (Exception e) { - return new ResolvePathResultWrapper(null, isMainDescriptor); + return null; } } + private boolean hasNestedTestDirectory(File moduleDir) { + File testClassesDir = getTestClassesDirectory(); + return testClassesDir != null && new File(testClassesDir, moduleDir.getName()).isDirectory(); + } + /** * Whether the main build output uses the Maven 4 Module Source Hierarchy layout: * no module-info.class at the root of the build output directory, but at least one @@ -1503,17 +1571,31 @@ private static boolean isNestedModuleLayout(File buildPath) { * @return the subdirectory containing module-info.class, or null if not found */ private static File findNestedModuleDescriptor(File buildPath) { + List moduleDirs = findNestedModuleDescriptors(buildPath); + return moduleDirs.isEmpty() ? null : moduleDirs.get(0); + } + + /** + * Searches for all immediate subdirectories of the given directory containing a + * module-info.class. A Maven 4 Module Source Hierarchy build may compile several + * Java modules into one build output directory, one subdirectory per module. + * + * @param buildPath the build output directory (e.g., target/classes) + * @return the subdirectories containing a module-info.class, sorted by name, may be empty + */ + private static List findNestedModuleDescriptors(File buildPath) { + List moduleDirs = new ArrayList<>(); File[] subdirs = buildPath.listFiles(File::isDirectory); if (subdirs != null) { - // deterministic pick independent of filesystem iteration order + // deterministic order independent of filesystem iteration order Arrays.sort(subdirs); for (File subdir : subdirs) { if (new File(subdir, "module-info.class").exists()) { - return subdir; + moduleDirs.add(subdir); } } } - return null; + return moduleDirs; } private static boolean isModule(File jar) { @@ -2056,23 +2138,21 @@ private StartupConfiguration newStartupConfigWithModularPath( final ProviderRequirements providerRequirements; final Classpath testModulepath; + List additionalModules = moduleDescriptor.getAdditionalResults(); if (isMainDescriptor) { providerRequirements = new ProviderRequirements(true, true, false); - ResolvePathsRequest req = ResolvePathsRequest.ofStrings(testClasspath.getClassPath()) - .setIncludeAllProviders(true) - .setJdkHome(javaHome) - .setIncludeStatic(true) - .setModuleDescriptor(javaModuleDescriptor); - - ResolvePathsResult result = getLocationManager().resolvePaths(req); - for (Entry entry : result.getPathExceptions().entrySet()) { - // Probably JDK version < 9. Other known causes: passing a non-jar or a corrupted jar. - getConsoleLogger().warning("Exception for '" + entry.getKey() + "'.", entry.getValue()); + if (additionalModules.isEmpty()) { + ResolvePathsResult result = + resolveTestClasspath(testClasspath.getClassPath(), javaModuleDescriptor, javaHome); + testClasspath = new Classpath(result.getClasspathElements()); + testModulepath = new Classpath(result.getModulepathElements().keySet()); + } else { + ModulePathSplit split = resolveModulePathSplitForAllModules( + testClasspath.getClassPath(), javaModuleDescriptor, additionalModules, javaHome); + testClasspath = split.classpath; + testModulepath = split.modulepath; } - testClasspath = new Classpath(result.getClasspathElements()); - testModulepath = new Classpath(result.getModulepathElements().keySet()); - for (String className : scanResult.getClasses()) { packages.add(substringBeforeLast(className, ".")); } @@ -2085,17 +2165,28 @@ private StartupConfiguration newStartupConfigWithModularPath( getConsoleLogger().debug("main module descriptor name: " + javaModuleDescriptor.name()); File patchFile = null; + List additionalModuleArgs = new ArrayList<>(); if (isMainDescriptor) { File testDir = getTestClassesDirectory(); + boolean nestedLayout = false; if (testDir != null && testDir.isDirectory() && isNestedModuleLayout(getMainBuildPath())) { // Maven 4 Module Source Hierarchy: test classes nested under /. // Only the main output layout decides — a test-classes subdirectory merely // sharing the module name (module named after its root package) must not. File nestedTestDir = new File(testDir, javaModuleDescriptor.name()); - patchFile = nestedTestDir.isDirectory() ? nestedTestDir : testDir; + nestedLayout = nestedTestDir.isDirectory(); + patchFile = nestedLayout ? nestedTestDir : testDir; } else { patchFile = testDir; } + + if (nestedLayout) { + // The primary module should only open its own test packages, not those + // of sibling modules scanned from the other nested test directories. + packages.clear(); + packages.addAll(scanTestPackages(patchFile)); + additionalModuleArgs = createAdditionalModuleArgs(testDir, additionalModules); + } } ModularClasspath modularClasspath = new ModularClasspath( @@ -2131,9 +2222,117 @@ private StartupConfiguration newStartupConfigWithModularPath( getConsoleLogger().debug(inProcClasspath.getCompactLogMessage("in-process(compact) classpath:")); ProcessCheckerType processCheckerType = ProcessCheckerType.toEnum(getEnableProcessChecker()); - List jpmsArgs = providerInfo.getJpmsArguments(providerRequirements); + List javaModulesArgs = new ArrayList<>(providerInfo.getJpmsArguments(providerRequirements)); + javaModulesArgs.addAll(additionalModuleArgs); return new StartupConfiguration( - providerName, classpathConfiguration, classLoaderConfiguration, processCheckerType, jpmsArgs); + providerName, classpathConfiguration, classLoaderConfiguration, processCheckerType, javaModulesArgs); + } + + /** + * Splits the test classpath of a multi-module source hierarchy build into classpath + * and module path. The fork has a single boot layer, so an element required on the + * module path by ANY of the modules must end up there. + * + * @param testClasspathElements the unsplit test classpath elements + * @param primary the primary module descriptor + * @param additionalModules the sibling module descriptors + * @param javaHome the JDK used for path resolution + * @return the classpath/module-path split + * @throws IOException if the location manager fails to resolve the paths + */ + private ModulePathSplit resolveModulePathSplitForAllModules( + List testClasspathElements, + JavaModuleDescriptor primary, + List additionalModules, + String javaHome) + throws IOException { + Set modulepathElements = new LinkedHashSet<>(); + Set classpathElements = new LinkedHashSet<>(); + List allDescriptors = new ArrayList<>(); + allDescriptors.add(primary); + for (ResolvePathResult additional : additionalModules) { + allDescriptors.add(additional.getModuleDescriptor()); + } + for (JavaModuleDescriptor descriptor : allDescriptors) { + ResolvePathsResult result = resolveTestClasspath(testClasspathElements, descriptor, javaHome); + modulepathElements.addAll(result.getModulepathElements().keySet()); + classpathElements.addAll(result.getClasspathElements()); + } + classpathElements.removeAll(modulepathElements); + return new ModulePathSplit( + new Classpath(new ArrayList<>(classpathElements)), new Classpath(new ArrayList<>(modulepathElements))); + } + + /** + * Classpath and module path resulting from {@link #resolveModulePathSplitForAllModules}. + */ + private static final class ModulePathSplit { + private final Classpath classpath; + private final Classpath modulepath; + + private ModulePathSplit(Classpath classpath, Classpath modulepath) { + this.classpath = classpath; + this.modulepath = modulepath; + } + } + + /** + * Java Modules arguments patching each sibling module of a module source hierarchy + * build with its own test classes ({@code --patch-module}), letting it read the + * unnamed module ({@code --add-reads}, JUnit is on the classpath) and opening its + * test packages for reflection ({@code --add-opens}). Passed to the fork through the + * argument file via {@link StartupConfiguration#getJpmsArguments()}. + * + * @param testDir the test classes directory containing the nested per-module directories + * @param additionalModules the sibling module descriptors + * @return the Java Modules arguments, one option/value pair per entry + * @throws MojoExecutionException if scanning a nested test directory fails + */ + private List createAdditionalModuleArgs(File testDir, List additionalModules) + throws MojoExecutionException { + List args = new ArrayList<>(); + for (ResolvePathResult additional : additionalModules) { + String additionalName = additional.getModuleDescriptor().name(); + File additionalTestDir = new File(testDir, additionalName); + if (additionalTestDir.isDirectory()) { + String escapedPath = additionalTestDir.getPath().replace("\\", "\\\\"); + args.add(new String[] {"--patch-module", additionalName + "=\"" + escapedPath + "\""}); + args.add(new String[] {"--add-reads", additionalName + "=ALL-UNNAMED"}); + for (String pkg : scanTestPackages(additionalTestDir)) { + args.add(new String[] {"--add-opens", additionalName + "/" + pkg + "=ALL-UNNAMED"}); + } + } + } + return args; + } + + private ResolvePathsResult resolveTestClasspath( + List testClasspathElements, JavaModuleDescriptor descriptor, String javaHome) throws IOException { + ResolvePathsRequest req = ResolvePathsRequest.ofStrings(testClasspathElements) + .setIncludeAllProviders(true) + .setJdkHome(javaHome) + .setIncludeStatic(true) + .setModuleDescriptor(descriptor); + + ResolvePathsResult result = getLocationManager().resolvePaths(req); + for (Entry entry : result.getPathExceptions().entrySet()) { + // Probably JDK version < 9. Other known causes: passing a non-jar or a corrupted jar. + getConsoleLogger().warning("Exception for '" + entry.getKey() + "'.", entry.getValue()); + } + return result; + } + + private SortedSet scanTestPackages(File testClassesDir) throws MojoExecutionException { + SortedSet testPackages = new TreeSet<>(); + try { + DefaultScanResult scan = new DirectoryScanner(testClassesDir, getIncludedAndExcludedTests()).scan(); + for (String className : scan.getClasses()) { + testPackages.add(substringBeforeLast(className, ".")); + } + } catch (MojoFailureException e) { + throw new MojoExecutionException(e.getLocalizedMessage(), e); + } + return testPackages; } private Artifact getCommonArtifact() { diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/ResolvePathResultWrapper.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/ResolvePathResultWrapper.java index b13bdc7bb7..0f01096f37 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/ResolvePathResultWrapper.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/ResolvePathResultWrapper.java @@ -18,18 +18,32 @@ */ package org.apache.maven.plugin.surefire; +import java.util.List; + import org.codehaus.plexus.languages.java.jpms.ResolvePathResult; +import static java.util.Collections.emptyList; +import static java.util.Collections.unmodifiableList; + /** * Wraps {@link ResolvePathResult} and place marker. */ final class ResolvePathResultWrapper { private final ResolvePathResult resolvePathResult; private final boolean isMainModuleDescriptor; + private final List additionalResults; ResolvePathResultWrapper(ResolvePathResult resolvePathResult, boolean isMainModuleDescriptor) { + this(resolvePathResult, isMainModuleDescriptor, emptyList()); + } + + ResolvePathResultWrapper( + ResolvePathResult resolvePathResult, + boolean isMainModuleDescriptor, + List additionalResults) { this.resolvePathResult = resolvePathResult; this.isMainModuleDescriptor = isMainModuleDescriptor; + this.additionalResults = additionalResults; } ResolvePathResult getResolvePathResult() { @@ -42,4 +56,15 @@ ResolvePathResult getResolvePathResult() { boolean isMainModuleDescriptor() { return isMainModuleDescriptor; } + + /** + * Descriptors of further Java modules beyond the primary one, present when a Maven 4 + * module source hierarchy build produces several modules under one build output directory + * ({@code target/classes//}). + * + * @return additional module descriptors, empty for single-module or flat layouts + */ + List getAdditionalResults() { + return unmodifiableList(additionalResults); + } } diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java index 5b7fbb23ea..e14063dd37 100644 --- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java +++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java @@ -577,6 +577,34 @@ public void shouldTreatModuleSourceHierarchyLayoutAsNested() throws Exception { } } + @Test + public void shouldFindAllNestedModuleDescriptors() throws Exception { + // target/classes/{com.example.one,com.example.two}/module-info.class plus a non-module dir + File tempDir = Files.createTempDirectory("surefire-test-nested-modules").toFile(); + try { + for (String module : new String[] {"com.example.two", "com.example.one"}) { + File moduleDir = new File(tempDir, module); + moduleDir.mkdirs(); + new File(moduleDir, "module-info.class").createNewFile(); + } + File plainDir = new File(tempDir, "META-INF"); + plainDir.mkdirs(); + + List result = invokeMethod(AbstractSurefireMojo.class, "findNestedModuleDescriptors", tempDir); + assertThat(result).hasSize(2); + // sorted by name for deterministic module ordering + assertThat(result.get(0).getName()).isEqualTo("com.example.one"); + assertThat(result.get(1).getName()).isEqualTo("com.example.two"); + } finally { + for (String module : new String[] {"com.example.one", "com.example.two"}) { + new File(new File(tempDir, module), "module-info.class").delete(); + new File(tempDir, module).delete(); + } + new File(tempDir, "META-INF").delete(); + tempDir.delete(); + } + } + private static File mockFile(String absolutePath) { File f = mock(File.class); when(f.getAbsolutePath()).thenReturn(absolutePath); diff --git a/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire3393MultiModuleSourceHierarchyIT.java b/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire3393MultiModuleSourceHierarchyIT.java new file mode 100644 index 0000000000..f5a97c5197 --- /dev/null +++ b/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire3393MultiModuleSourceHierarchyIT.java @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.surefire.its.jiras; + +import java.io.File; + +import org.apache.maven.surefire.its.fixture.AbstractJava9PlusIT; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +/** + * Integration test for whitebox testing of SEVERAL Java modules declared in one POM + * using Maven 4 Module Source Hierarchy and module-info-patch.maven. + *

+ * This test requires Maven 4 and is skipped when running with Maven 3. + * It verifies that surefire correctly: + *

    + *
  • Detects all modules in nested target/classes/<module>/ directories
    • + *
  • Scans test classes from every nested target/test-classes/<module>/ directory
    • + *
  • Patches each module with its own test classes in a single execution
    • + *
  • Places dependencies required by ANY of the modules on the module path + * (here: jakarta.json, required by com.example.core only)
    • + *
+ */ +class Surefire3393MultiModuleSourceHierarchyIT extends AbstractJava9PlusIT { + + @Test + void testWhiteboxWithMultiModuleSourceHierarchy() { + assumeTrue(isMaven4Plus(), "This test requires Maven 4."); + // 5 tests: core CalculatorTest.testAdd + testKindOfJsonValue + MathHelperWhiteboxTest.testAdd + // extra DoublerTest.testDoubled + TwiceHelperWhiteboxTest.testTwice + assumeJava9().debugLogging().executeTest().verifyErrorFreeLog().assertTestSuiteResults(5); + } + + @Override + protected String getProjectDirectoryName() { + return "surefire-3393-multi-module-source-hierarchy"; + } + + private static boolean isMaven4Plus() { + String mavenHome = System.getProperty("maven.home"); + if (mavenHome == null) { + return false; + } + File mavenLib = new File(mavenHome, "lib"); + if (!mavenLib.isDirectory()) { + return false; + } + // Maven 4 ships maven-api-core; Maven 3 does not + File[] files = mavenLib.listFiles((dir, name) -> name.startsWith("maven-api-core-") && name.endsWith(".jar")); + return files != null && files.length > 0; + } +} diff --git a/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/pom.xml b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/pom.xml new file mode 100644 index 0000000000..72f07df944 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/pom.xml @@ -0,0 +1,69 @@ + + + 4.1.0 + + com.example + modulepath-whitebox-multi-msh + 1.0.0-SNAPSHOT + + + ${java.specification.version} + UTF-8 + + + + + jakarta.json + jakarta.json-api + 2.1.3 + + + org.junit.jupiter + junit-jupiter-api + 5.9.1 + test + + + org.junit.jupiter + junit-jupiter-engine + 5.9.1 + test + + + + + + + com.example.core + + + com.example.extra + + + com.example.core + test + + + com.example.extra + test + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 4.0.0-beta-4 + + + org.apache.maven.plugins + maven-surefire-plugin + ${surefire.version} + + + + diff --git a/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/main/java/com/example/core/Calculator.java b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/main/java/com/example/core/Calculator.java new file mode 100644 index 0000000000..e83e7d376e --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/main/java/com/example/core/Calculator.java @@ -0,0 +1,14 @@ +package com.example.core; + +import com.example.core.internal.MathHelper; +import jakarta.json.JsonValue; + +public class Calculator { + public long add(long a, long b) { + return MathHelper.add(a, b); + } + + public String kindOf(JsonValue value) { + return value.getValueType().name(); + } +} diff --git a/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/main/java/com/example/core/internal/MathHelper.java b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/main/java/com/example/core/internal/MathHelper.java new file mode 100644 index 0000000000..4fdf0652e3 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/main/java/com/example/core/internal/MathHelper.java @@ -0,0 +1,9 @@ +package com.example.core.internal; + +public final class MathHelper { + private MathHelper() {} + + public static long add(long a, long b) { + return a + b; + } +} diff --git a/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/main/java/module-info.java b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/main/java/module-info.java new file mode 100644 index 0000000000..17ee912db2 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/main/java/module-info.java @@ -0,0 +1,5 @@ +module com.example.core { + requires transitive jakarta.json; + + exports com.example.core; +} diff --git a/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/test/java/com/example/core/CalculatorTest.java b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/test/java/com/example/core/CalculatorTest.java new file mode 100644 index 0000000000..a426809ca3 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/test/java/com/example/core/CalculatorTest.java @@ -0,0 +1,19 @@ +package com.example.core; + +import jakarta.json.JsonValue; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class CalculatorTest { + @Test + void testAdd() { + assertEquals(42L, new Calculator().add(40L, 2L)); + } + + @Test + void testKindOfJsonValue() { + // exercises the jakarta.json module dependency on the module path + assertEquals("TRUE", new Calculator().kindOf(JsonValue.TRUE)); + } +} diff --git a/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/test/java/com/example/core/internal/MathHelperWhiteboxTest.java b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/test/java/com/example/core/internal/MathHelperWhiteboxTest.java new file mode 100644 index 0000000000..56c714d062 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/test/java/com/example/core/internal/MathHelperWhiteboxTest.java @@ -0,0 +1,16 @@ +package com.example.core.internal; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Whitebox test accessing the non-exported internal package of com.example.core. + */ +class MathHelperWhiteboxTest { + @Test + void testAdd() { + assertEquals(0L, MathHelper.add(0L, 0L)); + assertEquals(42L, MathHelper.add(21L, 21L)); + } +} diff --git a/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/test/java/module-info-patch.maven b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/test/java/module-info-patch.maven new file mode 100644 index 0000000000..2fd4c39790 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.core/test/java/module-info-patch.maven @@ -0,0 +1,9 @@ +/* + * Whitebox test patch for the com.example.core module. + * Allows test code to access the non-exported com.example.core.internal package. + */ +patch-module com.example.core { + add-modules TEST-MODULE-PATH; + add-reads TEST-MODULE-PATH; + add-exports com.example.core.internal to ALL-UNNAMED; +} diff --git a/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/main/java/com/example/extra/Doubler.java b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/main/java/com/example/extra/Doubler.java new file mode 100644 index 0000000000..712fbf5c26 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/main/java/com/example/extra/Doubler.java @@ -0,0 +1,11 @@ +package com.example.extra; + +import com.example.core.Calculator; +import com.example.extra.internal.TwiceHelper; + +public class Doubler { + public long doubled(long value) { + // cross-module call into com.example.core + return new Calculator().add(TwiceHelper.twice(value), 0L); + } +} diff --git a/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/main/java/com/example/extra/internal/TwiceHelper.java b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/main/java/com/example/extra/internal/TwiceHelper.java new file mode 100644 index 0000000000..e1aeb68617 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/main/java/com/example/extra/internal/TwiceHelper.java @@ -0,0 +1,9 @@ +package com.example.extra.internal; + +public final class TwiceHelper { + private TwiceHelper() {} + + public static long twice(long value) { + return 2L * value; + } +} diff --git a/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/main/java/module-info.java b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/main/java/module-info.java new file mode 100644 index 0000000000..d0e98a1bf6 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/main/java/module-info.java @@ -0,0 +1,5 @@ +module com.example.extra { + requires com.example.core; + + exports com.example.extra; +} diff --git a/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/test/java/com/example/extra/DoublerTest.java b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/test/java/com/example/extra/DoublerTest.java new file mode 100644 index 0000000000..3bbd5bd518 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/test/java/com/example/extra/DoublerTest.java @@ -0,0 +1,12 @@ +package com.example.extra; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class DoublerTest { + @Test + void testDoubled() { + assertEquals(42L, new Doubler().doubled(21L)); + } +} diff --git a/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/test/java/com/example/extra/internal/TwiceHelperWhiteboxTest.java b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/test/java/com/example/extra/internal/TwiceHelperWhiteboxTest.java new file mode 100644 index 0000000000..25d73e6c70 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/test/java/com/example/extra/internal/TwiceHelperWhiteboxTest.java @@ -0,0 +1,16 @@ +package com.example.extra.internal; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Whitebox test accessing the non-exported internal package of com.example.extra. + */ +class TwiceHelperWhiteboxTest { + @Test + void testTwice() { + assertEquals(0L, TwiceHelper.twice(0L)); + assertEquals(42L, TwiceHelper.twice(21L)); + } +} diff --git a/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/test/java/module-info-patch.maven b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/test/java/module-info-patch.maven new file mode 100644 index 0000000000..b0911eef04 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3393-multi-module-source-hierarchy/src/com.example.extra/test/java/module-info-patch.maven @@ -0,0 +1,9 @@ +/* + * Whitebox test patch for the com.example.extra module. + * Allows test code to access the non-exported com.example.extra.internal package. + */ +patch-module com.example.extra { + add-modules TEST-MODULE-PATH; + add-reads TEST-MODULE-PATH; + add-exports com.example.extra.internal to ALL-UNNAMED; +} From bef0da56dec78d65bac0609bd0308412971b72be Mon Sep 17 00:00:00 2001 From: Gerd Aschemann Date: Tue, 21 Jul 2026 11:23:46 +0200 Subject: [PATCH 07/10] Address review: dedupe module scan, use Path API Compute the nested module directories once and pass them down instead of the isNestedModuleLayout/findNestedModuleDescriptor double invocation, switch the module-info-patch.args helpers from File to java.nio.file.Path, and create test File objects before the try block so the finally block deletes them directly. Addresses review comments by @desruisseaux on #3392. Co-Authored-By: Claude Fable 5 --- .../plugin/surefire/AbstractSurefireMojo.java | 62 ++++------ .../ModularClasspathForkConfiguration.java | 24 ++-- .../AbstractSurefireMojoJava7PlusTest.java | 117 ++++++++---------- 3 files changed, 91 insertions(+), 112 deletions(-) diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java index 609068b2e2..13fc07e104 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java @@ -1067,8 +1067,9 @@ private DefaultScanResult scanForTestClasses() throws MojoFailureException { private DefaultScanResult scanDirectories() throws MojoFailureException { File scanDir = getTestClassesDirectory(); // Maven 4 Module Source Hierarchy: test classes may be nested under / - if (scanDir != null && isNestedModuleLayout(getMainBuildPath())) { - DefaultScanResult nestedResult = scanNestedModuleTestDirectories(scanDir); + if (scanDir != null) { + DefaultScanResult nestedResult = + scanNestedModuleTestDirectories(scanDir, nestedModuleDirectories(getMainBuildPath())); if (nestedResult != null) { return nestedResult; } @@ -1082,12 +1083,14 @@ private DefaultScanResult scanDirectories() throws MojoFailureException { * module source hierarchy build and unions the results. * * @param scanDir the test classes directory + * @param nestedModuleDirectories the per-module build output directories, may be empty * @return the union of all nested scans, or null if no nested test directory exists * @throws MojoFailureException if the include/exclude filter is malformed */ - private DefaultScanResult scanNestedModuleTestDirectories(File scanDir) throws MojoFailureException { + private DefaultScanResult scanNestedModuleTestDirectories(File scanDir, List nestedModuleDirectories) + throws MojoFailureException { DefaultScanResult nestedResult = null; - for (File nestedModule : findNestedModuleDescriptors(getMainBuildPath())) { + for (File nestedModule : nestedModuleDirectories) { File nestedTestDir = new File(scanDir, nestedModule.getName()); if (nestedTestDir.isDirectory()) { DefaultScanResult scan = new DirectoryScanner(nestedTestDir, getIncludedAndExcludedTests()).scan(); @@ -1474,9 +1477,9 @@ private ResolvePathResultWrapper findModuleDescriptor(File jdkHome, File buildPa // Maven 4 Module Source Hierarchy: classes may be in target/classes// if (!isJpmsModule && buildPath.isDirectory()) { - List nestedModuleDirs = findNestedModuleDescriptors(buildPath); - if (!nestedModuleDirs.isEmpty()) { - return resolveNestedModuleDescriptors(jdkHome, nestedModuleDirs, isMainDescriptor); + List nestedModuleDirectories = findNestedModuleDescriptors(buildPath); + if (!nestedModuleDirectories.isEmpty()) { + return resolveNestedModuleDescriptors(jdkHome, nestedModuleDirectories, isMainDescriptor); } } @@ -1495,15 +1498,15 @@ private ResolvePathResultWrapper findModuleDescriptor(File jdkHome, File buildPa * additional results in the wrapper. * * @param jdkHome the JDK to parse module descriptors with - * @param nestedModuleDirs the nested module directories, each containing a module-info.class + * @param nestedModuleDirectories the nested module directories, each containing a module-info.class * @param isMainDescriptor whether the descriptors stem from the main build output * @return wrapper with the primary descriptor and all sibling descriptors */ private ResolvePathResultWrapper resolveNestedModuleDescriptors( - File jdkHome, List nestedModuleDirs, boolean isMainDescriptor) { + File jdkHome, List nestedModuleDirectories, boolean isMainDescriptor) { ResolvePathResult primary = null; List additional = new ArrayList<>(); - for (File moduleDir : orderModulesWithTestsFirst(nestedModuleDirs)) { + for (File moduleDir : orderModulesWithTestsFirst(nestedModuleDirectories)) { ResolvePathResult result = resolveModuleDescriptor(jdkHome, moduleDir); if (result != null) { if (primary == null) { @@ -1546,33 +1549,20 @@ private boolean hasNestedTestDirectory(File moduleDir) { } /** - * Whether the main build output uses the Maven 4 Module Source Hierarchy layout: - * no module-info.class at the root of the build output directory, but at least one - * immediate subdirectory containing one ({@code target/classes//}). - * A classic modular build (module-info.class at the root) is never nested, even if - * a subdirectory happens to share the module's name. + * The per-module subdirectories of a Maven 4 Module Source Hierarchy build output + * ({@code target/classes//}, each containing a module-info.class), sorted by + * name. Empty for the classic layout: a module-info.class at the root wins, even if a + * subdirectory happens to share the module's name (module named after its root + * package), and a missing directory yields no modules either. * * @param buildPath the build output directory (e.g., target/classes) - * @return {@code true} for the nested module source hierarchy layout + * @return the nested module directories, may be empty */ - private static boolean isNestedModuleLayout(File buildPath) { - return buildPath != null - && buildPath.isDirectory() - && !new File(buildPath, "module-info.class").exists() - && findNestedModuleDescriptor(buildPath) != null; - } - - /** - * Searches for a module-info.class in immediate subdirectories of the given directory. - * This supports Maven 4 Module Source Hierarchy where compiled classes are placed - * in {@code target/classes//} instead of directly in {@code target/classes/}. - * - * @param buildPath the build output directory (e.g., target/classes) - * @return the subdirectory containing module-info.class, or null if not found - */ - private static File findNestedModuleDescriptor(File buildPath) { - List moduleDirs = findNestedModuleDescriptors(buildPath); - return moduleDirs.isEmpty() ? null : moduleDirs.get(0); + private static List nestedModuleDirectories(File buildPath) { + if (buildPath == null || !buildPath.isDirectory() || new File(buildPath, "module-info.class").exists()) { + return emptyList(); + } + return findNestedModuleDescriptors(buildPath); } /** @@ -2169,7 +2159,9 @@ private StartupConfiguration newStartupConfigWithModularPath( if (isMainDescriptor) { File testDir = getTestClassesDirectory(); boolean nestedLayout = false; - if (testDir != null && testDir.isDirectory() && isNestedModuleLayout(getMainBuildPath())) { + if (testDir != null + && testDir.isDirectory() + && !nestedModuleDirectories(getMainBuildPath()).isEmpty()) { // Maven 4 Module Source Hierarchy: test classes nested under /. // Only the main output layout decides — a test-classes subdirectory merely // sharing the module name (module named after its root package) must not. diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java index 45429339c8..21866f84b4 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java @@ -27,6 +27,7 @@ import java.io.FileWriter; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.Path; import java.util.Collection; import java.util.Iterator; import java.util.List; @@ -204,7 +205,7 @@ File createArgsFile( .append(NL); // Check for module-info-patch.args generated by maven-compiler-plugin 4.x - File patchArgs = findModuleInfoPatchArgs(patchFile); + Path patchArgs = findModuleInfoPatchArgs(patchFile); if (patchArgs != null) { // Merge the file's directives, except --add-reads/--add-modules // which surefire manages itself (see appendModuleInfoPatchArgs) @@ -269,17 +270,18 @@ File createArgsFile( * @return the module-info-patch.args file, or null if not found */ @Nullable - private static File findModuleInfoPatchArgs(File patchFile) { - // classic layout: patchFile == target/test-classes - File argsFile = new File(patchFile, "META-INF/maven/module-info-patch.args"); - if (argsFile.isFile()) { + private static Path findModuleInfoPatchArgs(File patchFile) { + Path patchDir = patchFile.toPath(); + // classic layout: patchDir == target/test-classes + Path argsFile = patchDir.resolve("META-INF/maven/module-info-patch.args"); + if (Files.isRegularFile(argsFile)) { return argsFile; } - // module source hierarchy: patchFile == target/test-classes/ - File parent = patchFile.getParentFile(); + // module source hierarchy: patchDir == target/test-classes/ + Path parent = patchDir.getParent(); if (parent != null) { - argsFile = new File(parent, "META-INF/maven/module-info-patch.args"); - if (argsFile.isFile()) { + argsFile = parent.resolve("META-INF/maven/module-info-patch.args"); + if (Files.isRegularFile(argsFile)) { return argsFile; } } @@ -299,10 +301,10 @@ private static File findModuleInfoPatchArgs(File patchFile) { * @param moduleName the module name for surefire's own --add-reads * @throws IOException if the file cannot be read */ - private static void appendModuleInfoPatchArgs(StringBuilder args, File patchArgs, String moduleName) + private static void appendModuleInfoPatchArgs(StringBuilder args, Path patchArgs, String moduleName) throws IOException { // Files.newBufferedReader(Path) reads UTF-8, matching the compiler-written file - try (BufferedReader reader = Files.newBufferedReader(patchArgs.toPath())) { + try (BufferedReader reader = Files.newBufferedReader(patchArgs)) { String line; while ((line = reader.readLine()) != null) { line = line.trim(); diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java index e14063dd37..e60e9313c0 100644 --- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java +++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java @@ -488,50 +488,60 @@ public void shouldJoinStrings() throws Exception { } @Test - public void shouldFindNestedModuleDescriptor() throws Exception { - // Create a temp directory structure: target/classes/com.example/module-info.class - File tempDir = Files.createTempDirectory("surefire-test-nested-module").toFile(); + public void shouldFindNestedModuleDescriptors() throws Exception { + // target/classes/{com.example.one,com.example.two}/module-info.class plus a non-module dir + File tempDir = Files.createTempDirectory("surefire-test-nested-modules").toFile(); + File moduleOne = new File(tempDir, "com.example.one"); + File moduleTwo = new File(tempDir, "com.example.two"); + File descriptorOne = new File(moduleOne, "module-info.class"); + File descriptorTwo = new File(moduleTwo, "module-info.class"); + File plainDir = new File(tempDir, "META-INF"); try { - File moduleDir = new File(tempDir, "com.example"); - moduleDir.mkdirs(); - new File(moduleDir, "module-info.class").createNewFile(); + moduleOne.mkdirs(); + moduleTwo.mkdirs(); + descriptorOne.createNewFile(); + descriptorTwo.createNewFile(); + plainDir.mkdirs(); - File result = invokeMethod(AbstractSurefireMojo.class, "findNestedModuleDescriptor", tempDir); - assertThat(result).isNotNull(); - assertThat(result.getName()).isEqualTo("com.example"); + List result = invokeMethod(AbstractSurefireMojo.class, "findNestedModuleDescriptors", tempDir); + // sorted by name for deterministic module ordering + assertThat(result).extracting(File::getName).containsExactly("com.example.one", "com.example.two"); } finally { - // Cleanup - new File(new File(tempDir, "com.example"), "module-info.class").delete(); - new File(tempDir, "com.example").delete(); + descriptorOne.delete(); + descriptorTwo.delete(); + moduleOne.delete(); + moduleTwo.delete(); + plainDir.delete(); tempDir.delete(); } } @Test - public void shouldNotFindNestedModuleDescriptorInFlatLayout() throws Exception { - // Create a temp directory structure without nested module-info.class + public void shouldFindNoNestedModuleDescriptorsInFlatLayout() throws Exception { + // plain package directories without module-info.class must not count as modules File tempDir = Files.createTempDirectory("surefire-test-flat").toFile(); + File pkgDir = new File(tempDir, "com/example"); + File classFile = new File(pkgDir, "Foo.class"); try { - File pkgDir = new File(tempDir, "com/example"); pkgDir.mkdirs(); - new File(pkgDir, "Foo.class").createNewFile(); + classFile.createNewFile(); - File result = invokeMethod(AbstractSurefireMojo.class, "findNestedModuleDescriptor", tempDir); - assertThat(result).isNull(); + List result = invokeMethod(AbstractSurefireMojo.class, "findNestedModuleDescriptors", tempDir); + assertThat(result).isEmpty(); } finally { - new File(new File(tempDir, "com/example"), "Foo.class").delete(); - new File(tempDir, "com/example").delete(); - new File(tempDir, "com").delete(); + classFile.delete(); + pkgDir.delete(); + pkgDir.getParentFile().delete(); tempDir.delete(); } } @Test - public void shouldReturnNullForEmptyDirectory() throws Exception { + public void shouldFindNoNestedModuleDescriptorsInEmptyDirectory() throws Exception { File tempDir = Files.createTempDirectory("surefire-test-empty").toFile(); try { - File result = invokeMethod(AbstractSurefireMojo.class, "findNestedModuleDescriptor", tempDir); - assertThat(result).isNull(); + List result = invokeMethod(AbstractSurefireMojo.class, "findNestedModuleDescriptors", tempDir); + assertThat(result).isEmpty(); } finally { tempDir.delete(); } @@ -544,18 +554,20 @@ public void shouldNotTreatClassicModularLayoutAsNested() throws Exception { // directory sharing the module name must NOT switch surefire to the nested layout. File tempDir = Files.createTempDirectory("surefire-test-classic-modular").toFile(); + File rootDescriptor = new File(tempDir, "module-info.class"); + File pkgDir = new File(tempDir, "it"); + File classFile = new File(pkgDir, "Main.class"); try { - new File(tempDir, "module-info.class").createNewFile(); - File pkgDir = new File(tempDir, "it"); + rootDescriptor.createNewFile(); pkgDir.mkdirs(); - new File(pkgDir, "Main.class").createNewFile(); + classFile.createNewFile(); - boolean result = invokeMethod(AbstractSurefireMojo.class, "isNestedModuleLayout", tempDir); - assertThat(result).isFalse(); + List result = invokeMethod(AbstractSurefireMojo.class, "nestedModuleDirectories", tempDir); + assertThat(result).isEmpty(); } finally { - new File(new File(tempDir, "it"), "Main.class").delete(); - new File(tempDir, "it").delete(); - new File(tempDir, "module-info.class").delete(); + classFile.delete(); + pkgDir.delete(); + rootDescriptor.delete(); tempDir.delete(); } } @@ -563,44 +575,17 @@ public void shouldNotTreatClassicModularLayoutAsNested() throws Exception { @Test public void shouldTreatModuleSourceHierarchyLayoutAsNested() throws Exception { File tempDir = Files.createTempDirectory("surefire-test-nested-layout").toFile(); + File moduleDir = new File(tempDir, "com.example"); + File descriptor = new File(moduleDir, "module-info.class"); try { - File moduleDir = new File(tempDir, "com.example"); moduleDir.mkdirs(); - new File(moduleDir, "module-info.class").createNewFile(); + descriptor.createNewFile(); - boolean result = invokeMethod(AbstractSurefireMojo.class, "isNestedModuleLayout", tempDir); - assertThat(result).isTrue(); + List result = invokeMethod(AbstractSurefireMojo.class, "nestedModuleDirectories", tempDir); + assertThat(result).extracting(File::getName).containsExactly("com.example"); } finally { - new File(new File(tempDir, "com.example"), "module-info.class").delete(); - new File(tempDir, "com.example").delete(); - tempDir.delete(); - } - } - - @Test - public void shouldFindAllNestedModuleDescriptors() throws Exception { - // target/classes/{com.example.one,com.example.two}/module-info.class plus a non-module dir - File tempDir = Files.createTempDirectory("surefire-test-nested-modules").toFile(); - try { - for (String module : new String[] {"com.example.two", "com.example.one"}) { - File moduleDir = new File(tempDir, module); - moduleDir.mkdirs(); - new File(moduleDir, "module-info.class").createNewFile(); - } - File plainDir = new File(tempDir, "META-INF"); - plainDir.mkdirs(); - - List result = invokeMethod(AbstractSurefireMojo.class, "findNestedModuleDescriptors", tempDir); - assertThat(result).hasSize(2); - // sorted by name for deterministic module ordering - assertThat(result.get(0).getName()).isEqualTo("com.example.one"); - assertThat(result.get(1).getName()).isEqualTo("com.example.two"); - } finally { - for (String module : new String[] {"com.example.one", "com.example.two"}) { - new File(new File(tempDir, module), "module-info.class").delete(); - new File(tempDir, module).delete(); - } - new File(tempDir, "META-INF").delete(); + descriptor.delete(); + moduleDir.delete(); tempDir.delete(); } } From d6a1bff25ebec7810142010c866de9f2a98ee02a Mon Sep 17 00:00:00 2001 From: Gerd Aschemann Date: Tue, 21 Jul 2026 16:33:24 +0200 Subject: [PATCH 08/10] Move handoff modules to module path (#3090) When maven-compiler-plugin 4.x hands off module-info-patch.args, treat it as the single source of truth for the forked JVM: emit its directives verbatim and drop surefire's auto-generated --add-reads ALL-UNNAMED / --add-modules ALL-MODULE-PATH, which remain as fallback when the file is absent. Precondition: the modules listed by the file's --add-modules (the test-scope dependencies, e.g. the JUnit engine) must be named modules in the fork's boot layer, so their elements move from the class-path to the module path - together with their transitive requires closure, every test or provider classpath element requiring one of the moved modules, and an explicit resolution root per moved module. Surefire's --add-opens for reflective test access now targets ALL-UNNAMED plus the moved named modules instead of ALL-UNNAMED alone. The new ModuleInfoPatchArgsFile parses the handoff file (same-line and two-line option form) and exposes its --add-modules names. Verified: maven-surefire-common 857/857 unit tests green; ITs green on Maven 3.10.0-rc-1 (fallback byte-identical) and 4.0.0-rc-5; acceptance: jigsaw example_test/m4 (1/1) and Vidocq champollion-json (602/602) zero-config under the enforced module system. Fixes #3090. Co-Authored-By: Claude Fable 5 --- .../plugin/surefire/AbstractSurefireMojo.java | 236 +++++++++++++++++- .../surefire/ModuleInfoPatchArgsFile.java | 144 +++++++++++ .../ModularClasspathForkConfiguration.java | 104 +++----- .../AbstractSurefireMojoJava7PlusTest.java | 57 ++++- .../surefire/ModuleInfoPatchArgsFileTest.java | 80 ++++++ ...ModularClasspathForkConfigurationTest.java | 43 ++-- 6 files changed, 566 insertions(+), 98 deletions(-) create mode 100644 maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/ModuleInfoPatchArgsFile.java create mode 100644 maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/ModuleInfoPatchArgsFileTest.java diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java index 13fc07e104..f6e04e5575 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java @@ -27,13 +27,17 @@ import java.math.BigDecimal; import java.nio.file.Files; import java.text.ChoiceFormat; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Deque; import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -2129,19 +2133,46 @@ private StartupConfiguration newStartupConfigWithModularPath( final ProviderRequirements providerRequirements; final Classpath testModulepath; List additionalModules = moduleDescriptor.getAdditionalResults(); + ModuleInfoPatchArgsFile moduleInfoPatchArgs = null; + String reflectiveOpensTargets = null; + Set movedTestScopeModules = new LinkedHashSet<>(); if (isMainDescriptor) { providerRequirements = new ProviderRequirements(true, true, false); + moduleInfoPatchArgs = ModuleInfoPatchArgsFile.load(getTestClassesDirectory()); + + List classpathElements; + List modulepathElements; + Map elementDescriptors; if (additionalModules.isEmpty()) { ResolvePathsResult result = resolveTestClasspath(testClasspath.getClassPath(), javaModuleDescriptor, javaHome); - testClasspath = new Classpath(result.getClasspathElements()); - testModulepath = new Classpath(result.getModulepathElements().keySet()); + classpathElements = new ArrayList<>(result.getClasspathElements()); + modulepathElements = + new ArrayList<>(result.getModulepathElements().keySet()); + elementDescriptors = result.getPathElements(); } else { ModulePathSplit split = resolveModulePathSplitForAllModules( testClasspath.getClassPath(), javaModuleDescriptor, additionalModules, javaHome); - testClasspath = split.classpath; - testModulepath = split.modulepath; + classpathElements = new ArrayList<>(split.classpath.getClassPath()); + modulepathElements = new ArrayList<>(split.modulepath.getClassPath()); + elementDescriptors = split.elementDescriptors; + } + + if (moduleInfoPatchArgs != null) { + List providerClasspathElements = new ArrayList<>(providerClasspath.getClassPath()); + movedTestScopeModules = moveTestScopeModulesToModulePath( + moduleInfoPatchArgs, + classpathElements, + modulepathElements, + providerClasspathElements, + elementDescriptors, + javaModuleDescriptor, + javaHome); + providerClasspath = new Classpath(providerClasspathElements); + reflectiveOpensTargets = opensTargets(moduleInfoPatchArgs.getAddedModules(), movedTestScopeModules); } + testClasspath = new Classpath(classpathElements); + testModulepath = new Classpath(modulepathElements); for (String className : scanResult.getClasses()) { packages.add(substringBeforeLast(className, ".")); @@ -2177,7 +2208,25 @@ private StartupConfiguration newStartupConfigWithModularPath( // of sibling modules scanned from the other nested test directories. packages.clear(); packages.addAll(scanTestPackages(patchFile)); - additionalModuleArgs = createAdditionalModuleArgs(testDir, additionalModules); + additionalModuleArgs = createAdditionalModuleArgs(testDir, additionalModules, reflectiveOpensTargets); + } + + if (moduleInfoPatchArgs != null) { + if (!movedTestScopeModules.isEmpty()) { + // Classpath consumers (e.g. the surefire provider using the JUnit + // launcher API) can only reach boot-layer modules that are resolved — + // make every moved module a resolution root. + additionalModuleArgs.add(new String[] {"--add-modules", String.join(",", movedTestScopeModules)}); + } + // With the handoff file present the fork configuration emits no --add-opens + // itself — generate them here, where the moved module closure is known + // (the reflecting engine, e.g. org.junit.platform.commons, may only be on + // the module path via a transitive requires of the file's added modules). + for (String pkg : packages) { + additionalModuleArgs.add(new String[] { + "--add-opens", javaModuleDescriptor.name() + "/" + pkg + "=" + reflectiveOpensTargets + }); + } } } @@ -2240,6 +2289,7 @@ private ModulePathSplit resolveModulePathSplitForAllModules( throws IOException { Set modulepathElements = new LinkedHashSet<>(); Set classpathElements = new LinkedHashSet<>(); + Map elementDescriptors = new LinkedHashMap<>(); List allDescriptors = new ArrayList<>(); allDescriptors.add(primary); for (ResolvePathResult additional : additionalModules) { @@ -2249,23 +2299,74 @@ private ModulePathSplit resolveModulePathSplitForAllModules( ResolvePathsResult result = resolveTestClasspath(testClasspathElements, descriptor, javaHome); modulepathElements.addAll(result.getModulepathElements().keySet()); classpathElements.addAll(result.getClasspathElements()); + elementDescriptors.putAll(result.getPathElements()); } classpathElements.removeAll(modulepathElements); return new ModulePathSplit( - new Classpath(new ArrayList<>(classpathElements)), new Classpath(new ArrayList<>(modulepathElements))); + new Classpath(new ArrayList<>(classpathElements)), + new Classpath(new ArrayList<>(modulepathElements)), + elementDescriptors); } /** - * Classpath and module path resulting from {@link #resolveModulePathSplitForAllModules}. + * Classpath, module path and per-element module descriptors resulting from + * {@link #resolveModulePathSplitForAllModules}. */ private static final class ModulePathSplit { private final Classpath classpath; private final Classpath modulepath; + private final Map elementDescriptors; - private ModulePathSplit(Classpath classpath, Classpath modulepath) { + private ModulePathSplit( + Classpath classpath, Classpath modulepath, Map elementDescriptors) { this.classpath = classpath; this.modulepath = modulepath; + this.elementDescriptors = elementDescriptors; + } + } + + /** + * Moves the classpath elements providing the requested modules — plus the transitive + * {@code requires} closure available on the classpath — to the module path. Requested + * modules without a matching classpath element (unknown, or already on the module path) + * are skipped silently. + * + * @param requestedModules module names from the handoff file's {@code --add-modules} + * @param classpathElements mutable classpath element list (elements are removed) + * @param modulepathElements mutable module path element list (elements are added) + * @param elementDescriptors module descriptor per element, from the path resolution + * @return the names of the modules actually moved, in resolution order + */ + static Collection moveHandoffModulesToModulePath( + Set requestedModules, + List classpathElements, + List modulepathElements, + Map elementDescriptors) { + Map classpathElementByModule = new LinkedHashMap<>(); + for (String element : classpathElements) { + JavaModuleDescriptor descriptor = elementDescriptors.get(element); + if (descriptor != null && descriptor.name() != null) { + classpathElementByModule.putIfAbsent(descriptor.name(), element); + } + } + + List moved = new ArrayList<>(); + Deque queue = new ArrayDeque<>(requestedModules); + while (!queue.isEmpty()) { + String moduleName = queue.removeFirst(); + String element = classpathElementByModule.remove(moduleName); + if (element == null) { + continue; + } + classpathElements.remove(element); + modulepathElements.add(element); + moved.add(moduleName); + for (JavaModuleDescriptor.JavaRequires requires : + elementDescriptors.get(element).requires()) { + queue.addLast(requires.name()); + } } + return moved; } /** @@ -2280,7 +2381,8 @@ private ModulePathSplit(Classpath classpath, Classpath modulepath) { * @return the Java Modules arguments, one option/value pair per entry * @throws MojoExecutionException if scanning a nested test directory fails */ - private List createAdditionalModuleArgs(File testDir, List additionalModules) + private List createAdditionalModuleArgs( + File testDir, List additionalModules, String reflectiveOpensTargets) throws MojoExecutionException { List args = new ArrayList<>(); for (ResolvePathResult additional : additionalModules) { @@ -2289,15 +2391,127 @@ private List createAdditionalModuleArgs(File testDir, List modulePathModules, + List classpathElements, + List modulepathElements, + Map elementDescriptors) { + boolean changed = true; + while (changed) { + changed = false; + for (Iterator it = classpathElements.iterator(); it.hasNext(); ) { + String element = it.next(); + JavaModuleDescriptor descriptor = elementDescriptors.get(element); + if (descriptor == null || descriptor.name() == null) { + continue; + } + for (JavaModuleDescriptor.JavaRequires requires : descriptor.requires()) { + if (modulePathModules.contains(requires.name())) { + it.remove(); + modulepathElements.add(element); + modulePathModules.add(descriptor.name()); + changed = true; + break; + } + } + } + } + } + + /** + * Target list for the auto-generated {@code --add-opens} of test packages when the + * handoff file is present: the unnamed module (provider/booter on the classpath) plus + * the file's added modules and every module moved to the module path with them — the + * reflecting engine (e.g. {@code org.junit.platform.commons}) may only be there via a + * transitive {@code requires}. + * + * @param addedModules module names from the handoff file's {@code --add-modules} + * @param movedModules module names moved by {@link #moveHandoffModulesToModulePath} + * @return comma-separated target module list, starting with ALL-UNNAMED + */ + static String opensTargets(Set addedModules, Collection movedModules) { + Set targets = new LinkedHashSet<>(addedModules); + targets.addAll(movedModules); + StringBuilder result = new StringBuilder("ALL-UNNAMED"); + for (String target : targets) { + result.append(',').append(target); + } + return result.toString(); + } + private ResolvePathsResult resolveTestClasspath( List testClasspathElements, JavaModuleDescriptor descriptor, String javaHome) throws IOException { ResolvePathsRequest req = ResolvePathsRequest.ofStrings(testClasspathElements) diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/ModuleInfoPatchArgsFile.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/ModuleInfoPatchArgsFile.java new file mode 100644 index 0000000000..3c21f179e6 --- /dev/null +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/ModuleInfoPatchArgsFile.java @@ -0,0 +1,144 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.plugin.surefire; + +import java.io.BufferedReader; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; + +import static java.util.Collections.unmodifiableList; +import static java.util.Collections.unmodifiableSet; + +/** + * The runtime handoff file {@code META-INF/maven/module-info-patch.args} written by + * maven-compiler-plugin 4.x from {@code module-info-patch.maven}. It contains the Java + * Modules options the compiler used for the test compilation (one option per line, value + * either on the same line separated by whitespace or on the following line). + * + * @since 3.6.0 + */ +public final class ModuleInfoPatchArgsFile { + public static final String RELATIVE_PATH = "META-INF/maven/module-info-patch.args"; + + private final File file; + private final List lines; + private final Set addedModules; + + private ModuleInfoPatchArgsFile(File file, List lines, Set addedModules) { + this.file = file; + this.lines = lines; + this.addedModules = addedModules; + } + + /** + * Loads the handoff file from the test output directory. + * + * @param testClassesDirectory the test output directory (e.g. target/test-classes) + * @return the parsed file, or null if it does not exist + * @throws IOException if the file exists but cannot be read + */ + public static ModuleInfoPatchArgsFile load(File testClassesDirectory) throws IOException { + if (testClassesDirectory == null || !testClassesDirectory.isDirectory()) { + return null; + } + return parse(new File(testClassesDirectory, RELATIVE_PATH)); + } + + /** + * Parses the given handoff file. + * + * @param file the module-info-patch.args file + * @return the parsed file, or null if it does not exist + * @throws IOException if the file exists but cannot be read + */ + public static ModuleInfoPatchArgsFile parse(File file) throws IOException { + if (file == null || !file.isFile()) { + return null; + } + + List lines = new ArrayList<>(); + Set addedModules = new LinkedHashSet<>(); + try (BufferedReader reader = Files.newBufferedReader(file.toPath())) { + String line; + while ((line = reader.readLine()) != null) { + line = line.trim(); + if (line.isEmpty() || line.startsWith("#")) { + continue; + } + lines.add(line); + } + } + for (int i = 0; i < lines.size(); i++) { + String line = lines.get(i); + String value = optionValue(line, "--add-modules", lines, i); + if (value != null) { + for (String module : value.split(",")) { + String name = module.trim(); + if (!name.isEmpty()) { + addedModules.add(name); + } + } + } + } + return new ModuleInfoPatchArgsFile(file, unmodifiableList(lines), unmodifiableSet(addedModules)); + } + + /** + * The value of the given option at index {@code i}, supporting both the same-line form + * ({@code --add-modules a,b}) and the two-line argfile form (value on the next line). + * + * @return the option value, or null if the line is not the given option + */ + private static String optionValue(String line, String option, List lines, int i) { + if (line.equals(option)) { + return i + 1 < lines.size() ? lines.get(i + 1) : null; + } + if (line.startsWith(option) && Character.isWhitespace(line.charAt(option.length()))) { + return line.substring(option.length()).trim(); + } + return null; + } + + public File getFile() { + return file; + } + + /** + * @return all non-empty, non-comment lines of the file, in order + */ + public List getLines() { + return lines; + } + + /** + * Module names listed by the file's {@code --add-modules} directives — with + * {@code module-info-patch.maven}'s {@code add-modules TEST-MODULE-PATH} these are the + * test-scope dependencies that belong on the module path. + * + * @return the module names, in file order + */ + public Set getAddedModules() { + return addedModules; + } +} diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java index 21866f84b4..a64b354c7c 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfiguration.java @@ -22,7 +22,6 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; -import java.io.BufferedReader; import java.io.File; import java.io.FileWriter; import java.io.IOException; @@ -34,6 +33,7 @@ import java.util.Map; import java.util.Properties; +import org.apache.maven.plugin.surefire.ModuleInfoPatchArgsFile; import org.apache.maven.plugin.surefire.booterclient.lazytestprovider.Commandline; import org.apache.maven.plugin.surefire.booterclient.output.InPluginProcessDumpSingleton; import org.apache.maven.plugin.surefire.log.api.ConsoleLogger; @@ -194,6 +194,7 @@ File createArgsFile( args.append('"').append(NL); } + ModuleInfoPatchArgsFile patchArgs = null; if (isMainDescriptor) { args.append("--patch-module") .append(NL) @@ -204,29 +205,40 @@ File createArgsFile( .append('"') .append(NL); - // Check for module-info-patch.args generated by maven-compiler-plugin 4.x - Path patchArgs = findModuleInfoPatchArgs(patchFile); - if (patchArgs != null) { - // Merge the file's directives, except --add-reads/--add-modules - // which surefire manages itself (see appendModuleInfoPatchArgs) - appendModuleInfoPatchArgs(args, patchArgs, moduleName); + // module-info-patch.args generated by maven-compiler-plugin 4.x + Path patchArgsPath = findModuleInfoPatchArgs(patchFile); + if (patchArgsPath != null) { + patchArgs = ModuleInfoPatchArgsFile.parse(patchArgsPath.toFile()); } - - // Always auto-generate --add-opens for test packages (JUnit needs reflection access). - // module-info-patch.maven cannot use ALL-UNNAMED in add-opens, so surefire handles this. - for (String pkg : packages) { - args.append("--add-opens") - .append(NL) - .append(moduleName) - .append('/') - .append(pkg) - .append('=') - .append("ALL-UNNAMED") - .append(NL); + if (patchArgs != null) { + // #3090: the developer-controlled handoff file is the + // single source of truth — pass it through verbatim. Its --add-modules + // dependencies are named modules on the module path (moved there by + // AbstractSurefireMojo), so the directives resolve in the boot layer. + for (String line : patchArgs.getLines()) { + args.append(line).append(NL); + } + // the patched module itself must still be a resolution root + args.append("--add-modules").append(NL).append(moduleName).append(NL); } if (patchArgs == null) { - // Without module-info-patch.args, also auto-generate --add-reads + // Without module-info-patch.args, auto-generate --add-opens for test + // packages (JUnit needs reflection access; module-info-patch.maven cannot + // express ALL-UNNAMED opens) and --add-reads. With the file present these + // arrive as pass-through arguments from AbstractSurefireMojo, which knows + // the modules moved to the module path. + for (String pkg : packages) { + args.append("--add-opens") + .append(NL) + .append(moduleName) + .append('/') + .append(pkg) + .append('=') + .append("ALL-UNNAMED") + .append(NL); + } + args.append("--add-reads") .append(NL) .append(moduleName) @@ -236,7 +248,12 @@ File createArgsFile( } } - args.append("--add-modules").append(NL).append("ALL-MODULE-PATH").append(NL); + if (patchArgs == null) { + args.append("--add-modules") + .append(NL) + .append("ALL-MODULE-PATH") + .append(NL); + } for (String[] entries : providerJpmsArguments) { for (String entry : entries) { @@ -287,49 +304,4 @@ private static Path findModuleInfoPatchArgs(File patchFile) { } return null; } - - /** - * Reads the module-info-patch.args file and appends its directives (e.g. --add-exports, - * --add-opens) to the args builder. Exceptions are --add-reads and --add-modules, which - * are skipped: they may reference named modules that surefire places on the classpath - * rather than the module path. Surefire appends its own - * {@code --add-reads =ALL-UNNAMED} instead, and {@code --add-modules - * ALL-MODULE-PATH} is generated by the caller. - * - * @param args the args builder to append to - * @param patchArgs the module-info-patch.args file - * @param moduleName the module name for surefire's own --add-reads - * @throws IOException if the file cannot be read - */ - private static void appendModuleInfoPatchArgs(StringBuilder args, Path patchArgs, String moduleName) - throws IOException { - // Files.newBufferedReader(Path) reads UTF-8, matching the compiler-written file - try (BufferedReader reader = Files.newBufferedReader(patchArgs)) { - String line; - while ((line = reader.readLine()) != null) { - line = line.trim(); - if (line.isEmpty() || line.startsWith("#")) { - continue; - } - // Skip --add-reads and --add-modules from the file — surefire manages these itself. - // The compiler-generated args may reference named modules that surefire places on - // the classpath rather than the module-path, causing boot layer errors. - if (line.startsWith("--add-reads") || line.startsWith("--add-modules")) { - if (line.equals("--add-reads") || line.equals("--add-modules")) { - // two-line form: the value is on the following line - reader.readLine(); - } - continue; - } - args.append(line).append(NL); - } - } - // Surefire always needs --add-reads =ALL-UNNAMED for its classpath-based runner - args.append("--add-reads") - .append(NL) - .append(moduleName) - .append('=') - .append("ALL-UNNAMED") - .append(NL); - } } diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java index e60e9313c0..be5cfacaf3 100644 --- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java +++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoJava7PlusTest.java @@ -21,8 +21,11 @@ import java.io.File; import java.lang.reflect.Method; import java.nio.file.Files; +import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -59,6 +62,7 @@ import static java.util.Arrays.asList; import static java.util.Collections.singleton; +import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static org.apache.maven.artifact.versioning.VersionRange.createFromVersion; import static org.assertj.core.api.Assertions.assertThat; @@ -236,7 +240,8 @@ public void shouldHaveStartupConfigForModularClasspath() throws Exception { verify(mojo, times(1)).effectiveIsEnableAssertions(); verify(mojo, times(1)).isChildDelegation(); - verify(mojo, times(1)).getTestClassesDirectory(); + // once for the patch directory, once probing for module-info-patch.args + verify(mojo, times(2)).getTestClassesDirectory(); verify(scanResult, times(1)).getClasses(); ArgumentCaptor argument1 = ArgumentCaptor.forClass(String.class); ArgumentCaptor argument2 = ArgumentCaptor.forClass(Exception.class); @@ -590,6 +595,56 @@ public void shouldTreatModuleSourceHierarchyLayoutAsNested() throws Exception { } } + @Test + public void shouldMoveHandoffModulesWithTransitiveRequires() { + JavaModuleDescriptor api = JavaModuleDescriptor.newModule("org.junit.jupiter.api") + .requires("org.junit.platform.commons") + .build(); + JavaModuleDescriptor commons = + JavaModuleDescriptor.newModule("org.junit.platform.commons").build(); + + List classpath = new ArrayList<>(asList("api.jar", "commons.jar", "plain.jar")); + List modulepath = new ArrayList<>(singletonList("classes")); + Map descriptors = new HashMap<>(); + descriptors.put("api.jar", api); + descriptors.put("commons.jar", commons); + + Collection moved = AbstractSurefireMojo.moveHandoffModulesToModulePath( + new LinkedHashSet<>(singletonList("org.junit.jupiter.api")), classpath, modulepath, descriptors); + + // the requested module and its transitive requires available on the classpath move + assertThat(moved).containsExactly("org.junit.jupiter.api", "org.junit.platform.commons"); + assertThat(classpath).containsExactly("plain.jar"); + assertThat(modulepath).containsExactly("classes", "api.jar", "commons.jar"); + } + + @Test + public void shouldMoveClasspathModulesRequiringMovedModules() { + JavaModuleDescriptor launcher = JavaModuleDescriptor.newModule("org.junit.platform.launcher") + .requires("org.junit.platform.commons") + .build(); + List classpath = new ArrayList<>(asList("launcher.jar", "provider.jar")); + List modulepath = new ArrayList<>(singletonList("commons.jar")); + Map descriptors = new HashMap<>(); + descriptors.put("launcher.jar", launcher); + + Set onModulePath = new LinkedHashSet<>(singletonList("org.junit.platform.commons")); + AbstractSurefireMojo.moveModulesRequiringMovedModules(onModulePath, classpath, modulepath, descriptors); + + // launcher requires a moved module and must follow it to the module path; + // the automatic-module provider jar (no requires) stays on the classpath + assertThat(classpath).containsExactly("provider.jar"); + assertThat(modulepath).containsExactly("commons.jar", "launcher.jar"); + assertThat(onModulePath).contains("org.junit.platform.launcher"); + } + + @Test + public void shouldBuildOpensTargetsFromAddedAndMovedModules() { + String targets = + AbstractSurefireMojo.opensTargets(new LinkedHashSet<>(asList("a.b", "c.d")), asList("c.d", "e.f")); + assertThat(targets).isEqualTo("ALL-UNNAMED,a.b,c.d,e.f"); + } + private static File mockFile(String absolutePath) { File f = mock(File.class); when(f.getAbsolutePath()).thenReturn(absolutePath); diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/ModuleInfoPatchArgsFileTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/ModuleInfoPatchArgsFileTest.java new file mode 100644 index 0000000000..301c6f8a51 --- /dev/null +++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/ModuleInfoPatchArgsFileTest.java @@ -0,0 +1,80 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.plugin.surefire; + +import java.io.File; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Test for {@link ModuleInfoPatchArgsFile}. + */ +class ModuleInfoPatchArgsFileTest { + @TempDir + Path tempDir; + + private File writeArgs(String content) throws Exception { + Path metaInf = tempDir.resolve("META-INF/maven"); + Files.createDirectories(metaInf); + Files.write(metaInf.resolve("module-info-patch.args"), content.getBytes(StandardCharsets.UTF_8)); + return tempDir.toFile(); + } + + @Test + void shouldReturnNullWithoutFile() throws Exception { + assertThat(ModuleInfoPatchArgsFile.load(tempDir.toFile())).isNull(); + assertThat(ModuleInfoPatchArgsFile.load(null)).isNull(); + } + + @Test + void shouldParseSameLineForm() throws Exception { + File dir = writeArgs("--add-modules org.junit.jupiter.api,org.junit.jupiter.engine\n" + + "--add-reads com.example=org.junit.jupiter.api\n" + + "--add-exports com.example/com.example.internal=ALL-UNNAMED\n"); + + ModuleInfoPatchArgsFile args = ModuleInfoPatchArgsFile.load(dir); + assertThat(args).isNotNull(); + assertThat(args.getAddedModules()).containsExactly("org.junit.jupiter.api", "org.junit.jupiter.engine"); + assertThat(args.getLines()).hasSize(3); + } + + @Test + void shouldParseTwoLineForm() throws Exception { + File dir = writeArgs("--add-modules\norg.junit.jupiter.api\n--add-reads\ncom.example=ALL-UNNAMED\n"); + + ModuleInfoPatchArgsFile args = ModuleInfoPatchArgsFile.load(dir); + assertThat(args).isNotNull(); + assertThat(args.getAddedModules()).containsExactly("org.junit.jupiter.api"); + } + + @Test + void shouldIgnoreCommentsAndUnionMultipleAddModules() throws Exception { + File dir = writeArgs("# generated\n--add-modules a.b\n\n--add-modules c.d, e.f\n"); + + ModuleInfoPatchArgsFile args = ModuleInfoPatchArgsFile.load(dir); + assertThat(args).isNotNull(); + assertThat(args.getAddedModules()).containsExactly("a.b", "c.d", "e.f"); + } +} diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfigurationTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfigurationTest.java index bc8ba14f2f..bb1ea71dcb 100644 --- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfigurationTest.java +++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ModularClasspathForkConfigurationTest.java @@ -226,22 +226,25 @@ public void shouldUseModuleInfoPatchArgsWhenPresent() throws Exception { // Should contain --patch-module (always generated by surefire) assertThat(lines).anyMatch(l -> l.equals("--patch-module")); - // Should contain --add-exports from module-info-patch.args (not auto-generated) - assertThat(lines).anyMatch(l -> l.equals("--add-exports mymod/com.example.internal=ALL-UNNAMED")); - - // Should NOT contain --add-modules from module-info-patch.args (surefire skips these) + // The handoff file is passed through verbatim, in order (single source of truth) assertThat(lines) - .noneMatch(l -> l.contains("junit,org.junit.jupiter.api") && !l.contains("ALL-MODULE-PATH")); + .containsSubsequence( + "--add-modules junit,org.junit.jupiter.api", + "--add-reads mymod=junit,org.junit.jupiter.api,ALL-UNNAMED", + "--add-exports mymod/com.example.internal=ALL-UNNAMED"); - // Should contain --add-reads with ALL-UNNAMED (surefire's own, not from file) - assertThat(lines).anyMatch(l -> l.equals("mymod=ALL-UNNAMED")); + // The patched module is still made a resolution root + assertThat(lines).containsSubsequence("--add-modules", "mymod"); - // Should STILL contain auto-generated --add-opens for test packages - // (surefire always generates these for JUnit reflection access) - assertThat(lines).anyMatch(l -> l.equals("mymod/com.example.test=ALL-UNNAMED")); + // No auto-generated --add-reads (the file's --add-reads is authoritative) + assertThat(lines).noneMatch(l -> l.equals("mymod=ALL-UNNAMED")); - // Should still have --add-modules ALL-MODULE-PATH - assertThat(lines).anyMatch(l -> l.equals("ALL-MODULE-PATH")); + // No fork-side --add-opens with the handoff file present — they arrive as + // pass-through arguments from the mojo, which knows the moved module closure + assertThat(lines).noneMatch(l -> l.contains("com.example.test")); + + // No ALL-MODULE-PATH — roots are explicit with the handoff file present + assertThat(lines).noneMatch(l -> l.equals("ALL-MODULE-PATH")); } finally { // Cleanup new File(new File(patchFile, "META-INF/maven"), "module-info-patch.args").delete(); @@ -312,7 +315,7 @@ public void shouldFallbackWithoutModuleInfoPatchArgs() throws Exception { @Test @SuppressWarnings("ResultOfMethodCallIgnored") - public void shouldNotSwallowDirectiveAfterSameLineSkippedOption() throws Exception { + public void shouldPassEveryHandoffDirectiveThroughVerbatim() throws Exception { Classpath booter = new Classpath(asList("booter.jar", "non-modular.jar")); File target = new File("target").getCanonicalFile(); File tmp = new File(target, "surefire"); @@ -335,8 +338,8 @@ public void shouldNotSwallowDirectiveAfterSameLineSkippedOption() throws Excepti new NullConsoleLogger(), mock(ForkNodeFactory.class)); - // maven-compiler-plugin 4.x writes option and value on the SAME line. - // A directive directly after a skipped option must not be swallowed. + // maven-compiler-plugin 4.x writes option and value on the SAME line; + // every directive must reach the forked JVM verbatim and in order. File patchFile = Files.createTempDirectory("surefire-test-patch-order").toFile(); try { File metaInf = new File(patchFile, "META-INF/maven"); @@ -366,12 +369,12 @@ public void shouldNotSwallowDirectiveAfterSameLineSkippedOption() throws Excepti assertThat(jigsawArgsFile).isNotNull(); List lines = readAllLines(jigsawArgsFile.toPath(), UTF_8); - // The --add-exports following the skipped --add-modules must survive - assertThat(lines).anyMatch(l -> l.equals("--add-exports mymod/com.example.internal=ALL-UNNAMED")); - - // The skipped options must not leak through from the file + // All directives arrive verbatim, in file order assertThat(lines) - .noneMatch(l -> l.contains("junit,org.junit.jupiter.api") && !l.contains("ALL-MODULE-PATH")); + .containsSubsequence( + "--add-modules junit,org.junit.jupiter.api", + "--add-exports mymod/com.example.internal=ALL-UNNAMED", + "--add-reads mymod=junit,org.junit.jupiter.api,ALL-UNNAMED"); } finally { new File(new File(patchFile, "META-INF/maven"), "module-info-patch.args").delete(); new File(patchFile, "META-INF/maven").delete(); From d5a46b6e6ddea303c60aebf13761eeffa460c48d Mon Sep 17 00:00:00 2001 From: Gerd Aschemann Date: Tue, 21 Jul 2026 16:49:00 +0200 Subject: [PATCH 09/10] Split newStartupConfigWithModularPath by cases Extract three private helpers: resolveModulePathSplit (single-module resolution vs multi-module union split), resolveModularPatchDirectory (classic vs nested test output layout) and createHandoffPassThroughArgs (resolution roots + --add-opens for the verbatim module-info-patch.args case). No behavior change; maven-surefire-common tests 857/857 green. Co-Authored-By: Claude Fable 5 --- .../plugin/surefire/AbstractSurefireMojo.java | 140 ++++++++++++------ 1 file changed, 94 insertions(+), 46 deletions(-) diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java index f6e04e5575..29e05ac518 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java @@ -2140,23 +2140,11 @@ private StartupConfiguration newStartupConfigWithModularPath( providerRequirements = new ProviderRequirements(true, true, false); moduleInfoPatchArgs = ModuleInfoPatchArgsFile.load(getTestClassesDirectory()); - List classpathElements; - List modulepathElements; - Map elementDescriptors; - if (additionalModules.isEmpty()) { - ResolvePathsResult result = - resolveTestClasspath(testClasspath.getClassPath(), javaModuleDescriptor, javaHome); - classpathElements = new ArrayList<>(result.getClasspathElements()); - modulepathElements = - new ArrayList<>(result.getModulepathElements().keySet()); - elementDescriptors = result.getPathElements(); - } else { - ModulePathSplit split = resolveModulePathSplitForAllModules( - testClasspath.getClassPath(), javaModuleDescriptor, additionalModules, javaHome); - classpathElements = new ArrayList<>(split.classpath.getClassPath()); - modulepathElements = new ArrayList<>(split.modulepath.getClassPath()); - elementDescriptors = split.elementDescriptors; - } + ModulePathSplit split = + resolveModulePathSplit(testClasspath, javaModuleDescriptor, additionalModules, javaHome); + List classpathElements = new ArrayList<>(split.classpath.getClassPath()); + List modulepathElements = new ArrayList<>(split.modulepath.getClassPath()); + Map elementDescriptors = split.elementDescriptors; if (moduleInfoPatchArgs != null) { List providerClasspathElements = new ArrayList<>(providerClasspath.getClassPath()); @@ -2189,19 +2177,8 @@ private StartupConfiguration newStartupConfigWithModularPath( List additionalModuleArgs = new ArrayList<>(); if (isMainDescriptor) { File testDir = getTestClassesDirectory(); - boolean nestedLayout = false; - if (testDir != null - && testDir.isDirectory() - && !nestedModuleDirectories(getMainBuildPath()).isEmpty()) { - // Maven 4 Module Source Hierarchy: test classes nested under /. - // Only the main output layout decides — a test-classes subdirectory merely - // sharing the module name (module named after its root package) must not. - File nestedTestDir = new File(testDir, javaModuleDescriptor.name()); - nestedLayout = nestedTestDir.isDirectory(); - patchFile = nestedLayout ? nestedTestDir : testDir; - } else { - patchFile = testDir; - } + patchFile = resolveModularPatchDirectory(testDir, javaModuleDescriptor.name()); + boolean nestedLayout = patchFile != null && !patchFile.equals(testDir); if (nestedLayout) { // The primary module should only open its own test packages, not those @@ -2212,21 +2189,8 @@ private StartupConfiguration newStartupConfigWithModularPath( } if (moduleInfoPatchArgs != null) { - if (!movedTestScopeModules.isEmpty()) { - // Classpath consumers (e.g. the surefire provider using the JUnit - // launcher API) can only reach boot-layer modules that are resolved — - // make every moved module a resolution root. - additionalModuleArgs.add(new String[] {"--add-modules", String.join(",", movedTestScopeModules)}); - } - // With the handoff file present the fork configuration emits no --add-opens - // itself — generate them here, where the moved module closure is known - // (the reflecting engine, e.g. org.junit.platform.commons, may only be on - // the module path via a transitive requires of the file's added modules). - for (String pkg : packages) { - additionalModuleArgs.add(new String[] { - "--add-opens", javaModuleDescriptor.name() + "/" + pkg + "=" + reflectiveOpensTargets - }); - } + additionalModuleArgs.addAll(createHandoffPassThroughArgs( + movedTestScopeModules, packages, javaModuleDescriptor.name(), reflectiveOpensTargets)); } } @@ -2269,6 +2233,90 @@ private StartupConfiguration newStartupConfigWithModularPath( providerName, classpathConfiguration, classLoaderConfiguration, processCheckerType, javaModulesArgs); } + /** + * Splits the test classpath into classpath and module path: a plain resolution + * against the primary module descriptor for the single-module case, the union split + * over all modules for a multi-module source hierarchy build. + * + * @param testClasspath the unsplit test classpath + * @param javaModuleDescriptor the primary module descriptor + * @param additionalModules the sibling module descriptors, empty for a single module + * @param javaHome the JDK used for path resolution + * @return the classpath/module-path split + * @throws IOException if the location manager fails to resolve the paths + */ + private ModulePathSplit resolveModulePathSplit( + Classpath testClasspath, + JavaModuleDescriptor javaModuleDescriptor, + List additionalModules, + String javaHome) + throws IOException { + if (additionalModules.isEmpty()) { + ResolvePathsResult result = + resolveTestClasspath(testClasspath.getClassPath(), javaModuleDescriptor, javaHome); + return new ModulePathSplit( + new Classpath(new ArrayList<>(result.getClasspathElements())), + new Classpath(new ArrayList<>(result.getModulepathElements().keySet())), + result.getPathElements()); + } + return resolveModulePathSplitForAllModules( + testClasspath.getClassPath(), javaModuleDescriptor, additionalModules, javaHome); + } + + /** + * The directory patched into the module under test: the per-module nested test + * output directory ({@code target/test-classes//}) for a Maven 4 Module + * Source Hierarchy build, the test output directory itself otherwise. Only the main + * build output layout decides — a test-classes subdirectory merely sharing the + * module name (module named after its root package) must not switch the layout. + * + * @param testDir the test output directory, may be null + * @param moduleName the name of the module under test + * @return the patch directory, or null without a test output directory + */ + private File resolveModularPatchDirectory(File testDir, String moduleName) { + if (testDir != null + && testDir.isDirectory() + && !nestedModuleDirectories(getMainBuildPath()).isEmpty()) { + File nestedTestDir = new File(testDir, moduleName); + if (nestedTestDir.isDirectory()) { + return nestedTestDir; + } + } + return testDir; + } + + /** + * The pass-through arguments accompanying a verbatim {@code module-info-patch.args} + * emission. Two cases: every moved module becomes an explicit resolution root + * (classpath consumers, e.g. the surefire provider using the JUnit launcher API, can + * only reach boot-layer modules that are resolved), and the {@code --add-opens} for + * reflective test access are generated here — not in the fork configuration — because + * only this side knows the moved module closure (the reflecting engine, e.g. + * org.junit.platform.commons, may only be on the module path via a transitive + * requires of the file's added modules). + * + * @param movedTestScopeModules the module names moved to the module path + * @param packages the test packages of the module under test + * @param moduleName the name of the module under test + * @param reflectiveOpensTargets the target module list for {@code --add-opens} + * @return the pass-through arguments for the forked JVM + */ + private static List createHandoffPassThroughArgs( + Set movedTestScopeModules, + Collection packages, + String moduleName, + String reflectiveOpensTargets) { + List args = new ArrayList<>(); + if (!movedTestScopeModules.isEmpty()) { + args.add(new String[] {"--add-modules", String.join(",", movedTestScopeModules)}); + } + for (String pkg : packages) { + args.add(new String[] {"--add-opens", moduleName + "/" + pkg + "=" + reflectiveOpensTargets}); + } + return args; + } + /** * Splits the test classpath of a multi-module source hierarchy build into classpath * and module path. The fork has a single boot layer, so an element required on the @@ -2310,7 +2358,7 @@ private ModulePathSplit resolveModulePathSplitForAllModules( /** * Classpath, module path and per-element module descriptors resulting from - * {@link #resolveModulePathSplitForAllModules}. + * {@link #resolveModulePathSplit}. */ private static final class ModulePathSplit { private final Classpath classpath; From 4ea4f281aedf5571172a480c7dc7c9a2b01957de Mon Sep 17 00:00:00 2001 From: Gerd Aschemann Date: Tue, 21 Jul 2026 17:21:52 +0200 Subject: [PATCH 10/10] Add Surefire3090TestDepsOnModulePathIT New fixture surefire-3090-test-deps-on-module-path: a Maven 4 module source hierarchy project with module-info-patch.maven whose tests assert behaviorally that the JUnit test-scope dependencies run as named modules on the module path (Test.class.getModule() must be org.junit.jupiter.api) and that whitebox access to a non-exported package keeps working. The IT additionally asserts surefire's debug line listing the moved module closure and skips on Maven 3. Verified: green under Maven 4.0.0-rc-5 (3/3 fixture tests, moved closure = api, engine, apiguardian, platform.commons, opentest4j, platform.engine, launcher), skipped under Maven 3.10.0-rc-1. Co-Authored-By: Claude Fable 5 --- .../Surefire3090TestDepsOnModulePathIT.java | 77 +++++++++++++++++++ .../pom.xml | 57 ++++++++++++++ .../main/java/com/example/probe/Widget.java | 10 +++ .../com/example/probe/internal/Secret.java | 12 +++ .../main/java/module-info.java | 3 + .../example/probe/ModulePlacementTest.java | 29 +++++++ .../probe/internal/SecretWhiteboxTest.java | 15 ++++ .../test/java/module-info-patch.maven | 10 +++ 8 files changed, 213 insertions(+) create mode 100644 surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire3090TestDepsOnModulePathIT.java create mode 100644 surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/pom.xml create mode 100644 surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/main/java/com/example/probe/Widget.java create mode 100644 surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/main/java/com/example/probe/internal/Secret.java create mode 100644 surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/main/java/module-info.java create mode 100644 surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/test/java/com/example/probe/ModulePlacementTest.java create mode 100644 surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/test/java/com/example/probe/internal/SecretWhiteboxTest.java create mode 100644 surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/test/java/module-info-patch.maven diff --git a/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire3090TestDepsOnModulePathIT.java b/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire3090TestDepsOnModulePathIT.java new file mode 100644 index 0000000000..7caedd6421 --- /dev/null +++ b/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire3090TestDepsOnModulePathIT.java @@ -0,0 +1,77 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.surefire.its.jiras; + +import java.io.File; + +import org.apache.maven.surefire.its.fixture.AbstractJava9PlusIT; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +/** + * Integration test for running test-scope dependencies as named modules on the module + * path when a module-info-patch.args handoff file is present. + *

+ * This test requires Maven 4 and is skipped when running with Maven 3. + * It verifies that surefire correctly: + *

    + *
  • Moves the modules referenced by the file's {@code add-modules} (the JUnit + * test-scope dependencies) from the classpath to the module path
    • + *
  • Passes the handoff file's directives through to the forked JVM verbatim
    • + *
  • Keeps reflective test access working with the moved named modules
    • + *
+ * The fixture asserts the placement behaviorally: {@code Test.class.getModule()} must be + * the named module {@code org.junit.jupiter.api}, which fails when the engine is left on + * the classpath in the unnamed module. + */ +class Surefire3090TestDepsOnModulePathIT extends AbstractJava9PlusIT { + + @Test + void testTestDepsRunAsNamedModules() { + assumeTrue(isMaven4Plus(), "This test requires Maven 4."); + // 3 tests: ModulePlacementTest.junitApiIsNamedModuleOnModulePath + // + testRunsInsidePatchedModule + SecretWhiteboxTest.testReveal + assumeJava9() + .debugLogging() + .executeTest() + .verifyErrorFreeLog() + .verifyTextInLog("Moved test-scope modules to the module path") + .assertTestSuiteResults(3); + } + + @Override + protected String getProjectDirectoryName() { + return "surefire-3090-test-deps-on-module-path"; + } + + private static boolean isMaven4Plus() { + String mavenHome = System.getProperty("maven.home"); + if (mavenHome == null) { + return false; + } + File mavenLib = new File(mavenHome, "lib"); + if (!mavenLib.isDirectory()) { + return false; + } + // Maven 4 ships maven-api-core; Maven 3 does not + File[] files = mavenLib.listFiles((dir, name) -> name.startsWith("maven-api-core-") && name.endsWith(".jar")); + return files != null && files.length > 0; + } +} diff --git a/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/pom.xml b/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/pom.xml new file mode 100644 index 0000000000..ad63d8eaf4 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/pom.xml @@ -0,0 +1,57 @@ + + + 4.1.0 + + com.example + modulepath-test-deps-probe + 1.0.0-SNAPSHOT + + + ${java.specification.version} + UTF-8 + + + + + org.junit.jupiter + junit-jupiter-api + 5.9.1 + test + + + org.junit.jupiter + junit-jupiter-engine + 5.9.1 + test + + + + + + + com.example.probe + + + com.example.probe + test + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 4.0.0-beta-4 + + + org.apache.maven.plugins + maven-surefire-plugin + ${surefire.version} + + + + diff --git a/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/main/java/com/example/probe/Widget.java b/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/main/java/com/example/probe/Widget.java new file mode 100644 index 0000000000..b3048256fb --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/main/java/com/example/probe/Widget.java @@ -0,0 +1,10 @@ +package com.example.probe; + +/** + * Trivial exported class of the module under test. + */ +public class Widget { + public int size() { + return 1; + } +} diff --git a/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/main/java/com/example/probe/internal/Secret.java b/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/main/java/com/example/probe/internal/Secret.java new file mode 100644 index 0000000000..5582bf9842 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/main/java/com/example/probe/internal/Secret.java @@ -0,0 +1,12 @@ +package com.example.probe.internal; + +/** + * Non-exported internal helper, reachable for tests only by patching them into the module. + */ +public final class Secret { + private Secret() {} + + public static String reveal() { + return "42"; + } +} diff --git a/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/main/java/module-info.java b/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/main/java/module-info.java new file mode 100644 index 0000000000..c847e0e2d0 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/main/java/module-info.java @@ -0,0 +1,3 @@ +module com.example.probe { + exports com.example.probe; +} diff --git a/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/test/java/com/example/probe/ModulePlacementTest.java b/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/test/java/com/example/probe/ModulePlacementTest.java new file mode 100644 index 0000000000..2f361b4638 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/test/java/com/example/probe/ModulePlacementTest.java @@ -0,0 +1,29 @@ +package com.example.probe; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Probes that the test-scope dependencies referenced by module-info-patch.args run as + * NAMED modules on the module path (#3090) instead of classpath jars in the unnamed + * module. + */ +class ModulePlacementTest { + @Test + void junitApiIsNamedModuleOnModulePath() { + Module junitApi = Test.class.getModule(); + assertTrue(junitApi.isNamed(), "junit-jupiter-api must run as a named module, but was: " + junitApi); + assertEquals("org.junit.jupiter.api", junitApi.getName()); + } + + @Test + void testRunsInsidePatchedModule() { + Module own = ModulePlacementTest.class.getModule(); + assertTrue(own.isNamed(), "the test must run inside the patched module, but was: " + own); + assertEquals("com.example.probe", own.getName()); + assertTrue(own.canRead(Test.class.getModule()), "the patched module must read the moved JUnit API module"); + assertEquals(1, new Widget().size()); + } +} diff --git a/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/test/java/com/example/probe/internal/SecretWhiteboxTest.java b/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/test/java/com/example/probe/internal/SecretWhiteboxTest.java new file mode 100644 index 0000000000..9198b3726b --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/test/java/com/example/probe/internal/SecretWhiteboxTest.java @@ -0,0 +1,15 @@ +package com.example.probe.internal; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Whitebox test accessing the non-exported internal package of com.example.probe. + */ +class SecretWhiteboxTest { + @Test + void testReveal() { + assertEquals("42", Secret.reveal()); + } +} diff --git a/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/test/java/module-info-patch.maven b/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/test/java/module-info-patch.maven new file mode 100644 index 0000000000..0056aae102 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-3090-test-deps-on-module-path/src/com.example.probe/test/java/module-info-patch.maven @@ -0,0 +1,10 @@ +/* + * Whitebox test patch for the com.example.probe module. The TEST-MODULE-PATH + * tokens make maven-compiler-plugin 4.x expand the test-scope dependencies + * (JUnit) into module-info-patch.args, which surefire passes through verbatim + * after moving those dependencies to the module path (#3090). + */ +patch-module com.example.probe { + add-modules TEST-MODULE-PATH; + add-reads TEST-MODULE-PATH; +}