Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
ajeydudhe opened this issue Mar 4, 2018 · 19 comments

Comments

@ajeydudhe
Copy link

ajeydudhe commented Mar 4, 2018

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

Versions

JUnit 5.1.0
JUnit Platform 1.1.0
Maven Surefire Plugin 2.19.1

@sbrannen sbrannen changed the title maven-surefire-plugin reports does not use dynamic test name but lists the tests as JUnit factory method name with indices maven-surefire-plugin report does not use dynamic test name but lists the tests as factory method name with indices Mar 4, 2018
@sbrannen
Copy link
Member

sbrannen commented Mar 4, 2018

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
Copy link
Author

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

TEST-ParamTest.txt

@ajeydudhe
Copy link
Author

@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
Copy link
Member

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
Copy link
Member

sbrannen commented Mar 10, 2018

Reopening for team discussion.

@sbrannen sbrannen reopened this Mar 10, 2018
@sbrannen sbrannen modified the milestones: 5.2 Backlog, 5.2 M1 Mar 10, 2018
@sbrannen sbrannen changed the title maven-surefire-plugin report does not use dynamic test name but lists the tests as factory method name with indices Surefire provider reports @TestFactory method name with invocation index instead of display name Mar 23, 2018
@marcphilipp marcphilipp modified the milestones: 5.2 M1, 5.3 M1 Apr 13, 2018
@k1w1m8
Copy link

k1w1m8 commented Jun 13, 2018

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

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.

@sormuras sormuras modified the milestones: 5.3 M1, 5.3 M2 Jun 15, 2018
@k1w1m8
Copy link

k1w1m8 commented Jun 20, 2018

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
Copy link
Member

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.

@marcphilipp marcphilipp removed this from the 5.3 RC1 milestone Jun 29, 2018
@Korop
Copy link

Korop commented Jul 9, 2018

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

@dblevins
Copy link

dblevins commented Jul 22, 2018

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
Copy link
Member

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

k1w1m8 commented Sep 13, 2018

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
Copy link
Member

sbrannen commented Sep 13, 2018

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

Tibor17 commented Sep 13, 2018

@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
Copy link

Tibor17 commented Sep 13, 2018

@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
Copy link

k1w1m8 commented Sep 13, 2018

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

@sormuras sormuras closed this as completed Feb 1, 2019
@krusche
Copy link

krusche commented Sep 24, 2019

@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
Copy link

k1w1m8 commented Jan 1, 2020

@krusche M4 was released in November.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants