[GitHub] [maven-surefire] br0nstein commented on a diff in pull request #516: [SUREFIRE-2065] Test Reports Inconsistencies with Parameterized and junit4

[GitBox <gi...@apache.org>]

br0nstein commented on code in PR #516:
URL: https://github.com/apache/maven-surefire/pull/516#discussion_r889499114


##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/TestSetRunListener.java:
##########
@@ -312,7 +312,7 @@ private void addTestMethodStats()
         for ( WrappedReportEntry reportEntry : detailsForThis.getReportEntries() )
         {
             TestMethodStats methodStats =
-                new TestMethodStats( reportEntry.getClassMethodName(), reportEntry.getReportEntryType(),
+                new TestMethodStats( reportEntry.getReportClassMethodName(), reportEntry.getReportEntryType(),

Review Comment:
   Make sure you understand b2cce57a5e1dbd9a3a9e79c2eadd866e289e6f7f and especially e8f65b99aaaa7c04e1ad3770309cd626d3fe5ab0. I believe the fix for this issue should be in RunListenerAdapter. The logic there is hard to reason about without comments describing intent but ultimately I think you'll want methodDesc to be set to the legacy reporting name (given by junit in the testidentifier directly) for these junit4 parameterized tests, not methodSource.getMethodName() as it is today. It's using the method name because displayName and legacyReportingName are found to be equal (both e.g. methodName[0]).
   
   When running junit5 parameterized tests, this does not repro because junit creates the testidentifier setting the legacy reporting name to methodName(ParameterDataType1, ..)[testInvocationNum] (e.g. "methodName(Long)[0]") and the display name if not overridden (@ParameterizedTest(name="blah")) to [testInvocationNum] argList e.g. "[0] 100" if the given param is 100L and it's the first parameterized case. Since the code finds them unequal it sets methodDesc to the legacyReportingName. The basic issue here is that the handling for parameterized tests makes an implicit assumption that they will always have a different legacy reporting name from display name which isn't the case for junit 4 parameterized tests without the @Parameters name overridden.



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