[SUREFIRE-2298] fix xml output with junit 5 nested classes (fix integration with Cucumber and Archunit) #828

olamy · 2025-04-04T09:32:11Z

Signed-off-by: Olivier Lamy [email protected]

Following this checklist to help us incorporate your
contribution quickly and easily:

Make sure there is a JIRA issue 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 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
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
In any other case, please file an Apache Individual Contributor License Agreement.

surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire2298IT.java

marcphilipp · 2025-04-04T12:12:27Z

...junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/RunListenerAdapter.java

+        Optional<UniqueId.Segment> classSegment = testIdentifier.getUniqueIdObject().getSegments().stream()
+                .filter(segment -> "class".equals(segment.getType()))
+                .findFirst();


You shouldn't parse the UniqueId because it's considered to be an implementation detail. Instead, you should use testPlan.getParent(testIdentifier) to find the right level. If this should only support class-based engines, finding the last one with a TestSource of type ClassSource should work. If this should also work for file-based engines like Cucumber, you might want to use a differeny heuristic instead. Maybe always use the level immediate below the root (engine descriptor)? If it has a ClassSource, use that; otherwise, use the display name (similar to what toClassMethodName does)?

Yeah, Cucumber was also broken by the last patch, so the alternative heuristic should be used.

You shouldn't parse the UniqueId because it's considered to be an implementation detail. Instead, you should use testPlan.getParent(testIdentifier) to find the right level. If this should only support class-based engines, finding the last one with a TestSource of type ClassSource should work. If this should also work for file-based engines like Cucumber, you might want to use a differeny heuristic instead. Maybe always use the level immediate below the root (engine descriptor)? If it has a ClassSource, use that; otherwise, use the display name (similar to what toClassMethodName does)?

I tried this approach but it looks not possible with the Expected reporting from here #2601 (comment)

every single time I see I will have to modify this method I'm just crying 😆

maven-surefire/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/RunListenerAdapter.java

Line 297 in 9f49866

private String[] toClassMethodName(TestIdentifier testIdentifier) {

olamy · 2025-05-18T23:13:40Z

Hi @olamy

you might be able to see progress there #828

But... is there any?

yup sorry been busy with other stuff.

I'm thinking of possible quick fix which is something more dynamic here https://github.com/apache/maven-surefire/pull/815/files#diff-38e379eb63d7dcd2deb45902b4a494517e4ce54c29913bcf74e7d9243ebb7011R2077

Switching this flag may fix the current issue

JanMosigItemis · 2025-06-18T10:01:50Z

Hello everyone, is there an ETA for this fix?

jira-importer · 2025-07-04T05:16:37Z

Resolve #2601

olamy · 2025-07-23T06:33:23Z

I have actually reverted the change (well make this happening only for parallel execution)
But even if test number is fine but even with 3.5.2 or even now, generated XML files are/were not correct (there are 3 instead of 2 with the added test with this PR).

olamy · 2025-07-23T10:24:46Z

Sorry for the delay, winter here, so finally back on this.
Some progress. Last changes should have fixed the problem, but I need to check what else I just broke :)

Signed-off-by: Olivier Lamy <[email protected]>

…arallel is enabled Signed-off-by: Olivier Lamy <[email protected]>

Signed-off-by: Olivier Lamy <[email protected]>

olamy · 2025-09-04T06:01:09Z

Oh yeah. we might be finally good now :)
it took a while but finally managed to work on this.
Sorry for delay

Signed-off-by: Olivier Lamy <[email protected]>

olamy · 2025-09-04T10:22:50Z

The faulty m-sf-p ran cucumber tests fine, but didn't produce expected summary.

I see cucumber test was added, but I can't see its build output to be verified for expected result. Does the new test fail without other changes in this PR? Or would it pass then (with 3.9.3) just like now (hence not providing new check)?

build output not verified? you sure?

https://github.com/apache/maven-surefire/pull/828/files#diff-b64af33c9a98c2d43c52cfca7f3bb89af9a833feae8489c0738560350c4592caR55

github-actions · 2025-09-04T22:59:03Z

@olamy The PR can't be associated to a milestone, because there are multiple open milestones. Please add the text "branch: master" to the description to the milestone where this PR belongs to.

surefire-its/src/test/resources/cucumber-tests/src/test/java/cz/fafejta/RunCucumberTest.java

With Surefire 3.5.4 the heuristic for naming tests changed[1]. As a result the `surefire` strategy no longer works as expected. But the `long` strategy now does provide reasonable results. On Surefire <= 3.5.2 with the `surefire` strategy the test results would be rendered as: ``` <testcase name="a few cukes" classname="Belly" time="0.062"> ``` On Surefire >= 3.5.4 with the `long` strategy the test results will be rendered as: ``` <testcase name="Belly - a few cukes" classname="RunCucumberTest" time="0.06"> ``` 1. apache/maven-surefire#828

jesperancinha · 2025-09-11T16:50:19Z

When is the new release coming out? I have a PR open that I want to merge once the release is out: jesperancinha/jeorg-spring-test-drives#890

slawekjaranowski · 2025-09-11T20:04:35Z

When is the new release coming out? I have a PR open that I want to merge once the release is out: jesperancinha/jeorg-spring-test-drives#890

will be soon: https://lists.apache.org/thread/r905v9hrvz34f5t3vzbvnd64fbvpb1no

With Surefire 3.5.4 the heuristic for naming tests changed[1]. As a result the `surefire` strategy no longer works as expected. But the `long` strategy now does provide reasonable results. On Surefire <= 3.5.2 with the `surefire` strategy the test results would be rendered as: ``` <testcase name="a few cukes" classname="Belly" time="0.062"> ``` This includes both the scenario and feature name. On Surefire >= 3.5.4 with the `surefire` strategy the test results will be rendered as: ``` <testcase name="a few cukes" classname="RunCucumberTest" time="0.06"> ``` This does not include the feature name. On Surefire >= 3.5.4 with the `long` strategy the test results will be rendered as: ``` <testcase name="Belly - a few cukes" classname="RunCucumberTest" time="0.06"> ``` This does the scenario and feature name and also the name of the Suite that ran Cucumber. This is the optimal outcome. 1. apache/maven-surefire#828 Closes: #2984

olamy added the bug Something isn't working label Apr 4, 2025

marcphilipp reviewed Apr 4, 2025

View reviewed changes

surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire2298IT.java Show resolved Hide resolved

marcphilipp reviewed Apr 4, 2025

View reviewed changes

olamy mentioned this pull request Apr 8, 2025

[SUREFIRE-1643] surefire junit5 parallel tests #815

Merged

8 tasks

hankem mentioned this pull request Apr 9, 2025

ArchTest on fields does not run under Maven Surefire Plugin 3.5.3+ TNG/ArchUnit#1442

Closed

pzygielo mentioned this pull request Apr 16, 2025

With cucumber integration tests not counted in total test summary in surefire-plugin 3.5.3 #834

Closed

mpkorstanje mentioned this pull request Apr 24, 2025

Surefire 3.5.3 does not include Cucumber scenarios in test summary count cucumber/cucumber-jvm#2984

Closed

This comment was marked as outdated.

Sign in to view

olamy marked this pull request as draft July 22, 2025 11:14

olamy force-pushed the SUREFIRE-2298-nested-classes branch 2 times, most recently from 2dc3506 to f6c974b Compare July 26, 2025 11:36

houssain-barouni mentioned this pull request Jul 29, 2025

Downgrade surefire version to 3.5.2 to allow running ArchUnit tests in the CI camunda/camunda#36115

Merged

1 task

MarkvanOsch mentioned this pull request Aug 1, 2025

Failing Cucumber-Tests are not detected #3166

Closed

jglick mentioned this pull request Aug 18, 2025

Revert "Bump maven-surefire-plugin.version from 3.5.2 to 3.5.3" jenkinsci/plugin-pom#1184

Merged

olamy added 11 commits August 28, 2025 10:46

[SUREFIRE-2298] fix xml output with junit 5 nested classes

603b6ba

Signed-off-by: Olivier Lamy <[email protected]>

fix assertions

8c49da9

Signed-off-by: Olivier Lamy <[email protected]>

revert changes to try something more simple with next commit

c88a539

Signed-off-by: Olivier Lamy <[email protected]>

only activate the StatPerSourceName mode if junit.jupiter.execution.p…

47c1a1b

…arallel is enabled Signed-off-by: Olivier Lamy <[email protected]>

fix typo

9fc2088

Signed-off-by: Olivier Lamy <[email protected]>

cannot support both now

1019394

Signed-off-by: Olivier Lamy <[email protected]>

naviguate into testPlan hierarchy to find root class for @nested

01342e7

Signed-off-by: Olivier Lamy <[email protected]>

reduce number of threads per core

1c9cbea

Signed-off-by: Olivier Lamy <[email protected]>

spotless

a4ce840

Signed-off-by: Olivier Lamy <[email protected]>

use parent child model for poms

17dfc06

Signed-off-by: Olivier Lamy <[email protected]>

restore dynamic

622dda5

Signed-off-by: Olivier Lamy <[email protected]>

those tests need java 17

3ce5e90

Signed-off-by: Olivier Lamy <[email protected]>

olamy mentioned this pull request Sep 4, 2025

No tests found from failsafe with newer JUnit5 and Cucumber serenity-bdd/serenity-cucumber-starter#160

Open

olamy linked an issue Sep 4, 2025 that may be closed by this pull request

Issue with custom JUnit 5 Engine and Surefire Reporting #837

Closed

olamy added 2 commits September 4, 2025 16:06

force compiler plugin version

9a440bc

Signed-off-by: Olivier Lamy <[email protected]>

IT need java 17

443a68c

Signed-off-by: Olivier Lamy <[email protected]>

This comment was marked as resolved.

Sign in to view

olamy merged commit 6c30bf1 into apache:master Sep 4, 2025
86 of 88 checks passed

jira-importer mentioned this pull request May 18, 2025

[SUREFIRE-2298] @Nested test classes are not reported properly #2601

Closed

github-actions bot assigned olamy Sep 4, 2025

olamy added this to the 3.5.4 milestone Sep 4, 2025

olamy deleted the SUREFIRE-2298-nested-classes branch September 4, 2025 22:59

olamy changed the title ~~[SUREFIRE-2298] fix xml output with junit 5 nested classes~~ [SUREFIRE-2298] fix xml output with junit 5 nested classes (fix integration with Cucumber and Archunit) Sep 4, 2025

mpkorstanje reviewed Sep 5, 2025

View reviewed changes

surefire-its/src/test/resources/cucumber-tests/src/test/java/cz/fafejta/RunCucumberTest.java Show resolved Hide resolved

mpkorstanje mentioned this pull request Sep 6, 2025

Warn when surefire naming strategy is used cucumber/cucumber-jvm#3067

Merged

9 tasks

jesperancinha mentioned this pull request Sep 11, 2025

Bump io.cucumber:cucumber-bom from 7.23.0 to 7.25.0 jesperancinha/jeorg-spring-test-drives#890

Open

jesperancinha mentioned this pull request Sep 12, 2025

It is not finding the "cucumber" tests. At least that is what it is saying cucumber/cucumber-jvm#3071

Closed

uhafner mentioned this pull request Sep 21, 2025

[SUREFIRE-2194] JUnit 5: Implementing tests in test class and nested class results in incorrect reporting #2682

Open

[SUREFIRE-2298] fix xml output with junit 5 nested classes (fix integration with Cucumber and Archunit) #828

[SUREFIRE-2298] fix xml output with junit 5 nested classes (fix integration with Cucumber and Archunit) #828

Uh oh!

Conversation

olamy commented Apr 4, 2025

Uh oh!

Uh oh!

marcphilipp Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

leonard84 Apr 7, 2025

Choose a reason for hiding this comment

Uh oh!

olamy Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

olamy Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

olamy commented May 18, 2025

Uh oh!

JanMosigItemis commented Jun 18, 2025

Uh oh!

jira-importer commented Jul 4, 2025

Uh oh!

olamy commented Jul 23, 2025

Uh oh!

olamy commented Jul 23, 2025

Uh oh!

olamy commented Sep 4, 2025

Uh oh!

This comment was marked as resolved.

olamy commented Sep 4, 2025

Uh oh!

This comment was marked as resolved.

Uh oh!

github-actions bot commented Sep 4, 2025

Uh oh!

Uh oh!

jesperancinha commented Sep 11, 2025

Uh oh!

slawekjaranowski commented Sep 11, 2025

Uh oh!

Uh oh!