Fix diff output for data types where -v would show less information#9661
Fix diff output for data types where -v would show less information#9661nicoddemus merged 1 commit intopytest-dev:mainfrom
-v would show less information#9661Conversation
`_compare_eq_verbose` doesn't seem to useful in the end, with the examples we have in the test suite. Close pytest-dev#5192
| if not verbose and not running_on_ci(): | ||
| return ["Use -v to get the full diff"] | ||
| if verbose <= 0 and not running_on_ci(): | ||
| return ["Use -v to get more diff"] |
There was a problem hiding this comment.
The "full diff" here is misleading, because the users would use -v, and in the next run we show Use -vv to get the full diff again if the output is too large.
|
@nicoddemus Yes, this is much better. This resolves the issue seen earlier. I'm currently testing various assertion combinations, and so far this change seems to have the desired outcome. |
| @@ -0,0 +1,3 @@ | |||
| Fixed test output for some data types where ``-v`` would show less information. | |||
There was a problem hiding this comment.
Marked this as an "improvement", but arguably this can be considered a bugfix.
bluetech
left a comment
bluetech
left a comment
There was a problem hiding this comment.
TL;DR - LGTM :)
IIUC the situation is this:
- Assertion rewrite calls the reprcompare mechanism, with custom "expls" for the assertion (the rewrite special sauce)
- But before using these special expls, it also calls the
pytest_assertrepr_comparehook to see if it wants to offer its own formatting (which is preferred). The hook only gets the op and values (no special stringification sauce). - The built-in
pytest_assertrepr_comparehookimpl in this case eventually reaches_compare_eq_any.
Without verbose, _compare_eq_any returns None for this, and the custom expls are used.
With verbose, before this fix, _compare_eq_verbose would be called, which ends up with
- Less nice formatting, because no special sauce.
- Less verbose than the expls, because no special sauce, only shows the actual values.
This PR removes _compare_eq_verbose so it falls back to the rewrite expls.
So I looked into why _compare_eq_any was added in the first place. AFAIU, it used to be that the rewrite formatting didn't take verbosity in account when truncating - it always truncated. So _compare_eq_any was added as a workaround. But the underlying problem has since been fixed by @nicoddemus in be8d63e. So this is probably no longer needed.
|
Thanks @bluetech for looking into why Do you agree this is an improvement, or should we treat it as bug fix and backport to 7.0.x? |
|
I think the compare-any is an improvement, I wouldn't backport it since it's not risk-free either. The We probably shouldn't wait too long with pytest 7.1 though. I will open an issue for a release schedule. |
3 participants
_compare_eq_verbosedoesn't seem to useful in the end, with the examples wehave in the test suite.
Close #5192
cc @mdmintz