-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Codex/assertion text diff blocks #14425
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
base: main
Are you sure you want to change the base?
Changes from all commits
e720397
5a2a014
2a40e38
ef90179
b9e20c4
499aaa9
e5f209e
84dc2f4
4529bf4
d093267
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| Added the :confval:`assertion_text_diff_style` configuration option, allowing | ||
| multiline string equality failures to be rendered as separate ``Left:`` and | ||
| ``Right:`` blocks instead of ``ndiff`` output. |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2699,6 +2699,32 @@ passed multiple times. The expected format is ``name=value``. For example:: | |||||
| A special value of ``"auto"`` can be used to explicitly use the global verbosity level. | ||||||
|
|
||||||
|
|
||||||
| .. confval:: assertion_text_diff_style | ||||||
| :type: ``str`` | ||||||
| :default: ``"ndiff"`` | ||||||
|
|
||||||
| Set how pytest renders diffs for string equality assertions. | ||||||
|
|
||||||
| Supported values are: | ||||||
|
|
||||||
| * ``ndiff``: use the default inline diff rendering. | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| * ``block``: render multiline string comparisons as separate ``Left:`` and ``Right:`` blocks. | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| .. tab:: toml | ||||||
|
|
||||||
| .. code-block:: toml | ||||||
|
|
||||||
| [pytest] | ||||||
| assertion_text_diff_style = "block" | ||||||
|
|
||||||
| .. tab:: ini | ||||||
|
|
||||||
| .. code-block:: ini | ||||||
|
|
||||||
| [pytest] | ||||||
| assertion_text_diff_style = block | ||||||
|
|
||||||
|
|
||||||
| .. confval:: verbosity_subtests | ||||||
| :type: ``str`` | ||||||
| :default: ``"auto"`` | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,6 +57,15 @@ def pytest_addoption(parser: Parser) -> None: | |
| default=None, | ||
| help=("Set threshold of CHARS after which truncation will take effect"), | ||
| ) | ||
| parser.addini( | ||
| "assertion_text_diff_style", | ||
| default=util.ASSERTION_TEXT_DIFF_STYLE_NDIFF, | ||
| help=( | ||
| "Choose how pytest renders diffs for string equality assertions: " | ||
| f"{util.ASSERTION_TEXT_DIFF_STYLE_NDIFF} or " | ||
| f"{util.ASSERTION_TEXT_DIFF_STYLE_BLOCK} for multiline strings" | ||
| ), | ||
| ) | ||
|
|
||
| Config._add_verbosity_ini( | ||
| parser, | ||
|
|
@@ -68,6 +77,10 @@ def pytest_addoption(parser: Parser) -> None: | |
| ) | ||
|
|
||
|
|
||
| def pytest_configure(config: Config) -> None: | ||
| util.validate_assertion_text_diff_style(config) | ||
|
|
||
|
|
||
| def register_assert_rewrite(*names: str) -> None: | ||
| """Register one or more module names to be rewritten on import. | ||
|
|
||
|
|
@@ -210,10 +223,15 @@ def pytest_assertrepr_compare( | |
| else: | ||
| # Keep it plaintext when not using terminalrepoterer (#14377). | ||
| highlighter = util.dummy_highlighter | ||
| return util.assertrepr_compare( | ||
| op=op, | ||
| left=left, | ||
| right=right, | ||
| verbose=config.get_verbosity(Config.VERBOSITY_ASSERTIONS), | ||
| highlighter=highlighter, | ||
| ) | ||
| saved_config = util._config | ||
| util._config = config | ||
|
Comment on lines
+226
to
+227
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this necessary? Is the config being passed here different from the one already in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, given this is the only place where |
||
| try: | ||
| return util.assertrepr_compare( | ||
| op=op, | ||
| left=left, | ||
| right=right, | ||
| verbose=config.get_verbosity(Config.VERBOSITY_ASSERTIONS), | ||
| highlighter=highlighter, | ||
| ) | ||
| finally: | ||
| util._config = saved_config | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -23,6 +23,7 @@ | |||||
| from _pytest._io.saferepr import saferepr_unlimited | ||||||
| from _pytest.compat import running_on_ci | ||||||
| from _pytest.config import Config | ||||||
| from _pytest.config import UsageError | ||||||
|
|
||||||
|
|
||||||
| # The _reprcompare attribute on the util module is used by the new assertion | ||||||
|
|
@@ -38,6 +39,14 @@ | |||||
| # Config object which is assigned during pytest_runtest_protocol. | ||||||
| _config: Config | None = None | ||||||
|
|
||||||
| ASSERTION_TEXT_DIFF_STYLE_INI = "assertion_text_diff_style" | ||||||
| ASSERTION_TEXT_DIFF_STYLE_NDIFF = "ndiff" | ||||||
| ASSERTION_TEXT_DIFF_STYLE_BLOCK = "block" | ||||||
| ASSERTION_TEXT_DIFF_STYLE_CHOICES = ( | ||||||
| ASSERTION_TEXT_DIFF_STYLE_NDIFF, | ||||||
| ASSERTION_TEXT_DIFF_STYLE_BLOCK, | ||||||
| ) | ||||||
|
|
||||||
|
|
||||||
| class _HighlightFunc(Protocol): | ||||||
| def __call__(self, source: str, lexer: Literal["diff", "python"] = "python") -> str: | ||||||
|
|
@@ -52,6 +61,22 @@ def dummy_highlighter(source: str, lexer: Literal["diff", "python"] = "python") | |||||
| return source | ||||||
|
|
||||||
|
|
||||||
| def get_assertion_text_diff_style(config: Config) -> str: | ||||||
| style = str(config.getini(ASSERTION_TEXT_DIFF_STYLE_INI)) | ||||||
| if style not in ASSERTION_TEXT_DIFF_STYLE_CHOICES: | ||||||
| choices = ", ".join( | ||||||
| repr(choice) for choice in ASSERTION_TEXT_DIFF_STYLE_CHOICES | ||||||
| ) | ||||||
| raise UsageError( | ||||||
| f"{ASSERTION_TEXT_DIFF_STYLE_INI} must be one of {choices}; got {style!r}" | ||||||
| ) | ||||||
| return style | ||||||
|
|
||||||
|
|
||||||
| def validate_assertion_text_diff_style(config: Config) -> None: | ||||||
| get_assertion_text_diff_style(config) | ||||||
|
|
||||||
|
|
||||||
| def format_explanation(explanation: str) -> str: | ||||||
| r"""Format an explanation. | ||||||
|
|
||||||
|
|
@@ -182,6 +207,11 @@ def assertrepr_compare( | |||||
| highlighter: _HighlightFunc, | ||||||
| ) -> list[str] | None: | ||||||
| """Return specialised explanations for some operators/operands.""" | ||||||
| assertion_text_diff_style = ( | ||||||
| get_assertion_text_diff_style(_config) | ||||||
| if _config is not None | ||||||
| else ASSERTION_TEXT_DIFF_STYLE_NDIFF | ||||||
| ) | ||||||
| # Strings which normalize equal are often hard to distinguish when printed; use ascii() to make this easier. | ||||||
| # See issue #3246. | ||||||
| use_ascii = ( | ||||||
|
|
@@ -208,7 +238,13 @@ def assertrepr_compare( | |||||
| explanation = None | ||||||
| try: | ||||||
| if op == "==": | ||||||
| explanation = _compare_eq_any(left, right, highlighter, verbose) | ||||||
| explanation = _compare_eq_any( | ||||||
| left, | ||||||
| right, | ||||||
| highlighter, | ||||||
| verbose, | ||||||
| assertion_text_diff_style, | ||||||
| ) | ||||||
| elif op == "not in": | ||||||
| if istext(left) and istext(right): | ||||||
| explanation = _notin_text(left, right, verbose) | ||||||
|
|
@@ -246,11 +282,21 @@ def assertrepr_compare( | |||||
|
|
||||||
|
|
||||||
| def _compare_eq_any( | ||||||
| left: object, right: object, highlighter: _HighlightFunc, verbose: int = 0 | ||||||
| left: object, | ||||||
| right: object, | ||||||
| highlighter: _HighlightFunc, | ||||||
| verbose: int = 0, | ||||||
| assertion_text_diff_style: str = ASSERTION_TEXT_DIFF_STYLE_NDIFF, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Suggested change
|
||||||
| ) -> list[str]: | ||||||
| explanation = [] | ||||||
| if istext(left) and istext(right): | ||||||
| explanation = _diff_text(left, right, highlighter, verbose) | ||||||
| explanation = _compare_eq_text( | ||||||
| left, | ||||||
| right, | ||||||
| highlighter, | ||||||
| verbose, | ||||||
| assertion_text_diff_style, | ||||||
| ) | ||||||
| else: | ||||||
| from _pytest.python_api import ApproxBase | ||||||
|
|
||||||
|
|
@@ -281,6 +327,40 @@ def _compare_eq_any( | |||||
| return explanation | ||||||
|
|
||||||
|
|
||||||
| def _compare_eq_text( | ||||||
| left: str, | ||||||
| right: str, | ||||||
| highlighter: _HighlightFunc, | ||||||
| verbose: int, | ||||||
| assertion_text_diff_style: str, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of typing this as |
||||||
| ) -> list[str]: | ||||||
| if ( | ||||||
| assertion_text_diff_style == ASSERTION_TEXT_DIFF_STYLE_BLOCK | ||||||
| and _is_multiline_text(left, right) | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||||||
| and not (left.isspace() or right.isspace()) | ||||||
| ): | ||||||
| return _diff_text_block(left, right) | ||||||
| return _diff_text(left, right, highlighter, verbose) | ||||||
|
|
||||||
|
|
||||||
| def _is_multiline_text(*texts: str) -> bool: | ||||||
| return any("\n" in text or "\r" in text for text in texts) | ||||||
|
|
||||||
|
|
||||||
| def _diff_text_block(left: str, right: str) -> list[str]: | ||||||
| return [ | ||||||
| "Left:", | ||||||
| *_format_text_block_lines(left), | ||||||
| "", | ||||||
| "Right:", | ||||||
| *_format_text_block_lines(right), | ||||||
| ] | ||||||
|
|
||||||
|
|
||||||
| def _format_text_block_lines(text: str) -> list[str]: | ||||||
| return [f" {line}" for line in text.split("\n")] | ||||||
|
|
||||||
|
|
||||||
| def _diff_text( | ||||||
| left: str, right: str, highlighter: _HighlightFunc, verbose: int = 0 | ||||||
| ) -> list[str]: | ||||||
|
|
||||||
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.
Let's clearly present the two options: