Surefire-1701: Fix @DisplayName breaking reruns #249
Conversation
...latform/src/test/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProviderTest.java
Outdated
Show resolved
Hide resolved
| { | ||
| // Extract quantified test name data | ||
| String[] classMethodName = adapter.toClassMethodNameWithoutPlan( identifier ); | ||
| String className = classMethodName[1]; | ||
| String methodName = classMethodName[3]; | ||
| String className = classMethodName[0]; |
There was a problem hiding this comment.
It would be worth to use OOP and not string array because now I can see how it is difficult to understand the indexes of the array and their meaning. So we have a couple of class + method, and one group is the traditional and second is named. So the class can have annotation as well.
There was a problem hiding this comment.
I agree, but I do not think that is within the scope of this PR (A simple bug fix).
There was a problem hiding this comment.
yes, right, we can do it after this PR. And we do not need to have Jira issue for a small internal code improvement.
| @@ -865,6 +866,7 @@ void testAlwaysSkipped() | |||
| throw new IllegalStateException( "this test should be never called" ); | |||
| } | |||
|
|
|||
| @org.junit.jupiter.api.DisplayName( "Fails twice" ) | |||
There was a problem hiding this comment.
well, okay it cover the test scenario but since we made two changes (array indexes and added HashSet) we should assert the display names in the listener too.
What if we coment the DisplayName? It will not rerun the tests as we found it in Jira?
There was a problem hiding this comment.
What if we coment the DisplayName? It will not rerun the tests as we found it in Jira?
I'm not sure I follow. I've updated the test to print the failing test names if that's what you're looking for.
There was a problem hiding this comment.
I meant something like this 4f16fcd#diff-183e6d2cde226a9744aa928c54694b2fR239 in the tests for this bug.
There was a problem hiding this comment.
I guess you have already added these assertion statements for handled display names in the latest commit. :-)
| for ( TestExecutionSummary.Failure failure : summary.getFailures() ) | ||
| failDisplays.add( failure.getTestIdentifier().getDisplayName() ); | ||
| assertEquals( 3, failDisplays.size() ); | ||
| assertTrue( failDisplays.contains( "Fails twice" ) ); |
...latform/src/test/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProviderTest.java
Show resolved
Hide resolved
…sses/methods for test reruns Fix test formatting Fix test formatting 2 a fix test c
|
@Col-E |
As discussed in Surefire-1584 there was a bug where
@DisplayNamewould break rerun logic. This PR resolves both this bug and addresses another potential issue (No triggering sample found though) where theadapterwas treated as if it were stateless, but it is stateful.The platform provider unit test has been updated with
@DisplayNameusages to verify the correctness of this solution.