diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ecbfceae..07daf4cf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,7 +17,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-latest] + os: [ubuntu-latest, macos-latest, windows-latest] # requires-python = ">=3.11" in pyproject.toml -- cover the floor # and the latest supported minor. python-version: ["3.11", "3.12"] @@ -49,6 +49,25 @@ jobs: strategy: fail-fast: false matrix: + # No windows-latest here, deliberately. Every test this job selects + # (`-m integration`) forks a real child process and, in most of the + # files, allocates a real pty pair via the POSIX-only `pty`/`termios` + # stdlib modules -- there is no Windows equivalent of either + # mechanism. `test_ctrlc_functional_integration.py` and + # `test_terminal_echo_integration.py` already `pytest.skip(..., + # allow_module_level=True)` on win32 (they import `pty`/`termios` at + # module scope, so skipping is the only way to avoid a collection + # error). `test_stdout_offload_freeze_integration.py` calls + # `os.fork()` directly with no platform guard at all, and `os.fork` + # simply does not exist on Windows -- it fails with + # `AttributeError: module 'os' has no attribute 'fork'` + # (confirmed on a real windows-latest run, job id 94301234220). + # A Windows leg of this job would therefore either run zero tests + # (all skipped) or hard-fail on the one file missing a guard -- pure + # CI theatre, burning runner minutes to report a "green" (or "red") + # that says nothing about Windows support. If a genuinely + # cross-platform integration test is ever added to this job, add + # windows-latest back for it specifically. os: [ubuntu-latest, macos-latest] steps: - uses: actions/checkout@v4 @@ -66,5 +85,6 @@ jobs: # -- the ones that fork a real pty child and probe real termios state # -- are skipped by default and, before this job existed, ran nowhere. # They are exactly the tests that guard the dedicated-tty-input - # mechanism, so they get their own job on both platforms. + # mechanism, so they get their own job (POSIX only -- see the + # `matrix.os` comment above for why Windows is excluded). run: uv run pytest -m integration -q diff --git a/amplifier_app_cli/lib/bundle_loader/resolvers.py b/amplifier_app_cli/lib/bundle_loader/resolvers.py index df4679b4..ad135987 100644 --- a/amplifier_app_cli/lib/bundle_loader/resolvers.py +++ b/amplifier_app_cli/lib/bundle_loader/resolvers.py @@ -16,6 +16,7 @@ import asyncio import logging import os +import re from importlib import metadata from pathlib import Path from typing import Any @@ -27,6 +28,17 @@ logger = logging.getLogger(__name__) +# The two absolute path forms Windows actually has: a drive letter +# (``C:\...`` or ``C:/...``) and a UNC share (``\\server\share``). Used to tell +# a local path from a package name in ``_parse_source`` -- see the comment at +# that call site for why a Windows path otherwise gets misread as a PyPI +# distribution name. +# +# Evaluated on every platform, which is safe: no legitimate package name +# contains a backslash or an ``X:`` drive prefix, and on POSIX the ``/`` and +# ``.`` prefixes short-circuit before this is ever reached. +_WINDOWS_ABSOLUTE_PATH_RE = re.compile(r"^(?:[A-Za-z]:[\\/]|\\\\)") + class ModuleResolutionError(Exception): """Error during module resolution.""" @@ -321,6 +333,23 @@ def _parse_source( source.startswith("file://") or source.startswith("/") or source.startswith(".") + # A Windows absolute path matches none of the prefixes above -- not + # "file://", not "/", not "." -- so it used to fall through to the + # package-name branch below. A user putting a local path override in + # settings.yaml on Windows (`C:\src\my-module`) had it silently + # treated as a PyPI distribution name, and the resulting failure + # named a package that was never mentioned anywhere. + # + # Matches the two absolute forms Windows actually has: a drive + # letter (`C:\...` or `C:/...`) and a UNC share (`\\server\share`). + # A bare leading backslash (`\foo`, drive-relative) is deliberately + # NOT matched -- it is not absolute, and treating it as a path would + # be a guess. + # + # POSIX is unaffected: `/` and `.` already short-circuit before + # this, and no legitimate package name contains a backslash or a + # `X:` drive prefix. + or _WINDOWS_ABSOLUTE_PATH_RE.match(source) is not None ): return FoundationFileSource(source) # Assume package name diff --git a/amplifier_app_cli/main.py b/amplifier_app_cli/main.py index d9a27f65..53e246fc 100644 --- a/amplifier_app_cli/main.py +++ b/amplifier_app_cli/main.py @@ -30,6 +30,24 @@ from prompt_toolkit.key_binding import KeyBindings from rich.panel import Panel +# Errors that mean the terminal can never satisfy the REPL, so retrying is +# pointless. See the REPL loop's handler for the full story: on Windows without +# a real console, prompt_toolkit's Win32Output raises before the loop reaches +# any await, so the "keep going" catch-all turns into an unkillable busy spin. +# +# `prompt_toolkit.output.win32` asserts `sys.platform == "win32"` at import, so +# this must stay guarded. On POSIX the tuple is empty and `except ()` catches +# nothing -- the POSIX path is byte-identical in effect to what shipped. +if sys.platform == "win32": # pragma: no cover - platform-specific + from prompt_toolkit.output.win32 import NoConsoleScreenBufferError + + _TERMINAL_UNUSABLE_ERRORS: tuple[type[BaseException], ...] = ( + NoConsoleScreenBufferError, + ) +else: + _TERMINAL_UNUSABLE_ERRORS = () + + from .commands.agents import agents as agents_group from .commands.allowed_dirs import allowed_dirs as allowed_dirs_group from .commands.bundle import bundle as bundle_group @@ -66,6 +84,36 @@ from .utils.error_format import escape_markup from .utils.version import get_core_version, get_version + +def _report_terminal_unusable(exc: BaseException, *, verbose: bool = False) -> None: + """Explain an unusable terminal in terms the user can act on. + + Shared by every site that can hit ``_TERMINAL_UNUSABLE_ERRORS`` so the + message stays identical no matter where the failure surfaces. Without this, + a user piping or redirecting ``amplifier`` on Windows got a raw + prompt_toolkit traceback naming ``Win32Output`` -- accurate, but it points + at a library internal rather than at what they did or what to do instead. + """ + # Local imports: this helper is defined above main.py's own import block so + # it can sit next to the _TERMINAL_UNUSABLE_ERRORS tuple it belongs to. + from .console import console as _console + from .utils.error_format import escape_markup as _escape + + _console.print(f"[red]Cannot run an interactive session:[/red] {_escape(exc)}") + _console.print( + "[yellow]The terminal has no console screen buffer. This happens when " + "output is piped or redirected, or when running without a real " + "console.[/yellow]" + ) + _console.print( + "Run interactively in a real terminal (Windows Terminal, conhost, or " + "cmd.exe), or use a non-interactive command such as " + "[cyan]amplifier run[/cyan]." + ) + if verbose: + _console.print_exception() + + logger = logging.getLogger(__name__) # --------------------------------------------------------------------------- @@ -3476,13 +3524,30 @@ async def interactive_chat( ) ) - # Create prompt session for history and advanced editing - prompt_session = _create_prompt_session( - get_active_mode=lambda: command_processor.session.coordinator.session_state.get( - "active_mode" - ), - get_pinned_provider=lambda: _pinned_provider_name(command_processor.session), - ) + # Create prompt session for history and advanced editing. + # + # This is the FIRST place an interactive session touches the terminal, and + # on Windows it is where an unusable terminal actually surfaces: building + # the prompt_toolkit Application resolves `get_app().output`, which + # constructs Win32Output, which raises NoConsoleScreenBufferError whenever + # stdout is not a real console (piped, redirected, CI, non-console parent). + # + # Guarding here rather than only at the REPL loop matters: measured on + # Windows, an unguarded `amplifier` with piped stdout died with a raw + # prompt_toolkit traceback out of this call, never reaching the loop. Unit + # tests miss it because they mock _create_prompt_session. + try: + prompt_session = _create_prompt_session( + get_active_mode=lambda: command_processor.session.coordinator.session_state.get( + "active_mode" + ), + get_pinned_provider=lambda: _pinned_provider_name(command_processor.session), + ) + except _TERMINAL_UNUSABLE_ERRORS as e: + _report_terminal_unusable(e, verbose=verbose) + await initialized.cleanup() + close_dedicated_tty_input() + return # Helper to extract model name from config def _extract_model_name() -> str: @@ -3806,24 +3871,50 @@ def sigint_handler(signum, frame): ): _streaming_hooks_instance.set_composing_source(None) - # Execute initial prompt if provided - if initial_prompt: - console.print( - f"\n[bold cyan]>[/bold cyan] {initial_prompt[:100]}{'...' if len(initial_prompt) > 100 else ''}" - ) - console.print("\n[dim]Processing... (Ctrl+C to cancel)[/dim]") - - # Process runtime @mentions in initial prompt - initial_prompt = await process_runtime_mentions(session, initial_prompt) - # NOTE: the /goal auto-continue loop lives in the orchestrator - # (loop-streaming's execute()), so the REPL calls the plain - # `_execute_with_interrupt` here -- the orchestrator drives - # auto-continuation internally via session_state["goal"]. See - # docs/GOAL_COMMAND.md. - await _execute_with_interrupt(initial_prompt) - - # === REPL LOOP === + # === REPL LOOP (and everything that must run under its cleanup) === + # + # The try/finally starts HERE rather than at the loop, so the terminal + # check and the initial-prompt turn are both covered by the finally's + # teardown. An early `return` from inside a try still runs the finally, + # so bailing on an unusable terminal still awaits initialized.cleanup() + # and closes the dedicated tty fd -- leaking those was a real bug caught + # by test_interactive_chat_teardown_does_not_raise_when_fd_never_opened. try: + # An interactive session needs a terminal prompt_toolkit can actually + # drive. Check ONCE, here, before any turn runs -- both the initial-prompt + # path below and the REPL loop wrap their work in `patch_stdout()`, and on + # Windows without a real console that raises NoConsoleScreenBufferError + # from Win32Output. Checking up front means one clear message instead of + # the same failure surfacing differently from two call sites. + if _TERMINAL_UNUSABLE_ERRORS: + try: + with patch_stdout(): + pass + except _TERMINAL_UNUSABLE_ERRORS as e: + _report_terminal_unusable(e, verbose=verbose) + # No explicit teardown here: this `return` is inside the try, + # so the finally below runs and does the whole teardown -- + # cleanup(), close_dedicated_tty_input(), the hook emits. + # Calling close_dedicated_tty_input() here as well double-fired + # it, which the teardown tests correctly caught. + return + + # Execute initial prompt if provided + if initial_prompt: + console.print( + f"\n[bold cyan]>[/bold cyan] {initial_prompt[:100]}{'...' if len(initial_prompt) > 100 else ''}" + ) + console.print("\n[dim]Processing... (Ctrl+C to cancel)[/dim]") + + # Process runtime @mentions in initial prompt + initial_prompt = await process_runtime_mentions(session, initial_prompt) + # NOTE: the /goal auto-continue loop lives in the orchestrator + # (loop-streaming's execute()), so the REPL calls the plain + # `_execute_with_interrupt` here -- the orchestrator drives + # auto-continuation internally via session_state["goal"]. See + # docs/GOAL_COMMAND.md. + await _execute_with_interrupt(initial_prompt) + while True: try: # Get user input with history, editing, and paste support. @@ -3923,6 +4014,44 @@ def sigint_handler(signum, frame): except LLMError as e: display_llm_error(console, e, verbose=verbose) + except _TERMINAL_UNUSABLE_ERRORS as e: + # MUST precede the catch-all below, and MUST break. + # + # On Windows with stdout not attached to a real console (piped, + # redirected, CI, a non-console parent), prompt_toolkit's + # Win32Output raises NoConsoleScreenBufferError. Critically it + # raises on ENTRY to `with patch_stdout():` -- before + # `await prompt_session.prompt_async()` -- so this loop + # iteration contains NO await point at all. + # + # Falling into the generic handler below therefore produced an + # infinite BUSY loop: raise, print, loop, raise... measured at + # 88% CPU on ALIENWARE-R13. And because the coroutine never + # yields, asyncio cannot interrupt it -- an + # `asyncio.wait_for(..., timeout=10)` around the whole call + # never fired. Not cancellable, not timeout-able; only SIGKILL + # ends it. + # + # A terminal that is not a console will not become one by + # trying again, so this is fatal to the REPL by definition. + # Fail loud and leave, rather than spin in a lesser state. + console.print( + f"[red]Cannot run an interactive session:[/red] {escape_markup(e)}" + ) + console.print( + "[yellow]The terminal has no console screen buffer. This " + "happens when output is piped or redirected, or when " + "running without a real console.[/yellow]" + ) + console.print( + "Run interactively in a real terminal (Windows Terminal, " + "conhost, or cmd.exe), or use a non-interactive command " + "such as [cyan]amplifier run[/cyan]." + ) + if verbose: + console.print_exception() + break + except Exception as e: console.print(f"[red]Error:[/red] {escape_markup(e)}") if verbose: @@ -4119,9 +4248,7 @@ async def execute_single( # condition is re-sent to the evaluator model every turn; # without this it would see the literal "@file" token # forever instead of the file's content. - goal_condition = await process_runtime_mentions( - session, goal_condition - ) + goal_condition = await process_runtime_mentions(session, goal_condition) session.coordinator.session_state["goal"] = { "condition": goal_condition, diff --git a/tests/lib/mention_loading/test_deduplicator.py b/tests/lib/mention_loading/test_deduplicator.py index 942fdb39..bf76ca74 100644 --- a/tests/lib/mention_loading/test_deduplicator.py +++ b/tests/lib/mention_loading/test_deduplicator.py @@ -35,9 +35,16 @@ def test_deduplicator_duplicate_content(): ctx_file = files[0] assert ctx_file.content == content assert len(ctx_file.paths) == 3 - assert Path("/path1/file.md") in ctx_file.paths - assert Path("/path2/file.md") in ctx_file.paths - assert Path("/path3/file.md") in ctx_file.paths + # Compare against RESOLVED paths. add_file() deliberately calls + # path.resolve() so a relative and an absolute reference to the same file + # deduplicate. On POSIX "/path1/file.md" is already canonical so the + # unresolved form happened to match; on Windows resolve() prepends the + # current drive, giving WindowsPath("C:/path1/file.md") and the bare + # comparison failed. Resolving both sides tests the actual contract -- + # "the path I added is tracked" -- instead of an accident of POSIX. + assert Path("/path1/file.md").resolve() in ctx_file.paths + assert Path("/path2/file.md").resolve() in ctx_file.paths + assert Path("/path3/file.md").resolve() in ctx_file.paths def test_deduplicator_same_path_twice(): diff --git a/tests/test_always_render_final_response.py b/tests/test_always_render_final_response.py index 717e101a..9056175c 100644 --- a/tests/test_always_render_final_response.py +++ b/tests/test_always_render_final_response.py @@ -12,6 +12,7 @@ GREEN phase: Once the gate is removed and _streaming_overlay_active is deleted the calls go through and both assertions pass. """ + from __future__ import annotations import sys @@ -19,10 +20,33 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest +from prompt_toolkit.application import create_app_session +from prompt_toolkit.output import DummyOutput _MODULE = "amplifier_app_cli.main" +@pytest.fixture(autouse=True) +def _dummy_app_session(): + """Give prompt_toolkit an output it can use without a real console. + + These tests drive the REAL ``interactive_chat``, whose REPL wraps each turn + in ``patch_stdout()``. That reaches ``get_app().output``; with no app + session it builds a platform Output, and on Windows + ``Win32Output.__init__`` raises ``NoConsoleScreenBufferError`` whenever + stdout is not a real console (piped, redirected, CI). ``interactive_chat`` + then correctly declines to start an interactive session, so no turn runs + and ``render_message`` is never called -- the tests failed for a reason + that has nothing to do with the always-render contract they exist to guard. + + A ``DummyOutput`` app session removes that incidental dependency on the + host terminal. The assertions are unchanged and now hold on every platform + rather than only where a console happens to be attached. + """ + with create_app_session(output=DummyOutput()): + yield + + # --------------------------------------------------------------------------- # Session / initialized mock helpers (mirrors test_overlay_active_detection.py) # --------------------------------------------------------------------------- @@ -143,9 +167,7 @@ async def test_render_message_called_even_when_overlay_would_be_active( mock_render.assert_called_once() @pytest.mark.asyncio - async def test_render_message_called_when_no_streaming_config( - self, tmp_path: Path - ): + async def test_render_message_called_when_no_streaming_config(self, tmp_path: Path): """render_message IS called when no streaming-ui hook is configured. Sanity-check: the non-streaming path must also always render. diff --git a/tests/test_ctrlc_functional_integration.py b/tests/test_ctrlc_functional_integration.py index 6e4e0898..d47efee0 100644 --- a/tests/test_ctrlc_functional_integration.py +++ b/tests/test_ctrlc_functional_integration.py @@ -46,6 +46,27 @@ from __future__ import annotations +import sys + +import pytest + +# These tests drive a real POSIX pseudo-terminal. `pty`, `termios` and `fcntl` +# are POSIX-only stdlib modules with no Windows equivalent, and they are +# imported at module scope -- so on Windows this file fails at COLLECTION, +# surfacing as a hard error rather than a skip. An `ImportError` during +# collection is indistinguishable in CI output from a genuine breakage, which +# is exactly the noise that trains people to ignore a red run. +# +# `allow_module_level=True` is required: a plain `pytestmark` is evaluated +# AFTER the module body executes, which is far too late to prevent the import +# itself from raising. +if sys.platform == "win32": + pytest.skip( + "POSIX-only: requires pty, which have no Windows equivalent", + allow_module_level=True, + ) + + import json import os import signal diff --git a/tests/test_dead_code_removal.py b/tests/test_dead_code_removal.py index c7149cd6..80504388 100644 --- a/tests/test_dead_code_removal.py +++ b/tests/test_dead_code_removal.py @@ -16,7 +16,12 @@ def test_no_cancel_requested_global_variable(self): from pathlib import Path source = Path(__file__).parent.parent / "amplifier_app_cli" / "main.py" - content = source.read_text() + # encoding="utf-8" is REQUIRED: Python on Windows defaults to the + # locale codec (cp1252 here), and main.py contains non-cp1252 bytes, + # so a bare read_text() dies with UnicodeDecodeError before the + # assertion is ever reached. Every other read_text() in this file + # already passes it -- these two were simply missed. + content = source.read_text(encoding="utf-8") assert "_cancel_requested" not in content, ( "_cancel_requested is dead code (CancellationToken is used instead)" ) @@ -29,7 +34,12 @@ def test_key_manager_not_stored_as_variable(self): from pathlib import Path source = Path(__file__).parent.parent / "amplifier_app_cli" / "main.py" - content = source.read_text() + # encoding="utf-8" is REQUIRED: Python on Windows defaults to the + # locale codec (cp1252 here), and main.py contains non-cp1252 bytes, + # so a bare read_text() dies with UnicodeDecodeError before the + # assertion is ever reached. Every other read_text() in this file + # already passes it -- these two were simply missed. + content = source.read_text(encoding="utf-8") assert "_key_manager" not in content, ( "_key_manager is never referenced; only the constructor side-effect matters" ) diff --git a/tests/test_dedicated_tty_input.py b/tests/test_dedicated_tty_input.py index c86950bc..2082585a 100644 --- a/tests/test_dedicated_tty_input.py +++ b/tests/test_dedicated_tty_input.py @@ -35,6 +35,27 @@ from __future__ import annotations +import sys + +import pytest + +# These tests drive a real POSIX pseudo-terminal. `pty`, `termios` and `fcntl` +# are POSIX-only stdlib modules with no Windows equivalent, and they are +# imported at module scope -- so on Windows this file fails at COLLECTION, +# surfacing as a hard error rather than a skip. An `ImportError` during +# collection is indistinguishable in CI output from a genuine breakage, which +# is exactly the noise that trains people to ignore a red run. +# +# `allow_module_level=True` is required: a plain `pytestmark` is evaluated +# AFTER the module body executes, which is far too late to prevent the import +# itself from raising. +if sys.platform == "win32": + pytest.skip( + "POSIX-only: requires fcntl, pty, which have no Windows equivalent", + allow_module_level=True, + ) + + import fcntl import os import pty diff --git a/tests/test_general_config_overrides.py b/tests/test_general_config_overrides.py index c6b79abe..412765e7 100644 --- a/tests/test_general_config_overrides.py +++ b/tests/test_general_config_overrides.py @@ -622,9 +622,15 @@ async def test_custom_routing_dir_injected_when_it_exists(self): assert len(routing_entries) == 1 cfg = routing_entries[0]["config"] assert cfg.get("default_matrix") == "ornith" - assert cfg.get("custom_routing_dirs") == ["/fake/home/.amplifier/routing"], ( - f"Expected custom_routing_dirs to be injected, got: {cfg}" - ) + # Compare against str(Path(...)), not a hardcoded POSIX string. The + # source does `str(custom_routing_dir)`, which correctly yields native + # separators -- the value is handed to a hook that opens the directory, + # so native is what it must be. On Windows that is + # "\\fake\\home\\.amplifier\\routing", and the literal comparison failed + # for a reason that had nothing to do with the injection being tested. + assert cfg.get("custom_routing_dirs") == [ + str(Path("/fake/home/.amplifier/routing")) + ], f"Expected custom_routing_dirs to be injected, got: {cfg}" @pytest.mark.asyncio async def test_custom_routing_dir_not_injected_when_absent(self): diff --git a/tests/test_provider_command.py b/tests/test_provider_command.py index a244c710..276a34dc 100644 --- a/tests/test_provider_command.py +++ b/tests/test_provider_command.py @@ -6,6 +6,7 @@ -- it never selects a provider itself (REQUIRED BEHAVIORS in the task spec). """ +import contextlib import sys from pathlib import Path from types import SimpleNamespace @@ -1065,6 +1066,26 @@ def isolated_home(tmp_path, monkeypatch): return tmp_path +@contextlib.contextmanager +def _headless_app_session(): + """Run prompt_toolkit with a console-free output backend. + + On Windows, pytest captures stdout as a pipe rather than a real console, + so prompt_toolkit selects Win32Output, whose GetConsoleScreenBufferInfo + call raises NoConsoleScreenBufferError. Binding a DummyOutput to the + ambient AppSession makes PromptSession resolve output via the session + (Application.__init__: ``self.output = output or session.output``) + instead of probing the console. POSIX behaviour is unchanged. + """ + from prompt_toolkit.application.current import create_app_session + from prompt_toolkit.input import create_pipe_input + from prompt_toolkit.output import DummyOutput + + with create_pipe_input() as pipe_input: + with create_app_session(input=pipe_input, output=DummyOutput()): + yield + + def _prompt_session(mode=None, pinned=None): """Build the REAL PromptSession through the REAL factory.""" from amplifier_app_cli.main import _create_prompt_session @@ -1088,6 +1109,18 @@ def _render_via_prompt_session(session) -> str: @pytest.mark.usefixtures("isolated_home") class TestPromptSessionWiring: + @pytest.fixture(autouse=True) + def _console_free_output(self): + """Every test in this class builds a real PromptSession. + + Applied at class scope rather than inside the _prompt_session helper + because four of these tests call _create_prompt_session directly, and + a class-scoped fixture cannot be bypassed by a future test that does + the same. + """ + with _headless_app_session(): + yield + def test_message_is_a_callable_not_a_prebuilt_value(self): """A static value would freeze the indicator at construction time -- the pin must be re-read on every render.""" diff --git a/tests/test_stdout_offload_gaps.py b/tests/test_stdout_offload_gaps.py index 653fa860..3ea02573 100644 --- a/tests/test_stdout_offload_gaps.py +++ b/tests/test_stdout_offload_gaps.py @@ -23,6 +23,31 @@ from amplifier_app_cli.stdout_offload import _run_in_terminal_forcing_executor from amplifier_app_cli.stdout_offload import patch_stdout_offloaded +from prompt_toolkit.application import create_app_session +from prompt_toolkit.output import DummyOutput + + +@pytest.fixture(autouse=True) +def _dummy_app_session(): + """Give prompt_toolkit an output it can use without a real console. + + These tests exercise ``patch_stdout_offloaded()``'s own install/restore + machinery, but entering it also enters prompt_toolkit's ``patch_stdout``, + which reaches ``get_app().output``. With no app session that builds a + platform Output -- and on Windows ``Win32Output.__init__`` calls + ``GetConsoleScreenBufferInfo`` and raises ``NoConsoleScreenBufferError`` + whenever stdout is not a real console (piped, redirected, CI). The context + manager then dies on ENTRY and the body never runs, so all three tests + failed on Windows for a reason that has nothing to do with what they test. + + A ``DummyOutput`` app session removes that incidental dependency on the + host terminal without weakening the assertions: the monkeypatch + install/restore logic under test is untouched, and the tests now run + identically on every platform rather than only where a console happens to + be attached. + """ + with create_app_session(output=DummyOutput()): + yield def _current_run_in_terminal() -> Callable[..., Any]: diff --git a/tests/test_terminal_echo_integration.py b/tests/test_terminal_echo_integration.py index a8549945..8abdd1e5 100644 --- a/tests/test_terminal_echo_integration.py +++ b/tests/test_terminal_echo_integration.py @@ -32,6 +32,27 @@ from __future__ import annotations +import sys + +import pytest + +# These tests drive a real POSIX pseudo-terminal. `pty`, `termios` and `fcntl` +# are POSIX-only stdlib modules with no Windows equivalent, and they are +# imported at module scope -- so on Windows this file fails at COLLECTION, +# surfacing as a hard error rather than a skip. An `ImportError` during +# collection is indistinguishable in CI output from a genuine breakage, which +# is exactly the noise that trains people to ignore a red run. +# +# `allow_module_level=True` is required: a plain `pytestmark` is evaluated +# AFTER the module body executes, which is far too late to prevent the import +# itself from raising. +if sys.platform == "win32": + pytest.skip( + "POSIX-only: requires pty, termios, which have no Windows equivalent", + allow_module_level=True, + ) + + import os import signal import sys