Copilot commented on code in PR #3547: URL: https://github.com/apache/maven-surefire/pull/3547#discussion_r4441621040
########## surefire-providers/surefire-junit-platform/src/test/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProviderRunOrderTest.java: ########## @@ -0,0 +1,261 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.surefire.junitplatform; + +import java.io.PrintStream; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; + +import org.apache.maven.surefire.api.provider.ProviderParameters; +import org.apache.maven.surefire.api.report.ReporterFactory; +import org.apache.maven.surefire.api.report.TestOutputReceiver; +import org.apache.maven.surefire.api.report.TestOutputReportEntry; +import org.apache.maven.surefire.api.report.TestReportListener; +import org.apache.maven.surefire.api.report.TestSetReportEntry; +import org.apache.maven.surefire.api.testset.RunOrderParameters; +import org.apache.maven.surefire.api.testset.TestListResolver; +import org.apache.maven.surefire.api.testset.TestRequest; +import org.apache.maven.surefire.api.testset.TestSetFailedException; +import org.apache.maven.surefire.api.util.DefaultRunOrderCalculator; +import org.apache.maven.surefire.api.util.RunOrderCalculator; +import org.apache.maven.surefire.api.util.ScanResult; +import org.apache.maven.surefire.api.util.TestsToRun; +import org.junit.jupiter.api.Test; +import org.junit.platform.launcher.LauncherSession; +import org.junit.platform.launcher.core.LauncherConfig; +import org.junit.platform.launcher.core.LauncherFactory; +import org.mockito.ArgumentCaptor; + +import static org.apache.maven.surefire.api.booter.ProviderParameterNames.JUNIT4_ONLY_DETECTED; +import static org.apache.maven.surefire.api.booter.ProviderParameterNames.JUNIT_VINTAGE_DETECTED; +import static org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.CLASS_ORDER_DEFAULT; +import static org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.CONFIGURATION_PARAMETERS; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.withSettings; + +/** + * Tests how the provider splits the class ordering with Jupiter for {@code runOrder=random} + * (issue 3544: both sides shuffled with the same seed, which composed the permutation with + * itself and made some class orders unreachable, two classes always ran alphabetically). + */ +public class JUnitPlatformProviderRunOrderTest { Review Comment: The root POM runs test classes concurrently, but `invokeProvider()` replaces and restores JVM-wide stdout/stderr, so overlapping execution can misroute output and fail `JUnitPlatformProviderTest.outputIsCaptured()`. These tests can also race that class's temporary class-orderer system property, unexpectedly disabling the new ordering branch. Add `@Isolated` to prevent both races. ########## surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProvider.java: ########## @@ -281,9 +294,49 @@ private TestsToRun scanClasspath(LauncherAdapter launcher) { TestPlanScannerFilter filter = new TestPlanScannerFilter(launcher, allFilters(), candidateClassNames, testNgClassNames); TestsToRun scannedClasses = scanResult.applyFilter(filter, parameters.getTestClassLoader()); + return orderTestClasses(scannedClasses, classesReachJupiterInOneRequest && testNgClassNames.isEmpty()); + } + + /** + * Orders the scanned classes for the discovery request(s) they run in. + * <p> + * When all classes reach Jupiter in one discovery request and this provider configured its class orderer + * for {@code runOrder=random}, Jupiter shuffles the whole request with the seed of + * {@code runOrder.random.seed}. The classes must be handed over sorted then, so that Jupiter applies the + * single shuffle: the run order calculator shuffles with the same seed, so letting it shuffle as well + * would compose the permutation with itself, which can never produce some class orders and made two test + * classes always run alphabetically. + * + * @param jupiterShufflesTheClasses whether the class orderer of Jupiter re-orders every class, which is + * the case when they all go to the platform in one discovery request and none of them is a TestNG + * test + * @see <a href="https://github.com/apache/maven-surefire/issues/3544";>Issue 3544</a> + */ + private TestsToRun orderTestClasses(TestsToRun scannedClasses, boolean jupiterShufflesTheClasses) { + if (jupiterShufflesTheClasses + && classOrdererSetByProvider + && !runsOneClassAtATime() + && !isVintageEngineDetected()) { + return sortedAlphabetically(scannedClasses); Review Comment: `JUNIT_VINTAGE_DETECTED` is set only when `AbstractSurefireMojo` automatically adds Vintage, so an explicit `junit-vintage-engine` dependency passes this guard. Those JUnit 4 classes now receive alphabetical ordering instead of the calculator's shuffle, and Jupiter cannot randomize them; other non-Jupiter engines can also pass these guards. Bypass the calculator only when the discovered classes are confirmed to be Jupiter-owned, and add a regression test with an explicit Vintage 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
