You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "gnodet (via GitHub)" <gi...@apache.org> on 2026/09/25 19:43:29 UTC

[PR] fix: decouple test-jar skip from maven.test.skip [MJAR-138] [maven-jar-plugin]

gnodet opened a new pull request, #612:
URL: https://github.com/apache/maven-jar-plugin/pull/612

   ## Problem
   
   Fixes #308 / MJAR-138.
   
   The `test-jar` goal's `skip` parameter was bound to `${maven.test.skip}`. This meant passing `-Dmaven.test.skip=true` — which also skips test compilation — silently prevented the test-jar from being built. In multi-module projects where a downstream module has a `test-jar` dependency, this broke the build.
   
   This is a long-standing (2010) tension between two intentional decisions:
   - **MJAR-90**: skip the test-jar when `maven.test.skip=true` to avoid producing an *empty* test-jar (tests weren't compiled)
   - **MJAR-138**: but skipping the test-jar breaks cross-module test-jar dependencies
   
   ## Solution
   
   Rebind `skip` from `${maven.test.skip}` to `${maven.test.jar.skip}` (default `false`).
   
   The MJAR-90 concern (empty test-jar) is already handled by `skipIfEmpty` in modern versions. There is no longer a reason to couple test-jar packaging to `maven.test.skip`.
   
   **Users who want to skip test-jar packaging** should now use `-Dmaven.test.jar.skip=true` (or `<skip>true</skip>` in POM config — unchanged).
   
   **Users who want to skip test execution** without breaking downstream modules should use:
   - `-DskipTests` (Maven 3+)
   - `--skip-tests` (Maven 4.1+, see #13233)
   
   ## Changes
   
   - `TestJarMojo`: rebind `skip` property from `maven.test.skip` → `maven.test.jar.skip`
   - New IT `mjar-138`: multi-module build with `-DskipTests` — verifies model test-jar is built and client can compile against it via the reactor


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: decouple test-jar skip from maven.test.skip [MJAR-138] [maven-jar-plugin]

Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #612:
URL: https://github.com/apache/maven-jar-plugin/pull/612#discussion_r4114999332


##########
src/it/mjar-138/verify.groovy:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.
+ */
+
+// MJAR-138: -DskipTests must NOT prevent the test-jar from being built.
+// The model module's test-jar must be produced so the client module can
+// resolve it as a reactor artifact and compile its test sources against it.
+
+import java.io.*;
+
+boolean result = true;
+
+try {
+    // 1. The model test-jar must have been produced despite -DskipTests.
+    File testJar = new File(basedir, "model/target/model-1.0-SNAPSHOT-tests.jar");
+    if (!testJar.exists() || testJar.isDirectory()) {

Review Comment:
   The two conditions can be replaced by `!testJar.isFile()`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: decouple test-jar skip from maven.test.skip [MJAR-138] [maven-jar-plugin]

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #612:
URL: https://github.com/apache/maven-jar-plugin/pull/612


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: decouple test-jar skip from maven.test.skip [MJAR-138] [maven-jar-plugin]

Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #612:
URL: https://github.com/apache/maven-jar-plugin/pull/612#discussion_r4108284465


##########
src/main/java/org/apache/maven/plugins/jar/TestJarMojo.java:
##########
@@ -35,9 +35,16 @@ public class TestJarMojo extends AbstractJarMojo {
 
     /**
      * Set this to {@code true} to bypass test-jar generation.
-     * Its use is not recommended, but quite convenient on occasion.
+     *
+     * <p>Note: unlike {@code -Dmaven.test.skip=true} (which also skips test compilation and makes
+     * the test-jar unavailable to downstream modules), this flag suppresses only the packaging step
+     * while leaving test classes compiled and accessible within the reactor.
+     * Use {@code -DskipTests} or, with Maven&nbsp;4.1+, {@code --skip-tests} to skip test
+     * <em>execution</em> without preventing the test-jar from being built.</p>
+     *
+     * @since 2.2
      */

Review Comment:
   💡 `@since 2.2` is incorrect — the `skip` field itself predates 2.2 (it's been in TestJarMojo since the class was created). The current development version is `4.0.0-beta-2-SNAPSHOT`, and the rest of this codebase uses `@since 4.0.0-beta-2` for new features (see `AbstractJarMojo.java`).
   
   Since the field isn't new — only its property binding changed — the `@since` tag should either document the property change explicitly or be dropped. The Javadoc body already describes the new behavior.
   
   ```suggestion
        * @since 4.0.0-beta-2
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: decouple test-jar skip from maven.test.skip [MJAR-138] [maven-jar-plugin]

Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #612:
URL: https://github.com/apache/maven-jar-plugin/pull/612#discussion_r4115002413


##########
src/it/mjar-138/verify.groovy:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.
+ */
+
+// MJAR-138: -DskipTests must NOT prevent the test-jar from being built.
+// The model module's test-jar must be produced so the client module can
+// resolve it as a reactor artifact and compile its test sources against it.
+
+import java.io.*;
+
+boolean result = true;
+
+try {
+    // 1. The model test-jar must have been produced despite -DskipTests.
+    File testJar = new File(basedir, "model/target/model-1.0-SNAPSHOT-tests.jar");
+    if (!testJar.exists() || testJar.isDirectory()) {
+        System.err.println("MJAR-138: model test-jar is missing: " + testJar);
+        return false;
+    }
+
+    // 2. The client's test classes must have compiled (they depend on TestComponent from the test-jar).
+    File clientTestClasses = new File(basedir, "client/target/test-classes/org/apache/maven/its/mjar138/ClientTest.class");
+    if (!clientTestClasses.exists()) {

Review Comment:
   Use `!clientTestClasses.isFile()` for consistency with above.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: decouple test-jar skip from maven.test.skip [MJAR-138] [maven-jar-plugin]

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #612:
URL: https://github.com/apache/maven-jar-plugin/pull/612#discussion_r4110810803


##########
src/main/java/org/apache/maven/plugins/jar/TestJarMojo.java:
##########
@@ -35,9 +35,16 @@ public class TestJarMojo extends AbstractJarMojo {
 
     /**
      * Set this to {@code true} to bypass test-jar generation.
-     * Its use is not recommended, but quite convenient on occasion.
+     *
+     * <p>Note: unlike {@code -Dmaven.test.skip=true} (which also skips test compilation and makes
+     * the test-jar unavailable to downstream modules), this flag suppresses only the packaging step
+     * while leaving test classes compiled and accessible within the reactor.
+     * Use {@code -DskipTests} or, with Maven&nbsp;4.1+, {@code --skip-tests} to skip test
+     * <em>execution</em> without preventing the test-jar from being built.</p>
+     *
+     * @since 2.2
      */

Review Comment:
   Fixed in 7dee44e.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: decouple test-jar skip from maven.test.skip [MJAR-138] [maven-jar-plugin]

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #612:
URL: https://github.com/apache/maven-jar-plugin/pull/612#discussion_r4115016863


##########
src/it/mjar-138/verify.groovy:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.
+ */
+
+// MJAR-138: -DskipTests must NOT prevent the test-jar from being built.
+// The model module's test-jar must be produced so the client module can
+// resolve it as a reactor artifact and compile its test sources against it.
+
+import java.io.*;
+
+boolean result = true;
+
+try {
+    // 1. The model test-jar must have been produced despite -DskipTests.
+    File testJar = new File(basedir, "model/target/model-1.0-SNAPSHOT-tests.jar");
+    if (!testJar.exists() || testJar.isDirectory()) {
+        System.err.println("MJAR-138: model test-jar is missing: " + testJar);
+        return false;
+    }
+
+    // 2. The client's test classes must have compiled (they depend on TestComponent from the test-jar).
+    File clientTestClasses = new File(basedir, "client/target/test-classes/org/apache/maven/its/mjar138/ClientTest.class");
+    if (!clientTestClasses.exists()) {
+        System.err.println("MJAR-138: client test classes not compiled (test-jar was not available in reactor): " + clientTestClasses);

Review Comment:
   Good point — switched to `assert` style for consistency with newer ITs. Fixed in ec583d2.



##########
src/it/mjar-138/verify.groovy:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.
+ */
+
+// MJAR-138: -DskipTests must NOT prevent the test-jar from being built.
+// The model module's test-jar must be produced so the client module can
+// resolve it as a reactor artifact and compile its test sources against it.
+
+import java.io.*;
+
+boolean result = true;
+
+try {
+    // 1. The model test-jar must have been produced despite -DskipTests.
+    File testJar = new File(basedir, "model/target/model-1.0-SNAPSHOT-tests.jar");
+    if (!testJar.exists() || testJar.isDirectory()) {

Review Comment:
   Fixed in 1a571bc.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: decouple test-jar skip from maven.test.skip [MJAR-138] [maven-jar-plugin]

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #612:
URL: https://github.com/apache/maven-jar-plugin/pull/612#discussion_r4115004426


##########
src/it/mjar-138/verify.groovy:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.
+ */
+
+// MJAR-138: -DskipTests must NOT prevent the test-jar from being built.
+// The model module's test-jar must be produced so the client module can
+// resolve it as a reactor artifact and compile its test sources against it.
+
+import java.io.*;
+
+boolean result = true;
+
+try {
+    // 1. The model test-jar must have been produced despite -DskipTests.
+    File testJar = new File(basedir, "model/target/model-1.0-SNAPSHOT-tests.jar");
+    if (!testJar.exists() || testJar.isDirectory()) {

Review Comment:
   Fixed in 1a571bc.



##########
src/it/mjar-138/verify.groovy:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.
+ */
+
+// MJAR-138: -DskipTests must NOT prevent the test-jar from being built.
+// The model module's test-jar must be produced so the client module can
+// resolve it as a reactor artifact and compile its test sources against it.
+
+import java.io.*;
+
+boolean result = true;
+
+try {
+    // 1. The model test-jar must have been produced despite -DskipTests.
+    File testJar = new File(basedir, "model/target/model-1.0-SNAPSHOT-tests.jar");
+    if (!testJar.exists() || testJar.isDirectory()) {
+        System.err.println("MJAR-138: model test-jar is missing: " + testJar);
+        return false;
+    }
+
+    // 2. The client's test classes must have compiled (they depend on TestComponent from the test-jar).
+    File clientTestClasses = new File(basedir, "client/target/test-classes/org/apache/maven/its/mjar138/ClientTest.class");
+    if (!clientTestClasses.exists()) {

Review Comment:
   Fixed in 1a571bc.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: decouple test-jar skip from maven.test.skip [MJAR-138] [maven-jar-plugin]

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #612:
URL: https://github.com/apache/maven-jar-plugin/pull/612#discussion_r4115017701


##########
src/it/mjar-138/verify.groovy:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.
+ */
+
+// MJAR-138: -DskipTests must NOT prevent the test-jar from being built.
+// The model module's test-jar must be produced so the client module can
+// resolve it as a reactor artifact and compile its test sources against it.
+
+import java.io.*;
+
+boolean result = true;
+
+try {
+    // 1. The model test-jar must have been produced despite -DskipTests.
+    File testJar = new File(basedir, "model/target/model-1.0-SNAPSHOT-tests.jar");
+    if (!testJar.exists() || testJar.isDirectory()) {
+        System.err.println("MJAR-138: model test-jar is missing: " + testJar);
+        return false;
+    }
+
+    // 2. The client's test classes must have compiled (they depend on TestComponent from the test-jar).
+    File clientTestClasses = new File(basedir, "client/target/test-classes/org/apache/maven/its/mjar138/ClientTest.class");
+    if (!clientTestClasses.exists()) {

Review Comment:
   Fixed in 1a571bc.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: decouple test-jar skip from maven.test.skip [MJAR-138] [maven-jar-plugin]

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #612:
URL: https://github.com/apache/maven-jar-plugin/pull/612#discussion_r4108625063


##########
src/main/java/org/apache/maven/plugins/jar/TestJarMojo.java:
##########
@@ -35,9 +35,16 @@ public class TestJarMojo extends AbstractJarMojo {
 
     /**
      * Set this to {@code true} to bypass test-jar generation.
-     * Its use is not recommended, but quite convenient on occasion.
+     *
+     * <p>Note: unlike {@code -Dmaven.test.skip=true} (which also skips test compilation and makes
+     * the test-jar unavailable to downstream modules), this flag suppresses only the packaging step
+     * while leaving test classes compiled and accessible within the reactor.
+     * Use {@code -DskipTests} or, with Maven&nbsp;4.1+, {@code --skip-tests} to skip test
+     * <em>execution</em> without preventing the test-jar from being built.</p>
+     *
+     * @since 2.2
      */

Review Comment:
   Fixed in 7dee44e.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: decouple test-jar skip from maven.test.skip [MJAR-138] [maven-jar-plugin]

Posted by "desruisseaux (via GitHub)" <gi...@apache.org>.
desruisseaux commented on code in PR #612:
URL: https://github.com/apache/maven-jar-plugin/pull/612#discussion_r4115003906


##########
src/it/mjar-138/verify.groovy:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.
+ */
+
+// MJAR-138: -DskipTests must NOT prevent the test-jar from being built.
+// The model module's test-jar must be produced so the client module can
+// resolve it as a reactor artifact and compile its test sources against it.
+
+import java.io.*;
+
+boolean result = true;
+
+try {
+    // 1. The model test-jar must have been produced despite -DskipTests.
+    File testJar = new File(basedir, "model/target/model-1.0-SNAPSHOT-tests.jar");
+    if (!testJar.exists() || testJar.isDirectory()) {
+        System.err.println("MJAR-138: model test-jar is missing: " + testJar);
+        return false;
+    }
+
+    // 2. The client's test classes must have compiled (they depend on TestComponent from the test-jar).
+    File clientTestClasses = new File(basedir, "client/target/test-classes/org/apache/maven/its/mjar138/ClientTest.class");
+    if (!clientTestClasses.exists()) {
+        System.err.println("MJAR-138: client test classes not compiled (test-jar was not available in reactor): " + clientTestClasses);

Review Comment:
   Some integration tests use `assert` instead. Maybe we should be consistent?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org