ajohnstonTE commented on a change in pull request #267: [SUREFIRE-1741] JUnit5:
Detect failed containers
URL: https://github.com/apache/maven-surefire/pull/267#discussion_r371430596
##########
File path:
surefire-providers/surefire-junit-platform/src/test/java/org/apache/maven/surefire/junitplatform/RunListenerAdapterTest.java
##########
@@ -425,7 +425,7 @@ public void notifiedOfContainerFailure()
throws Exception
{
adapter.executionFinished( newContainerIdentifier(), failed( new
RuntimeException() ) );
- verify( listener ).testFailed( any() );
+ verify( listener ).testError( any() );
Review comment:
Right. I initially just re-used the existing logical flow when I made my
fix, which led to it being a failure, not an error. So I changed that in this
latest set of changes. I can add a check [like the one a few lines below my
changes](https://github.com/apache/maven-surefire/pull/267/commits/6df45c50e06b396504b0a846a59a60052f0177ff#diff-ad07b0c74885ecb3d687dcc85495e390R136-R137)
to see whether or not it's an `AssertionError` if you'd prefer.
I was actually planning on adding that check for my latest commit, but
noticed that the `isClass` (previously `!isTest`) doesn't check that, so
instead I limited the scope of my changes and decided I'd just leave it for
another PR.
`isClass` block:
https://github.com/apache/maven-surefire/blob/6df45c50e06b396504b0a846a59a60052f0177ff/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/RunListenerAdapter.java#L121-L126
If I do add that `AssertionError` check, should I add it to the `isClass`
block as well? Or just the `isContainer` block?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
