Junit5: Vintage engine does not report multiple errors collected by ErrorCollector

[smoyer64]

Overview

• (X) Bug report. When JUnit 4 tests are run using the vintage engine, failures collected by ErrorCollector are not reported as a list of failures as shown below. Test code for this issue is provide in the javadocs for the JUnit 4 ErrorCollector at http://junit.org/junit4/javadoc/latest/org/junit/rules/ErrorCollector.html.

│  ├─ example.testrule.ErrorCollectorVintageDemo
│  │  ├─ example ✘ 
Expected: not a string containing "ERROR!"
     but: was "ERROR! - something is broke"

Deliverables

• [ ] All failures collected by the ErrorCollector should be displayed. The output below shows how the same test executed via the JUnit 4 directly produces the expected three results.

java.lang.Throwable: first thing went wrong
java.lang.Throwable: second thing went wrong
java.lang.AssertionError: 
Expected: not a string containing "ERROR!"
     but: was "ERROR! - something is broke"

[marcphilipp]

Thanks for reporting! This is indeed a bug in the Vintage engine. JUnit 4 special-cases MultipleFailureException and calls RunListener#testFailure() once for each failure. With the Platform's EngineExecutionListener it's not possible to report multiple failures separately. Thus, I see the following options:

1. Call Throwable#addSuppressed on the first exception with all remaining exceptions.
2. Create a org.junit.runners.model.MultipleFailureException with all exceptions.
3. Create a org.opentest4j.MultipleFailuresError with all exceptions. However, it can currently only contain AssertionErrors. So we would need to change that.

Opinions?

[smoyer64]

My opinion?

I think I'm more in favor of (3) above as it uses the MultipleFailuresError directly which is how the equivalent code in JUnit 5 works. I had noted that as I was writing the vintage and jupiter demo code for ErrorCollector that I realized the example in the javadocs was very contrived. I actually like how the JUnit 5 code works better so I'd suggest the following:

1. For each Throwable create an AssertionError setting the message to the message of the offending Throwable and adding the throwable as the cause.
2. Add all the AsssertionErrors (whether directly reported or created from Throwables) to the MultipleFailuresError

That's essentially what the code I've committed for the '''ErrorCollectorJupiterDemo``` (a9889db) is going to produce anyway.

[gaganis]

Hello all!

This issue has a pretty straightforward spec, even though @marcphillips comment hints the implementation to be more interesting. I was watching for something that I could maybe try tackling.

Is this up for grabs?

Thanks
Giorgos

[marcphilipp]

Thanks for the offer! Please wait until we've decided how to implement it, though.

[marcphilipp]

Team decision:

• Change org.opentest4j.MultipleFailuresError to support Throwable instead of AssertionError
• Fix Vintage Engine to store multiple failures instead of just the last one

[gaganis]

Thanks @marcphilipp for the implementation details.

I am starting to look into this. :)