You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "wilx (via GitHub)" <gi...@apache.org> on 2026/08/08 21:23:02 UTC
[PR] [SUREFIRE-2241] Preserve JUnit Platform test identity across reruns [maven-surefire]
wilx opened a new pull request, #3418:
URL: https://github.com/apache/maven-surefire/pull/3418
Following this checklist to help us incorporate your
contribution quickly and easily:
- [x] Each commit in the pull request should have a meaningful subject line and body.
- [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.
- [x] 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).
## Summary
Fixes #2686.
Fixes #2670.
JUnit Platform tests were aggregated by their human-readable class and method names. Engines can legitimately report multiple logical tests with the same names, and JUnitParams on the Vintage engine reports actual tests with a `ClassSource` but no method source. During reruns this collapsed skipped, failing, and successful executions into one apparent flake, could generate an empty XML element name, and could hide real failures.
This change:
- uses the JUnit Platform unique ID as the internal test-run identity while retaining readable report names;
- propagates that identity into XML and global result aggregation;
- restores cached IDs to the output association;
- gives `ClassSource`-backed tests a readable test name;
- serializes skipped executions defensively in a flake history; and
- adds exact JUnitParams and Cucumber regression projects.
An existing integration fixture containing two distinct tests with the same method name now correctly expects both tests in its report.
## User impact
Identically named tests and scenarios remain distinct. The JUnitParams reproducer reports its failures, returns a failing Maven result, writes valid XML, and continues to the following test class. The Cucumber reproducer reports three scenarios as one skipped, one failed, and one passed test.
## Tests
- `mvn clean install`
- `JAVA_HOME=/opt/jdks/latest-17 mvn -Prun-its clean install` (761 integration tests, 0 failures, 0 errors)
- focused affected-module unit tests (953 tests, 0 failures, 0 errors)
- focused rerun-related integration tests (89 tests, 0 failures, 0 errors)
- exact external JUnitParams reproducer
--
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-2239, SUREFIRE-2241, SUREFIRE-2260, SUREFIRE-2274, SUREFIRE-2300] Preserve JUnit Platform test identity across reruns [maven-surefire]
Posted by "olamy (via GitHub)" <gi...@apache.org>.
olamy merged PR #3418:
URL: https://github.com/apache/maven-surefire/pull/3418
--
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-2239, SUREFIRE-2241, SUREFIRE-2260, SUREFIRE-2274, SUREFIRE-2300] Preserve JUnit Platform test identity across reruns [maven-surefire]
Posted by "wilx (via GitHub)" <gi...@apache.org>.
wilx commented on PR #3418:
URL: https://github.com/apache/maven-surefire/pull/3418#issuecomment-5249555609
> The PR has some conflicts
Rebased.
--
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-2239, SUREFIRE-2241, SUREFIRE-2260, SUREFIRE-2274, SUREFIRE-2300] Preserve JUnit Platform test identity across reruns [maven-surefire]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3418:
URL: https://github.com/apache/maven-surefire/pull/3418#discussion_r3762657306
##########
surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProvider.java:
##########
@@ -316,12 +317,41 @@ private TestExecutionListener createJUnit4Listeners() {
return null;
}
- private LauncherDiscoveryRequest buildLauncherDiscoveryRequestForRerunFailures(RunListenerAdapter adapter) {
+ LauncherDiscoveryRequest buildLauncherDiscoveryRequestForRerunFailures(RunListenerAdapter adapter) {
LauncherDiscoveryRequestBuilder builder = newRequest();
- // Iterate over recorded failures
- for (TestIdentifier identifier :
- new LinkedHashSet<>(adapter.getFailures().keySet())) {
- builder.selectors(selectUniqueId(identifier.getUniqueId()));
+ LinkedHashSet<TestIdentifier> failures =
+ new LinkedHashSet<>(adapter.getFailures().keySet());
+ LinkedHashSet<UniqueId> failureIds = failures.stream()
+ .map(TestIdentifier::getUniqueId)
+ .map(UniqueId::parse)
+ .collect(Collectors.toCollection(LinkedHashSet::new));
+ adapter.setRerunTestIds(failureIds);
+ LinkedHashSet<String> classNames = new LinkedHashSet<>();
+
+ for (TestIdentifier identifier : failures) {
+ // Runner-backed Vintage tests may expose only a ClassSource and may not
+ // support rediscovery from a leaf UniqueId. Rediscover their owning class
+ // and retain the failed branch with a post-discovery filter instead.
+ Optional<ClassSource> classSource =
+ identifier.getSource().filter(ClassSource.class::isInstance).map(ClassSource.class::cast);
+ if (classSource.isPresent()) {
+ classNames.add(classSource.get().getClassName());
+ } else {
+ builder.selectors(selectUniqueId(identifier.getUniqueId()));
+ }
+ }
+
+ if (!classNames.isEmpty()) {
+ classNames.forEach(className -> builder.selectors(selectClass(className)));
+ builder.filters((PostDiscoveryFilter) testDescriptor -> {
+ UniqueId candidateId = testDescriptor.getUniqueId();
+ boolean isFailureOrRelatedContainer = failureIds.stream()
+ .anyMatch(failureId -> candidateId.hasPrefix(failureId) || failureId.hasPrefix(candidateId));
Review Comment:
This post-discovery filter scans every failed ID for every discovered descriptor, so rerunning F failed tests performs O(F²) prefix checks. Large failing suites can spend substantial time in discovery before executing tests. Precompute an ancestor/prefix index with constant- or logarithmic-time membership checks.
##########
surefire-its/src/test/resources/surefire-2274-failsafe-flake-count/pom.xml:
##########
@@ -0,0 +1,66 @@
+<?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 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
Review Comment:
This new POM uses four-space nesting, but `.editorconfig:10-13` requires two-space indentation for XML. Reformat the XML hierarchy to match the repository convention.
##########
surefire-its/src/test/resources/surefire-2300-before-all-assumption/pom.xml:
##########
@@ -0,0 +1,56 @@
+<?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 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
Review Comment:
This new POM uses four-space nesting, but `.editorconfig:10-13` requires two-space indentation for XML. Reformat the XML hierarchy to match the repository convention.
##########
surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProvider.java:
##########
@@ -316,12 +317,41 @@ private TestExecutionListener createJUnit4Listeners() {
return null;
}
- private LauncherDiscoveryRequest buildLauncherDiscoveryRequestForRerunFailures(RunListenerAdapter adapter) {
+ LauncherDiscoveryRequest buildLauncherDiscoveryRequestForRerunFailures(RunListenerAdapter adapter) {
LauncherDiscoveryRequestBuilder builder = newRequest();
- // Iterate over recorded failures
- for (TestIdentifier identifier :
- new LinkedHashSet<>(adapter.getFailures().keySet())) {
- builder.selectors(selectUniqueId(identifier.getUniqueId()));
+ LinkedHashSet<TestIdentifier> failures =
+ new LinkedHashSet<>(adapter.getFailures().keySet());
+ LinkedHashSet<UniqueId> failureIds = failures.stream()
+ .map(TestIdentifier::getUniqueId)
+ .map(UniqueId::parse)
+ .collect(Collectors.toCollection(LinkedHashSet::new));
+ adapter.setRerunTestIds(failureIds);
+ LinkedHashSet<String> classNames = new LinkedHashSet<>();
+
+ for (TestIdentifier identifier : failures) {
+ // Runner-backed Vintage tests may expose only a ClassSource and may not
+ // support rediscovery from a leaf UniqueId. Rediscover their owning class
+ // and retain the failed branch with a post-discovery filter instead.
+ Optional<ClassSource> classSource =
+ identifier.getSource().filter(ClassSource.class::isInstance).map(ClassSource.class::cast);
+ if (classSource.isPresent()) {
+ classNames.add(classSource.get().getClassName());
+ } else {
+ builder.selectors(selectUniqueId(identifier.getUniqueId()));
+ }
Review Comment:
`ClassSource` identifies where a test came from; it does not guarantee that the engine supports `ClassSelector`. Dropping the existing unique-ID selector can therefore make reruns discover no test for third-party engines that expose `ClassSource` but only support unique-ID rediscovery. Keep the unique-ID selector for every failure and add the class selector as the Vintage-runner fallback.
##########
surefire-its/src/test/resources/surefire-2239-testng-rerun/pom.xml:
##########
@@ -0,0 +1,72 @@
+<?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 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
Review Comment:
This new POM uses four-space nesting, but `.editorconfig:10-13` requires two-space indentation for XML. Reformat the XML hierarchy to match the repository convention.
##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/TestMethodKey.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * 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.plugin.surefire.report;
+
+import java.util.Objects;
+
+/**
+ * Identity used to aggregate the executions of one test across reruns. Providers
+ * without a test run ID retain the legacy class-and-method-name grouping.
+ */
+final class TestMethodKey implements Comparable<TestMethodKey> {
+ private final String testClassMethodName;
+
+ private final Long testRunId;
+
+ TestMethodKey(String testClassMethodName, Long testRunId) {
+ this.testClassMethodName = testClassMethodName;
+ this.testRunId = testRunId;
+ }
+
+ String getTestClassMethodName() {
+ return testClassMethodName;
+ }
+
+ @Override
+ public int compareTo(TestMethodKey other) {
+ int byName = compare(testClassMethodName, other.testClassMethodName);
+ return byName == 0 ? compare(testRunId, other.testRunId) : byName;
+ }
+
+ private static <T extends Comparable<T>> int compare(T left, T right) {
+ if (left == null) {
+ return right == null ? 0 : -1;
+ }
+ return right == null ? 1 : left.compareTo(right);
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (this == other) {
+ return true;
+ }
+ if (!(other instanceof TestMethodKey)) {
+ return false;
+ }
+ TestMethodKey that = (TestMethodKey) other;
+ return Objects.equals(testClassMethodName, that.testClassMethodName)
+ && Objects.equals(testRunId, that.testRunId);
Review Comment:
This key still requires the human-readable name to match even when a test-run ID is present. If an engine reports a different display/legacy name when rediscovering the same unique ID, the failure and successful rerun become separate tests instead of a flake. Scope the numeric ID with a stable source/class identity, use that as the key when present, and retain class-and-method-name grouping only for null IDs.
##########
surefire-its/src/test/resources/surefire-2241-junitparams-rerun/pom.xml:
##########
@@ -0,0 +1,88 @@
+<?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 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
Review Comment:
This new POM uses four-space nesting, but `.editorconfig:10-13` requires two-space indentation for XML. Reformat the XML hierarchy to match the repository convention.
##########
surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/RunListenerAdapter.java:
##########
@@ -603,6 +693,20 @@ Map<TestIdentifier, TestExecutionResult> getFailures() {
return failures;
}
+ void setRerunTestIds(Set<UniqueId> rerunTestIds) {
+ this.rerunTestIds = unmodifiableSet(new LinkedHashSet<>(rerunTestIds));
+ }
+
+ private boolean isRerunTarget(TestIdentifier testIdentifier) {
+ if (runMode != RERUN_TEST_AFTER_FAILURE || rerunTestIds.isEmpty()) {
+ return true;
+ }
+
+ UniqueId candidateId = UniqueId.parse(testIdentifier.getUniqueId());
+ return rerunTestIds.stream()
+ .anyMatch(failureId -> candidateId.hasPrefix(failureId) || failureId.hasPrefix(candidateId));
Review Comment:
Every JUnit execution callback linearly scans all failed IDs. Since reruns emit O(F) target events when F tests fail, this adds another O(F²) path during execution. Reuse a precomputed ancestor/prefix index rather than streaming the full failure set for each callback.
##########
surefire-its/src/test/resources/surefire-2260-non-unique-scenarios/pom.xml:
##########
@@ -0,0 +1,91 @@
+<?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 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
Review Comment:
This new POM uses four-space nesting, but `.editorconfig:10-13` requires two-space indentation for XML. Reformat the XML hierarchy to match the repository convention.
--
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-2239, SUREFIRE-2241, SUREFIRE-2260, SUREFIRE-2274, SUREFIRE-2300] Preserve JUnit Platform test identity across reruns [maven-surefire]
Posted by "olamy (via GitHub)" <gi...@apache.org>.
olamy commented on PR #3418:
URL: https://github.com/apache/maven-surefire/pull/3418#issuecomment-5248118151
The PR has some conflicts
--
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-2239, SUREFIRE-2241, SUREFIRE-2260, SUREFIRE-2274, SUREFIRE-2300] Preserve JUnit Platform test identity across reruns [maven-surefire]
Posted by "wilx (via GitHub)" <gi...@apache.org>.
wilx commented on PR #3418:
URL: https://github.com/apache/maven-surefire/pull/3418#issuecomment-5232183585
I have pushed [2d29ea4](https://github.com/apache/maven-surefire/pull/3418/commits/2d29ea448de7d623cb923e9e6b89a5032af5192a) to try to avoid some sort of threading issue in possibly TestNG. I have seen the failure in `TestMultipleMethodPatternsTestNGIT.shouldMatchMultiplePatternsAsParallel` on two separate occasions now:
- https://github.com/wilx/maven-surefire/actions/runs/31283536632/job/93169115946
- https://github.com/apache/maven-surefire/actions/runs/31283538328/job/93244980155
--
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