diff --git a/cw/testing.py b/cw/testing.py index c95a7d7..2e6f9e3 100644 --- a/cw/testing.py +++ b/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), } diff --git a/tests/test_testing.py b/tests/test_testing.py index 2c3b8a3..8854ac8 100644 --- a/tests/test_testing.py +++ b/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 = ""\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 "" 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`."""