diff --git a/meshterm/tools/config.py b/meshterm/tools/config.py index ae0e5cf..3fba131 100644 --- a/meshterm/tools/config.py +++ b/meshterm/tools/config.py @@ -488,7 +488,9 @@ async def _export_key( # `meshterm config export-key > key.hex` should hold the key and nothing else. The # warning that comes with it belongs beside it on screen, not in the file. ctx.ui.ack("[warn]private key (keep secret):[/warn]") - ctx.ui.note(f"[muted]{key_hex}[/muted]") + # Bare, on both faces: this line *is* the export, and `config export-key` with no + # `--out` is expected to be redirected into a file. + ctx.ui.note(key_hex) async def _read_channels(device: Device) -> list[dict]: diff --git a/meshterm/ui/script.py b/meshterm/ui/script.py index e822d56..5ada9f1 100644 --- a/meshterm/ui/script.py +++ b/meshterm/ui/script.py @@ -72,16 +72,36 @@ GUTTER = 2 NONE = "-" -#: The characters that would end a record early, and the escape each one takes. A record -#: is a line, so a value carrying a newline is not a value this format can hold — and the -#: fields most likely to carry one (a node name, a message body) are exactly the fields a -#: stranger fills in. The convention is the ordinary backslash one, so it reads back. +#: The characters with a readable escape of their own. Everything else that cannot travel +#: in a record falls through to ``\xNN``/``\uNNNN`` below. _BREAKS = {"\\": "\\\\", "\n": "\\n", "\r": "\\r", "\t": "\\t"} +#: Codepoints that must never reach stdout unescaped, as a *category* rather than a list: +#: every C0 and C1 control, plus the two Unicode separators a terminal also breaks a line +#: on. A four-entry table caught the newline and the tab and let ESC through — and an ESC +#: inside a node name is a live colour run written into the caller's file, which is the +#: one thing "no colour, not a single escape sequence" exists to prevent. A BEL was worse: +#: silently dropped, so the printed name was not the advertised one. +_UNPRINTABLE = frozenset([*range(0x00, 0x20), 0x7F, *range(0x80, 0xA0), 0x2028, 0x2029]) + def _escaped(text: str) -> str: - """``text`` with its backslashes doubled and its line breaks folded into escapes.""" - return "".join(_BREAKS.get(ch, ch) for ch in text) + r"""``text`` with everything that cannot travel inside one record folded into an escape. + + A record is a line and a field is a run of printable cells; this is what makes both + true of a value that arrived over the air. Named escapes where one reads (``\n``), + numeric escapes everywhere else, and the backslash doubled so the whole thing reads + back unambiguously. + """ + out: list[str] = [] + for ch in text: + if ch in _BREAKS: + out.append(_BREAKS[ch]) + elif ord(ch) in _UNPRINTABLE: + out.append(f"\\x{ord(ch):02x}" if ord(ch) < 0x100 else f"\\u{ord(ch):04x}") + else: + out.append(ch) + return "".join(out) def quote(value: str | None) -> str: diff --git a/meshterm/ui/surface.py b/meshterm/ui/surface.py index 5c68349..a26bc10 100644 --- a/meshterm/ui/surface.py +++ b/meshterm/ui/surface.py @@ -328,8 +328,26 @@ class PlainUi(Ui): self.console.print(item) def note(self, markup: str) -> None: - """Print a markup line to the console immediately.""" - self.console.print(markup) + """Print a markup line to the console immediately, as plain text. + + The parameter is markup — that is the shared surface's contract, and forty-odd + callers write it — but the scripted console does not interpret markup: a node + broadcasts its own name, and Rich would read ``[...]`` in one as a style tag. So + the tags are resolved *here* rather than by the console, and what reaches stdout + is the text they were wrapping. Printing them raw put ``[muted]`` and ``[/muted]`` + around the private key that ``config export-key > key.hex`` is supposed to be the + whole content of. + + Malformed markup — which is what a *name* holding a bracket looks like — is not an + error to report but a string to print, so it falls through verbatim. + """ + from rich.errors import MarkupError + from rich.markup import render + + try: + self.console.print(render(markup).plain) + except MarkupError: + self.console.print(markup) def ack(self, markup: str) -> None: """Drop it: on the command line the exit status is the acknowledgement.""" diff --git a/tests/conftest.py b/tests/conftest.py index 5fbb391..2f5c6c6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,6 +18,7 @@ always see the colours the app really emits. from __future__ import annotations +import os import re from collections.abc import Callable, Iterable, Iterator @@ -87,6 +88,33 @@ def _reset_transmit_gate() -> Iterator[None]: transmit_clock().reset() +@pytest.fixture(autouse=True) +def _isolate_config_dir(tmp_path_factory: pytest.TempPathFactory) -> Iterator[None]: + """No test may reach the developer's own ``~/.meshterm``. + + ``$MESHTERM_HOME`` moves the whole directory — the history, the contact and channel + caches, the outbox, the stored admin passwords, the device profiles, the preferences + and the log. ``--db`` moves only the first of those, which is why isolating a CLI test + with ``--db`` alone was not isolation at all: a synthetic contact written by a probe + sat in the real cache and made ``test_cli_contract`` fail on one machine and pass on + every other, and every run appended to the developer's real ``meshterm.log`` and read + their real ``log_level``. + + Autouse and unconditional, because the exposure is not the CLI's: anything that + touches ``Settings.load()``, a preference, a store or the logger has it. A test that + wants a *populated* directory sets the variable itself; ``monkeypatch.setenv`` inside + a test still wins over this. + """ + home = tmp_path_factory.mktemp("meshterm-home") + previous = os.environ.get("MESHTERM_HOME") + os.environ["MESHTERM_HOME"] = str(home) + yield + if previous is None: + os.environ.pop("MESHTERM_HOME", None) + else: + os.environ["MESHTERM_HOME"] = previous + + @pytest.fixture(autouse=True) def _reset_platform() -> Iterator[None]: """Every test starts and ends on :data:`~meshterm.platforms.REGULAR`. diff --git a/tests/test_cli_contract.py b/tests/test_cli_contract.py index a57f7c0..66dfac4 100644 --- a/tests/test_cli_contract.py +++ b/tests/test_cli_contract.py @@ -21,6 +21,11 @@ from meshterm.core import exitcodes #: Any ANSI escape sequence. Nothing on the CLI's stdout may match this. _ANSI = re.compile(r"\x1b\[") +#: A Rich console-markup tag. The scripted console no longer *interprets* markup — a node +#: name is remote data and `[bold]` in one is a name, not a style — which means a tag +#: MeshTerm writes itself is no longer consumed either, and lands in the caller's pipe. +_MARKUP = re.compile(r"\[/?(?:ok|err|warn|muted|accent|brand|bold|dim|reverse)\b[^\]]*\]") + #: The shape of a scripted path line: one or more quoted node names, each optionally #: followed by its hash in parentheses *outside* the quotes, joined by a bare comma. This #: is the whole grammar — a line matching it splits, whatever the names hold. @@ -40,6 +45,35 @@ _LISTINGS: list[tuple[str, list[str], list[str]]] = [ ] +def _leaf_commands() -> list[tuple[str, ...]]: + """Every registered subcommand path, walked out of the real Typer app. + + Derived rather than listed: a hand-kept roster covers whatever was reworked the day it + was written, and a command added afterwards inherits none of it. Two rule violations + shipped inside commands the roster did not name — a 16384-cell rule in ``about`` and + markup tags in ``config export-key``. + """ + from typer.main import get_command + + from meshterm.cli import app + + def walk(command, prefix: tuple[str, ...] = ()) -> list[tuple[str, ...]]: # noqa: ANN001 + found: list[tuple[str, ...]] = [] + for name, sub in sorted(getattr(command, "commands", {}).items()): + here = (*prefix, name) + found.extend(walk(sub, here) if getattr(sub, "commands", None) else [here]) + return found + + return walk(get_command(app)) + + +#: Commands that own the terminal or run until interrupted: they have no one-shot output. +_NOT_ONE_SHOT = {("monitor",), ("chat", "listen"), ("tx-optimize",)} + +#: The one command whose output *is* its colour, and says so (:mod:`meshterm.ui.specimen`). +_KEEPS_ITS_COLOUR = {("specimen",)} + + @pytest.fixture() def run(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): # noqa: ANN201 - a closure """Invoke the real CLI against the simulator, in a config directory of its own. @@ -71,7 +105,7 @@ def test_a_command_prints_no_escape_sequence(run, command, args, fields) -> None """Nothing is coloured: stdout is a pipe more often than a terminal.""" result = run(command, *args) assert result.exit_code == exitcodes.OK, result.output - assert not _ANSI.search(result.output) + assert not _ANSI.search(result.stdout) @pytest.mark.parametrize("command,args,fields", _LISTINGS, ids=[c for c, _, _ in _LISTINGS]) @@ -79,14 +113,14 @@ def test_a_command_carries_the_fields_a_caller_looks_for(run, command, args, fie """The headings and keys are the contract; renaming one silently breaks a script.""" result = run(command, *args) for field in fields: - assert field in result.output, f"{command} lost {field}" + assert field in result.stdout, f"{command} lost {field}" @pytest.mark.parametrize("command,args,fields", _LISTINGS, ids=[c for c, _, _ in _LISTINGS]) def test_a_command_draws_no_frame_and_no_rule(run, command, args, fields) -> None: # noqa: ANN001 """No borders, no boxes, no header rules — the command typed is the title.""" result = run(command, *args) - assert not set(result.output) & set("─│┌┐└┘├┤━┃") + assert not set(result.stdout) & set("─│┌┐└┘├┤━┃") @pytest.mark.parametrize("command,args,fields", _LISTINGS, ids=[c for c, _, _ in _LISTINGS]) @@ -97,8 +131,47 @@ def test_a_command_leaves_no_trailing_whitespace(run, command, args, fields) -> def result_lines(result) -> list[str]: # noqa: ANN001 - """The command's stdout as lines, with the trailing blank dropped.""" - return result.output.splitlines() + """The command's stdout as lines, with the trailing blank dropped. + + ``result.output`` is a third buffer that *both* streams copy into, so a rule about + stdout asserted against it passes when the field it wants only ever appeared on + stderr — and fails for the wrong reason the moment anything logs. + """ + return result.stdout.splitlines() + + +@pytest.mark.parametrize( + "leaf", + [c for c in _leaf_commands() if c not in _NOT_ONE_SHOT | _KEEPS_ITS_COLOUR], + ids=lambda leaf: "-".join(leaf), +) +def test_every_registered_command_obeys_the_rules(run, leaf) -> None: # noqa: ANN001 + """The rules belong to the CLI, not to the six commands somebody listed by hand. + + Run bare, a command that needs arguments fails harmlessly as a usage error and still + proves it wrote nothing awkward on the way out; a command that can answer without + arguments is exercised for real. That is where both shipped violations were — the + ``---`` in ``about`` drawn as a rule the width of a 16384-cell console, and the + ``[muted]`` tags around what ``config export-key`` exists to emit. + """ + out = run(*leaf).stdout + assert not _ANSI.search(out), f"{' '.join(leaf)} put an escape sequence on stdout" + assert not set(out) & set("─│┌┐└┘├┤━┃╭╮╰╯"), f"{' '.join(leaf)} drew a frame or a rule" + assert not _MARKUP.search(out), f"{' '.join(leaf)} printed a console markup tag" + for line in out.splitlines(): + assert line == line.rstrip(), f"{' '.join(leaf)} padded a line: {line!r}" + + +def test_the_sweep_actually_covers_the_whole_command_surface() -> None: + """A derived list is only a guarantee while it is still finding the commands. + + If the walk breaks — a Typer upgrade moving ``commands``, say — every parametrised + case silently becomes zero cases and the sweep passes by covering nothing. + """ + leaves = _leaf_commands() + assert len(leaves) > 40, f"only found {len(leaves)} commands: {leaves}" + for expected in [("about",), ("config", "export-key"), ("contacts",), ("trace",)]: + assert expected in leaves, expected # -- listings ------------------------------------------------------------------------- diff --git a/tests/test_script_output.py b/tests/test_script_output.py index 55c8b59..ca97b5d 100644 --- a/tests/test_script_output.py +++ b/tests/test_script_output.py @@ -239,7 +239,10 @@ def test_the_gutter_still_separates_columns_after_the_trimming() -> None: header, record = _rendered(table).splitlines() assert header.split() == ["NAME", "TYPE", "PKTS"] assert record.split() == ['"Alice"', "node", "12"] - assert " " * script.GUTTER in record + # Spelled out, not written as `" " * script.GUTTER`: an assertion that reads the + # constant it is checking narrows to `" " in record` when the gutter narrows, which + # is exactly the state the gutter exists to prevent. + assert '"Alice" node' in record def test_columns_align_so_every_record_splits_the_same_way() -> None: @@ -282,6 +285,50 @@ def test_no_column_elides_however_long_its_value_runs() -> None: assert "d4" * 32 in out +def test_the_trimmer_keeps_the_gutter_when_the_stream_flushes_between_columns() -> None: + """Rich's legacy-Windows renderer writes a row one segment at a time, flushing each. + + ``LegacyWindowsTerm.write_text`` is ``write(text); flush()``, so on that path a flush + lands *between two columns* as often as at the end of a line. A wrapper that emptied + its held run at flush destroyed every gutter in real use while the in-process tests — + which see one write and one flush per ``print`` — went on passing. That is not a + hypothetical: it shipped, and this drives the stream the way that renderer does. + """ + buffer = io.StringIO() + stream = script._Trimmed(buffer) + for segment in ('"Alice"', " ", "node", " ", "12", "\n"): + stream.write(segment) + stream.flush() + assert buffer.getvalue() == '"Alice" node 12\n' + + +def test_the_trimmer_still_drops_the_run_that_ends_a_line() -> None: + """The other half of the rule: holding padding back is only right if it is still cut. + + Held whitespace is emitted when something follows it on the line, and dropped at the + newline and at the end of the stream — otherwise every record would end in the + invisible padding of its last column. + """ + buffer = io.StringIO() + stream = script._Trimmed(buffer) + for segment in ("row", " ", "\n", "next", " "): + stream.write(segment) + stream.flush() + assert buffer.getvalue() == "row\nnext" + + +def test_a_value_wider_than_the_console_is_cut_rather_than_elided() -> None: + """Past :data:`~meshterm.ui.script.WIDTH` cells something gives; it must not be a "…". + + An elision is indistinguishable from a value that really ends there, so a key read out + of a listing would go back into ``--to`` subtly wrong. A 64-hex key never reaches the + overflow path at all, so only an over-width value tests the guard that stands there. + """ + table = script.columns("KEY") + table.add_row("d" * (script.WIDTH + 500)) + assert "…" not in _rendered(table) + + # -- framing -------------------------------------------------------------------------- @@ -337,26 +384,21 @@ def test_a_pairs_value_is_unquoted_because_it_is_the_rest_of_the_line() -> None: # -- exit statuses -------------------------------------------------------------------- -def test_every_exit_code_is_documented() -> None: - """A documented return value is half of what makes the CLI scriptable.""" - codes = { - exitcodes.OK, - exitcodes.FAILURE, - exitcodes.USAGE, - exitcodes.NO_DEVICE, - exitcodes.DEVICE, - exitcodes.NO_RESULT, - } - assert set(exitcodes.MEANINGS) == codes +def test_the_exit_statuses_are_the_numbers_a_caller_hard_codes() -> None: + """``case $? in 3)`` is written against the digits, so the digits are the contract. + + Stated as literals on purpose. Asserting ``set(MEANINGS) == {OK, FAILURE, …}``, or + ``len(set(d)) == len(d)``, cannot fail however the constants are renumbered: a dict + has no duplicate keys, so two statuses colliding *removes* a row and both sides of + the comparison lose the same one together. Renumbering ``NO_DEVICE`` to ``4`` left + that whole family of assertions green while status 3 vanished from ``--help``. + """ + assert (exitcodes.OK, exitcodes.FAILURE, exitcodes.USAGE) == (0, 1, 2) + assert (exitcodes.NO_DEVICE, exitcodes.DEVICE, exitcodes.NO_RESULT) == (3, 4, 5) + assert sorted(exitcodes.MEANINGS) == [0, 1, 2, 3, 4, 5] assert all(meaning for meaning in exitcodes.MEANINGS.values()) -def test_the_codes_are_distinct_and_ok_is_zero() -> None: - """The shell's own convention, and no two outcomes share a status.""" - assert exitcodes.OK == 0 - assert len(set(exitcodes.MEANINGS)) == len(exitcodes.MEANINGS) - - def test_the_help_epilog_names_every_status() -> None: """A caller should not have to find the table in a README.""" from meshterm.cli import EXIT_STATUS_EPILOG