Less discarding of test failure throwables#183
Less discarding of test failure throwables#183Baccata merged 1 commit intotypelevel:mainfrom dubinsky:less-discarding-of-test-failure-throwables
Conversation
| case Result.Failure(_, maybeCause, _) => maybeCause | ||
| case _ => None | ||
| case Result.Failures(failures) => | ||
| failures.map(_.source).find(_.isDefined).flatten |
There was a problem hiding this comment.
| failures.map(_.source).find(_.isDefined).flatten | |
| failures.collectFirst { | |
| case Result.Failure(_, Some(cause), _) => cause | |
| } |
|
Thanks! My only concern (which doesn't need to be addressed) is that we're only taking the first failure and discarding others. If this ends up being a problem, we could define |
... and more propagating thereof. On one end, it seems a shame to discard information that we have. On the other end, receiver of the `sbt.testing.Event` - in my case, [Gradle plugin for multi-backend Scala](https://github.com/dubinsky/scalajs-gradle) - has to provide a throwable to Gradle, which requires one to accompany a test failure, and if it is not propagated by the test framework, a fake throwable has to be concocted (ugh). This change makes the ends meet ;)
Yes, this asymmetricity bothers me too; but for test failure reporting, one - any one! - is better than none... |
Baccata
left a comment
There was a problem hiding this comment.
I think this is good enough for now, in the sense that it makes the library better for a rather niche usecase without impacting the majority.
|
Reporting the cause of the test failure does not seem like an niche use case to me; do I need to do anything for this to get merged? |
|
A little more context: there is no throwable in the Thanks! |
That is what I meant. My point is that if mistake there is, you're the first reporter in years being noticeably impacted by it, hence the "niche" denomination. I presume the use of Gradle is what led for this to be noticed, and Gradle is a somewhat niche build-tool in the Scala community. Didn't mean anything more by it, providing a mitigation is indeed important. We can start with the mitigation you implemented in this PR and take the time to assess what should be done for a more proper fix. |
|
Thanks! |
... and more propagating thereof.
On one end, it seems a shame to discard information that we have.
On the other end, receiver of the
sbt.testing.Event- in my case, Gradle plugin for multi-backend Scala - has to provide a throwable to Gradle, which requires one to accompany a test failure, and if it is not propagated by the test framework, a fake throwable has to be concocted (ugh).This change makes the ends meet ;)