Codex/assertion text diff blocks#14425
Conversation
Closes pytest-dev#8395 Co-authored-by: Antigravity <antigravity@google.com>
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
|
@RonnyPfannschmidt I'm still waiting on your feedback :) |
RonnyPfannschmidt
left a comment
There was a problem hiding this comment.
This. Technically looks good but is also at odds with a number of comments on the original issue
As such id like to defer to @nicoddemus
nicoddemus
left a comment
There was a problem hiding this comment.
Thanks @hamza-mobeen for the PR, overall great work!
I left some comments, please take a look.
| :confval:`assertion_text_diff_style`: Controls how pytest renders ``str == str`` failures. The default ``ndiff`` output | ||
| keeps the existing inline diff markers. Setting it to ``block`` prints multiline string comparisons as separate | ||
| ``Left:`` and ``Right:`` blocks, which can be easier to read when whitespace or indentation differences dominate. |
There was a problem hiding this comment.
Let's clearly present the two options:
| :confval:`assertion_text_diff_style`: Controls how pytest renders ``str == str`` failures. The default ``ndiff`` output | |
| keeps the existing inline diff markers. Setting it to ``block`` prints multiline string comparisons as separate | |
| ``Left:`` and ``Right:`` blocks, which can be easier to read when whitespace or indentation differences dominate. | |
| :confval:`assertion_text_diff_style`: Controls how pytest renders ``str == str`` failures. | |
| * ``ndiff`` (the default) outputs the differences using inline diff markers. | |
| * ``block`` prints multiline string comparisons as separate ``Left:`` and ``Right:`` blocks, which can be easier to read when whitespace or indentation differences dominate. | |
| Note that it is possible to set this option (as any other configuration option) directly in the command line using ``-o assertion_text_diff_style=block``. |
|
|
||
| Supported values are: | ||
|
|
||
| * ``ndiff``: use the default inline diff rendering. |
There was a problem hiding this comment.
| * ``ndiff``: use the default inline diff rendering. | |
| * ``ndiff``: use the inline diff rendering markers. |
| Supported values are: | ||
|
|
||
| * ``ndiff``: use the default inline diff rendering. | ||
| * ``block``: render multiline string comparisons as separate ``Left:`` and ``Right:`` blocks. |
There was a problem hiding this comment.
| * ``block``: render multiline string comparisons as separate ``Left:`` and ``Right:`` blocks. | |
| * ``block``: render each string in separate ``Left:`` and ``Right:`` blocks. |
| saved_config = util._config | ||
| util._config = config |
There was a problem hiding this comment.
Why is this necessary? Is the config being passed here different from the one already in util._config?
There was a problem hiding this comment.
Also, given this is the only place where util.assertrepr_compare is called, consider just passing the text diff style directly to util.assertrepr_compare.
| right: object, | ||
| highlighter: _HighlightFunc, | ||
| verbose: int = 0, | ||
| assertion_text_diff_style: str = ASSERTION_TEXT_DIFF_STYLE_NDIFF, |
There was a problem hiding this comment.
Lets remove this default value and force callers to always pass it; it is an internal function, so breaking the API is actually a good thing because it will ensure every caller is actually passing the right thing.
| assertion_text_diff_style: str = ASSERTION_TEXT_DIFF_STYLE_NDIFF, | |
| assertion_text_diff_style: str, |
| ) -> list[str]: | ||
| if ( | ||
| assertion_text_diff_style == ASSERTION_TEXT_DIFF_STYLE_BLOCK | ||
| and _is_multiline_text(left, right) |
There was a problem hiding this comment.
Not sure we should special case being multiline text or not; I think it is reasonable to always honor the block configuration, even for single line texts.
| right: str, | ||
| highlighter: _HighlightFunc, | ||
| verbose: int, | ||
| assertion_text_diff_style: str, |
There was a problem hiding this comment.
Instead of typing this as str, let's use Literal["ndiff", "block"]. This way we are more explicit, plus we can use match below.
closes #6757
Description
This PR introduces a new configuration option,
assertion_text_diff_style, to allow users to customize how multiline string assertion failures are displayed.As highlighted in #6757, when
pytestusesndiffto display differences for multiline strings (like output fromcapsys), it can sometimes become an unreadable "soup", especially when changes involve indentation or leading/trailing whitespace. The current behavior often results in a confusing line-by-line diff that is difficult to parse.This PR adds an optional
blockdiff style that displays the exact contents of the "Left" and "Right" text blocks intact and sequentially, making it significantly easier to read the literal multiline strings being compared without the interspersedndiffnoise.Key changes:
assertion_text_diff_styleconfiguration option. Defaults tondiffto preserve backwards compatibility.blockdiff style (_diff_text_block) for multiline string comparisons, solving the readability issue raised in assertion diffs for multiline-string can become unreadable soup #6757.ndiffeven ifblockis selected.assertion_text_diff_styleis eitherndifforblock.testing/test_assertion.pyverifying block formatting, fallback logic, blank line handling, and configuration validation.Checklist
closes #XYZWto the PR description and/or commits (whereXYZWis the issue number).Co-authored-bycommit trailers.changelogdirectory, with a name like<ISSUE NUMBER>.<TYPE>.rst.AUTHORSin alphabetical order.