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

Fix ANSI escape codes for colored output not handled correctly with pytest.fail(..., pytrace=False) #12959

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ Kristoffer Nordström
Kyle Altendorf
Lawrence Mitchell
Lee Kamentsky
Leonardus Chen
Lev Maximov
Levon Saldamli
Lewis Cowles
Expand Down
1 change: 1 addition & 0 deletions changelog/12849.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ANSI escape codes for colored output now handled correctly in :func:`pytest.fail` with `pytrace=False`.
13 changes: 8 additions & 5 deletions src/_pytest/_code/code.py
Original file line number Diff line number Diff line change
Expand Up @@ -1221,6 +1221,12 @@
if not self.lines:
return

if self.style == "value":
for line in self.lines:
tw.write(line)
Copy link
Author

Choose a reason for hiding this comment

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

Using TerminalWriter.write instead of TerminalWriter.line because when using TWMock . we can't distinguish lines written with TWMock._write_source and lines written with line directly, whereas with TWMock.write we can, with the flag.

We could add a flag for TWMock._write_source as well, like so:

class TWMock:
    WRITE = object()
    WRITE_SOURCE = object()

    def _write_source(self, lines, indents=()):
        if not indents:
            indents = [""] * len(lines)
        for indent, line in zip(indents, lines):
            newline = indent + line
            self.line((TWMock.WRITE_SOURCE, newline))

But then we'll have to change around half the test cases in test_excinfo.py

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanation! I guess this rationale would be nice to have as a comment in the code itself, to help future readers.

Copy link
Author

Choose a reason for hiding this comment

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

sure

tw.write("\n")
return

Check warning on line 1228 in src/_pytest/_code/code.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/_code/code.py#L1226-L1228

Added lines #L1226 - L1228 were not covered by tests

# separate indents and source lines that are not failures: we want to
# highlight the code but not the indentation, which may contain markers
# such as "> assert 0"
Expand All @@ -1236,11 +1242,8 @@
failure_lines.extend(self.lines[index:])
break
else:
if self.style == "value":
source_lines.append(line)
else:
indents.append(line[:indent_size])
source_lines.append(line[indent_size:])
indents.append(line[:indent_size])
source_lines.append(line[indent_size:])

tw._write_source(source_lines, indents)

Expand Down
17 changes: 17 additions & 0 deletions testing/code/test_excinfo.py
Original file line number Diff line number Diff line change
Expand Up @@ -1194,6 +1194,23 @@ def f():
line = tw_mock.lines[-1]
assert line == ":3: ValueError"

def test_toterminal_value(self, importasmod, tw_mock):
mod = importasmod(
"""
def g(x):
raise ValueError(x)
def f():
g('some_value')
"""
)
excinfo = pytest.raises(ValueError, mod.f)
excinfo.traceback = excinfo.traceback.filter(excinfo)
repr = excinfo.getrepr(style="value")
repr.toterminal(tw_mock)

assert tw_mock.get_write_msg(0) == "some_value"
assert tw_mock.get_write_msg(1) == "\n"

@pytest.mark.parametrize(
"reproptions",
[
Expand Down
4 changes: 2 additions & 2 deletions testing/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@
return text

def get_write_msg(self, idx):
flag, msg = self.lines[idx]
assert flag == TWMock.WRITE
assert self.lines[idx][0] == TWMock.WRITE
Copy link
Author

Choose a reason for hiding this comment

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

Added this so that calling get_write_msg raises AssertionError instead of ValueError: Not enough values to unpack

I can revert it back if needed

Copy link
Member

Choose a reason for hiding this comment

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

That's fine, thanks!

msg = self.lines[idx][1]

Check warning on line 123 in testing/conftest.py

View check run for this annotation

Codecov / codecov/patch

testing/conftest.py#L122-L123

Added lines #L122 - L123 were not covered by tests
return msg

fullwidth = 80
Expand Down
Loading