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

Revert change to traceback repr #7535

Merged
merged 3 commits into from Jul 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/7534.bugfix.rst
@@ -0,0 +1 @@
Restored the previous formatting of ``TracebackEntry.__str__`` which was changed by accident.
10 changes: 9 additions & 1 deletion src/_pytest/_code/code.py
Expand Up @@ -262,7 +262,15 @@ def __str__(self) -> str:
raise
except BaseException:
line = "???"
return " File %r:%d in %s\n %s\n" % (self.path, self.lineno + 1, name, line)
# This output does not quite match Python's repr for traceback entries,
# but changing it to do so would break certain plugins. See
# https://github.com/pytest-dev/pytest/pull/7535/ for details.
return " File %r:%d in %s\n %s\n" % (
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 we should show the non-repr of the path here:

Suggested change
return " File %r:%d in %s\n %s\n" % (
return " File %s:%d in %s\n %s\n" % (

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 think so - Python also quotes the path in its tracebacks:

Traceback (most recent call last):
  File "test.py", line 1, in <module>
    raise Exception

Copy link
Member

Choose a reason for hiding this comment

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

Ah, though Python uses " in any case:

  File "test"foo.py", line 1, in <module>

so I suppose we could do File \"%s\" here instead (and drop the str(...) again?).

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 think so - Python also quotes the path in its tracebacks:

Ahh you are right. 👍

so I suppose we could do File "%s" here instead (and drop the str(...) again?).

Right on. 😁

Copy link
Member

Choose a reason for hiding this comment

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

@The-Compiler

File "test.py", line 1, in

The code seems to (before my oversight) do

File "test.py":1 in

Which looks kinda weird. Should we indeed change it to match the Python format, i.e File "%s", line %d, in %s?

Though that would break werkzeug as well, so not for this PR.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I hadn't noticed that. I guess let's just stay with the previous format for now then?

Copy link
Member

Choose a reason for hiding this comment

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

Actually I noticed previously this used ' instead of ", based on the failure reported in #7534:

        traceback = "".join(str(line) for line in baz_exc.traceback)
>       assert "bb.py':1" in traceback  # a bit different than typical python tb

(Nevermind this, replied to a stale comment hehehe)

str(self.path),
self.lineno + 1,
name,
line,
)

@property
def name(self) -> str:
Expand Down
10 changes: 10 additions & 0 deletions testing/code/test_code.py
@@ -1,3 +1,4 @@
import re
import sys
from types import FrameType
from unittest import mock
Expand Down Expand Up @@ -170,6 +171,15 @@ def test_getsource(self) -> None:
assert len(source) == 6
assert "assert False" in source[5]

def test_tb_entry_str(self):
try:
assert False
except AssertionError:
exci = ExceptionInfo.from_current()
pattern = r" File '.*test_code.py':\d+ in test_tb_entry_str\n assert False"
entry = str(exci.traceback[0])
assert re.match(pattern, entry)


class TestReprFuncArgs:
def test_not_raise_exception_with_mixed_encoding(self, tw_mock) -> None:
Expand Down