From 5aea5e2abf37456ecc775836ed3b97cd16dc5ad7 Mon Sep 17 00:00:00 2001 From: Henning Schmiedehausen Date: Wed, 23 Aug 2023 09:41:09 -0700 Subject: [PATCH] [SUREFIRE-2190] Fix module dependencies for compile only dependencies (#668) * [SUREFIRE-2190] Fix module dependencies for compile only dependencies Include all modules on the module path to the root modules. This fixes test code that uses dependencies that are declared as optional (`require static`) for the main artifact to access these dependencies. --- .../ModularClasspathForkConfiguration.java | 6 +- .../booterclient/ForkConfigurationTest.java | 2 +- ...ModularClasspathForkConfigurationTest.java | 8 +-- .../its/jiras/Surefire2190JUnitIT.java | 37 ++++++++++ .../src/test/resources/surefire-2190/pom.xml | 68 +++++++++++++++++++ .../src/main/java/module-info.java | 5 ++ .../src/main/java/surefirebug/thing/Main.java | 44 ++++++++++++ .../test/java/surefirebug/thing/MainTest.java | 55 +++++++++++++++ 8 files changed, 216 insertions(+), 9 deletions(-) create mode 100644 surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire2190JUnitIT.java create mode 100644 surefire-its/src/test/resources/surefire-2190/pom.xml create mode 100644 surefire-its/src/test/resources/surefire-2190/src/main/java/module-info.java create mode 100644 surefire-its/src/test/resources/surefire-2190/src/main/java/surefirebug/thing/Main.java create mode 100644 surefire-its/src/test/resources/surefire-2190/src/test/java/surefirebug/thing/MainTest.java 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 7473349208..5a25761176 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 @@ -194,18 +194,16 @@ File createArgsFile( .append(NL); } - args.append("--add-modules").append(NL).append(moduleName).append(NL); - args.append("--add-reads") .append(NL) .append(moduleName) .append('=') .append("ALL-UNNAMED") .append(NL); - } else { - args.append("--add-modules").append(NL).append(moduleName).append(NL); } + args.append("--add-modules").append(NL).append("ALL-MODULE-PATH").append(NL); + for (String[] entries : providerJpmsArguments) { for (String entry : entries) { args.append(entry).append(NL); diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ForkConfigurationTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ForkConfigurationTest.java index 752a64ef93..708ec12608 100644 --- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ForkConfigurationTest.java +++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/ForkConfigurationTest.java @@ -192,7 +192,7 @@ public void testCliArgs() throws Exception { assertThat(endOfFileArg).isPositive(); Path argFile = Paths.get(cliAsString.substring(beginOfFileArg + 1, endOfFileArg)); String argFileText = new String(readAllBytes(argFile)); - assertThat(argFileText).contains("arg2").contains("arg3").contains("--add-modules" + NL + "test.module"); + assertThat(argFileText).contains("arg2").contains("arg3").contains("--add-modules" + NL + "ALL-MODULE-PATH"); } @Test 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 d049efc097..fbf3e54fd7 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 @@ -127,13 +127,13 @@ public void shouldCreateModularArgsFile() throws Exception { assertThat(argsFileLines.get(7)).isEqualTo("abc/org.apache.abc=ALL-UNNAMED"); - assertThat(argsFileLines.get(8)).isEqualTo("--add-modules"); + assertThat(argsFileLines.get(8)).isEqualTo("--add-reads"); - assertThat(argsFileLines.get(9)).isEqualTo("abc"); + assertThat(argsFileLines.get(9)).isEqualTo("abc=ALL-UNNAMED"); - assertThat(argsFileLines.get(10)).isEqualTo("--add-reads"); + assertThat(argsFileLines.get(10)).isEqualTo("--add-modules"); - assertThat(argsFileLines.get(11)).isEqualTo("abc=ALL-UNNAMED"); + assertThat(argsFileLines.get(11)).isEqualTo("ALL-MODULE-PATH"); assertThat(argsFileLines.get(12)).isEqualTo(ForkedBooter.class.getName()); diff --git a/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire2190JUnitIT.java b/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire2190JUnitIT.java new file mode 100644 index 0000000000..7b04a61500 --- /dev/null +++ b/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire2190JUnitIT.java @@ -0,0 +1,37 @@ +/* + * 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 org.apache.maven.surefire.its.fixture.AbstractJava9PlusIT; +import org.junit.Test; + +/** + * Integration test for SUREFIRE-2190. + */ +public class Surefire2190JUnitIT extends AbstractJava9PlusIT { + @Test + public void test() throws Exception { + assumeJava9().debugLogging().executeVerify().assertTestSuiteResults(4, 0, 0, 0); + } + + @Override + protected String getProjectDirectoryName() { + return "surefire-2190"; + } +} diff --git a/surefire-its/src/test/resources/surefire-2190/pom.xml b/surefire-its/src/test/resources/surefire-2190/pom.xml new file mode 100644 index 0000000000..3cdfd1e819 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-2190/pom.xml @@ -0,0 +1,68 @@ + + + + 4.0.0 + + org.apache.maven.plugins.surefire + surefire-2190-it + 1.0 + + + ${java.specification.version} + ${java.specification.version} + UTF-8 + + + + + org.junit.jupiter + junit-jupiter-api + 5.9.1 + test + + + + org.junit.jupiter + junit-jupiter-params + 5.9.1 + test + + + + jakarta.annotation + jakarta.annotation-api + 2.1.1 + provided + true + + + + + + + maven-compiler-plugin + 3.8.1 + + UTF-8 + + + + org.apache.maven.plugins + maven-surefire-plugin + ${surefire.version} + + + + diff --git a/surefire-its/src/test/resources/surefire-2190/src/main/java/module-info.java b/surefire-its/src/test/resources/surefire-2190/src/main/java/module-info.java new file mode 100644 index 0000000000..2ec4f1a66c --- /dev/null +++ b/surefire-its/src/test/resources/surefire-2190/src/main/java/module-info.java @@ -0,0 +1,5 @@ +module surefire.bug.thing3 { + requires static jakarta.annotation; + + exports surefirebug.thing; +} diff --git a/surefire-its/src/test/resources/surefire-2190/src/main/java/surefirebug/thing/Main.java b/surefire-its/src/test/resources/surefire-2190/src/main/java/surefirebug/thing/Main.java new file mode 100644 index 0000000000..e78261e02c --- /dev/null +++ b/surefire-its/src/test/resources/surefire-2190/src/main/java/surefirebug/thing/Main.java @@ -0,0 +1,44 @@ +package surefirebug.thing; + +import java.util.concurrent.Callable; +import java.util.function.Supplier; + +public final class Main implements Callable { + + private static final Supplier DEFAULT_PROVIDER = () -> "Hello, World"; + + + public static void main(String... args) throws Exception { + Main main = new Main(); + String text = main.call(); + + System.out.println(text); + } + + private final Supplier supplier; + + public Main(Supplier supplier) { + this.supplier = supplier; + } + + public Main() { + this(DEFAULT_PROVIDER); + } + + @Override + public String call() { + var value = supplier.get(); + + if (value == null) { + var annotations = supplier.getClass().getAnnotations(); + for (var annotation : annotations) { + if (annotation.annotationType().getSimpleName().equals("Nullable")) { + return ""; + } + } + throw new IllegalStateException("null without @Nullable annotation"); + } + + return value; + } +} diff --git a/surefire-its/src/test/resources/surefire-2190/src/test/java/surefirebug/thing/MainTest.java b/surefire-its/src/test/resources/surefire-2190/src/test/java/surefirebug/thing/MainTest.java new file mode 100644 index 0000000000..7d33e5eef0 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-2190/src/test/java/surefirebug/thing/MainTest.java @@ -0,0 +1,55 @@ +package surefirebug.thing; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import jakarta.annotation.Nullable; +import java.util.function.Supplier; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class MainTest { + + @Test + public void testDefault() { + Main main = new Main(); + assertEquals("Hello, World", main.call()); + } + + @Test + public void testNonStandard() { + Main main = new Main(new NonstandardSupplier()); + assertEquals("Hello, People", main.call()); + } + + @Test + public void testNullSupplierWithNullable() { + Main main = new Main(new NullSupplierWithNullable()); + assertEquals("", main.call()); + } + + @Test + public void testNullSupplierWithoutNullable() { + Main main = new Main(new NullSupplierWithoutNullable()); + IllegalStateException e = Assertions.assertThrows(IllegalStateException.class, main::call); + assertEquals("null without @Nullable annotation", e.getMessage()); + } + + public static class NonstandardSupplier implements Supplier { + public String get() { + return "Hello, People"; + } + } + + @Nullable + public static class NullSupplierWithNullable implements Supplier { + public String get() { + return null; + } + } + + public static class NullSupplierWithoutNullable implements Supplier { + public String get() { + return null; + } + } +}