testing: pin stdin closed while recording

cw/testing.py

@@ -55,11 +55,15 @@
 Recorded text is stored newline-normalised (``\\r\\n`` and ``\\r`` both become ``\\n``) and
 every comparison normalises both sides, so a golden recorded on a Mac asserts cleanly on a
 Windows runner. The subprocess environment pins ``COLUMNS``, ``PYTHONUTF8``,
-``PYTHONIOENCODING``, ``PYTHONHASHSEED`` and ``TERM`` so the bytes are reproducible rather
-than merely comparable. :func:`read_cases` reads a JSON-list form as well as a ``shlex``
-line, because ``shlex`` is POSIX-only and a Windows user must never need it. And
-:func:`parity` spawns no subprocess at all -- it runs in-process against shipped fixtures --
-so the ``.exe`` console-script shim and the cp1252 console never enter the picture.
+``PYTHONIOENCODING``, ``PYTHONHASHSEED`` and ``TERM``, and ``stdin`` is pinned closed, so
+the bytes are reproducible rather than merely comparable. :func:`read_cases` reads a JSON-list form as well as a ``shlex``
+line, because ``shlex`` is POSIX-only and a Windows user must never need it. The ``.exe``
+a console script is installed as on Windows is scrubbed out of recorded text by
+:func:`scrub_exe_suffix`, because ``argparse`` takes its ``prog`` from
+``basename(sys.argv[0])`` and would otherwise report ``usage: opsward.EXE`` on the runner
+and ``usage: opsward`` everywhere else. And :func:`parity` spawns no subprocess at all --
+it runs in-process against shipped fixtures -- so the console-script shim and the cp1252
+console never enter the picture there either.
 
     >>> normalise_text('a\\r\\nb\\r\\n')
     'a\\nb\\n'
@@ -92,6 +96,8 @@
     "pinned_env",
     "read_cases",
     "replay",
+    "scrub_addresses",
+    "scrub_exe_suffix",
 ]
 
 #: Bumped when the golden JSON schema changes incompatibly. A golden that does not carry
@@ -266,6 +272,59 @@ def scrub_addresses(text: str) -> str:
     return _ADDRESS.sub("0xADDR", text) if text else text
 
 
+def scrub_exe_suffix(text: str, program) -> str:
+    """Strip Windows' ``.exe`` off the program name a CLI prints about itself.
+
+    ``argparse`` derives ``prog`` from ``os.path.basename(sys.argv[0])``, and on Windows a
+    console script is installed as ``opsward.exe``. So the *same* CLI, at the same commit,
+    reports ``usage: opsward ...`` on a Mac and ``usage: opsward.EXE ...`` on a Windows
+    runner -- and every case that prints a usage line or an error prefix differs. That is a
+    fact about packaging, not about the command line, and this module promises that "a
+    golden recorded on a Mac asserts cleanly on a Windows runner".
+
+    Only the *program's own* stem is rewritten, so a CLI that talks about some other
+    ``.exe`` is left alone. The match is case-insensitive because the shim's extension is
+    reported as ``.EXE`` on some Windows configurations and ``.exe`` on others -- which
+    would otherwise make two Windows runners disagree with each other.
+
+    ``program`` is the **whole command**, not just its first word, because the console
+    script is not always the first word: a caller may pass ``['python', 'toy.exe']`` as
+    readily as ``['opsward.exe']``, and ``argparse`` names whichever of them landed in
+    ``sys.argv[0]``. Every part is considered; a part that is not the program yields a stem
+    that appears nowhere in the text, so considering it costs nothing.
+
+    Both path separators are handled, and the replacement is a function rather than a
+    template string, because a Windows path reaches this on a POSIX host -- through a
+    golden's recorded ``prog`` -- where ``os.path.basename`` does not split on a backslash
+    and ``re.sub`` would read the remaining ``\\p`` as a bad escape.
+
+    >>> scrub_exe_suffix('usage: opsward.EXE [-h]', r'C:\\Scripts\\opsward.exe')
+    'usage: opsward [-h]'
+    >>> scrub_exe_suffix('usage: toy.EXE [-h]', ['/usr/bin/python', '/tmp/toy.exe'])
+    'usage: toy [-h]'
+    >>> scrub_exe_suffix('usage: opsward [-h]', '/usr/local/bin/opsward')
+    'usage: opsward [-h]'
+    >>> scrub_exe_suffix('run setup.exe first', '/usr/local/bin/opsward')
+    'run setup.exe first'
+    """
+    if not text or not program:
+        return text
+    parts = [program] if isinstance(program, str) else list(program)
+    for part in parts:
+        stem = re.split(r"[\\/]", str(part))[-1]
+        if stem[-4:].lower() == ".exe":
+            stem = stem[:-4]
+        if not stem:
+            continue
+        text = re.sub(
+            re.escape(stem) + r"\.exe\b",
+            lambda _match, matched=stem: matched,
+            text,
+            flags=re.IGNORECASE,
+        )
+    return text
+
+
 def _usage_of(stdout: str, stderr: str) -> str:
     """The normalised ``usage:`` line, from wherever the CLI put it.
 
@@ -437,8 +496,14 @@ def capture(call) -> dict:
     _flush(sys.stdout, sys.stderr)
     with tempfile.TemporaryFile() as out_file, tempfile.TemporaryFile() as err_file:
         saved_fds = (os.dup(1), os.dup(2))
+        # Descriptor 0 is pinned closed for the same reason the subprocess path pins it --
+        # see _run_subprocess. A command with an interactive path must read EOF here, not
+        # whatever console the recorder happens to be attached to.
+        saved_stdin_fd = os.dup(0)
+        null_fd = os.open(os.devnull, os.O_RDONLY)
         trailer = ""
         try:
+            os.dup2(null_fd, 0)
             os.dup2(out_file.fileno(), 1)
             os.dup2(err_file.fileno(), 2)
             sys.stdout = _stream_on_fd(1)
@@ -465,6 +530,9 @@ def capture(call) -> dict:
             for saved, fd in zip(saved_fds, (1, 2)):
                 os.dup2(saved, fd)
                 os.close(saved)
+            os.dup2(saved_stdin_fd, 0)
+            os.close(saved_stdin_fd)
+            os.close(null_fd)
         out_file.seek(0)
         err_file.seek(0)
         stdout = out_file.read().decode("utf-8", "replace")
@@ -516,7 +584,17 @@ def _as_command(prog) -> list:
 
 
 def _run_subprocess(command, argv, *, env, cwd, timeout) -> dict:
-    """One case, run as a real subprocess. The heart of the standalone half."""
+    """One case, run as a real subprocess. The heart of the standalone half.
+
+    ``stdin`` is :data:`subprocess.DEVNULL`, not the recorder's own. A CLI with an
+    interactive path -- a REPL entered when a query is omitted, a ``confirm()`` prompt --
+    reads stdin, and inheriting the recorder's makes the *recording itself* depend on where
+    it was run: at a terminal the child blocks on a prompt nobody answers and the case is
+    recorded as a timeout, while under CI or pytest the same case sees an immediate EOF and
+    records the real behaviour. A golden is a claim about a CLI, so it must not be a claim
+    about the console that recorded it; pinning stdin closed is the same measure as pinning
+    ``COLUMNS``, and it belongs for the same reason.
+    """
     try:
         done = subprocess.run(
             command + list(argv),
@@ -527,6 +605,7 @@ def _run_subprocess(command, argv, *, env, cwd, timeout) -> dict:
             env=env,
             cwd=cwd,
             timeout=timeout,
+            stdin=subprocess.DEVNULL,
         )
     except subprocess.TimeoutExpired:
         return {
@@ -536,8 +615,8 @@ def _run_subprocess(command, argv, *, env, cwd, timeout) -> dict:
         }
     return {
         "returncode": done.returncode,
-        "stdout": normalise_text(done.stdout),
-        "stderr": normalise_text(done.stderr),
+        "stdout": scrub_exe_suffix(normalise_text(done.stdout), command),
+        "stderr": scrub_exe_suffix(normalise_text(done.stderr), command),
     }
 
 

tests/test_testing.py

@@ -9,6 +9,7 @@
 import io
 import json
 import os
+import pathlib
 import subprocess
 import sys
 import textwrap
@@ -403,6 +404,134 @@ def test_argcomplete_is_removed_so_it_cannot_hijack_a_recording(self):
             os.environ.pop("_ARGCOMPLETE", None)
 
 
+class TestPinnedStdin:
+    """A recording must describe the CLI, not the console that recorded it.
+
+    A CLI with an interactive path -- grub drops into a REPL when the query is omitted,
+    `cw.confirm` asks a question -- reads stdin. If the recorded child inherits the
+    recorder's stdin, the *same case* records two different facts: at a terminal the child
+    blocks on a prompt nobody will answer and the case is recorded as a timeout, while
+    under CI or pytest it sees an immediate EOF and records the real behaviour. Then a
+    golden recorded in CI fails when a developer replays it locally, for a reason that has
+    nothing to do with the CLI.
+    """
+
+    READS_STDIN = (
+        'import sys\n'
+        'try:\n'
+        '    line = input("prompt> ")\n'
+        'except EOFError:\n'
+        '    line = "<EOF>"\n'
+        'print("read:", line)\n'
+    )
+
+    @pytest.fixture
+    def reads_stdin(self, tmp_path):
+        path = tmp_path / "reads_stdin.py"
+        path.write_text(self.READS_STDIN, encoding="utf-8")
+        return [sys.executable, str(path)]
+
+    def test_a_command_that_reads_stdin_records_eof_not_a_timeout(self, reads_stdin):
+        golden = testing.characterize(reads_stdin, [[]], timeout=10)
+        case = golden["cases"][0]
+        assert case["returncode"] == 0
+        assert "<EOF>" in case["stdout"]
+
+    def test_the_recording_does_not_depend_on_the_recorder_s_own_stdin(
+        self, reads_stdin, tmp_path
+    ):
+        """Record the same case twice, under two different stdins, and compare."""
+        driver = tmp_path / "driver.py"
+        driver.write_text(
+            "import json, sys\n"
+            f"sys.path.insert(0, {str(pathlib.Path(testing.__file__).parent.parent)!r})\n"
+            "from cw import testing\n"
+            f"golden = testing.characterize({reads_stdin!r}, [[]], timeout=10)\n"
+            "print(json.dumps(golden['cases'][0]))\n",
+            encoding="utf-8",
+        )
+
+        def record_with(stdin):
+            done = subprocess.run(
+                [sys.executable, str(driver)],
+                stdin=stdin,
+                capture_output=True,
+                text=True,
+                timeout=60,
+            )
+            assert done.returncode == 0, done.stderr
+            return json.loads(done.stdout)
+
+        # An open pipe nobody writes to is what a terminal looks like to the child.
+        read_fd, write_fd = os.pipe()
+        try:
+            from_terminal = record_with(read_fd)
+        finally:
+            os.close(read_fd)
+            os.close(write_fd)
+        with open(os.devnull, "rb") as devnull:
+            from_devnull = record_with(devnull)
+
+        assert from_terminal == from_devnull
+        assert from_terminal["returncode"] == 0
+
+
+class TestWindowsConsoleScriptShim:
+    """A golden recorded on a Mac must assert on a Windows runner -- as promised.
+
+    `argparse` takes its `prog` from `basename(sys.argv[0])`, and a console script is
+    installed as `opsward.exe` on Windows. Without scrubbing, the same CLI at the same
+    commit reports `usage: opsward ...` on a Mac and `usage: opsward.EXE ...` on the
+    runner, and every case that prints a usage line or an error prefix differs -- 22 of
+    31 in the case that found this.
+
+    The simulation is exact rather than mocked: the same toy CLI is written under two
+    names, one carrying the Windows extension, and the golden recorded from the plain one
+    is replayed against the `.exe` one.
+
+    Both are run as `[sys.executable, path]` rather than as executables in their own
+    right. A first version wrote a shebang and `chmod +x`, which is exactly the sort of
+    POSIX assumption this class exists to catch -- it failed on the Windows runner with
+    `OSError: [WinError 193] %1 is not a valid Win32 application`. It also made the test
+    weaker than it looks: with the interpreter in front, the console script is *not* the
+    command's first word, so this now covers the harder shape too.
+    """
+
+    DERIVES_PROG = (
+        "import argparse\n"
+        "parser = argparse.ArgumentParser(description='A toy.')\n"
+        "parser.add_argument('name')\n"
+        "parser.parse_args()\n"
+    )
+
+    @pytest.fixture
+    def two_names(self, tmp_path):
+        """The same CLI as `toy` and as `toy.EXE`, both letting argparse derive prog."""
+        plain = tmp_path / "toy"
+        plain.write_text(self.DERIVES_PROG, encoding="utf-8")
+        windows = tmp_path / "toy.EXE"
+        windows.write_text(self.DERIVES_PROG, encoding="utf-8")
+        return (
+            [sys.executable, str(plain)],
+            [sys.executable, str(windows)],
+        )
+
+    def test_the_exe_suffix_does_not_make_a_recording_os_specific(self, two_names):
+        plain, windows = two_names
+        golden = testing.characterize(plain, [["--help"], []], timeout=30)
+        assert "toy.EXE" not in json.dumps(golden)
+        # Replaying the plain-name golden against the .exe shim must be clean.
+        testing.assert_replay(golden, prog=windows, strict_help=True)
+
+    def test_only_the_program_s_own_exe_is_scrubbed(self):
+        assert testing.scrub_exe_suffix("run setup.exe", "/bin/toy") == "run setup.exe"
+        assert testing.scrub_exe_suffix("toy.exe ran", "/bin/toy") == "toy ran"
+
+    def test_the_program_need_not_be_the_command_s_first_word(self):
+        command = ["/usr/bin/python", "/tmp/toy.exe"]
+        assert testing.scrub_exe_suffix("usage: toy.EXE [-h]", command) == "usage: toy [-h]"
+
+
 class TestExitStatus:
     """`capture` reproduces what the interpreter does with a `SystemExit`."""