[GitHub] eolivelli commented on a change in pull request #212: [SUREFIRE-1546] JUnit 5 runner does not honor JUnit 5 display names

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

eolivelli commented on a change in pull request #212: [SUREFIRE-1546] JUnit 5 runner does not honor JUnit 5 display names
URL: https://github.com/apache/maven-surefire/pull/212#discussion_r244215312
 
 

 ##########
 File path: surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/RunListenerAdapter.java
 ##########
 @@ -185,88 +181,72 @@ private void reportFailedTest(
 
     private SimpleReportEntry createTestSetReportEntry( TestIdentifier testIdentifier )
     {
-        return new SimpleReportEntry(
-                        JUnitPlatformProvider.class.getName(), testIdentifier.getLegacyReportingName() );
+        String[] classMethodName = toClassMethodName( testIdentifier );
+        String className = classMethodName[0];
+        String methodName = classMethodName[1];
+        return new SimpleReportEntry( className, methodName );
     }
 
     private SimpleReportEntry createReportEntry( TestIdentifier testIdentifier )
     {
         return createReportEntry( testIdentifier, (StackTraceWriter) null );
     }
 
-    private SimpleReportEntry createReportEntry(
-                    TestIdentifier testIdentifier, TestExecutionResult testExecutionResult )
-    {
-        return createReportEntry(
-                        testIdentifier, getStackTraceWriter( testIdentifier, testExecutionResult ) );
-    }
-
-    private SimpleReportEntry createReportEntry(
-                    TestIdentifier testIdentifier, StackTraceWriter stackTraceWriter )
-    {
-        String source = getLegacyReportingClassName( testIdentifier );
-        String name = getLegacyReportingName( testIdentifier );
-
-        return SimpleReportEntry.withException( source, name, stackTraceWriter );
-    }
-
-    private String getLegacyReportingName( TestIdentifier testIdentifier )
+    private SimpleReportEntry createReportEntry( TestIdentifier testIdentifier,
+                                                 TestExecutionResult testExecutionResult )
     {
-        // Surefire cuts off the name at the first '(' character. Thus, we have to pick a different
-        // character to represent parentheses. "()" are removed entirely to maximize compatibility with
-        // existing reporting tools because in the old days test methods used to not have parameters.
-        return testIdentifier
-                        .getLegacyReportingName()
-                        .replace( "()", "" )
-                        .replace( '(', '{' )
-                        .replace( ')', '}' );
+        return createReportEntry( testIdentifier, toStackTraceWriter( testIdentifier, testExecutionResult ) );
     }
 
-    private String getLegacyReportingClassName( TestIdentifier testIdentifier )
+    private SimpleReportEntry createReportEntry( TestIdentifier testIdentifier, StackTraceWriter stackTraceWriter )
     {
-        return LegacyReportingUtils.getClassName( testPlan, testIdentifier );
+        String[] classMethodName = toClassMethodName( testIdentifier );
+        String className = classMethodName[0];
+        String methodName = classMethodName[1];
+        return withException( className, methodName, stackTraceWriter );
     }
 
-    private StackTraceWriter getStackTraceWriter(
-                    TestIdentifier testIdentifier, TestExecutionResult testExecutionResult )
+    private StackTraceWriter toStackTraceWriter( TestIdentifier testIdentifier,
+                                                 TestExecutionResult testExecutionResult )
     {
         Optional<Throwable> throwable = testExecutionResult.getThrowable();
         if ( testExecutionResult.getStatus() == FAILED )
         {
             // Failed tests must have a StackTraceWriter, otherwise Surefire will fail
-            return getStackTraceWriter( testIdentifier, throwable.orElse( null ) );
+            return toStackTraceWriter( testIdentifier, throwable.orElse( null ) );
         }
-        return throwable.map( t -> getStackTraceWriter( testIdentifier, t ) ).orElse( null );
+        return throwable.map( t -> toStackTraceWriter( testIdentifier, t ) )
+                .orElse( null );
     }
 
-    private StackTraceWriter getStackTraceWriter( TestIdentifier testIdentifier, Throwable throwable )
+    private StackTraceWriter toStackTraceWriter( TestIdentifier testIdentifier, Throwable throwable )
     {
-        String className = getClassName( testIdentifier );
-        String methodName = getMethodName( testIdentifier ).orElse( "" );
+        String[] classMethodName = toClassMethodName( testIdentifier );
+        String className = classMethodName[0];
+        String methodName = classMethodName[1];
         return new PojoStackTraceWriter( className, methodName, throwable );
     }
 
-    private String getClassName( TestIdentifier testIdentifier )
+    private String[] toClassMethodName( TestIdentifier testIdentifier )
 
 Review comment:
   Is there any particular reason for using an array and not a struct? It will be more clear and error prone

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services