Skip to content

Commit

Permalink
Fix serialization of 'None' reprcrashes
Browse files Browse the repository at this point in the history
Tracebacks coming from remote processes crated by the multiprocess module
will contain "RemoteTracebacks" which don't have a 'reprcrash' attribute

Fix pytest-dev#5971
  • Loading branch information
nicoddemus committed Jan 7, 2020
1 parent 26a2e1a commit 32bd9b9
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 4 deletions.
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 exception info
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
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

0 comments on commit 32bd9b9

Please sign in to comment.