ota4j-team / opentest4j

Open Test Alliance for the JVM
Apache License 2.0
279 stars 37 forks source link

Is it a good idea to override the message format for MultipleFailuresError? #46

Closed evant closed 6 years ago

evant commented 6 years ago

Hey, I'm looking at integrating opentest4j in my assertions lib. We already have a way we are formatting multiple errors. My current solution is to subclass MultipleFailuresError and override getMessage(). Is this a reasonable solution (will it affect tooling), or is there a strong reason to go with the default message?

sbrannen commented 6 years ago

Is this a reasonable solution (will it affect tooling),

Yes, this has the potential to affect tooling.

For example, Eclipse has built-in support for MultipleFailuresError. So if you override getMessage(), I can only assume that the Eclipse "Result Comparison" dialog will no longer work properly.

Though, to be honest, I don't know the details of how the "Result Comparison" feature in Eclipse is implemented. So you'd want to investigate that.

Other IDEs and reporting tools might depend on the built-in format for the failure message as well.

or is there a strong reason to go with the default message?

See above for reasons you would want to stay with the default message format.

Whether or not you override getMessage() really depends on your use case, your target audience, and what it is you hope to achieve.

In light of that, I am closing this issue since it is more of a question than an issue.

BenWoodworth commented 10 months ago

Just want to add that I've had no trouble overriding getMessage (and just passing null for the header). Obviously can't speak for all tooling, but IntelliJ and Eclipse both lean on getFailures() and recursively retrieve them all (and then AssertionFailedError's expected/actual functions for comparison):

https://github.com/JetBrains/intellij-community/blob/49ae60200a26cabd4ccf8de98609c95af262a54a/plugins/junit5_rt/src/com/intellij/junit5/JUnit5TestExecutionListener.java#L262-L264

https://github.com/eclipse-jdt/eclipse.jdt.ui/blob/c4f796f006242a2cb6d9422367d7d6c46fcf9b2d/org.eclipse.jdt.junit5.runtime/src/org/eclipse/jdt/internal/junit5/runner/JUnit5TestListener.java#L138