Twisted support: extract sys.exc_info during addError

Alternative fix for #13497: no longer monkeypatch `Failure.__init__`, instead delay obtaining the `sys.exc_info()` object until `addError` is called.

Fix #13497

Author: nicoddemus

changelog/13497.bugfix.rst

@@ -1 +1 @@
-Fixed compatibility with ``Twisted 25``.
+Fixed compatibility with ``Twisted``, however version ``25`` or later is required.

src/_pytest/unittest.py

@@ -10,7 +10,6 @@
 import sys
 import traceback
 import types
-from typing import Any
 from typing import TYPE_CHECKING
 from typing import Union
 
@@ -197,12 +196,6 @@ def unittest_setup_method_fixture(
         )
 
 
-# Name of the attribute in `twisted.python.Failure` instances that stores
-# the `sys.exc_info()` tuple.
-# See twisted.trial support in `pytest_runtest_protocol`.
-TWISTED_RAW_EXCINFO_ATTR = "_twisted_raw_excinfo"
-
-
 class TestCaseFunction(Function):
     nofuncargs = True
     _excinfo: list[_pytest._code.ExceptionInfo[BaseException]] | None = None
@@ -234,16 +227,7 @@ def startTest(self, testcase: unittest.TestCase) -> None:
         pass
 
     def _addexcinfo(self, rawexcinfo: _SysExcInfoType) -> None:
-        # Unwrap potential exception info (see twisted trial support below).
-        # Twisted calls addError() passing its own classes (like `twisted.python.Failure`), which violates
-        # the `addError()` signature, so we extract the original `sys.exc_info()` tuple which is stored
-        # in the object.
-        if hasattr(rawexcinfo, TWISTED_RAW_EXCINFO_ATTR):
-            saved_exc_info = getattr(rawexcinfo, TWISTED_RAW_EXCINFO_ATTR)
-            # Delete the attribute from the original object to avoid leaks.
-            delattr(rawexcinfo, TWISTED_RAW_EXCINFO_ATTR)
-            rawexcinfo = saved_exc_info
-            del saved_exc_info
+        rawexcinfo = _handle_twisted_exc_info(rawexcinfo)
         try:
             excinfo = _pytest._code.ExceptionInfo[BaseException].from_exc_info(
                 rawexcinfo  # type: ignore[arg-type]
@@ -399,54 +383,42 @@ def pytest_runtest_makereport(item: Item, call: CallInfo[None]) -> None:
         call.excinfo = call2.excinfo
 
 
-# Twisted trial support.
-classImplements_has_run = False
+def pytest_configure() -> None:
+    """Register the TestCaseFunction class as an IReporter if twisted.trial is available."""
+    if _is_twisted_trial_available():
+        from twisted.trial.itrial import IReporter
+        from zope.interface import classImplements
 
+        classImplements(TestCaseFunction, IReporter)
 
-@hookimpl(wrapper=True)
-def pytest_runtest_protocol(item: Item) -> Generator[None, object, object]:
-    if isinstance(item, TestCaseFunction) and "twisted.trial.unittest" in sys.modules:
-        ut: Any = sys.modules["twisted.python.failure"]
-        global classImplements_has_run
-        if not classImplements_has_run:
-            from twisted.trial.itrial import IReporter
-            from zope.interface import classImplements
 
-            classImplements(TestCaseFunction, IReporter)
-            classImplements_has_run = True
+def _is_skipped(obj) -> bool:
+    """Return True if the given object has been marked with @unittest.skip."""
+    return bool(getattr(obj, "__unittest_skip__", False))
 
-        # Monkeypatch `Failure.__init__` to store the raw exception info.
-        Failure__init__ = ut.Failure.__init__
 
-        def store_raw_exception_info(
-            self, exc_value=None, exc_type=None, exc_tb=None, captureVars=None
-        ):
-            if exc_value is None:
-                raw_exc_info = sys.exc_info()
-            else:
-                if exc_type is None:
-                    exc_type = type(exc_value)
-                if exc_tb is None:
-                    exc_tb = sys.exc_info()[2]
-                raw_exc_info = (exc_type, exc_value, exc_tb)
-            setattr(self, TWISTED_RAW_EXCINFO_ATTR, tuple(raw_exc_info))
-            try:
-                Failure__init__(
-                    self, exc_value, exc_type, exc_tb, captureVars=captureVars
-                )
-            except TypeError:
-                Failure__init__(self, exc_value, exc_type, exc_tb)
+def _is_twisted_trial_available() -> bool:
+    return "twisted.trial.unittest" in sys.modules
 
-        ut.Failure.__init__ = store_raw_exception_info
-        try:
-            res = yield
-        finally:
-            ut.Failure.__init__ = Failure__init__
-    else:
-        res = yield
-    return res
 
+def _handle_twisted_exc_info(
+    rawexcinfo: _SysExcInfoType | BaseException,
+) -> _SysExcInfoType:
+    """
+    Twisted passes a custom Failure instance to `addError()` instead of using `sys.exc_info()`.
+    Therefore, if `rawexcinfo` is a `Failure` instance, convert it into the equivalent `sys.exc_info()` tuple
+    as expected by pytest.
+    """
+    if isinstance(rawexcinfo, BaseException) and _is_twisted_trial_available():
+        import twisted.python.failure
 
-def _is_skipped(obj) -> bool:
-    """Return True if the given object has been marked with @unittest.skip."""
-    return bool(getattr(obj, "__unittest_skip__", False))
+        if isinstance(rawexcinfo, twisted.python.failure.Failure):
+            tb = rawexcinfo.__traceback__
+            if tb is None:
+                tb = sys.exc_info()[2]
+            return type(rawexcinfo.value), rawexcinfo.value, tb
+
+    # Unfortunately, because we cannot import `twisted.python.failure` at the top of the file
+    # and use it in the signature, we need to use `type:ignore` here because we cannot narrow
+    # the type properly in the `if` statement above.
+    return rawexcinfo  # type:ignore[return-value]