Improve failure message of expect.all
#166
Conversation
f63024c to
e65a3ba
Compare
e65a3ba to
d300c32
Compare
| |- (all) 0ms | ||
| | [0] assertion 2 of 5 failed: clue(x) == clue(y) (modules/framework-cats/shared/src/test/scala/Meta.scala:102) | ||
| | [0] | ||
| | [0] Clues { | ||
| | [0] x: Int = 1 | ||
| | [0] y: Int = 2 | ||
| | [0] } | ||
| | | ||
| | [1] assertion 4 of 5 failed: clue(y) == clue(z) (modules/framework-cats/shared/src/test/scala/Meta.scala:102) | ||
| | [1] | ||
| | [1] Clues { | ||
| | [1] y: Int = 2 | ||
| | [1] z: Int = 3 | ||
| | [1] } |
There was a problem hiding this comment.
I wonder how useful the 2 out of 5 and 4 out of 5 are. Should we have an extra log line like:
assertions:
1: x == x
2: clue(x) == clue(y)
3: y == y
4: clue(y) == clue(z)
5: z == z
Or maybe get rid of the numbering altogether?
There was a problem hiding this comment.
Getting rid of the numbering is easiest to maintain. I added the numbering at first, thinking the source code would be challenging.
We can work on formatting source code in error messages in future PRs.
build.sbt
Outdated
| @@ -12,7 +12,7 @@ import laika.config.LinkValidation | |||
| import org.typelevel.sbt.site.TypelevelSiteSettings | |||
| import sbt.librarymanagement.Configurations.ScalaDocTool | |||
| // https://typelevel.org/sbt-typelevel/faq.html#what-is-a-base-version-anyway | |||
| ThisBuild / tlBaseVersion := "0.9" // your current series x.y | |||
| ThisBuild / tlBaseVersion := "0.10" // your current series x.y | |||
There was a problem hiding this comment.
The Clues.toExpectations signature has changed from accepting a Boolean* to a Boolean. This is source compatible, since no one should be calling it directly. It's not binary compatible, as libraries with expect compiled against 0.9.1 will refer to the old Clues.toExpectations signature.
Since we're breaking binary compatibility and using early semver, we need to bump the minor version.
An alternative approach (just to keep the same minor version) is to keep the Boolean* signature as is, and only pass in one argument. We could move it to Boolean when we next need to bump the minor version.
There was a problem hiding this comment.
I decided on keeping Boolean* for the moment to preserve compatibility.
9760c81 to
24cf553
Compare
Problem
As described in #163 ,
expect.allcurrently displays a generalassertion failedmessage when any assertion fails:It doesn't indicate which of the assertions in
expect.allfailed.Improvement
This PR adds the source code and index of the assertion into the failure message.
In addition, the collection of clues is assertion-specific.
Example failure message
The failing test:
Has the message:
Binary compatibility
The
Clues.toExpectationssignature has changed from accepting aBoolean*to aBoolean. This is source compatible, since no one should be calling it directly. It's not binary compatible, as libraries withexpectcompiled against0.9.1will refer to the oldClues.toExpectationssignature.