Surefire provider reports @TestFactory method name with invocation index instead of display name

ajeydudhe commented 6 years ago

Bug report

The maven-surefire-plugin is able to run the JUnit 5 experimental dynamic tests but in the generate report *.xml file the test case names are not reflected.

Steps to reproduce

pom.xml: Add the required dependencies as per attachement: pom.xml.txt

Create a dynamic test as follows:

public class SampleDynamicTest
{
@TestFactory
public Collection<DynamicTest> test_dynamic()
{
return Arrays.asList(

      dynamicTest("DynamicTest_01", () -> {System.out.println("Dynamic Test 01 executed...");}),
      dynamicTest("DynamicTest_02", () -> {System.out.println("Dynamic Test 02 executed...");})
    );
}
}

Compile the project using: mvn clean test
Check the surefire plugin output at: target/surefire-reports/TEST-my.pocs.junit5.SampleDynamicTest.xml
The dynamic test names are not as expected but uses the test factory method name i.e. test name appears as test_dynamic[1] instead of DynamicTest_01 and test_dynamic[2] instead of DynamicTest_02 TEST-my.pocs.junit5.SampleDynamicTest.xml.txt
If you run the test from Eclipse IDE then the test names are as expected:

Versions

JUnit 5.1.0 JUnit Platform 1.1.0 Maven Surefire Plugin 2.19.1

sbrannen commented 6 years ago

Hi @ajeydudhe,

Thanks for raising the issue.

We are in fact aware of this, and it is by design due to a limitation of the reporting format used by Surefire. See #1182 for details.

I am therefore closing this issue.

ajeydudhe commented 6 years ago

I have verified the parameterized tests using JUnit4 and there the actual parameter names are getting used instead of just the index names. Also the surefire report uses the name as [parameter_name]

TEST-ParamTest.txt

ajeydudhe commented 6 years ago

@sbrannen can you take a look at this? If the parameterized tests use the parameter name properly to identify the test as test_method_name><parameter_name_from_@Parameters(name=something) then we should have similar behavior for dynamic tests too.

sbrannen commented 6 years ago

I have verified the parameterized tests using JUnit4 and there the actual parameter names are getting used instead of just the index names.

Hmmm... interesting.

@marcphilipp, since you invested so much effort in #1182, what are your thoughts on including custom display names (if present) vs. the current indexed format?

sbrannen commented 6 years ago

Reopening for team discussion.

k1w1m8 commented 6 years ago

I have also found this issue.

In my Jenkins, in the HTML report which is generated from Surefire I get a bunch of links to failures which have a synthetic id where you would expect the DynamicTest name, like so:

Test Result (25 failures / +2)
com.clear2pay.jetqarun.junit5.JUnit5TestFactory.scan[2][1][18]
com.clear2pay.jetqarun.junit5.JUnit5TestFactory.scan[4][1][1][15]
com.clear2pay.jetqarun.junit5.JUnit5TestFactory.scan[4][1][1][18]
com.clear2pay.jetqarun.junit5.JUnit5TestFactory.scan[4][1][1][19]
...

Unfortunately, this makes using any kind of dynamic or parameterised test with Jenkins very painful, and is a powerful reason to avoid teams adopting JUnit 5 dynamic or parameterised tests for real projects.

As a result I have had to back out my changes to use JUnit 5 in our Jenkins CI pipeline. We will need to stick to JUnit 3 suite/test simulation of dynamic tests until this issue is addressed.

I also noted that IntelliJ IDEA has similar problems with using synthetic ids as names, with similar impacts on real world usage. I've created a ticket for that too:

https://youtrack.jetbrains.com/issue/IDEA-193770

valodzka commented 6 years ago

Just want to add that for my team this issue is main reason to postpone upgrade from junit 4. It's much harder to use jenkins reports when test names isn't readable.

k1w1m8 commented 6 years ago

Will this be fixed in the maven repo now that the junit 5 surefire reporter has been donated to maven and maven surefire plugin 22.0 has been released?

sormuras commented 6 years ago

Will this be fixed in the maven repo now that the junit 5 surefire reporter has been donated to maven and maven surefire plugin 22.0 has been released?

Yes, that's the plan.

Korop commented 6 years ago

The same issue with the @DisplayName ignoring is valid for maven-failsafe-plugin

dblevins commented 6 years ago

Read this thread and didn't quite understand the resolution. It seems this is a maven-surefire-plugin limitation that was expected to be fixed in version 2.22.0. I can confirm that with JUnit Jupiter 5.3.0-M1 and Maven-Surefire-Plugin 2.22.0, the problem is still there.

What is the expected JUnit 5 and Maven-Surefire-Plugin combination that we expect will someday work? (is it any JUnit 5.3.x with a patched Maven-Surefire-Plugin of version TBD?)

marcphilipp commented 6 years ago

Surefire does not yet support it. Please open an issue with the Apache Maven Surefire project at https://issues.apache.org/jira/browse/SUREFIRE.

k1w1m8 commented 6 years ago

I was wondering what was happening in Surefire wrt JUnit5.

It appears there is a single committer on the project. They are obviously doing an heroic job there, but there is a limit to what one person can do. Therefore, tickets for fixing the reporting of new JUnit5 features are not being worked on.

https://issues.apache.org/jira/browse/SUREFIRE-1567 https://issues.apache.org/jira/browse/SUREFIRE-1546

IMHO, having JUnit5 without first class support for new features in Surefire is pretty pointless given the importance of Maven to the Java ecosystem.

Is this something that the JUnit5 team are concerned about? Is there a plan to address this issue?

I've never worked on Surefire, but I'm considering getting involved...

sbrannen commented 6 years ago

Is this something that the JUnit5 team are concerned about? Is there a plan to address this issue?

Yes, we are concerned about such issues, but the entire JUnit 5 Team is simply not capable of getting involved in all IDEs and build tools.

Having said that, however, Christian Stein (@sormuras) is a core committer on the JUnit 5 Team, and he joined the Maven Surefire Team to help with the Surefire 2.22.0 release. In addition, he is still working on improving JUnit Platform support in Surefire as time permits.

I've never worked on Surefire, but I'm considering getting involved...

I think that would be great! The Surefire Team could certainly use some extra support.

Tibor17 commented 6 years ago

@k1w1m8 Do not worry ;-) we are glad to work on JUnit5. For instance we are refactoring a patch from our user, refactoring it which is part of https://issues.apache.org/jira/browse/SUREFIRE-1546 We try out in several branches.

Till now I had to fix CI build because the CI build was was still unstable, failed on new issues, and new failures were randomly caused after accepting user's patch of SUREFIRE-1535 and I did not want to revert it. So one fix came after 1535 and one feature as a clean solution and some concurrency issues fixed, etc. And then a new patch for Java 11. The only analysis of code take days due to concurrency issues. Now the CI is green again and we continue on JUnit5 with @sormuras . I know it looks like we are idle but this is not truth. I am working on Surefire almost every day.

Tibor17 commented 6 years ago

@k1w1m8 Before getting involved as a committer join us on Mailing List for Dev, on GitHub in form of issues/pullrequest and on the chat https://the-asf.slack.com/messages/C7Q9JB404/ One advice, rather talk with us and then do some coding but never in opposite order; otherwise it may cost you more energy than necessary.

k1w1m8 commented 6 years ago

Hi @Tibor17, thanks for the advice.

I appreciate all the good work that the Surefire and JUnit 5 team are doing.

And, glad to hear that there is still some intentions to improve the JUnit 5 support in Surefire.

My focus is getting the DynamicTests (and DynamicContainers) displaying properly in the set of tools I use with JUnit, so that means IDEA and Surefire. Intellij's support for Dynamic has similar issues to Surefire, see https://youtrack.jetbrains.com/issue/IDEA-193770. Until they all work properly, I am stuck on JUnit3 using TestSuite/TestCase.

Hopefully I will manage to get some time to get involved. So many competing priorities ;-)

krusche commented 5 years ago

@Tibor17 I ran into this issue as well. Any chance that maven-surefire-plugin gets a new milestone release 3.0.0-M4 officially published (not as snapshot) soon? This would make it much easier to use it in our projects without custom plugin repositories...

k1w1m8 commented 4 years ago

@krusche M4 was released in November.

junit-team / junit5