From eace6c4cada55fe7ae51a1b7a6a6c477f05eee18 Mon Sep 17 00:00:00 2001 From: Benjamin Schubert Date: Sat, 21 Oct 2023 10:27:13 +0100 Subject: [PATCH 1/7] Color the full diff that pytest shows as a diff Previously, it would get printed all in red, which makes it hard to read and actually understand. However, the diffs shown are standard and have a supported lexer in Pygments. As such, use this to color the output when pygments is available. --- AUTHORS | 1 + changelog/11520.improvement.rst | 1 + src/_pytest/_io/terminalwriter.py | 15 ++++-- src/_pytest/assertion/util.py | 31 ++++++++---- testing/conftest.py | 9 ++++ testing/test_assertion.py | 78 +++++++++++++++++++++++++++++++ 6 files changed, 123 insertions(+), 12 deletions(-) create mode 100644 changelog/11520.improvement.rst diff --git a/AUTHORS b/AUTHORS index f3461addfb7..5ccff644c1c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -56,6 +56,7 @@ Barney Gale Ben Gartner Ben Webb Benjamin Peterson +Benjamin Schubert Bernard Pratz Bo Wu Bob Ippolito diff --git a/changelog/11520.improvement.rst b/changelog/11520.improvement.rst new file mode 100644 index 00000000000..7cfb45b0b38 --- /dev/null +++ b/changelog/11520.improvement.rst @@ -0,0 +1 @@ +* Improved very verbose diff output to color it as a diff instead of only red. diff --git a/src/_pytest/_io/terminalwriter.py b/src/_pytest/_io/terminalwriter.py index eb1b469395e..934278b93ed 100644 --- a/src/_pytest/_io/terminalwriter.py +++ b/src/_pytest/_io/terminalwriter.py @@ -3,6 +3,7 @@ import shutil import sys from typing import final +from typing import Literal from typing import Optional from typing import Sequence from typing import TextIO @@ -193,15 +194,21 @@ def _write_source(self, lines: Sequence[str], indents: Sequence[str] = ()) -> No for indent, new_line in zip(indents, new_lines): self.line(indent + new_line) - def _highlight(self, source: str) -> str: - """Highlight the given source code if we have markup support.""" + def _highlight( + self, source: str, lexer: Literal["diff", "python"] = "python" + ) -> str: + """Highlight the given source if we have markup support.""" from _pytest.config.exceptions import UsageError if not self.hasmarkup or not self.code_highlight: return source try: from pygments.formatters.terminal import TerminalFormatter - from pygments.lexers.python import PythonLexer + + if lexer == "python": + from pygments.lexers.python import PythonLexer as Lexer + elif lexer == "diff": + from pygments.lexers.diff import DiffLexer as Lexer from pygments import highlight import pygments.util except ImportError: @@ -210,7 +217,7 @@ def _highlight(self, source: str) -> str: try: highlighted: str = highlight( source, - PythonLexer(), + Lexer(), TerminalFormatter( bg=os.getenv("PYTEST_THEME_MODE", "dark"), style=os.getenv("PYTEST_THEME"), diff --git a/src/_pytest/assertion/util.py b/src/_pytest/assertion/util.py index 057352cd387..2d8067294a3 100644 --- a/src/_pytest/assertion/util.py +++ b/src/_pytest/assertion/util.py @@ -17,6 +17,7 @@ from _pytest._io.saferepr import _pformat_dispatch from _pytest._io.saferepr import saferepr from _pytest._io.saferepr import saferepr_unlimited +from _pytest._io.terminalwriter import TerminalWriter from _pytest.config import Config # The _reprcompare attribute on the util module is used by the new assertion @@ -189,7 +190,8 @@ def assertrepr_compare( explanation = None try: if op == "==": - explanation = _compare_eq_any(left, right, verbose) + writer = config.get_terminal_writer() + explanation = _compare_eq_any(left, right, writer, verbose) elif op == "not in": if istext(left) and istext(right): explanation = _notin_text(left, right, verbose) @@ -225,7 +227,9 @@ def assertrepr_compare( return [summary] + explanation -def _compare_eq_any(left: Any, right: Any, verbose: int = 0) -> List[str]: +def _compare_eq_any( + left: Any, right: Any, writer: TerminalWriter, verbose: int = 0 +) -> List[str]: explanation = [] if istext(left) and istext(right): explanation = _diff_text(left, right, verbose) @@ -245,7 +249,7 @@ def _compare_eq_any(left: Any, right: Any, verbose: int = 0) -> List[str]: # field values, not the type or field names. But this branch # intentionally only handles the same-type case, which was often # used in older code bases before dataclasses/attrs were available. - explanation = _compare_eq_cls(left, right, verbose) + explanation = _compare_eq_cls(left, right, writer, verbose) elif issequence(left) and issequence(right): explanation = _compare_eq_sequence(left, right, verbose) elif isset(left) and isset(right): @@ -254,7 +258,7 @@ def _compare_eq_any(left: Any, right: Any, verbose: int = 0) -> List[str]: explanation = _compare_eq_dict(left, right, verbose) if isiterable(left) and isiterable(right): - expl = _compare_eq_iterable(left, right, verbose) + expl = _compare_eq_iterable(left, right, writer, verbose) explanation.extend(expl) return explanation @@ -321,7 +325,10 @@ def _surrounding_parens_on_own_lines(lines: List[str]) -> None: def _compare_eq_iterable( - left: Iterable[Any], right: Iterable[Any], verbose: int = 0 + left: Iterable[Any], + right: Iterable[Any], + writer: TerminalWriter, + verbose: int = 0, ) -> List[str]: if verbose <= 0 and not running_on_ci(): return ["Use -v to get more diff"] @@ -346,7 +353,13 @@ def _compare_eq_iterable( # "right" is the expected base against which we compare "left", # see https://github.com/pytest-dev/pytest/issues/3333 explanation.extend( - line.rstrip() for line in difflib.ndiff(right_formatting, left_formatting) + writer._highlight( + "\n".join( + line.rstrip() + for line in difflib.ndiff(right_formatting, left_formatting) + ), + lexer="diff", + ).splitlines() ) return explanation @@ -496,7 +509,9 @@ def _compare_eq_dict( return explanation -def _compare_eq_cls(left: Any, right: Any, verbose: int) -> List[str]: +def _compare_eq_cls( + left: Any, right: Any, writer: TerminalWriter, verbose: int +) -> List[str]: if not has_default_eq(left): return [] if isdatacls(left): @@ -542,7 +557,7 @@ def _compare_eq_cls(left: Any, right: Any, verbose: int) -> List[str]: ] explanation += [ indent + line - for line in _compare_eq_any(field_left, field_right, verbose) + for line in _compare_eq_any(field_left, field_right, writer, verbose) ] return explanation diff --git a/testing/conftest.py b/testing/conftest.py index 06116fee49d..6a50e810fbd 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -160,6 +160,9 @@ class ColorMapping: "red": "\x1b[31m", "green": "\x1b[32m", "yellow": "\x1b[33m", + "light-gray": "\x1b[90m", + "light-red": "\x1b[91m", + "light-green": "\x1b[92m", "bold": "\x1b[1m", "reset": "\x1b[0m", "kw": "\x1b[94m", @@ -171,6 +174,7 @@ class ColorMapping: "endline": "\x1b[90m\x1b[39;49;00m", } RE_COLORS = {k: re.escape(v) for k, v in COLORS.items()} + NO_COLORS = {k: "" for k in COLORS.keys()} @classmethod def format(cls, lines: List[str]) -> List[str]: @@ -187,6 +191,11 @@ def format_for_rematch(cls, lines: List[str]) -> List[str]: """Replace color names for use with LineMatcher.re_match_lines""" return [line.format(**cls.RE_COLORS) for line in lines] + @classmethod + def strip_colors(cls, lines: List[str]) -> List[str]: + """Entirely remove every color code""" + return [line.format(**cls.NO_COLORS) for line in lines] + return ColorMapping diff --git a/testing/test_assertion.py b/testing/test_assertion.py index d3caa5e4848..bde19038086 100644 --- a/testing/test_assertion.py +++ b/testing/test_assertion.py @@ -18,12 +18,19 @@ def mock_config(verbose=0): + class TerminalWriter: + def _highlight(self, source, lexer): + return source + class Config: def getoption(self, name): if name == "verbose": return verbose raise KeyError("Not mocked out: %s" % name) + def get_terminal_writer(self): + return TerminalWriter() + return Config() @@ -1784,3 +1791,74 @@ def test_reprcompare_verbose_long() -> None: "{'v0': 0, 'v1': 1, 'v2': 12, 'v3': 3, 'v4': 4, 'v5': 5, " "'v6': 6, 'v7': 7, 'v8': 8, 'v9': 9, 'v10': 10}" ) + + +@pytest.mark.parametrize("enable_colors", [True, False]) +@pytest.mark.parametrize( + ("code", "expected_lines"), + ( + ( + """ + def test(): + assert [0, 1] == [0, 2] + """, + [ + "{bold}{red}E assert [0, 1] == [0, 2]{reset}", + "{bold}{red}E At index 1 diff: 1 != 2{reset}", + "{bold}{red}E Full diff:{reset}", + "{bold}{red}E {light-red}- [0, 2]{hl-reset}{endline}{reset}", + "{bold}{red}E ? ^{endline}{reset}", + "{bold}{red}E {light-green}+ [0, 1]{hl-reset}{endline}{reset}", + "{bold}{red}E ? ^{endline}{reset}", + ], + ), + ( + """ + def test(): + assert {f"number-is-{i}": i for i in range(1, 6)} == { + f"number-is-{i}": i for i in range(5) + } + """, + [ + ( + "{bold}{red}E AssertionError: assert " + "{{'number-is-1': 1, 'number-is-2': 2, 'number-is-3': 3, 'number-is-4': 4, 'number-is-5': 5}}" + " == {{'number-is-0': 0, 'number-is-1': 1, 'number-is-2': 2, 'number-is-3': 3, 'number-is-4': 4}}" + "{reset}" + ), + "{bold}{red}E Common items:{reset}", + ( + "{bold}{red}E " + "{{'number-is-1': 1, 'number-is-2': 2, 'number-is-3': 3, 'number-is-4': 4}}{reset}" + ), + "{bold}{red}E Left contains 1 more item:{reset}", + "{bold}{red}E {{'number-is-5': 5}}{reset}", + "{bold}{red}E Right contains 1 more item:{reset}", + "{bold}{red}E {{'number-is-0': 0}}{reset}", + "{bold}{red}E Full diff:{reset}", + "{bold}{red}E {light-gray} {hl-reset} {{{endline}{reset}", + "{bold}{red}E {light-red}- 'number-is-0': 0,{hl-reset}{endline}{reset}", + "{bold}{red}E {light-gray} {hl-reset} 'number-is-1': 1,{endline}{reset}", + "{bold}{red}E {light-gray} {hl-reset} 'number-is-2': 2,{endline}{reset}", + "{bold}{red}E {light-gray} {hl-reset} 'number-is-3': 3,{endline}{reset}", + "{bold}{red}E {light-gray} {hl-reset} 'number-is-4': 4,{endline}{reset}", + "{bold}{red}E {light-green}+ 'number-is-5': 5,{hl-reset}{endline}{reset}", + "{bold}{red}E {light-gray} {hl-reset} }}{endline}{reset}", + ], + ), + ), +) +def test_comparisons_handle_colors( + pytester: Pytester, color_mapping, enable_colors, code, expected_lines +) -> None: + p = pytester.makepyfile(code) + result = pytester.runpytest( + f"--color={'yes' if enable_colors else 'no'}", "-vv", str(p) + ) + formatter = ( + color_mapping.format_for_fnmatch + if enable_colors + else color_mapping.strip_colors + ) + + result.stdout.fnmatch_lines(formatter(expected_lines)) From 2f649bf5c22696aef8331ab55937bf18f6822a7f Mon Sep 17 00:00:00 2001 From: Benjamin Schubert Date: Sat, 21 Oct 2023 12:53:48 +0100 Subject: [PATCH 2/7] fixup! Color the full diff that pytest shows as a diff --- testing/test_assertion.py | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/testing/test_assertion.py b/testing/test_assertion.py index bde19038086..3598dfeb086 100644 --- a/testing/test_assertion.py +++ b/testing/test_assertion.py @@ -1803,13 +1803,8 @@ def test(): assert [0, 1] == [0, 2] """, [ - "{bold}{red}E assert [0, 1] == [0, 2]{reset}", - "{bold}{red}E At index 1 diff: 1 != 2{reset}", - "{bold}{red}E Full diff:{reset}", "{bold}{red}E {light-red}- [0, 2]{hl-reset}{endline}{reset}", - "{bold}{red}E ? ^{endline}{reset}", "{bold}{red}E {light-green}+ [0, 1]{hl-reset}{endline}{reset}", - "{bold}{red}E ? ^{endline}{reset}", ], ), ( @@ -1820,30 +1815,9 @@ def test(): } """, [ - ( - "{bold}{red}E AssertionError: assert " - "{{'number-is-1': 1, 'number-is-2': 2, 'number-is-3': 3, 'number-is-4': 4, 'number-is-5': 5}}" - " == {{'number-is-0': 0, 'number-is-1': 1, 'number-is-2': 2, 'number-is-3': 3, 'number-is-4': 4}}" - "{reset}" - ), - "{bold}{red}E Common items:{reset}", - ( - "{bold}{red}E " - "{{'number-is-1': 1, 'number-is-2': 2, 'number-is-3': 3, 'number-is-4': 4}}{reset}" - ), - "{bold}{red}E Left contains 1 more item:{reset}", - "{bold}{red}E {{'number-is-5': 5}}{reset}", - "{bold}{red}E Right contains 1 more item:{reset}", - "{bold}{red}E {{'number-is-0': 0}}{reset}", - "{bold}{red}E Full diff:{reset}", "{bold}{red}E {light-gray} {hl-reset} {{{endline}{reset}", - "{bold}{red}E {light-red}- 'number-is-0': 0,{hl-reset}{endline}{reset}", "{bold}{red}E {light-gray} {hl-reset} 'number-is-1': 1,{endline}{reset}", - "{bold}{red}E {light-gray} {hl-reset} 'number-is-2': 2,{endline}{reset}", - "{bold}{red}E {light-gray} {hl-reset} 'number-is-3': 3,{endline}{reset}", - "{bold}{red}E {light-gray} {hl-reset} 'number-is-4': 4,{endline}{reset}", "{bold}{red}E {light-green}+ 'number-is-5': 5,{hl-reset}{endline}{reset}", - "{bold}{red}E {light-gray} {hl-reset} }}{endline}{reset}", ], ), ), @@ -1861,4 +1835,4 @@ def test_comparisons_handle_colors( else color_mapping.strip_colors ) - result.stdout.fnmatch_lines(formatter(expected_lines)) + result.stdout.fnmatch_lines(formatter(expected_lines), consecutive=False) From 7cbe6ef913d4fdd2b126ff2f23323029d25f7723 Mon Sep 17 00:00:00 2001 From: Benjamin Schubert Date: Sat, 21 Oct 2023 14:06:06 +0100 Subject: [PATCH 3/7] fixup! fixup! Color the full diff that pytest shows as a diff --- testing/test_assertion.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/testing/test_assertion.py b/testing/test_assertion.py index 3598dfeb086..62c465d8a34 100644 --- a/testing/test_assertion.py +++ b/testing/test_assertion.py @@ -1795,7 +1795,7 @@ def test_reprcompare_verbose_long() -> None: @pytest.mark.parametrize("enable_colors", [True, False]) @pytest.mark.parametrize( - ("code", "expected_lines"), + ("test_code", "expected_lines"), ( ( """ @@ -1823,9 +1823,9 @@ def test(): ), ) def test_comparisons_handle_colors( - pytester: Pytester, color_mapping, enable_colors, code, expected_lines + pytester: Pytester, color_mapping, enable_colors, test_code, expected_lines ) -> None: - p = pytester.makepyfile(code) + p = pytester.makepyfile(test_code) result = pytester.runpytest( f"--color={'yes' if enable_colors else 'no'}", "-vv", str(p) ) From afccacb3b8ae88c790a30df2bdbb81d42a9f33c8 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sat, 21 Oct 2023 15:35:07 -0300 Subject: [PATCH 4/7] Ensure logging tests always cleanup after themselves Logging has many global states, and we did foresee this by creating a ``cleanup_disabled_logging`` fixture, however one might still forget to use it and failures leak later -- sometimes not even in the same PR, because the order of the tests might change in the future, specially when running under xdist. This problem surfaced during pytest-dev/pytest#11530, where tests unrelated to the change started to fail. --- testing/logging/test_fixture.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/testing/logging/test_fixture.py b/testing/logging/test_fixture.py index 8eaa2de96a8..193535bace5 100644 --- a/testing/logging/test_fixture.py +++ b/testing/logging/test_fixture.py @@ -1,5 +1,6 @@ # mypy: disable-error-code="attr-defined" import logging +from typing import Iterator import pytest from _pytest.logging import caplog_records_key @@ -9,8 +10,8 @@ sublogger = logging.getLogger(__name__ + ".baz") -@pytest.fixture -def cleanup_disabled_logging(): +@pytest.fixture(autouse=True) +def cleanup_disabled_logging() -> Iterator[None]: """Simple fixture that ensures that a test doesn't disable logging. This is necessary because ``logging.disable()`` is global, so a test disabling logging @@ -42,7 +43,7 @@ def test_change_level(caplog): assert "CRITICAL" in caplog.text -def test_change_level_logging_disabled(caplog, cleanup_disabled_logging): +def test_change_level_logging_disabled(caplog): logging.disable(logging.CRITICAL) assert logging.root.manager.disable == logging.CRITICAL caplog.set_level(logging.WARNING) @@ -85,9 +86,7 @@ def test2(caplog): result.stdout.no_fnmatch_line("*log from test2*") -def test_change_disabled_level_undo( - pytester: Pytester, cleanup_disabled_logging -) -> None: +def test_change_disabled_level_undo(pytester: Pytester) -> None: """Ensure that '_force_enable_logging' in 'set_level' is undone after the end of the test. Tests the logging output themselves (affected by disabled logging level). @@ -159,7 +158,7 @@ def test_with_statement(caplog): assert "CRITICAL" in caplog.text -def test_with_statement_logging_disabled(caplog, cleanup_disabled_logging): +def test_with_statement_logging_disabled(caplog): logging.disable(logging.CRITICAL) assert logging.root.manager.disable == logging.CRITICAL with caplog.at_level(logging.WARNING): @@ -197,9 +196,7 @@ def test_with_statement_logging_disabled(caplog, cleanup_disabled_logging): ("NOTVALIDLEVEL", logging.NOTSET), ], ) -def test_force_enable_logging_level_string( - caplog, cleanup_disabled_logging, level_str, expected_disable_level -): +def test_force_enable_logging_level_string(caplog, level_str, expected_disable_level): """Test _force_enable_logging using a level string. ``expected_disable_level`` is one level below ``level_str`` because the disabled log level From c8369195af351d0b19502944903558918afef3fb Mon Sep 17 00:00:00 2001 From: Benjamin Schubert Date: Sun, 22 Oct 2023 12:42:05 +0100 Subject: [PATCH 5/7] Update changelog --- changelog/11520.improvement.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/11520.improvement.rst b/changelog/11520.improvement.rst index 7cfb45b0b38..46e4992dd39 100644 --- a/changelog/11520.improvement.rst +++ b/changelog/11520.improvement.rst @@ -1 +1 @@ -* Improved very verbose diff output to color it as a diff instead of only red. +Improved very verbose diff output to color it as a diff instead of only red. From 13459c35e8d92232f8169a06dcedf24085b6acf6 Mon Sep 17 00:00:00 2001 From: Benjamin Schubert Date: Mon, 23 Oct 2023 19:31:12 +0100 Subject: [PATCH 6/7] fixup! Color the full diff that pytest shows as a diff --- src/_pytest/assertion/util.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/_pytest/assertion/util.py b/src/_pytest/assertion/util.py index 2d8067294a3..14e2fa7ea85 100644 --- a/src/_pytest/assertion/util.py +++ b/src/_pytest/assertion/util.py @@ -7,8 +7,10 @@ from typing import Callable from typing import Iterable from typing import List +from typing import Literal from typing import Mapping from typing import Optional +from typing import Protocol from typing import Sequence from unicodedata import normalize @@ -17,7 +19,6 @@ from _pytest._io.saferepr import _pformat_dispatch from _pytest._io.saferepr import saferepr from _pytest._io.saferepr import saferepr_unlimited -from _pytest._io.terminalwriter import TerminalWriter from _pytest.config import Config # The _reprcompare attribute on the util module is used by the new assertion @@ -34,6 +35,11 @@ _config: Optional[Config] = None +class _HighlightFunc(Protocol): + def __call__(self, source: str, lexer: Literal["diff", "python"] = "python") -> str: + """Apply highlighting to the given source.""" + + def format_explanation(explanation: str) -> str: r"""Format an explanation. @@ -228,7 +234,7 @@ def assertrepr_compare( def _compare_eq_any( - left: Any, right: Any, writer: TerminalWriter, verbose: int = 0 + left: Any, right: Any, highlighter: _HighlightFunc, verbose: int = 0 ) -> List[str]: explanation = [] if istext(left) and istext(right): @@ -249,7 +255,7 @@ def _compare_eq_any( # field values, not the type or field names. But this branch # intentionally only handles the same-type case, which was often # used in older code bases before dataclasses/attrs were available. - explanation = _compare_eq_cls(left, right, writer, verbose) + explanation = _compare_eq_cls(left, right, highlighter, verbose) elif issequence(left) and issequence(right): explanation = _compare_eq_sequence(left, right, verbose) elif isset(left) and isset(right): @@ -258,7 +264,7 @@ def _compare_eq_any( explanation = _compare_eq_dict(left, right, verbose) if isiterable(left) and isiterable(right): - expl = _compare_eq_iterable(left, right, writer, verbose) + expl = _compare_eq_iterable(left, right, highlighter, verbose) explanation.extend(expl) return explanation @@ -327,7 +333,7 @@ def _surrounding_parens_on_own_lines(lines: List[str]) -> None: def _compare_eq_iterable( left: Iterable[Any], right: Iterable[Any], - writer: TerminalWriter, + highligher: _HighlightFunc, verbose: int = 0, ) -> List[str]: if verbose <= 0 and not running_on_ci(): @@ -353,7 +359,7 @@ def _compare_eq_iterable( # "right" is the expected base against which we compare "left", # see https://github.com/pytest-dev/pytest/issues/3333 explanation.extend( - writer._highlight( + highligher( "\n".join( line.rstrip() for line in difflib.ndiff(right_formatting, left_formatting) @@ -510,7 +516,7 @@ def _compare_eq_dict( def _compare_eq_cls( - left: Any, right: Any, writer: TerminalWriter, verbose: int + left: Any, right: Any, highlighter: _HighlightFunc, verbose: int ) -> List[str]: if not has_default_eq(left): return [] @@ -557,7 +563,9 @@ def _compare_eq_cls( ] explanation += [ indent + line - for line in _compare_eq_any(field_left, field_right, writer, verbose) + for line in _compare_eq_any( + field_left, field_right, highlighter, verbose + ) ] return explanation From 7e477ac23d426def89e945484e036920b781ac68 Mon Sep 17 00:00:00 2001 From: Benjamin Schubert Date: Mon, 23 Oct 2023 19:43:37 +0100 Subject: [PATCH 7/7] fixup! fixup! Color the full diff that pytest shows as a diff --- src/_pytest/assertion/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/assertion/util.py b/src/_pytest/assertion/util.py index 14e2fa7ea85..b9123c97dbe 100644 --- a/src/_pytest/assertion/util.py +++ b/src/_pytest/assertion/util.py @@ -197,7 +197,7 @@ def assertrepr_compare( try: if op == "==": writer = config.get_terminal_writer() - explanation = _compare_eq_any(left, right, writer, verbose) + explanation = _compare_eq_any(left, right, writer._highlight, verbose) elif op == "not in": if istext(left) and istext(right): explanation = _notin_text(left, right, verbose)