diff --git a/changelog/8258.bugfix.rst b/changelog/8258.bugfix.rst new file mode 100644 index 00000000000..6518ec0b738 --- /dev/null +++ b/changelog/8258.bugfix.rst @@ -0,0 +1,3 @@ +Fixed issue where pytest's ``faulthandler`` support would not dump traceback on crashes +if the :mod:`faulthandler` module was already enabled during pytest startup (using +``python -X dev -m pytest`` for example). diff --git a/src/_pytest/faulthandler.py b/src/_pytest/faulthandler.py index ff673b5b164..f82348ecfef 100644 --- a/src/_pytest/faulthandler.py +++ b/src/_pytest/faulthandler.py @@ -25,92 +25,71 @@ def pytest_addoption(parser: Parser) -> None: def pytest_configure(config: Config) -> None: import faulthandler - if not faulthandler.is_enabled(): - # faulthhandler is not enabled, so install plugin that does the actual work - # of enabling faulthandler before each test executes. - config.pluginmanager.register(FaultHandlerHooks(), "faulthandler-hooks") - else: - # Do not handle dumping to stderr if faulthandler is already enabled, so warn - # users that the option is being ignored. - timeout = FaultHandlerHooks.get_timeout_config_value(config) - if timeout > 0: - config.issue_config_time_warning( - pytest.PytestConfigWarning( - "faulthandler module enabled before pytest configuration step, " - "'faulthandler_timeout' option ignored" - ), - stacklevel=2, - ) - - -class FaultHandlerHooks: - """Implements hooks that will actually install fault handler before tests execute, - as well as correctly handle pdb and internal errors.""" - - def pytest_configure(self, config: Config) -> None: - import faulthandler + stderr_fd_copy = os.dup(get_stderr_fileno()) + config._store[fault_handler_stderr_key] = open(stderr_fd_copy, "w") + faulthandler.enable(file=config._store[fault_handler_stderr_key]) + - stderr_fd_copy = os.dup(self._get_stderr_fileno()) - config._store[fault_handler_stderr_key] = open(stderr_fd_copy, "w") - faulthandler.enable(file=config._store[fault_handler_stderr_key]) +def pytest_unconfigure(config: Config) -> None: + import faulthandler - def pytest_unconfigure(self, config: Config) -> None: + faulthandler.disable() + # close our dup file installed during pytest_configure + # re-enable the faulthandler, attaching it to the default sys.stderr + # so we can see crashes after pytest has finished, usually during + # garbage collection during interpreter shutdown + config._store[fault_handler_stderr_key].close() + del config._store[fault_handler_stderr_key] + faulthandler.enable(file=get_stderr_fileno()) + + +def get_stderr_fileno() -> int: + try: + fileno = sys.stderr.fileno() + # The Twisted Logger will return an invalid file descriptor since it is not backed + # by an FD. So, let's also forward this to the same code path as with pytest-xdist. + if fileno == -1: + raise AttributeError() + return fileno + except (AttributeError, io.UnsupportedOperation): + # pytest-xdist monkeypatches sys.stderr with an object that is not an actual file. + # https://docs.python.org/3/library/faulthandler.html#issue-with-file-descriptors + # This is potentially dangerous, but the best we can do. + return sys.__stderr__.fileno() + + +def get_timeout_config_value(config: Config) -> float: + return float(config.getini("faulthandler_timeout") or 0.0) + + +@pytest.hookimpl(hookwrapper=True, trylast=True) +def pytest_runtest_protocol(item: Item) -> Generator[None, None, None]: + timeout = get_timeout_config_value(item.config) + stderr = item.config._store[fault_handler_stderr_key] + if timeout > 0 and stderr is not None: import faulthandler - faulthandler.disable() - # close our dup file installed during pytest_configure - # re-enable the faulthandler, attaching it to the default sys.stderr - # so we can see crashes after pytest has finished, usually during - # garbage collection during interpreter shutdown - config._store[fault_handler_stderr_key].close() - del config._store[fault_handler_stderr_key] - faulthandler.enable(file=self._get_stderr_fileno()) - - @staticmethod - def _get_stderr_fileno(): + faulthandler.dump_traceback_later(timeout, file=stderr) try: - fileno = sys.stderr.fileno() - # The Twisted Logger will return an invalid file descriptor since it is not backed - # by an FD. So, let's also forward this to the same code path as with pytest-xdist. - if fileno == -1: - raise AttributeError() - return fileno - except (AttributeError, io.UnsupportedOperation): - # pytest-xdist monkeypatches sys.stderr with an object that is not an actual file. - # https://docs.python.org/3/library/faulthandler.html#issue-with-file-descriptors - # This is potentially dangerous, but the best we can do. - return sys.__stderr__.fileno() - - @staticmethod - def get_timeout_config_value(config): - return float(config.getini("faulthandler_timeout") or 0.0) - - @pytest.hookimpl(hookwrapper=True, trylast=True) - def pytest_runtest_protocol(self, item: Item) -> Generator[None, None, None]: - timeout = self.get_timeout_config_value(item.config) - stderr = item.config._store[fault_handler_stderr_key] - if timeout > 0 and stderr is not None: - import faulthandler - - faulthandler.dump_traceback_later(timeout, file=stderr) - try: - yield - finally: - faulthandler.cancel_dump_traceback_later() - else: yield + finally: + faulthandler.cancel_dump_traceback_later() + else: + yield - @pytest.hookimpl(tryfirst=True) - def pytest_enter_pdb(self) -> None: - """Cancel any traceback dumping due to timeout before entering pdb.""" - import faulthandler - faulthandler.cancel_dump_traceback_later() +@pytest.hookimpl(tryfirst=True) +def pytest_enter_pdb() -> None: + """Cancel any traceback dumping due to timeout before entering pdb.""" + import faulthandler - @pytest.hookimpl(tryfirst=True) - def pytest_exception_interact(self) -> None: - """Cancel any traceback dumping due to an interactive exception being - raised.""" - import faulthandler + faulthandler.cancel_dump_traceback_later() + + +@pytest.hookimpl(tryfirst=True) +def pytest_exception_interact() -> None: + """Cancel any traceback dumping due to an interactive exception being + raised.""" + import faulthandler - faulthandler.cancel_dump_traceback_later() + faulthandler.cancel_dump_traceback_later() diff --git a/testing/test_faulthandler.py b/testing/test_faulthandler.py index 370084c125f..411e841a31a 100644 --- a/testing/test_faulthandler.py +++ b/testing/test_faulthandler.py @@ -94,7 +94,7 @@ def test_cancel_timeout_on_hook(monkeypatch, hook_name) -> None: to timeout before entering pdb (pytest-dev/pytest-faulthandler#12) or any other interactive exception (pytest-dev/pytest-faulthandler#14).""" import faulthandler - from _pytest.faulthandler import FaultHandlerHooks + from _pytest import faulthandler as faulthandler_plugin called = [] @@ -104,19 +104,18 @@ def test_cancel_timeout_on_hook(monkeypatch, hook_name) -> None: # call our hook explicitly, we can trust that pytest will call the hook # for us at the appropriate moment - hook_func = getattr(FaultHandlerHooks, hook_name) - hook_func(self=None) + hook_func = getattr(faulthandler_plugin, hook_name) + hook_func() assert called == [1] -@pytest.mark.parametrize("faulthandler_timeout", [0, 2]) -def test_already_initialized(faulthandler_timeout: int, pytester: Pytester) -> None: - """Test for faulthandler being initialized earlier than pytest (#6575).""" +def test_already_initialized_crash(pytester: Pytester) -> None: + """Even if faulthandler is already initialized, we still dump tracebacks on crashes (#8258).""" pytester.makepyfile( """ def test(): import faulthandler - assert faulthandler.is_enabled() + faulthandler._sigabrt() """ ) result = pytester.run( @@ -125,22 +124,14 @@ def test(): "faulthandler", "-mpytest", pytester.path, - "-o", - f"faulthandler_timeout={faulthandler_timeout}", ) - # ensure warning is emitted if faulthandler_timeout is configured - warning_line = "*faulthandler.py*faulthandler module enabled before*" - if faulthandler_timeout > 0: - result.stdout.fnmatch_lines(warning_line) - else: - result.stdout.no_fnmatch_line(warning_line) - result.stdout.fnmatch_lines("*1 passed*") - assert result.ret == 0 + result.stderr.fnmatch_lines(["*Fatal Python error*"]) + assert result.ret != 0 def test_get_stderr_fileno_invalid_fd() -> None: """Test for faulthandler being able to handle invalid file descriptors for stderr (#8249).""" - from _pytest.faulthandler import FaultHandlerHooks + from _pytest.faulthandler import get_stderr_fileno class StdErrWrapper(io.StringIO): """ @@ -159,4 +150,4 @@ def fileno(self): # Even when the stderr wrapper signals an invalid file descriptor, # ``_get_stderr_fileno()`` should return the real one. - assert FaultHandlerHooks._get_stderr_fileno() == 2 + assert get_stderr_fileno() == 2