Print source code for all failures #174
Conversation
a05c8a2 to
e6100aa
Compare
ae81b78 to
1b99541
Compare
1b99541 to
e1fae77
Compare
| case LoggedEvent.Error(msg) => msg | ||
| }) { actual => | ||
| expect.same(actual, expected) | ||
| expect.same(expected, actual) |
There was a problem hiding this comment.
mmmmm, not your fault as I should probably pay closer attention to earlier reviews, but this is making me eyebrows . I'm pretty sure that unit considers the second argument to be the expected one. Are we differing from it ?
Even our error messages seem to validate that expected should come second 😅
There was a problem hiding this comment.
It wasn't something that crept into recent PRs. It was (expected, found) before the refactor, as seen in the old repo, so I kept it the same when doing the rewrite.
MUnit's assertEquals is (found, expected), as shown in the source code, so is unfortunately the opposite.
It would be great to align the two, but it would be very cumbersome for users to switch everything around. We could recommend people use named parameters (expected = ..., found = ...).
Digging into our error messages, I think they're ok, unless there's something I'm missing? The corresponding test has expected as 1 and obtained as 2, so the error message is correct. The guide also has found as the first argument in the error messages.
I changed the order here because I got very confused at the test error messages in the first place 😅 .
There was a problem hiding this comment.
Oh well, as long as we're consistent with ourselves, I think that's okay 😅
This PR adds source code to all failure messages.
This includes weaver expectations such as
expect.sameandfailure, but should also work for user-propagated source locations.Example
Pointing to the failure
For Scala 2, the source position obtained from the
SourceLocationMacrocorresponds to the opening brace of the expectation. For example, the◊in:For Scala 3, it corresponds to the closing brace:
We use the position to point to the failing expectation: