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 serialization of 'None' reprcrashes #6412

Merged
merged 2 commits into from Jan 7, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions changelog/5971.bugfix.rst
@@ -0,0 +1,2 @@
Fix a ``pytest-xdist`` crash when dealing with exceptions raised in subprocesses created by the
``multiprocessing`` module.
14 changes: 10 additions & 4 deletions src/_pytest/reports.py
Expand Up @@ -374,8 +374,11 @@ def serialize_repr_traceback(reprtraceback):
]
return result

def serialize_repr_crash(reprcrash):
return reprcrash.__dict__.copy()
def serialize_repr_crash(reprcrash: Optional[ReprFileLocation]):
if reprcrash is not None:
return reprcrash.__dict__.copy()
else:
return None

def serialize_longrepr(rep):
result = {
Expand Down Expand Up @@ -455,8 +458,11 @@ def deserialize_repr_traceback(repr_traceback_dict):
]
return ReprTraceback(**repr_traceback_dict)

def deserialize_repr_crash(repr_crash_dict):
return ReprFileLocation(**repr_crash_dict)
def deserialize_repr_crash(repr_crash_dict: Optional[dict]):
if repr_crash_dict is not None:
return ReprFileLocation(**repr_crash_dict)
else:
return None

if (
reportdict["longrepr"]
Expand Down
62 changes: 62 additions & 0 deletions testing/test_reports.py
Expand Up @@ -305,13 +305,75 @@ def check_longrepr(longrepr):

data = report._to_json()
loaded_report = report_class._from_json(data)

assert loaded_report.failed
check_longrepr(loaded_report.longrepr)

# make sure we don't blow up on ``toterminal`` call; we don't test the actual output because it is very
# brittle and hard to maintain, but we can assume it is correct because ``toterminal`` is already tested
# elsewhere and we do check the contents of the longrepr object after loading it.
loaded_report.longrepr.toterminal(tw_mock)

def test_chained_exceptions_no_reprcrash(
self, testdir, tw_mock,
):
"""Regression test for tracebacks without a reprcrash (#5971)

This happens notably on exceptions raised by multiprocess.pool: the exception transfer
from subprocess to main process creates an artificial exception which, ExceptionInfo
can't obtain the ReprFileLocation from.
"""
testdir.makepyfile(
"""
# equivalent of multiprocessing.pool.RemoteTraceback
class RemoteTraceback(Exception):
def __init__(self, tb):
self.tb = tb
def __str__(self):
return self.tb

def test_a():
try:
raise ValueError('value error')
except ValueError as e:
# equivalent to how multiprocessing.pool.rebuild_exc does it
nicoddemus marked this conversation as resolved.
Show resolved Hide resolved
e.__cause__ = RemoteTraceback('runtime error')
raise e
"""
)
reprec = testdir.inline_run()

reports = reprec.getreports("pytest_runtest_logreport")

def check_longrepr(longrepr):
assert isinstance(longrepr, ExceptionChainRepr)
assert len(longrepr.chain) == 2
entry1, entry2 = longrepr.chain
tb1, fileloc1, desc1 = entry1
tb2, fileloc2, desc2 = entry2

assert "RemoteTraceback: runtime error" in str(tb1)
assert "ValueError('value error')" in str(tb2)

assert fileloc1 is None
assert fileloc2.message == "ValueError: value error"

# 3 reports: setup/call/teardown: get the call report
assert len(reports) == 3
report = reports[1]

assert report.failed
check_longrepr(report.longrepr)

data = report._to_json()
loaded_report = TestReport._from_json(data)

assert loaded_report.failed
check_longrepr(loaded_report.longrepr)

# for same reasons as previous test, ensure we don't blow up here
loaded_report.longrepr.toterminal(tw_mock)


class TestHooks:
"""Test that the hooks are working correctly for plugins"""
Expand Down