-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
base: main
Are you sure you want to change the base?
Conversation
@@ -119,8 +119,8 @@ def markup(self, text, **kw): | |||
return text | |||
|
|||
def get_write_msg(self, idx): | |||
flag, msg = self.lines[idx] | |||
assert flag == TWMock.WRITE | |||
assert self.lines[idx][0] == TWMock.WRITE |
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.
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
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.
That's fine, thanks!
@@ -1221,6 +1221,12 @@ def _write_entry_lines(self, tw: TerminalWriter) -> None: | |||
if not self.lines: | |||
return | |||
|
|||
if self.style == "value": | |||
for line in self.lines: | |||
tw.write(line) |
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.
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
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.
Thanks for the explanation! I guess this rationale would be nice to have as a comment in the code itself, to help future readers.
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.
sure
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.
LGTM, thanks for the PR, appreciate it!
@@ -1221,6 +1221,12 @@ def _write_entry_lines(self, tw: TerminalWriter) -> None: | |||
if not self.lines: | |||
return | |||
|
|||
if self.style == "value": | |||
for line in self.lines: | |||
tw.write(line) |
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.
Thanks for the explanation! I guess this rationale would be nice to have as a comment in the code itself, to help future readers.
@@ -119,8 +119,8 @@ def markup(self, text, **kw): | |||
return text | |||
|
|||
def get_write_msg(self, idx): | |||
flag, msg = self.lines[idx] | |||
assert flag == TWMock.WRITE | |||
assert self.lines[idx][0] == TWMock.WRITE |
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.
That's fine, thanks!
Thanks @leonarduschen! Will squash/merge in a few days to give others a chance to review it. 👍 |
Sure thing @nicoddemus, thanks for the review! Excited for my first contribution :) |
Closes #12849
Description:
ReprEntry.style == "value"
(happens when callingpytest.fail(..., pytrace=False)
, the message should not be printed to terminal usingTerminalWriter._write_source
because then it'll try to highlight the message as source codeTerminalWriter.line
orTerminalWriter.write
, I went with the later for testing purposes Fix ANSI escape codes for colored output not handled correctly with pytest.fail(..., pytrace=False) #12959 (comment)closes #XYZW
to the PR description and/or commits (whereXYZW
is the issue number). See the github docs for more information.changelog
folder, with a name like<ISSUE NUMBER>.<TYPE>.rst
. See changelog/README.rst for details.AUTHORS
in alphabetical order.