You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/02/09 10:28:21 UTC

[GitHub] [maven-surefire] mthmulders opened a new pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

mthmulders opened a new pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463


   The `includeAllProviders` flag that we flip to "true" here lets Plexus Java detect all JPMS modules that "provide" a service that any module we already depend on "uses". This fixes [SUREFIRE-1993](https://issues.apache.org/jira/browse/SUREFIRE-1993)
   
    - [X] Each commit in the pull request should have a meaningful subject line and body.
    - [X] Format the pull request title like `[SUREFIRE-XXX] - Fixes bug in ApproximateQuantiles`,
          where you replace `SUREFIRE-XXX` with the appropriate JIRA issue. Best practice
          is to use the JIRA issue title in the pull request title and in the first line of the 
          commit message.
    - [X] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [X] Run `mvn clean install` to make sure basic checks pass. A more thorough check will 
          be performed on your pull request automatically.
    - [X] You have run the integration tests successfully (`mvn -Prun-its clean install`).
   
   If your pull request is about ~20 lines of code you don't need to sign an
   [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
   please ask on the developers list.
   
   To make clear that you license your contribution under 
   the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   you have to acknowledge this by using the following check-box.
   
    - [ ] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   
    - [X] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   


-- 
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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#discussion_r802542583



##########
File path: surefire-its/src/test/resources/surefire-1993-jpms-providing-modules/application/pom.xml
##########
@@ -0,0 +1,94 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+~ 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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>org.apache.maven.plugins.surefire</groupId>
+        <artifactId>surefire-1993-jpms-providing-modules</artifactId>
+        <version>1.0-SNAPSHOT</version>
+    </parent>
+
+    <groupId>org.apache.plugins.failsafe.its</groupId>
+    <artifactId>application</artifactId>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.plugins.failsafe.its</groupId>
+            <artifactId>studentservice</artifactId>
+            <version>1.0-SNAPSHOT</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.plugins.failsafe.its</groupId>
+            <artifactId>studentservice-provider</artifactId>
+            <version>1.0-SNAPSHOT</version>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-api</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-engine</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+        </dependency>
+    </dependencies>
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <configuration>
+                    <argLine>
+                        --add-modules ALL-DEFAULT
+                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED
+                    </argLine>
+                </configuration>
+            </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-failsafe-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>integration-test</goal>
+                            <goal>verify</goal>
+                        </goals>
+                    </execution>
+                </executions>
+                <configuration>
+                    <argLine>
+                        --add-modules ALL-DEFAULT
+                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED

Review comment:
       What would happen without `--add-opens`? We have another PR which opens modulepath to surefire libraries.




-- 
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



[GitHub] [maven-surefire] mthmulders commented on pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
mthmulders commented on pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#issuecomment-1036168881


   Closed with 6ea957ef.


-- 
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



[GitHub] [maven-surefire] mthmulders commented on a change in pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#discussion_r802996198



##########
File path: surefire-its/src/test/resources/surefire-1993-jpms-providing-modules/application/pom.xml
##########
@@ -59,16 +59,6 @@
     </dependencies>
     <build>
         <plugins>
-            <plugin>
-                <groupId>org.apache.maven.plugins</groupId>
-                <artifactId>maven-surefire-plugin</artifactId>
-                <configuration>
-                    <argLine>
-                        --add-modules ALL-DEFAULT
-                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED
-                    </argLine>
-                </configuration>
-            </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-failsafe-plugin</artifactId>

Review comment:
       Just to be sure: squash the commits and then force-push to the same branch?




-- 
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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#discussion_r802984399



##########
File path: surefire-its/src/test/resources/surefire-1993-jpms-providing-modules/application/pom.xml
##########
@@ -59,16 +59,6 @@
     </dependencies>
     <build>
         <plugins>
-            <plugin>
-                <groupId>org.apache.maven.plugins</groupId>
-                <artifactId>maven-surefire-plugin</artifactId>
-                <configuration>
-                    <argLine>
-                        --add-modules ALL-DEFAULT
-                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED
-                    </argLine>
-                </configuration>
-            </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-failsafe-plugin</artifactId>

Review comment:
       ah, ok, I understand that you removed [surefire plugin in the commit](https://github.com/apache/maven-surefire/pull/463/commits/a178700f52d4163deffac51b7bde5d18795c0bf6) so there is now only one plugin - Failsafe without the config. Just fine! I tought that the Surefire was intact - my bad!




-- 
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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#discussion_r802985031



##########
File path: surefire-its/src/test/resources/surefire-1993-jpms-providing-modules/application/pom.xml
##########
@@ -59,16 +59,6 @@
     </dependencies>
     <build>
         <plugins>
-            <plugin>
-                <groupId>org.apache.maven.plugins</groupId>
-                <artifactId>maven-surefire-plugin</artifactId>
-                <configuration>
-                    <argLine>
-                        --add-modules ALL-DEFAULT
-                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED
-                    </argLine>
-                </configuration>
-            </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-failsafe-plugin</artifactId>

Review comment:
       So, you can now squash the commits and we can make a final review.




-- 
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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#discussion_r802937554



##########
File path: surefire-its/src/test/resources/surefire-1993-jpms-providing-modules/application/pom.xml
##########
@@ -59,16 +59,6 @@
     </dependencies>
     <build>
         <plugins>
-            <plugin>
-                <groupId>org.apache.maven.plugins</groupId>
-                <artifactId>maven-surefire-plugin</artifactId>
-                <configuration>
-                    <argLine>
-                        --add-modules ALL-DEFAULT
-                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED
-                    </argLine>
-                </configuration>
-            </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-failsafe-plugin</artifactId>

Review comment:
       The Failsafe still has `--add-opens`?




-- 
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



[GitHub] [maven-surefire] mthmulders commented on a change in pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#discussion_r802579517



##########
File path: surefire-its/src/test/resources/surefire-1993-jpms-providing-modules/application/pom.xml
##########
@@ -0,0 +1,94 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+~ 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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>org.apache.maven.plugins.surefire</groupId>
+        <artifactId>surefire-1993-jpms-providing-modules</artifactId>
+        <version>1.0-SNAPSHOT</version>
+    </parent>
+
+    <groupId>org.apache.plugins.failsafe.its</groupId>
+    <artifactId>application</artifactId>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.plugins.failsafe.its</groupId>
+            <artifactId>studentservice</artifactId>
+            <version>1.0-SNAPSHOT</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.plugins.failsafe.its</groupId>
+            <artifactId>studentservice-provider</artifactId>
+            <version>1.0-SNAPSHOT</version>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-api</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-engine</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+        </dependency>
+    </dependencies>
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <configuration>
+                    <argLine>
+                        --add-modules ALL-DEFAULT
+                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED
+                    </argLine>
+                </configuration>
+            </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-failsafe-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>integration-test</goal>
+                            <goal>verify</goal>
+                        </goals>
+                    </execution>
+                </executions>
+                <configuration>
+                    <argLine>
+                        --add-modules ALL-DEFAULT
+                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED
+                    </argLine>
+                    <forkCount>1</forkCount>
+                    <forkMode>once</forkMode>

Review comment:
       When I remove them, the test fails with
   
   ```
   [WARNING] The parameter forkMode is deprecated since version 2.14. Use forkCount and reuseForks instead.
   [WARNING] useSystemClassLoader setting has no effect when not forking
   [WARNING] The parameter forkCount should likely not be 0. Forking a JVM for tests improves test accuracy. Ensure to have a <forkCount> >= 1.
   ```
   
   I guess this means that Failsafe is not forking a JVM and hence not setting the module path, could that be the case?




-- 
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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#discussion_r802540211



##########
File path: surefire-its/src/test/resources/surefire-1993-jpms-providing-modules/application/pom.xml
##########
@@ -0,0 +1,94 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+~ 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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>org.apache.maven.plugins.surefire</groupId>
+        <artifactId>surefire-1993-jpms-providing-modules</artifactId>
+        <version>1.0-SNAPSHOT</version>
+    </parent>
+
+    <groupId>org.apache.plugins.failsafe.its</groupId>
+    <artifactId>application</artifactId>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.plugins.failsafe.its</groupId>
+            <artifactId>studentservice</artifactId>
+            <version>1.0-SNAPSHOT</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.plugins.failsafe.its</groupId>
+            <artifactId>studentservice-provider</artifactId>
+            <version>1.0-SNAPSHOT</version>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-api</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-engine</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+        </dependency>
+    </dependencies>
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <configuration>
+                    <argLine>
+                        --add-modules ALL-DEFAULT
+                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED
+                    </argLine>
+                </configuration>
+            </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-failsafe-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>integration-test</goal>
+                            <goal>verify</goal>
+                        </goals>
+                    </execution>
+                </executions>
+                <configuration>
+                    <argLine>
+                        --add-modules ALL-DEFAULT
+                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED
+                    </argLine>
+                    <forkCount>1</forkCount>
+                    <forkMode>once</forkMode>

Review comment:
       These two parameters are not necessary because they have default values here.




-- 
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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#discussion_r802545449



##########
File path: surefire-its/src/test/resources/surefire-1993-jpms-providing-modules/application/pom.xml
##########
@@ -0,0 +1,94 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+~ 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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>org.apache.maven.plugins.surefire</groupId>
+        <artifactId>surefire-1993-jpms-providing-modules</artifactId>
+        <version>1.0-SNAPSHOT</version>
+    </parent>
+
+    <groupId>org.apache.plugins.failsafe.its</groupId>
+    <artifactId>application</artifactId>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.plugins.failsafe.its</groupId>
+            <artifactId>studentservice</artifactId>
+            <version>1.0-SNAPSHOT</version>

Review comment:
       All the `1.0-SNAPSHOT` can be nicely replaced by `${project.version}`. Why you use hard coded value?




-- 
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



[GitHub] [maven-surefire] mthmulders commented on a change in pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#discussion_r803399980



##########
File path: surefire-its/src/test/resources/surefire-1993-jpms-providing-modules/application/pom.xml
##########
@@ -59,16 +59,6 @@
     </dependencies>
     <build>
         <plugins>
-            <plugin>
-                <groupId>org.apache.maven.plugins</groupId>
-                <artifactId>maven-surefire-plugin</artifactId>
-                <configuration>
-                    <argLine>
-                        --add-modules ALL-DEFAULT
-                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED
-                    </argLine>
-                </configuration>
-            </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-failsafe-plugin</artifactId>

Review comment:
       Thanks @Tibor17 - squashed and force-pushed.




-- 
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



[GitHub] [maven-surefire] mthmulders commented on a change in pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#discussion_r802968319



##########
File path: surefire-its/src/test/resources/surefire-1993-jpms-providing-modules/application/pom.xml
##########
@@ -59,16 +59,6 @@
     </dependencies>
     <build>
         <plugins>
-            <plugin>
-                <groupId>org.apache.maven.plugins</groupId>
-                <artifactId>maven-surefire-plugin</artifactId>
-                <configuration>
-                    <argLine>
-                        --add-modules ALL-DEFAULT
-                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED
-                    </argLine>
-                </configuration>
-            </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-failsafe-plugin</artifactId>

Review comment:
       Not sure what you mean... there's no `--add-opens` left here, as far as I can see.




-- 
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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#discussion_r802589249



##########
File path: surefire-its/src/test/resources/surefire-1993-jpms-providing-modules/application/pom.xml
##########
@@ -0,0 +1,94 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+~ 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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>org.apache.maven.plugins.surefire</groupId>
+        <artifactId>surefire-1993-jpms-providing-modules</artifactId>
+        <version>1.0-SNAPSHOT</version>
+    </parent>
+
+    <groupId>org.apache.plugins.failsafe.its</groupId>
+    <artifactId>application</artifactId>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.plugins.failsafe.its</groupId>
+            <artifactId>studentservice</artifactId>
+            <version>1.0-SNAPSHOT</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.plugins.failsafe.its</groupId>
+            <artifactId>studentservice-provider</artifactId>
+            <version>1.0-SNAPSHOT</version>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-api</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-engine</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+        </dependency>
+    </dependencies>
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <configuration>
+                    <argLine>
+                        --add-modules ALL-DEFAULT
+                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED
+                    </argLine>
+                </configuration>
+            </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-failsafe-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>integration-test</goal>
+                            <goal>verify</goal>
+                        </goals>
+                    </execution>
+                </executions>
+                <configuration>
+                    <argLine>
+                        --add-modules ALL-DEFAULT
+                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED
+                    </argLine>
+                    <forkCount>1</forkCount>
+                    <forkMode>once</forkMode>

Review comment:
       Ah ok, I know what you mean. It's because of the [parent](https://github.com/apache/maven-surefire/blob/master/surefire-its/src/test/resources/pom.xml#L23). It would be nice to remove these parameters but nobody has spare time to make impact analysis in those ITs wjich inherit this parent and move these parameters into particular target POM of the ITs. If you like, you can keep it in this PR as it is and open another PR where all such cases will be fixed.
   
   ```
       <parent>
           <groupId>org.apache.maven.surefire</groupId>
           <artifactId>it-parent</artifactId>
           <version>1.0</version>
           <relativePath>../pom.xml</relativePath>
       </parent>
   ```




-- 
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



[GitHub] [maven-surefire] mthmulders commented on a change in pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
mthmulders commented on a change in pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#discussion_r802577435



##########
File path: surefire-its/src/test/resources/surefire-1993-jpms-providing-modules/application/pom.xml
##########
@@ -0,0 +1,94 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+~ 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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>org.apache.maven.plugins.surefire</groupId>
+        <artifactId>surefire-1993-jpms-providing-modules</artifactId>
+        <version>1.0-SNAPSHOT</version>
+    </parent>
+
+    <groupId>org.apache.plugins.failsafe.its</groupId>
+    <artifactId>application</artifactId>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.plugins.failsafe.its</groupId>
+            <artifactId>studentservice</artifactId>
+            <version>1.0-SNAPSHOT</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.plugins.failsafe.its</groupId>
+            <artifactId>studentservice-provider</artifactId>
+            <version>1.0-SNAPSHOT</version>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-api</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-engine</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+        </dependency>
+    </dependencies>
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <configuration>
+                    <argLine>
+                        --add-modules ALL-DEFAULT
+                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED
+                    </argLine>
+                </configuration>
+            </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-failsafe-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>integration-test</goal>
+                            <goal>verify</goal>
+                        </goals>
+                    </execution>
+                </executions>
+                <configuration>
+                    <argLine>
+                        --add-modules ALL-DEFAULT
+                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED

Review comment:
       For this integration test, it doesn't make a difference. It's a left-over from the original reproducer project. I'll remove the `--add-opens`. The same applies for the `--add-modules` by the way, so I'll remove that, too.

##########
File path: surefire-its/src/test/resources/surefire-1993-jpms-providing-modules/application/pom.xml
##########
@@ -0,0 +1,94 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+~ 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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>org.apache.maven.plugins.surefire</groupId>
+        <artifactId>surefire-1993-jpms-providing-modules</artifactId>
+        <version>1.0-SNAPSHOT</version>
+    </parent>
+
+    <groupId>org.apache.plugins.failsafe.its</groupId>
+    <artifactId>application</artifactId>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.plugins.failsafe.its</groupId>
+            <artifactId>studentservice</artifactId>
+            <version>1.0-SNAPSHOT</version>

Review comment:
       There is no particular reason, I'll change it to `${project.version}`.




-- 
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



[GitHub] [maven-surefire] Tibor17 commented on pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#issuecomment-1033680668


   @mthmulders 
   One IT failed because of the parameter `release` on JDK8, see `maven-compiler-plugin`. Pls have a look. thx


-- 
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



[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
Tibor17 edited a comment on pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#issuecomment-1033680668


   @mthmulders 
   One IT failed because of the parameter `release` on JDK8, see `maven-compiler-plugin`. Pls have a look. thx
   We use `assumeJavaVersion( 1.8d )` in the IT for these purposes.


-- 
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



[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
Tibor17 edited a comment on pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#issuecomment-1033680668


   @mthmulders 
   One IT failed because of the parameter `release` on JDK8, see `maven-compiler-plugin`. Pls have a look. thx
   We use `assumeJavaVersion( 9 )` in the IT for these purposes.


-- 
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



[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463#discussion_r803064447



##########
File path: surefire-its/src/test/resources/surefire-1993-jpms-providing-modules/application/pom.xml
##########
@@ -59,16 +59,6 @@
     </dependencies>
     <build>
         <plugins>
-            <plugin>
-                <groupId>org.apache.maven.plugins</groupId>
-                <artifactId>maven-surefire-plugin</artifactId>
-                <configuration>
-                    <argLine>
-                        --add-modules ALL-DEFAULT
-                        --add-opens application/org.apache.failsafe.its.serviceloader.application=ALL-UNNAMED
-                    </argLine>
-                </configuration>
-            </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-failsafe-plugin</artifactId>

Review comment:
       @mthmulders yes




-- 
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



[GitHub] [maven-surefire] mthmulders closed pull request #463: [SUREFIRE-1993] Discover providing modules from dependencies

Posted by GitBox <gi...@apache.org>.
mthmulders closed pull request #463:
URL: https://github.com/apache/maven-surefire/pull/463


   


-- 
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