Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions src/osw/cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from __future__ import annotations

import inspect
import sys
from typing import Any, Optional, get_type_hints

import click
Expand All @@ -36,6 +37,41 @@
app = typer.Typer(no_args_is_help=True, add_completion=False)


def _force_utf8_output() -> None:
"""Encode stdout and stderr as UTF-8, whatever the locale asks for.

Python encodes a redirected stream with the locale encoding, which on a
German Windows system is cp1252. A non-ASCII label then reaches the
consumer as bytes no JSON parser can read, and a character cp1252 has no
code point for -- Japanese, Greek, Cyrillic -- raises UnicodeEncodeError
and ends the command. A Windows console stream is UTF-8 already, so on
Windows only redirected output changes. Elsewhere a terminal uses the
locale encoding, so this overrides a deliberate non-UTF-8 LANG or
PYTHONIOENCODING too. stderr is covered as well as stdout, because
``Context.guard`` sends captured stdout to stderr under ``--json``.

Called from the app callback, so it covers every command. Click prints
help and rejects an unknown root-level name before any callback runs, so
those paths keep the locale encoding. They carry no wiki content: every
help string in this package is ASCII (held by a test), and rich
substitutes its box-drawing characters once the stream is not UTF-8. What
stays exposed is the name the user typed, echoed back in a usage error --
an unknown command name or an unknown root option name. A name typed
after the command is fine, because click resolves the command, runs this
callback, and only then parses the command's own arguments.
"""
for stream in (sys.stdout, sys.stderr):
reconfigure = getattr(stream, "reconfigure", None)
errors = getattr(stream, "errors", None)
# A stream a test harness or host application substituted may have
# neither, and then decides its own encoding. Both are required:
# errors= must be passed, because reconfigure() silently resets the
# handler to strict otherwise, which would let stderr raise while
# reporting a failure. Passing errors=None does exactly that too.
if reconfigure is not None and errors is not None:
reconfigure(encoding="utf-8", errors=errors)


@app.callback()
def _callback(
ctx: typer.Context,
Expand Down Expand Up @@ -66,6 +102,8 @@ def _callback(
# the prefix is always correct for any message printed on the way out,
# including one printed while handling set_env_file_discovery's error.
config.set_log_prefix("osw")
# Before any output, including the configuration banner.
_force_utf8_output()
# The CLI's working directory is the one the user typed the command in, so
# searching it upward for a .env is what they mean. The MCP server leaves
# this off: its working directory is chosen by the MCP client.
Expand Down
173 changes: 173 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import json
import logging
import re
import sys
from unittest.mock import MagicMock

import click
Expand Down Expand Up @@ -292,6 +293,178 @@ def test_render_dict_shows_key_value_lines():
assert "exists" in rendered


# -- output encoding ------------------------------------------------------------
# Redirected stdout on Windows is opened with the locale encoding, not UTF-8, so
# a German label used to reach the consumer as cp1252 bytes. CliRunner's charset
# gives the captured stream that same encoding, which reproduces the platform
# behaviour everywhere, so these run on Linux CI too.
_MISSING = object() # "do not set this attribute at all", distinct from None


@pytest.fixture
def cp1252_runner():
return CliRunner(mix_stderr=False, charset="cp1252")


def _fake_osw_labelled(monkeypatch, label: str):
"""Patch in an entity whose label slot holds ``label``."""
fake_osw, page = _fake_osw_with_page()
page.get_slot_content.return_value = {"label": [{"text": label}]}
page.get_url.return_value = "https://wiki.example.org/wiki/Item:OSW1"
monkeypatch.setattr("osw.service.context.OswExpress", lambda **kwargs: fake_osw)


def test_json_output_is_utf8_when_stdout_uses_the_locale_encoding(
cp1252_runner, configured_env, monkeypatch
):
_fake_osw_labelled(monkeypatch, "Änderungen")

result = cp1252_runner.invoke(app, ["--json", "entity", "get", "Item:OSW1"])

assert result.exit_code == 0, result.stderr
payload = json.loads(result.stdout_bytes.decode("utf-8"))
assert payload["jsondata"]["label"][0]["text"] == "Änderungen"


def test_human_output_is_utf8_when_stdout_uses_the_locale_encoding(
cp1252_runner, configured_env, monkeypatch
):
_fake_osw_labelled(monkeypatch, "Änderungen")

result = cp1252_runner.invoke(app, ["entity", "get", "Item:OSW1"])

assert result.exit_code == 0, result.stderr
assert "Änderungen" in result.stdout_bytes.decode("utf-8")


def test_error_message_is_utf8_when_stderr_uses_the_locale_encoding(
cp1252_runner, configured_env, monkeypatch
):
"""An error names the page it failed on, so stderr carries labels too."""
fake_osw, _page = _fake_osw_with_page(exists=False)
fake_osw.load_entity.return_value.entities = []
monkeypatch.setattr("osw.service.context.OswExpress", lambda **kwargs: fake_osw)

result = cp1252_runner.invoke(app, ["--json", "entity", "export", "Item:Änderung"])

assert result.exit_code == 2
assert "Item:Änderung" in result.stderr_bytes.decode("utf-8")


def test_forcing_utf8_keeps_the_error_handler_each_stream_was_given(monkeypatch):
"""``reconfigure`` resets ``errors`` to strict unless it is passed as well.

Python gives stderr ``backslashreplace`` precisely so that reporting a
failure cannot itself raise. Switching the encoding must not drop that.
"""
err = io.TextIOWrapper(io.BytesIO(), encoding="cp1252", errors="backslashreplace")
monkeypatch.setattr(sys, "stderr", err)
monkeypatch.setattr(
sys, "stdout", io.TextIOWrapper(io.BytesIO(), encoding="cp1252")
)

cli_main._force_utf8_output()

assert err.encoding == "utf-8"
err.write("\udc80") # a lone surrogate, which "strict" refuses to encode
err.flush()
assert err.buffer.getvalue() == rb"\udc80"


def test_every_help_string_is_ascii():
"""Guards the one gap ``_force_utf8_output`` cannot close.

Click prints help and rejects an unknown name before any callback runs,
so those paths keep the locale encoding. That is only harmless while no
help string contains a character the locale encoding may lack. Adding a
German option description would make it a real defect, and this test is
what reports it.
"""
offenders = []

def walk(command, path):
texts = {"help": command.help, "short_help": command.short_help}
for param in command.params:
texts[f"--{param.name}"] = getattr(param, "help", None)
for where, text in texts.items():
if text and not text.isascii():
offenders.append(f"{' '.join(path) or 'osw'} {where}: {text!r}")
for name, sub in getattr(command, "commands", {}).items():
walk(sub, [*path, name])

walk(typer.main.get_command(app), [])

assert offenders == []


def test_a_substituted_stream_with_no_usable_errors_value_is_left_alone(monkeypatch):
"""Both halves of the guard are needed, not just the ``reconfigure`` half.

A host application may put an object that is not a ``TextIOWrapper`` on
``sys.stdout``. Reading ``.errors`` on one that lacks it raises, which
would end the command. A ``.errors`` of ``None`` is no better: passing it
on means ``strict``, the handler this function exists to preserve.
"""

class Substituted:
def __init__(self, errors):
self.calls = []
if errors is not _MISSING:
self.errors = errors

def reconfigure(self, **kwargs):
self.calls.append(kwargs)

without = Substituted(_MISSING)
none_valued = Substituted(None)
monkeypatch.setattr(sys, "stdout", without)
monkeypatch.setattr(sys, "stderr", none_valued)

cli_main._force_utf8_output()

assert without.calls == []
assert none_valued.calls == []


def test_a_log_handler_holding_stderr_writes_utf8_after_the_switch(monkeypatch):
"""osw logs to ``sys.stderr``, and its handler is built at import time.

``logging.StreamHandler`` stores the stream object it was given, so the
handler osw attaches in ``enable_logging`` holds ``sys.stderr`` itself.
``reconfigure`` changes that object in place rather than replacing it,
which is why an already attached handler writes UTF-8 too. Replacing
``sys.stderr`` with a new object would leave the handler on the old one.
"""
err = io.TextIOWrapper(io.BytesIO(), encoding="cp1252", errors="backslashreplace")
monkeypatch.setattr(sys, "stderr", err)
handler = logging.StreamHandler(sys.stderr) # as osw.enable_logging does
logger = logging.getLogger("test_utf8_handler")
logger.addHandler(handler)
monkeypatch.setattr(
sys, "stdout", io.TextIOWrapper(io.BytesIO(), encoding="cp1252")
)

cli_main._force_utf8_output()
logger.warning("Änderungen")
handler.flush()

assert handler.stream is err
assert "Änderungen" in err.buffer.getvalue().decode("utf-8")


def test_label_the_locale_encoding_cannot_represent_is_written_not_raised(
cp1252_runner, configured_env, monkeypatch
):
"""cp1252 has no Japanese characters, so encoding used to raise, not corrupt."""
_fake_osw_labelled(monkeypatch, "文字")

result = cp1252_runner.invoke(app, ["--json", "entity", "get", "Item:OSW1"])

assert result.exit_code == 0, result.exception or result.stderr
payload = json.loads(result.stdout_bytes.decode("utf-8"))
assert payload["jsondata"]["label"][0]["text"] == "文字"


# -- CLI-only path-taking file commands (osw.cli.ops) ---------------------------
# These are the only operations in the codebase allowed to name a path; they
# are exercised here rather than in tests/test_service_ops_files.py.
Expand Down
Loading