Merge pull request #6412 from nicoddemus/remote-tb-5971

Fix serialization of 'None' reprcrashes

Author: asottile

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.

src/_pytest/reports.py

@@ -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 = {
@@ -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"]

testing/test_reports.py

@@ -305,13 +305,69 @@ 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(
+            """
+            from concurrent.futures import ProcessPoolExecutor
+
+            def func():
+                raise ValueError('value error')
+
+            def test_a():
+                with ProcessPoolExecutor() as p:
+                    p.submit(func).result()
+        """
+        )
+        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" 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"""