You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "michael-o (via GitHub)" <gi...@apache.org> on 2024/05/28 10:52:32 UTC

[PR] First try [maven-surefire]

michael-o opened a new pull request, #737:
URL: https://github.com/apache/maven-surefire/pull/737

   Following this checklist to help us incorporate your 
   contribution quickly and easily:
   
    - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SUREFIRE) filed 
          for the change (usually before you start working on it).  Trivial changes like typos do not 
          require a JIRA issue.  Your pull request should address just this issue, without 
          pulling in other changes.
    - [ ] Each commit in the pull request should have a meaningful subject line and body.
    - [ ] 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.
    - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [ ] Run `mvn clean install` to make sure basic checks pass. A more thorough check will 
          be performed on your pull request automatically.
    - [ ] You have run the integration tests successfully (`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)
   
    - [ ] 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


Re: [PR] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#discussion_r1623496679


##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java:
##########
@@ -935,31 +932,17 @@ protected final PluginConsoleLogger getConsoleLogger() {
         return consoleLogger;
     }
 
-    private static <T extends ToolchainManager> Toolchain getToolchainMaven33x(
-            Class<T> toolchainManagerType, T toolchainManager, MavenSession session, Map<String, String> toolchainArgs)
-            throws MojoFailureException {
-        Method getToolchainsMethod =
-                tryGetMethod(toolchainManagerType, "getToolchains", MavenSession.class, String.class, Map.class);
-        if (getToolchainsMethod != null) {
-            //noinspection unchecked
-            List<Toolchain> tcs =
-                    invokeMethodWithArray(toolchainManager, getToolchainsMethod, session, "jdk", toolchainArgs);
-            if (tcs.isEmpty()) {
-                throw new MojoFailureException(
-                        "Requested toolchain specification did not match any configured toolchain: " + toolchainArgs);
-            }
-            return tcs.get(0);
-        }
-        return null;
-    }
-
-    // TODO remove the part with ToolchainManager lookup once we depend on
-    // 3.0.9 (have it as prerequisite). Define as regular component field then.
     private Toolchain getToolchain() throws MojoFailureException {
         Toolchain tc = null;
 
         if (getJdkToolchain() != null) {
-            tc = getToolchainMaven33x(ToolchainManager.class, getToolchainManager(), getSession(), getJdkToolchain());
+            List<Toolchain> tcs = getToolchainManager().getToolchains(getSession(), "jdk", getJdkToolchain());
+            if (tcs.isEmpty()) {
+                throw new MojoFailureException(
+                        "Requested toolchain specification did not match any configured toolchain: "
+                                + getJdkToolchain());
+            }
+            tc = tcs.get(0);

Review Comment:
   looks similar to reflection way



-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "sormuras (via GitHub)" <gi...@apache.org>.
sormuras commented on PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#issuecomment-2140171588

   > Outdated in terms of what?
   In lacking a line for `args.add(new Object[] {"1.10.2", "5.10.2", "1.3.0", "1.1.2"});`
   
   > Tried to following:
   
   Going from `<artifactId>junit-jupiter-api</artifactId>` to `<artifactId>junit-jupiter</artifactId>` does not help either?
   


-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "sormuras (via GitHub)" <gi...@apache.org>.
sormuras commented on PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#issuecomment-2140059001

   This IT should include a 5.10.2/1.10.2 for JUnit 5.10.2: https://github.com/apache/maven-surefire/blob/19b16d9b457fca387ef24fdb27a52eeb8970f711/surefire-its/src/test/java/org/apache/maven/surefire/its/JUnitPlatformIT.java#L58
   And soon one for 5.11.0 (could already inserted as `5.11.0-M2`).
   
   > Maven Parent 42 updated to latest JUnit 1.10.x/5.10.x the use case started to fail because of https://github.com/junit-team/junit5/commit/b41ae69659e8dc3fa230f97d8a751d81e69d06d7. Mixed dependencies, ABI incompat.
   
   I don't see the "Mixed dependencies, ABI incompat." in the linked commit. Can you please expound?
   
   


-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#issuecomment-2139799953

   @sormuras and @juliette-derancourt: Can you have a look at my changes in:
   * surefire-its/src/test/resources/junit-4-5/pom.xml
   * surefire-its/src/test/resources/junit5-suite/pom.xml
   
   Maven Parent 42 updated to latest JUnit 1.10.x/5.10.x the use case started to fail because of https://github.com/junit-team/junit5/commit/b41ae69659e8dc3fa230f97d8a751d81e69d06d7. Mixed dependencies, ABI incompat.
   
   My questions to you both:
   * Is that correct to do so in that case?
   * We have documented in `./maven-surefire-plugin/src/site/apt/examples/jpms.apt.vm`, `./maven-surefire-plugin/src/site/apt/examples/junit-platform.apt.vm`, `./maven-surefire-plugin/src/site/apt/usage.apt.vm` that only two deps need to be supplied, in contrast to that what I have in those ITs. Should we modify docs and require `junit-platform-engine` to be set as well?


-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit closed pull request #737: [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3
URL: https://github.com/apache/maven-surefire/pull/737


-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#issuecomment-2140503023

   > > Outdated in terms of what?
   > 
   > In lacking a line for `args.add(new Object[] {"1.10.2", "5.10.2", "1.3.0", "1.1.2"});`
   > 
   > > Tried to following:
   > 
   > Going from `<artifactId>junit-jupiter-api</artifactId>` to `<artifactId>junit-jupiter</artifactId>` does not help either?
   
   Did, no change. fails with:
   ```
   Caused by: org.apache.maven.surefire.booter.SurefireBooterForkException: There was an error in the forked process
   org.junit.platform.engine.TestDescriptor.getAncestors()Ljava/util/Set;
   java.lang.NoSuchMethodError: org.junit.platform.engine.TestDescriptor.getAncestors()Ljava/util/Set;
           at org.junit.platform.launcher.core.StackTracePruningEngineExecutionListener.getTestClassNames(StackTracePruningEngineExecutionListener.java:50)
           at org.junit.platform.launcher.core.StackTracePruningEngineExecutionListener.executionFinished(StackTracePruningEngineExecutionListener.java:39)
   ```


-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#issuecomment-2140277555

   > IIRC there's some mapping magic in Surefire code that tries to find the correct Platform version based on the Jupiter version used in tests. Perhaps, that magic is outdated too? 🤔
   
   That maybe true, but that is out of scope for this issue.


-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#discussion_r1620863362


##########
maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoToolchainsTest.java:
##########
@@ -106,39 +94,17 @@ public void shouldFallthroughToBuildContextWhenNoSpecSet() throws Exception {
         assertThat(actual).isSameAs(expectedFromContext);
     }
 
-    @Test
-    public void shouldReturnNoToolchainInMaven32() throws Exception {
-        Toolchain toolchain = invokeMethod(
-                AbstractSurefireMojo.class,
-                "getToolchainMaven33x",
-                MockToolchainManagerMaven32.class,
-                new MockToolchainManagerMaven32(null),
-                mock(MavenSession.class),
-                emptyMap());
-        assertNull(toolchain);
-    }
-
     @Test(expected = MojoFailureException.class)
-    public void shouldThrowMaven33xToolchain() throws Exception {
-        invokeMethod(
-                AbstractSurefireMojo.class,
-                "getToolchainMaven33x",
-                MockToolchainManager.class,
-                new MockToolchainManager(null, null),
-                mock(MavenSession.class),
-                emptyMap());
+    @Ignore
+    public void shouldThrowToolchain() throws Exception {
+        invokeMethod(AbstractSurefireMojo.class, "getToolchain");
     }
 
     @Test
-    public void shouldGetMaven33xToolchain() throws Exception {
+    @Ignore
+    public void shouldGetToolchain() throws Exception {
         Toolchain expected = mock(Toolchain.class);
-        Toolchain actual = invokeMethod(
-                AbstractSurefireMojo.class,
-                "getToolchainMaven33x",
-                MockToolchainManager.class,
-                new MockToolchainManager(expected, null),
-                mock(MavenSession.class),
-                emptyMap());
+        Toolchain actual = invokeMethod(AbstractSurefireMojo.class, "getToolchain");

Review Comment:
   I have disabled a few tests because they do not work, but I believe that they are redundant now because we don't use any reflection. I am inclined to delete all disabled ones. WDYT?



-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "sormuras (via GitHub)" <gi...@apache.org>.
sormuras commented on PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#issuecomment-2140123639

   Just found another outdated IT: https://github.com/apache/maven-surefire/blob/19b16d9b457fca387ef24fdb27a52eeb8970f711/surefire-its/src/test/java/org/apache/maven/surefire/its/JUnitPlatformEnginesIT.java#L82


-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#discussion_r1623496947


##########
maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoToolchainsTest.java:
##########
@@ -106,39 +94,17 @@ public void shouldFallthroughToBuildContextWhenNoSpecSet() throws Exception {
         assertThat(actual).isSameAs(expectedFromContext);
     }
 
-    @Test
-    public void shouldReturnNoToolchainInMaven32() throws Exception {
-        Toolchain toolchain = invokeMethod(
-                AbstractSurefireMojo.class,
-                "getToolchainMaven33x",
-                MockToolchainManagerMaven32.class,
-                new MockToolchainManagerMaven32(null),
-                mock(MavenSession.class),
-                emptyMap());
-        assertNull(toolchain);
-    }
-
     @Test(expected = MojoFailureException.class)
-    public void shouldThrowMaven33xToolchain() throws Exception {
-        invokeMethod(
-                AbstractSurefireMojo.class,
-                "getToolchainMaven33x",
-                MockToolchainManager.class,
-                new MockToolchainManager(null, null),
-                mock(MavenSession.class),
-                emptyMap());
+    @Ignore
+    public void shouldThrowToolchain() throws Exception {
+        invokeMethod(AbstractSurefireMojo.class, "getToolchain");
     }
 
     @Test
-    public void shouldGetMaven33xToolchain() throws Exception {
+    @Ignore
+    public void shouldGetToolchain() throws Exception {
         Toolchain expected = mock(Toolchain.class);
-        Toolchain actual = invokeMethod(
-                AbstractSurefireMojo.class,
-                "getToolchainMaven33x",
-                MockToolchainManager.class,
-                new MockToolchainManager(expected, null),
-                mock(MavenSession.class),
-                emptyMap());
+        Toolchain actual = invokeMethod(AbstractSurefireMojo.class, "getToolchain");

Review Comment:
   delete it



-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "sormuras (via GitHub)" <gi...@apache.org>.
sormuras commented on PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#issuecomment-2140097468

   According to https://junit.org/junit5/docs/current/user-guide/#running-tests-build-maven "it is recommended to use the JUnit Platform BOM to align the versions of all JUnit 5 artifacts." Perhaps the Surefire ITs should follow this advice.
   
   Reading on [it says](https://junit.org/junit5/docs/current/user-guide/#running-tests-build-maven-engines-configure):
   > To configure support for JUnit Jupiter based tests, configure test scoped dependencies on the JUnit Jupiter API and the JUnit Jupiter TestEngine implementation similar to the following.
   
   ```xml
       <dependency>
           <groupId>org.junit.jupiter</groupId>
           <artifactId>junit-jupiter</artifactId>
           <version>5.10.2</version> <!-- can be omitted when using the BOM -->
           <scope>test</scope>
       </dependency>
   ```
   
   


-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#issuecomment-2140161031

   > According to https://junit.org/junit5/docs/current/user-guide/#running-tests-build-maven "it is recommended to use the JUnit Platform BOM to align the versions of all JUnit 5 artifacts." Perhaps the Surefire ITs should follow this advice.
   > 
   > Reading on [it says](https://junit.org/junit5/docs/current/user-guide/#running-tests-build-maven-engines-configure):
   > 
   > > To configure support for JUnit Jupiter based tests, configure test scoped dependencies on the JUnit Jupiter API and the JUnit Jupiter TestEngine implementation similar to the following.
   > 
   > ```
   >     <dependency>
   >         <groupId>org.junit.jupiter</groupId>
   >         <artifactId>junit-jupiter</artifactId>
   >         <version>5.10.2</version> <!-- can be omitted when using the BOM -->
   >         <scope>test</scope>
   >     </dependency>
   > ```
   
   Tried to following:
   ```
   osipovmi@deblndw011x:~/var/Projekte/maven-surefire/surefire-its/src/test/resources/junit-4-5 (parent-42-maven-3.6.3 *=)
   $ git diff
   diff --git a/surefire-its/src/test/resources/junit-4-5/pom.xml b/surefire-its/src/test/resources/junit-4-5/pom.xml
   index 9a9d69c27..9dddede92 100644
   --- a/surefire-its/src/test/resources/junit-4-5/pom.xml
   +++ b/surefire-its/src/test/resources/junit-4-5/pom.xml
   @@ -135,12 +135,26 @@
                </build>
            </profile>
            <profile>
   -            <id>select-junit5</id>
   +           <id>select-junit5</id>
   +
   +
   +<dependencyManagement>
   +    <dependencies>
   +        <dependency>
   +            <groupId>org.junit</groupId>
   +            <artifactId>junit-bom</artifactId>
   +            <version>5.9.1</version>
   +            <type>pom</type>
   +            <scope>import</scope>
   +        </dependency>
   +    </dependencies>
   +</dependencyManagement>
   +
   +
                <dependencies>
                    <dependency>
                        <groupId>org.junit.jupiter</groupId>
                        <artifactId>junit-jupiter-api</artifactId>
   -                    <version>5.9.1</version>
                        <scope>test</scope>
                    </dependency>
                    <dependency>
   @@ -161,11 +175,6 @@
                                    <artifactId>junit-jupiter-engine</artifactId>
                                    <version>5.9.1</version>
                                </dependency>
   -                            <dependency>
   -                                <groupId>org.junit.platform</groupId>
   -                                <artifactId>junit-platform-engine</artifactId>
   -                                <version>1.9.1</version>
   -                          </dependency>
                            </dependencies>
                        </plugin>
                    </plugins>
   ```
   and I am back to
   ```
   [INFO] ------------------------------------------------------------------------
   [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.2.6-SNAPSHOT:test (default-test) on project junit-4-5:
   [ERROR]
   [ERROR] Please refer to /var/osipovmi/Projekte/maven-surefire/surefire-its/src/test/resources/junit-4-5/target/surefire-reports for the individual test results.
   [ERROR] Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream.
   [ERROR] There was an error in the forked process
   [ERROR] org.junit.platform.commons.util.ExceptionUtils.findNestedThrowables(Ljava/lang/Throwable;)Ljava/util/List;
   [ERROR] org.apache.maven.surefire.booter.SurefireBooterForkException: There was an error in the forked process
   [ERROR] org.junit.platform.commons.util.ExceptionUtils.findNestedThrowables(Ljava/lang/Throwable;)Ljava/util/List;
   [ERROR]         at org.apache.maven.plugin.surefire.booterclient.ForkStarter.fork(ForkStarter.java:628)
   [ERROR]         at org.apache.maven.plugin.surefire.booterclient.ForkStarter.run(ForkStarter.java:285)
   [ERROR]         at org.apache.maven.plugin.surefire.booterclient.ForkStarter.run(ForkStarter.java:250)
   [ERROR]         at org.apache.maven.plugin.surefire.AbstractSurefireMojo.executeProvider(AbstractSurefireMojo.java:1224)
   [ERROR]         at org.apache.maven.plugin.surefire.AbstractSurefireMojo.executeAfterPreconditionsChecked(AbstractSurefireMojo.java:1073)
   ```
   because of the plugin classpath:
   ```
   [DEBUG] Populating class realm plugin>org.apache.maven.plugins:maven-surefire-plugin:3.2.6-SNAPSHOT
   [DEBUG]   Included: org.apache.maven.plugins:maven-surefire-plugin:jar:3.2.6-SNAPSHOT
   [DEBUG]   Included: org.junit.jupiter:junit-jupiter-engine:jar:5.9.1
   [DEBUG]   Included: org.junit.platform:junit-platform-engine:jar:1.10.2
   [DEBUG]   Included: org.opentest4j:opentest4j:jar:1.3.0
   [DEBUG]   Included: org.junit.platform:junit-platform-commons:jar:1.10.2
   [DEBUG]   Included: org.junit.jupiter:junit-jupiter-api:jar:5.10.2
   [DEBUG]   Included: org.apiguardian:apiguardian-api:jar:1.1.2
   [DEBUG]   Included: org.apache.maven.surefire:maven-surefire-common:jar:3.2.6-SNAPSHOT
   [DEBUG]   Included: org.apache.maven.surefire:surefire-api:jar:3.2.6-SNAPSHOT
   [DEBUG]   Included: org.apache.maven.surefire:surefire-logger-api:jar:3.2.6-SNAPSHOT
   [DEBUG]   Included: org.apache.maven.surefire:surefire-extensions-api:jar:3.2.6-SNAPSHOT
   [DEBUG]   Included: org.apache.maven.surefire:surefire-booter:jar:3.2.6-SNAPSHOT
   [DEBUG]   Included: org.apache.maven.surefire:surefire-extensions-spi:jar:3.2.6-SNAPSHOT
   [DEBUG]   Included: org.apache.maven.resolver:maven-resolver-util:jar:1.4.1
   [DEBUG]   Included: org.apache.maven.shared:maven-common-artifact-filters:jar:3.1.1
   [DEBUG]   Included: commons-io:commons-io:jar:2.16.1
   [DEBUG]   Included: org.codehaus.plexus:plexus-java:jar:1.2.0
   [DEBUG]   Included: org.ow2.asm:asm:jar:9.6
   [DEBUG]   Included: com.thoughtworks.qdox:qdox:jar:2.0.3
   [DEBUG]   Included: org.apache.maven.surefire:surefire-shared-utils:jar:3.2.6-SNAPSHOT
   ```
   with the fix it is:
   ```
   [DEBUG] Populating class realm plugin>org.apache.maven.plugins:maven-surefire-plugin:3.2.6-SNAPSHOT
   [DEBUG]   Included: org.apache.maven.plugins:maven-surefire-plugin:jar:3.2.6-SNAPSHOT
   [DEBUG]   Included: org.junit.jupiter:junit-jupiter-engine:jar:5.9.1
   [DEBUG]   Included: org.junit.jupiter:junit-jupiter-api:jar:5.10.2
   [DEBUG]   Included: org.apiguardian:apiguardian-api:jar:1.1.2
   [DEBUG]   Included: org.junit.platform:junit-platform-engine:jar:1.9.1
   [DEBUG]   Included: org.opentest4j:opentest4j:jar:1.2.0
   [DEBUG]   Included: org.junit.platform:junit-platform-commons:jar:1.10.2
   [DEBUG]   Included: org.apache.maven.surefire:maven-surefire-common:jar:3.2.6-SNAPSHOT
   [DEBUG]   Included: org.apache.maven.surefire:surefire-api:jar:3.2.6-SNAPSHOT
   [DEBUG]   Included: org.apache.maven.surefire:surefire-logger-api:jar:3.2.6-SNAPSHOT
   [DEBUG]   Included: org.apache.maven.surefire:surefire-extensions-api:jar:3.2.6-SNAPSHOT
   [DEBUG]   Included: org.apache.maven.surefire:surefire-booter:jar:3.2.6-SNAPSHOT
   [DEBUG]   Included: org.apache.maven.surefire:surefire-extensions-spi:jar:3.2.6-SNAPSHOT
   [DEBUG]   Included: org.apache.maven.resolver:maven-resolver-util:jar:1.4.1
   [DEBUG]   Included: org.apache.maven.shared:maven-common-artifact-filters:jar:3.1.1
   [DEBUG]   Included: commons-io:commons-io:jar:2.16.1
   [DEBUG]   Included: org.codehaus.plexus:plexus-java:jar:1.2.0
   [DEBUG]   Included: org.ow2.asm:asm:jar:9.6
   [DEBUG]   Included: com.thoughtworks.qdox:qdox:jar:2.0.3
   [DEBUG]   Included: org.apache.maven.surefire:surefire-shared-utils:jar:3.2.6-SNAPSHOT
   ```


-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#issuecomment-2140086579

   > This IT should include a 5.10.2/1.8.5 row for JUnit 5.10.2 and JQwik 1.8.5:
   > 
   > https://github.com/apache/maven-surefire/blob/19b16d9b457fca387ef24fdb27a52eeb8970f711/surefire-its/src/test/java/org/apache/maven/surefire/its/JUnitPlatformIT.java#L58
   > 
   > And soon one for JUnit 5.11.0 (could already inserted as `5.11.0-M2`) too.
   
   Will do.
   
   > > Maven Parent 42 updated to latest JUnit 1.10.x/5.10.x the use case started to fail because of [junit-team/junit5@b41ae69](https://github.com/junit-team/junit5/commit/b41ae69659e8dc3fa230f97d8a751d81e69d06d7). Mixed dependencies, ABI incompat.
   > 
   > I don't see the "Mixed dependencies, ABI incompat." in the linked commit. Can you please expound?
   
   When you run the ITs manually from the dir to analyze the failure you see this classpath:
   ```
   common-java5-3.2.6-SNAPSHOT.jar
   junit-jupiter-engine-5.9.3.jar
   junit-platform-engine-1.10.2.jar
   junit-platform-launcher-1.10.2.jar
   junit-platform-suite-1.9.3.jar
   junit-platform-suite-commons-1.9.3.jar
   junit-platform-suite-engine-1.9.3.jar
   maven-surefire-common-3.2.6-SNAPSHOT.jar
   surefire-api-3.2.6-SNAPSHOT.jar
   surefire-booter-3.2.6-SNAPSHOT.jar
   surefire-extensions-api-3.2.6-SNAPSHOT.jar
   surefire-extensions-spi-3.2.6-SNAPSHOT.jar
   surefire-junit-platform-3.2.6-SNAPSHOT.jar
   surefire-logger-api-3.2.6-SNAPSHOT.jar
   surefire-shared-utils-3.2.6-SNAPSHOT.jar
   ```
   
   The mix of 1.10.2 and 1.9.3/5.9.3 don't play nice together, if you want I can procude the error for you. I had this signature missing due to the broken CP: `org.junit.platform.commons.util.ExceptionUtils.findNestedThrowables(Ljava/lang/Throwable;)Ljava/util/List;`


-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#discussion_r1620861999


##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java:
##########
@@ -935,31 +932,17 @@ protected final PluginConsoleLogger getConsoleLogger() {
         return consoleLogger;
     }
 
-    private static <T extends ToolchainManager> Toolchain getToolchainMaven33x(
-            Class<T> toolchainManagerType, T toolchainManager, MavenSession session, Map<String, String> toolchainArgs)
-            throws MojoFailureException {
-        Method getToolchainsMethod =
-                tryGetMethod(toolchainManagerType, "getToolchains", MavenSession.class, String.class, Map.class);
-        if (getToolchainsMethod != null) {
-            //noinspection unchecked
-            List<Toolchain> tcs =
-                    invokeMethodWithArray(toolchainManager, getToolchainsMethod, session, "jdk", toolchainArgs);
-            if (tcs.isEmpty()) {
-                throw new MojoFailureException(
-                        "Requested toolchain specification did not match any configured toolchain: " + toolchainArgs);
-            }
-            return tcs.get(0);
-        }
-        return null;
-    }
-
-    // TODO remove the part with ToolchainManager lookup once we depend on
-    // 3.0.9 (have it as prerequisite). Define as regular component field then.
     private Toolchain getToolchain() throws MojoFailureException {
         Toolchain tc = null;
 
         if (getJdkToolchain() != null) {
-            tc = getToolchainMaven33x(ToolchainManager.class, getToolchainManager(), getSession(), getJdkToolchain());
+            List<Toolchain> tcs = getToolchainManager().getToolchains(getSession(), "jdk", getJdkToolchain());
+            if (tcs.isEmpty()) {
+                throw new MojoFailureException(
+                        "Requested toolchain specification did not match any configured toolchain: "
+                                + getJdkToolchain());
+            }
+            tc = tcs.get(0);

Review Comment:
   Guys, please check wether this is correct logically...



-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#issuecomment-2140151794

   > Just found another outdated IT:
   > 
   > https://github.com/apache/maven-surefire/blob/19b16d9b457fca387ef24fdb27a52eeb8970f711/surefire-its/src/test/java/org/apache/maven/surefire/its/JUnitPlatformEnginesIT.java#L82
   
   Outdated in terms of what?


-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#issuecomment-2140535713

   @sormuras, here is the magic:
   https://github.com/apache/maven-surefire/blob/19b16d9b457fca387ef24fdb27a52eeb8970f711/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java#L2929-L3090


-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#issuecomment-2140566960

   This works:
   ```
   $ git diff
   diff --git a/surefire-its/src/test/resources/junit-4-5/pom.xml b/surefire-its/src/test/resources/junit-4-5/pom.xml
   index 9a9d69c27..e55fd9b4e 100644
   --- a/surefire-its/src/test/resources/junit-4-5/pom.xml
   +++ b/surefire-its/src/test/resources/junit-4-5/pom.xml
   @@ -139,7 +139,7 @@
                <dependencies>
                    <dependency>
                        <groupId>org.junit.jupiter</groupId>
   -                    <artifactId>junit-jupiter-api</artifactId>
   +                    <artifactId>junit-jupiter</artifactId>
                        <version>5.9.1</version>
                        <scope>test</scope>
                    </dependency>
   @@ -155,18 +155,6 @@
                        <plugin>
                            <groupId>org.apache.maven.plugins</groupId>
                            <artifactId>maven-surefire-plugin</artifactId>
   -                        <dependencies>
   -                            <dependency>
   -                                <groupId>org.junit.jupiter</groupId>
   -                                <artifactId>junit-jupiter-engine</artifactId>
   -                                <version>5.9.1</version>
   -                            </dependency>
   -                            <dependency>
   -                                <groupId>org.junit.platform</groupId>
   -                                <artifactId>junit-platform-engine</artifactId>
   -                                <version>1.9.1</version>
   -                          </dependency>
   -                        </dependencies>
                        </plugin>
                    </plugins>
                </build>
   ```


-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "sormuras (via GitHub)" <gi...@apache.org>.
sormuras commented on PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#issuecomment-2140177155

   IIRC there's some mapping magic in Surefire code that tries to find the correct Platform version based on the Jupiter version used in tests. Perhaps, that magic is outdated too? 🤔 


-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#issuecomment-2140642939

   > This works:
   > 
   > ```
   > $ git diff
   > diff --git a/surefire-its/src/test/resources/junit-4-5/pom.xml b/surefire-its/src/test/resources/junit-4-5/pom.xml
   > index 9a9d69c27..e55fd9b4e 100644
   > --- a/surefire-its/src/test/resources/junit-4-5/pom.xml
   > +++ b/surefire-its/src/test/resources/junit-4-5/pom.xml
   > @@ -139,7 +139,7 @@
   >              <dependencies>
   >                  <dependency>
   >                      <groupId>org.junit.jupiter</groupId>
   > -                    <artifactId>junit-jupiter-api</artifactId>
   > +                    <artifactId>junit-jupiter</artifactId>
   >                      <version>5.9.1</version>
   >                      <scope>test</scope>
   >                  </dependency>
   > @@ -155,18 +155,6 @@
   >                      <plugin>
   >                          <groupId>org.apache.maven.plugins</groupId>
   >                          <artifactId>maven-surefire-plugin</artifactId>
   > -                        <dependencies>
   > -                            <dependency>
   > -                                <groupId>org.junit.jupiter</groupId>
   > -                                <artifactId>junit-jupiter-engine</artifactId>
   > -                                <version>5.9.1</version>
   > -                            </dependency>
   > -                            <dependency>
   > -                                <groupId>org.junit.platform</groupId>
   > -                                <artifactId>junit-platform-engine</artifactId>
   > -                                <version>1.9.1</version>
   > -                          </dependency>
   > -                        </dependencies>
   >                      </plugin>
   >                  </plugins>
   >              </build>
   > ```
   > 
   > I'll try that...
   
   Tried, while it works it gives a different classpath, thus different results. Profile `select-junit4` does not select 4. I will leave as-is now.


-- 
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] [SUREFIRE-2245] Upgrade to Parent 42 and Maven 3.6.3 [maven-surefire]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #737:
URL: https://github.com/apache/maven-surefire/pull/737#discussion_r1623497355


##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java:
##########
@@ -935,31 +932,17 @@ protected final PluginConsoleLogger getConsoleLogger() {
         return consoleLogger;
     }
 
-    private static <T extends ToolchainManager> Toolchain getToolchainMaven33x(
-            Class<T> toolchainManagerType, T toolchainManager, MavenSession session, Map<String, String> toolchainArgs)
-            throws MojoFailureException {
-        Method getToolchainsMethod =
-                tryGetMethod(toolchainManagerType, "getToolchains", MavenSession.class, String.class, Map.class);
-        if (getToolchainsMethod != null) {
-            //noinspection unchecked
-            List<Toolchain> tcs =
-                    invokeMethodWithArray(toolchainManager, getToolchainsMethod, session, "jdk", toolchainArgs);
-            if (tcs.isEmpty()) {
-                throw new MojoFailureException(
-                        "Requested toolchain specification did not match any configured toolchain: " + toolchainArgs);
-            }
-            return tcs.get(0);
-        }
-        return null;
-    }
-
-    // TODO remove the part with ToolchainManager lookup once we depend on
-    // 3.0.9 (have it as prerequisite). Define as regular component field then.
     private Toolchain getToolchain() throws MojoFailureException {
         Toolchain tc = null;
 
         if (getJdkToolchain() != null) {
-            tc = getToolchainMaven33x(ToolchainManager.class, getToolchainManager(), getSession(), getJdkToolchain());
+            List<Toolchain> tcs = getToolchainManager().getToolchains(getSession(), "jdk", getJdkToolchain());
+            if (tcs.isEmpty()) {
+                throw new MojoFailureException(
+                        "Requested toolchain specification did not match any configured toolchain: "
+                                + getJdkToolchain());
+            }
+            tc = tcs.get(0);

Review Comment:
   True, but simply hides code complexity. It is has no further use. I wouldn't necessarily delete it.



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