andpab commented on code in PR #516:
URL: https://github.com/apache/maven-surefire/pull/516#discussion_r1060357685
##########
surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/RunListenerAdapter.java:
##########
@@ -314,28 +313,26 @@ private String[] toClassMethodName( TestIdentifier
testIdentifier )
boolean needsSpaceSeparator = isNotBlank( parentDisplay ) &&
!display.startsWith( "[" );
String methodDisplay = parentDisplay + ( needsSpaceSeparator ? " "
: "" ) + display;
- String simpleClassNames = COMMA_PATTERN.splitAsStream(
methodSource.getMethodParameterTypes() )
- .map( s -> s.substring( 1 + s.lastIndexOf( '.' ) ) )
- .collect( joining( "," ) );
-
- boolean hasParams = isNotBlank(
methodSource.getMethodParameterTypes() );
Review Comment:
You're not only changing the behaviour for parameterized tests, but also for
normal tests which is why you then have to adjust a lot of test expectations.
That's not particularly nice.
I would suggest an approach that also addresses the other root cause of the
problem which is that the logic for determining `hasParams` only looks at
`methodSource.getMethodParameterTypes()`. That's not correct for JUnit 4
Parameterized tests since they do not have a method parameter. In JUnit 4 the
parameterization is handled on a level further up.
Consider something like this:
```java
boolean hasParameterizedParent =
collectAllTestIdentifiersInHierarchy( testIdentifier )
.filter( identifier -> !identifier.getSource().isPresent() )
.map( TestIdentifier::getLegacyReportingName )
.anyMatch( legacyReportingName -> legacyReportingName.matches(
"^\\[.+]$" ) );
boolean parameterized = isNotBlank( methodSource.getMethodParameterTypes() )
|| hasParameterizedParent;
String methodDesc = parameterized ? description : methodName;
```
This code relies on the fact that for JUnit 4 parameterized tests there is a
container-type test identifier with the legacyReportingName `[<index>]` or
`[<displayName>]` that has an empty source field in the test identifier
hierarchy between the test method and the test class.
The rest of the code could stay as it was to keep the behaviour for
non-parameterized tests the same - except for `simpleClassNames` and
`methodSign` which are not needed anymore.
Then you don't have to change any expectations of existing tests. You can
leave them as they are and only add your new tests.
--
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-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
