Avoid truncation when truncating means longer output#10446
Conversation
4faa5c0 to
25fbd3f
Compare
|
@nicoddemus anything I can do to make the review easier ? :) |
nicoddemus
left a comment
There was a problem hiding this comment.
Hi @Pierre-Sassoulas, sorry for the delay and thanks for the ping.
LGTM, left a few small comments, please take a look. 👍
| @@ -0,0 +1,2 @@ | |||
| The full output of a test is no longer truncated if the truncation message would be longer than | |||
There was a problem hiding this comment.
I think this file would be better marked as improvement in the changelog, so can you please rename it to 6267.improvement.rst?
src/_pytest/assertion/truncate.py
Outdated
| # Add ellipsis to final line | ||
| truncated_explanation[-1] = truncated_explanation[-1] + "..." | ||
| # We reevaluate the need to truncate following removal of some lines | ||
| if len("".join(input_lines)) > tolerable_max_chars: |
There was a problem hiding this comment.
Shouldn't you use truncated_explanation here?
| if len("".join(input_lines)) > tolerable_max_chars: | |
| if len("".join(truncated_explanation)) > tolerable_max_chars: |
There was a problem hiding this comment.
If so, probably best to update the comment above:
# We reevaluate the need to truncate chars following removal of some lines
There was a problem hiding this comment.
Nice catch ! This led to further improvement in 4d2c40f
|
I can cleanup the history and rebase if you want. I'm not sure if the current fail from the CI is due to my changes, so maybe rebasing would help ? |
The failure will be fixed in #10578, once we merge that don't worry that we can rebase this PR. 👍 |
nicoddemus
left a comment
There was a problem hiding this comment.
Thanks @Pierre-Sassoulas!
LGTM, great work! I will leave this up for a few more days in case anybody else wants to review it.
|
@Pierre-Sassoulas |
Also fixes the dislayed line hidden message Closes pytest-dev#6267
b3fddc4 to
a5f6d75
Compare
|
Strange, "allow edit by maintainer" is checked. I rebased, I'm going to check the macos fail later today :) |
|
I triggered the job again, and now it passed, so it seems like it was a flaky error, so do not worry. |
The full output of a test is no longer truncated if the truncation message would be longer than
the hidden text. The line number shown has also been fixed.
I also used f-string for faster string formatting in a previous commit, I don't think this warrant another MR though.
Closes #6267