diff --git a/CHANGELOG.md b/CHANGELOG.md index 45bff09ed..9e53a5b28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ ## 4.2.4 (TBD) - Bug Fixes + - Fixed output redirection and piping raising `UnicodeEncodeError` and leaving an empty file + behind. Command output is rendered by Rich and contains box-drawing characters, which no + `cp125x` Windows ANSI code page can represent, so redirecting or piping it failed. This + affects current Windows 11 with default settings on Python 3.11 through 3.14, not only older + systems. Redirection targets and pipes now use UTF-8 explicitly - Fixed the right prompt being redrawn beside every accepted command line in the scrollback. prompt-toolkit includes it in the final frame of each prompt, which is the frame left on the terminal; it is now hidden there, as the bottom toolbar already was, and stays on the live diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index c4a21c831..fcd26b2e4 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -3324,9 +3324,11 @@ def _redirect_output(self, statement: Statement) -> utils.RedirectionSavedState: # Create a pipe with read and write sides read_fd, write_fd = os.pipe() - # Open each side of the pipe - subproc_stdin = open(read_fd) # noqa: SIM115 - new_stdout: TextIO = cast(TextIO, open(write_fd, "w")) # noqa: SIM115 + # Open each side of the pipe. Both ends are given an explicit encoding: + # command output is rendered by Rich and routinely contains non-ASCII, which + # the locale encoding cannot always represent. + subproc_stdin = open(read_fd, encoding="utf-8") # noqa: SIM115 + new_stdout: TextIO = cast(TextIO, open(write_fd, "w", encoding="utf-8")) # noqa: SIM115 # Create pipe process in a separate group to isolate our signals from it. If a Ctrl-C event occurs, # our sigint handler will forward it only to the most recent pipe process. This makes sure pipe @@ -3375,8 +3377,15 @@ def _redirect_output(self, statement: Statement) -> utils.RedirectionSavedState: # statement.output can only contain REDIRECTION_APPEND or REDIRECTION_OUTPUT mode = "a" if statement.redirector == constants.REDIRECTION_APPEND else "w" try: - # Use line buffering - new_stdout = cast(TextIO, open(su.strip_quotes(statement.redirect_to), mode=mode, buffering=1)) # noqa: SIM115 + # Use line buffering. The encoding is explicit rather than the + # locale's: command output is rendered by Rich and routinely contains + # non-ASCII, so on a non-UTF-8 system -- a default Windows console, + # for instance -- redirection would otherwise fail and leave an empty + # file behind. + new_stdout = cast( + TextIO, + open(su.strip_quotes(statement.redirect_to), mode=mode, buffering=1, encoding="utf-8"), # noqa: SIM115 + ) except OSError as ex: raise RedirectionError("Failed to redirect output") from ex diff --git a/tests/conftest.py b/tests/conftest.py index 3a37e9856..e5e5ca732 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -80,6 +80,23 @@ def run_cmd(app: cmd2.Cmd, cmd: str) -> tuple[list[str], list[str]]: return normalize(out), normalize(err) +#: Environment variables that change how Rich and cmd2 render output. Left inherited, +#: they make unrelated tests fail depending on who runs the suite: NO_COLOR fails 15 +#: tests, FORCE_COLOR and TTY_COMPATIBLE 53 each. +COLOR_ENVIRONMENT = ("NO_COLOR", "FORCE_COLOR", "TTY_COMPATIBLE", "TTY_INTERACTIVE") + + +@pytest.fixture(autouse=True) +def neutral_color_environment(monkeypatch: pytest.MonkeyPatch) -> None: + """Render output the same way regardless of the caller's environment. + + Tests that exercise these variables set them explicitly, which still works because + a test's own monkeypatching runs after this fixture. + """ + for name in COLOR_ENVIRONMENT: + monkeypatch.delenv(name, raising=False) + + @pytest.fixture def base_app() -> cmd2.Cmd: return cmd2.Cmd(include_py=True, include_ipy=True) diff --git a/tests/test_run_pyscript.py b/tests/test_run_pyscript.py index 69b335fca..e4030ebde 100644 --- a/tests/test_run_pyscript.py +++ b/tests/test_run_pyscript.py @@ -246,7 +246,7 @@ def test_run_pyscript_print_redirection(base_app, request, tmp_path, capsys) -> out, err = capsys.readouterr() # Verify the output file contains what we expect from print() - content = pathlib.Path(out_file).read_text() + content = pathlib.Path(out_file).read_text(encoding="utf-8") # Look for everything written to self.stdout assert len(content.splitlines()) == 4 diff --git a/tests/test_suite_environment.py b/tests/test_suite_environment.py new file mode 100644 index 000000000..bb9cb8d1f --- /dev/null +++ b/tests/test_suite_environment.py @@ -0,0 +1,59 @@ +"""Guards that the suite renders output independently of the developer's environment. + +Rich and cmd2 both consult environment variables when deciding whether to emit styling. +Inheriting them makes large numbers of unrelated tests fail depending on who runs them, +which is expensive to diagnose because the failures look like product regressions. +""" + +import os +import sys + +import pytest + +import cmd2 + +#: Variables that change how output is rendered. Rich reads all of these; cmd2 reads +#: NO_COLOR directly. Tests that exercise them set them explicitly instead. +COLOR_ENVIRONMENT = ("NO_COLOR", "FORCE_COLOR", "TTY_COMPATIBLE", "TTY_INTERACTIVE") + + +@pytest.mark.parametrize("name", COLOR_ENVIRONMENT) +def test_color_environment_does_not_leak_into_tests(name: str) -> None: + """A developer exporting any of these must not change the suite's results.""" + assert name not in os.environ, ( + f"{name} leaked into the test environment; output-rendering assertions would depend on who is running the suite" + ) + + +class EncodingProbe(cmd2.Cmd): + """Reports the encoding of whatever stream output is currently going to.""" + + def do_show_encoding(self, _: str) -> None: + """Print the current output stream's encoding.""" + self.poutput(f"ENCODING={getattr(self.stdout, 'encoding', None)}") + + +def test_redirection_to_a_file_uses_utf8(tmp_path) -> None: + """cmd2 renders non-ASCII, so a redirect target must not use the locale encoding. + + Opened with the locale encoding, redirecting styled output raises UnicodeEncodeError + on any non-UTF-8 system -- which includes a default Windows console -- leaving the + user an empty file and an error. + """ + app = EncodingProbe(allow_cli_args=False) + target = tmp_path / "out.txt" + app.onecmd_plus_hooks(f'show_encoding > "{target}"') + assert "ENCODING=utf-8" in target.read_text(encoding="utf-8") + + +#: A pass-through filter, run with this interpreter so the test does not depend on Unix +#: utilities being installed. `cmd.exe` has no `cat`, and this fix exists for Windows. +PASS_THROUGH = "import sys; sys.stdin.reconfigure(encoding='utf-8'); sys.stdout.write(sys.stdin.read())" + + +def test_piping_uses_utf8(tmp_path) -> None: + """The same applies to the pipe the subprocess reads from.""" + app = EncodingProbe(allow_cli_args=False) + target = tmp_path / "piped.txt" + app.onecmd_plus_hooks(f'show_encoding | "{sys.executable}" -c "{PASS_THROUGH}" > "{target}"') + assert "ENCODING=utf-8" in target.read_text(encoding="utf-8")