-
Notifications
You must be signed in to change notification settings - Fork 340
Improve failure message for comparison expectations #2170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Using the digits calculation from waldo, displaying the actual calculation, comparison, and renaming "more" to "greater" Fixes #2006
R/expect-comparison.R
Outdated
| ">=" = "not greater than" | ||
| )[[operator]] | ||
|
|
||
| negated_op <- switch(operator, "<" = ">", "<=" = ">=", ">" = "<", ">=" = "<=") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't these wrong? e.g. I think we'll get
expect_snapshot_failure(expect_gt(x, 1.0 * x))
`x` is not strictly greater than 1.0 * x.
0.0000100 - 0.0000110 = 0 < 0
The negated open interval is a closed interval, and vice versa.
(even if I'm reading it wrong we'll still want a test of the edge case)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooops, yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also that revealed a bug when the number of digits in the difference is much smaller than the values.
MichaelChirico
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, output LGTM! It might be helpful to add another example where the values are totally different just to get a glimpse of what that will look like.
Using the digits calculation from waldo, displaying the actual calculation, comparison, and renaming "more" to "greater"
Fixes #2006
@MichaelChirico could you please take a look? This doesn't need to be perfect, but is hopefully better than the current display.