diff --git a/CHANGES.rst b/CHANGES.rst index 2e29b245d4..6c4b94a2c8 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -14,6 +14,10 @@ Unreleased with a dedicated CI job. :pr:`3139` - Fix callable ``flag_value`` being instantiated when used as a default via ``default=True``. :issue:`3121` :pr:`3201` :pr:`3213` :pr:`3225` +- ``CliRunner``'s redirected streams now expose the original file descriptor + via ``fileno()``, so that ``faulthandler``, ``subprocess``, and other + C-level consumers no longer crash with ``io.UnsupportedOperation``. + :issue:`2865` Version 8.3.1 -------------- diff --git a/docs/testing.md b/docs/testing.md index 6d437a2b0a..52ad4c0be1 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -196,3 +196,26 @@ def test_prompts(): Prompts will be emulated so they write the input data to the output stream as well. If hidden input is expected then this does not happen. + +## File Descriptors and Low-Level I/O + +{class}`CliRunner` captures output by replacing `sys.stdout` and `sys.stderr` with in-memory {class}`~io.BytesIO`-backed wrappers. This is Python-level redirection: calls to {func}`~click.echo`, {func}`print`, or `sys.stdout.write()` are captured, but the wrappers have no OS-level file descriptor. + +Code that calls `fileno()` on `sys.stdout` or `sys.stderr`, like {mod}`faulthandler`, {mod}`subprocess`, or C extensions, would normally crash with {exc}`io.UnsupportedOperation` inside {class}`CliRunner`. + +To avoid this, {class}`CliRunner` preserves the original stream's file descriptor and exposes it via `fileno()` on the replacement wrapper. + +This means: +- **Python-level writes** (`print()`, `click.echo()`, ...) are captured as usual. +- **fd-level writes** (C code writing directly to the file descriptor) go to the original terminal and are **not** captured. + +This is the same trade-off that [pytest](https://docs.pytest.org/en/stable/how-to/capture-stdout-stderr.html) makes with its two capture modes: + +- `capsys`, which captures Python-level output, where `fileno()` raises `UnsupportedOperation` and fd-level writes are not captured. +- `capfd`, which captures fd-level output via `os.dup2()`, where `fileno()` works and fd-level writes *are* captured. + +Rather than implementing a full `capfd`-style mechanism, {class}`CliRunner` takes the simpler path: expose the original `fd` so that standard library helpers keep working, while accepting that their output is not captured. + +```{versionchanged} 8.3.2 +`fileno()` on the redirected streams now returns the original stream's file descriptor instead of raising. +``` diff --git a/src/click/testing.py b/src/click/testing.py index ebfd54dc77..a0958d5da9 100644 --- a/src/click/testing.py +++ b/src/click/testing.py @@ -100,21 +100,55 @@ def __init__(self) -> None: class _NamedTextIOWrapper(io.TextIOWrapper): + """A :class:`~io.TextIOWrapper` with custom ``name`` and ``mode`` + that does not close its underlying buffer. + + An optional ``original_fd`` preserves the file descriptor of the + stream being replaced, so that C-level consumers that call + :meth:`fileno` (``faulthandler``, ``subprocess``, ...) still work. + Inspired by pytest's ``capsys``/``capfd`` split: see :doc:`/testing` + for details. + + .. versionchanged:: 8.3.2 + Added ``original_fd`` parameter and :meth:`fileno` override. + """ + def __init__( - self, buffer: t.BinaryIO, name: str, mode: str, **kwargs: t.Any + self, + buffer: t.BinaryIO, + name: str, + mode: str, + *, + original_fd: int = -1, + **kwargs: t.Any, ) -> None: super().__init__(buffer, **kwargs) self._name = name self._mode = mode + self._original_fd = original_fd def close(self) -> None: + """The buffer this object contains belongs to some other object, + so prevent the default ``__del__`` implementation from closing + that buffer. + + .. versionadded:: 8.3.2 """ - The buffer this object contains belongs to some other object, so - prevent the default __del__ implementation from closing that buffer. + + def fileno(self) -> int: + """Return the file descriptor of the original stream, if one was + provided at construction time. + + This allows C-level consumers (``faulthandler``, ``subprocess``, + signal handlers, ...) to obtain a valid fd without crashing, even + though the Python-level writes are redirected to an in-memory + buffer. .. versionadded:: 8.3.2 """ - ... + if self._original_fd >= 0: + return self._original_fd + return super().fileno() @property def name(self) -> str: @@ -320,6 +354,20 @@ def isolation( stream_mixer = StreamMixer() + # Preserve the original file descriptors so that C-level + # consumers (faulthandler, subprocess, etc.) can still obtain a + # valid fd from the redirected streams. The original streams + # may themselves lack a fileno() (e.g. when CliRunner is used + # inside pytest's capsys), so we fall back to -1. + def _safe_fileno(stream: t.IO[t.Any]) -> int: + try: + return stream.fileno() + except (AttributeError, io.UnsupportedOperation): + return -1 + + old_stdout_fd = _safe_fileno(old_stdout) + old_stderr_fd = _safe_fileno(old_stderr) + if self.echo_stdin: bytes_input = echo_input = t.cast( t.BinaryIO, EchoingStdin(bytes_input, stream_mixer.stdout) @@ -335,7 +383,11 @@ def isolation( text_input._CHUNK_SIZE = 1 # type: ignore sys.stdout = _NamedTextIOWrapper( - stream_mixer.stdout, encoding=self.charset, name="", mode="w" + stream_mixer.stdout, + encoding=self.charset, + name="", + mode="w", + original_fd=old_stdout_fd, ) sys.stderr = _NamedTextIOWrapper( @@ -344,6 +396,7 @@ def isolation( name="", mode="w", errors="backslashreplace", + original_fd=old_stderr_fd, ) @_pause_echo(echo_input) # type: ignore diff --git a/tests/test_testing.py b/tests/test_testing.py index 11fe29dc5d..0f32fcdebd 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -1,3 +1,4 @@ +import faulthandler import os import sys from io import BytesIO @@ -469,3 +470,28 @@ def cli(): result = runner.invoke(cli) assert result.stderr == "gyarados gyarados gyarados" + + +def test_faulthandler_enable(runner): + """``faulthandler.enable()`` inside ``CliRunner`` should not crash with + ``io.UnsupportedOperation: fileno``. + + ``faulthandler.enable()`` needs a real OS file descriptor to register + its signal handler. ``CliRunner`` replaces ``sys.stderr`` with a + ``BytesIO`` wrapper that has no ``fileno()``, causing the call to fail. + + Reproduce:https://github.com/pallets/click/issues/2865 + """ + + @click.command() + @click.option("--flag", type=bool, default=True) + def cli(flag): + click.echo("Executing main function...") + if flag: + click.echo("Registering faulthandler") + faulthandler.enable() + click.echo("Finished executing main function.") + + result = runner.invoke(cli, ["--flag", True]) + assert result.exit_code == 0, result.output + assert "Finished executing main function." in result.output