Skip to content
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

Color the full diff that pytest shows as a diff #11530

Merged
merged 8 commits into from
Oct 24, 2023
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ Barney Gale
Ben Gartner
Ben Webb
Benjamin Peterson
Benjamin Schubert
Bernard Pratz
Bo Wu
Bob Ippolito
Expand Down
1 change: 1 addition & 0 deletions changelog/11520.improvement.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improved very verbose diff output to color it as a diff instead of only red.
15 changes: 11 additions & 4 deletions src/_pytest/_io/terminalwriter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -193,15 +194,21 @@
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

Check warning on line 209 in src/_pytest/_io/terminalwriter.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/_io/terminalwriter.py#L209

Added line #L209 was not covered by tests
elif lexer == "diff":
from pygments.lexers.diff import DiffLexer as Lexer

Check warning on line 211 in src/_pytest/_io/terminalwriter.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/_io/terminalwriter.py#L211

Added line #L211 was not covered by tests
from pygments import highlight
import pygments.util
except ImportError:
Expand All @@ -210,7 +217,7 @@
try:
highlighted: str = highlight(
source,
PythonLexer(),
Lexer(),
TerminalFormatter(
bg=os.getenv("PYTEST_THEME_MODE", "dark"),
style=os.getenv("PYTEST_THEME"),
Expand Down
31 changes: 23 additions & 8 deletions src/_pytest/assertion/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like that we pass the full TerminalWriter to these functions, because it makes it seem like the functions actually write to the terminal, while they only need it for the pure _highlight function. But it's OK for now, not asking you to refactor :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you prefer, we could instead pass a highlight function (Callable[[str], str]) which would be TerminalWriter._highlight

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I have pushed a fixup for this. I wasn't sure where to add the type definition, feel free to update/let me know if there's a better place

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that looks great, thanks!

elif op == "not in":
if istext(left) and istext(right):
explanation = _notin_text(left, right, verbose)
Expand Down Expand Up @@ -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)
Expand All @@ -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):
Expand All @@ -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
Expand Down Expand Up @@ -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"]
Expand All @@ -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

Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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

Expand Down
9 changes: 9 additions & 0 deletions testing/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,9 @@
"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",
Expand All @@ -171,6 +174,7 @@
"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]:
Expand All @@ -187,6 +191,11 @@
"""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]:

Check warning on line 195 in testing/conftest.py

View check run for this annotation

Codecov / codecov/patch

testing/conftest.py#L194-L195

Added lines #L194 - L195 were not covered by tests
"""Entirely remove every color code"""
return [line.format(**cls.NO_COLORS) for line in lines]

return ColorMapping


Expand Down
17 changes: 7 additions & 10 deletions testing/logging/test_fixture.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down
52 changes: 52 additions & 0 deletions testing/test_assertion.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()


Expand Down Expand Up @@ -1784,3 +1791,48 @@ 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(
("test_code", "expected_lines"),
(
(
"""
def test():
assert [0, 1] == [0, 2]
""",
[
"{bold}{red}E {light-red}- [0, 2]{hl-reset}{endline}{reset}",
"{bold}{red}E {light-green}+ [0, 1]{hl-reset}{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 {light-gray} {hl-reset} {{{endline}{reset}",
"{bold}{red}E {light-gray} {hl-reset} 'number-is-1': 1,{endline}{reset}",
"{bold}{red}E {light-green}+ 'number-is-5': 5,{hl-reset}{endline}{reset}",
],
),
),
)
def test_comparisons_handle_colors(
pytester: Pytester, color_mapping, enable_colors, test_code, expected_lines
) -> None:
p = pytester.makepyfile(test_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), consecutive=False)