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

Display message from reprcrash in short test summary #5013

Merged
merged 14 commits into from
May 8, 2019
Merged
1 change: 1 addition & 0 deletions changelog/5013.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Messages from crash reports are displayed within test summaries now, truncated to the terminal width.
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
'pathlib2>=2.2.0;python_version<"3.6"',
'colorama;sys_platform=="win32"',
"pluggy>=0.9",
"wcwidth",
]


Expand Down
55 changes: 52 additions & 3 deletions src/_pytest/skipping.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
# coding=utf8
""" support for skip/xfail functions and markers. """
from __future__ import absolute_import
from __future__ import division
from __future__ import print_function

import six

from _pytest.config import hookimpl
from _pytest.mark.evaluate import MarkEvaluator
from _pytest.outcomes import fail
Expand Down Expand Up @@ -204,14 +207,60 @@ def pytest_terminal_summary(terminalreporter):
tr._tw.line(line)


def _get_line_with_reprcrash_message(config, rep, termwidth):
"""Get summary line for a report, trying to add reprcrash message."""
from wcwidth import wcswidth

verbose_word = _get_report_str(config, rep)
pos = _get_pos(config, rep)

line = "%s %s" % (verbose_word, pos)
len_line = wcswidth(line)
ellipsis, len_ellipsis = "...", 3
if len_line > termwidth - len_ellipsis:
# No space for an additional message.
return line

try:
msg = rep.longrepr.reprcrash.message
except AttributeError:
pass
else:
# Only use the first line.
i = msg.find("\n")
if i != -1:
msg = msg[:i]
len_msg = wcswidth(msg)

sep, len_sep = " - ", 3
max_len_msg = termwidth - len_line - len_sep
if max_len_msg >= len_ellipsis:
if len_msg > max_len_msg:
max_len_msg -= len_ellipsis
msg = msg[:max_len_msg]
while wcswidth(msg) > max_len_msg:
msg = msg[:-1]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could be optimized probably if we're taking it - there is also wcwidth.

if six.PY2:
nicoddemus marked this conversation as resolved.
Show resolved Hide resolved
# on python 2 systems with narrow unicode compilation, trying to
# get a single character out of a multi-byte unicode character such as
# u'😄' will result in a High Surrogate (U+D83D) character, which is
# rendered as u'�'; in this case we just strip that character out as it
# serves no purpose being rendered
while msg.endswith(u"\uD83D"):
msg = msg[:-1]
msg += ellipsis
line += sep + msg
Copy link
Member

Choose a reason for hiding this comment

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

Oh just realized this: is msg unicode-safe in Python 2? because we will be joining a str/bytes with a potential unicode message, so there's a good chance this might blow up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would appreciate a failing example.. do you mean we should prefix sep etc also with u?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm actually no, this is safe: if msg is unicode, Python 2 will try to elevate sep to unicode using ascii (which is fine), not the other way around. Sorry for the brain fart. 👍

Copy link
Member

Choose a reason for hiding this comment

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

If you want to play it safe and ensure we don't regress though, you can change the message to use a smiling face emoji instead (😄), but is up to you. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.. but it shows that we're not handling terminal cells there really - i.e. it would wrap due to this.

Copy link
Member

Choose a reason for hiding this comment

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

Oh. Hmm not even sure what the solution would be, TBH.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a commit using wcwidth.. not sure if it is worth the extra dependency, but it worked well.. ;)

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 adding wcwidth is fine: small, license is OK, seems to be maintained

return line


def show_simple(terminalreporter, lines, stat):
failed = terminalreporter.stats.get(stat)
if failed:
config = terminalreporter.config
termwidth = terminalreporter.writer.fullwidth
for rep in failed:
verbose_word = _get_report_str(config, rep)
pos = _get_pos(config, rep)
lines.append("%s %s" % (verbose_word, pos))
line = _get_line_with_reprcrash_message(config, rep, termwidth)
lines.append(line)


def show_xfailed(terminalreporter, lines):
Expand Down
4 changes: 3 additions & 1 deletion testing/acceptance_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,9 @@ def test_doctest_id(self, testdir):
_fail, _sep, testid = line.partition(" ")
break
result = testdir.runpytest(testid, "-rf")
result.stdout.fnmatch_lines([line, "*1 failed*"])
result.stdout.fnmatch_lines(
["FAILED test_doctest_id.txt::test_doctest_id.txt", "*1 failed*"]
)

def test_core_backward_compatibility(self):
"""Test backward compatibility for get_plugin_manager function. See #787."""
Expand Down
75 changes: 74 additions & 1 deletion testing/test_skipping.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# coding=utf8
from __future__ import absolute_import
from __future__ import division
from __future__ import print_function
Expand Down Expand Up @@ -1208,6 +1209,78 @@ def test_fail():
[
"=* FAILURES *=",
"*= short test summary info =*",
"FAILED test_summary_list_after_errors.py::test_fail",
"FAILED test_summary_list_after_errors.py::test_fail - assert 0",
]
)


def test_line_with_reprcrash(monkeypatch):
import _pytest.skipping
from _pytest.skipping import _get_line_with_reprcrash_message
from wcwidth import wcswidth

mocked_verbose_word = "FAILED"

def mock_get_report_str(*args):
return mocked_verbose_word

monkeypatch.setattr(_pytest.skipping, "_get_report_str", mock_get_report_str)

mocked_pos = "some::nodeid"

def mock_get_pos(*args):
return mocked_pos

monkeypatch.setattr(_pytest.skipping, "_get_pos", mock_get_pos)

class config:
pass

class rep:
class longrepr:
class reprcrash:
pass

def check(msg, width, expected):
__tracebackhide__ = True
if msg:
rep.longrepr.reprcrash.message = msg
actual = _get_line_with_reprcrash_message(config, rep, width)

assert actual == expected
if actual != "%s %s" % (mocked_verbose_word, mocked_pos):
assert len(actual) <= width
assert wcswidth(actual) <= width

# AttributeError with message
check(None, 80, "FAILED some::nodeid")

check("msg", 80, "FAILED some::nodeid - msg")
check("msg", 3, "FAILED some::nodeid")

check("msg", 24, "FAILED some::nodeid")
check("msg", 25, "FAILED some::nodeid - msg")

check("some longer msg", 24, "FAILED some::nodeid")
check("some longer msg", 25, "FAILED some::nodeid - ...")
check("some longer msg", 26, "FAILED some::nodeid - s...")

check("some\nmessage", 25, "FAILED some::nodeid - ...")
check("some\nmessage", 26, "FAILED some::nodeid - some")
check("some\nmessage", 80, "FAILED some::nodeid - some")

# Test unicode safety.
check(u"😄😄😄😄😄\n2nd line", 25, u"FAILED some::nodeid - ...")
check(u"😄😄😄😄😄\n2nd line", 26, u"FAILED some::nodeid - ...")
check(u"😄😄😄😄😄\n2nd line", 27, u"FAILED some::nodeid - 😄...")
check(u"😄😄😄😄😄\n2nd line", 28, u"FAILED some::nodeid - 😄...")
check(u"😄😄😄😄😄\n2nd line", 29, u"FAILED some::nodeid - 😄😄...")

# NOTE: constructed, not sure if this is supported.
# It would fail if not using u"" in Python 2 for mocked_pos.
mocked_pos = u"nodeid::😄::withunicode"
check(u"😄😄😄😄😄\n2nd line", 29, u"FAILED nodeid::😄::withunicode")
check(u"😄😄😄😄😄\n2nd line", 40, u"FAILED nodeid::😄::withunicode - 😄😄...")
check(u"😄😄😄😄😄\n2nd line", 41, u"FAILED nodeid::😄::withunicode - 😄😄...")
check(u"😄😄😄😄😄\n2nd line", 42, u"FAILED nodeid::😄::withunicode - 😄😄😄...")
check(u"😄😄😄😄😄\n2nd line", 80, u"FAILED nodeid::😄::withunicode - 😄😄😄😄😄")
12 changes: 9 additions & 3 deletions testing/test_terminal.py
Original file line number Diff line number Diff line change
Expand Up @@ -726,12 +726,18 @@ def test(i):
result.stdout.fnmatch_lines(["collected 3 items", "hello from hook: 3 items"])


def test_fail_extra_reporting(testdir):
testdir.makepyfile("def test_this(): assert 0")
def test_fail_extra_reporting(testdir, monkeypatch):
monkeypatch.setenv("COLUMNS", "80")
testdir.makepyfile("def test_this(): assert 0, 'this_failed' * 100")
result = testdir.runpytest()
assert "short test summary" not in result.stdout.str()
result = testdir.runpytest("-rf")
result.stdout.fnmatch_lines(["*test summary*", "FAIL*test_fail_extra_reporting*"])
result.stdout.fnmatch_lines(
[
"*test summary*",
"FAILED test_fail_extra_reporting.py::test_this - AssertionError: this_failedt...",
]
)


def test_fail_reporting_on_pass(testdir):
Expand Down