From d499f61f9fe40a10da8c20c2fafbed5eec9c7a62 Mon Sep 17 00:00:00 2001 From: Cole Gentry Date: Mon, 4 May 2026 21:42:35 -0400 Subject: [PATCH 1/5] Address PR #10 review feedback: data URIs, errors, source_path auto-fill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five gemini-code-assist comments on https://github.com/ClassicMiniDIY/WireViz/pull/10: * Data URI leading-space stripped in three places (svgembed.py data_URI_base64 and embed_svg_images, plus Harness._render's PNG b64 inline). The prior ``data:image/png;base64, `` form had a stray space after the comma which violates RFC 2397; most browsers tolerate it but stricter parsers (some PDF/EPUB tooling, lint tools) reject it. All three call sites now emit the canonical ``data:image/png;base64,``. * wv_cli.py: ``raise Exception("Unknown output format: ...")`` → ``raise click.UsageError(...)``. Matches the multi-format-stdout check elsewhere in the file and gives users the clean ``Try 'wireviz -h' for help.\nError: ...`` UX instead of a Python traceback. * wv_cli.py: stdin mode now defaults ``image_paths`` to ``{Path.cwd()}`` instead of an empty set, so relative ``image: src:`` references in piped YAML resolve against the invocation directory the same way file-based input resolves against the YAML's parent. * Harness._extend_tweak: tweak override conflict error now reports both the existing and new values (``"X1.tweak.override.X1.color: new value 'blue' conflicts with existing 'red'"``) instead of the prior vague ``"conflicts with another"``. * wireviz.parse(): ``source_path`` now auto-fills from ``yaml_file`` when the input was a Path. The docstring already promised this behavior; the code didn't actually do it. Confirmed via ``parse(Path('examples/demo01.yml'), output_formats=('svg',), ...)`` succeeding without explicit source_path. Verified against build_examples.py: deterministic outputs (.gv, .bom.tsv) byte-identical to baseline. Tweak conflict YAML correctly raises the new error. Bad CLI format flag now renders Click's UsageError formatting. Data URIs in re-rendered ex08.svg / .html now emit ``data:image/png;base64,iVB...`` (no space). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/wireviz/Harness.py | 5 +++-- src/wireviz/svgembed.py | 4 ++-- src/wireviz/wireviz.py | 4 ++++ src/wireviz/wv_cli.py | 8 ++++++-- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/wireviz/Harness.py b/src/wireviz/Harness.py index 96ea37aa..751960b9 100644 --- a/src/wireviz/Harness.py +++ b/src/wireviz/Harness.py @@ -172,7 +172,8 @@ def _extend_tweak(self, node: Union[Connector, Cable]) -> None: k, v = rph(k), rph(v) if k in s_dict and v != s_dict[k]: raise ValueError( - f"{node.name}.tweak.override.{ident}.{k} conflicts with another" + f"{node.name}.tweak.override.{ident}.{k}: new value " + f"{v!r} conflicts with existing {s_dict[k]!r}" ) s_dict[k] = v # Keep the empty dict rather than collapsing to None — the @@ -935,7 +936,7 @@ def _render( # rendered in this same call; otherwise let the template # fall back to reading {output_dir}/{output_name}.png. png_b64 = ( - f"data:image/png;base64, {base64.b64encode(png_bytes).decode('utf-8')}" + f"data:image/png;base64,{base64.b64encode(png_bytes).decode('utf-8')}" if png_bytes is not None else None ) diff --git a/src/wireviz/svgembed.py b/src/wireviz/svgembed.py index 3e33f631..37c98b4e 100644 --- a/src/wireviz/svgembed.py +++ b/src/wireviz/svgembed.py @@ -13,7 +13,7 @@ def data_URI_base64(file: Union[str, Path], media: str = "image") -> str: """Return Base64-encoded data URI of input file.""" file = Path(file) b64 = base64.b64encode(file.read_bytes()).decode("utf-8") - uri = f"data:{media}/{get_mime_subtype(file)};base64, {b64}" + uri = f"data:{media}/{get_mime_subtype(file)};base64,{b64}" # print(f"data_URI_base64('{file}', '{media}') -> {len(uri)}-character URI") if len(uri) > 65535: print( @@ -36,7 +36,7 @@ def replace(match: re.Match) -> str: images_b64[imgurl] = base64.b64encode(image).decode("utf-8") return image_tag( match["PRE"] or "", - f"data:image/{get_mime_subtype(imgurl)};base64, {images_b64[imgurl]}", + f"data:image/{get_mime_subtype(imgurl)};base64,{images_b64[imgurl]}", match["POST"] or "", ) diff --git a/src/wireviz/wireviz.py b/src/wireviz/wireviz.py index 82a55aec..4a300275 100755 --- a/src/wireviz/wireviz.py +++ b/src/wireviz/wireviz.py @@ -112,6 +112,10 @@ def parse( raise TypeError( f"Expected a dict as top-level YAML input, but got: {type(yaml_data)}" ) + # When inp was a Path, derive source_path automatically so callers + # don't have to pass it twice. Matches the docstring contract. + if source_path is None and yaml_file is not None: + source_path = yaml_file write_to_stdout = ( output_formats and (str(output_dir) == "-" or str(output_name) == "-") ) diff --git a/src/wireviz/wv_cli.py b/src/wireviz/wv_cli.py index ebbca72c..f3871089 100644 --- a/src/wireviz/wv_cli.py +++ b/src/wireviz/wv_cli.py @@ -116,7 +116,7 @@ def wireviz( if fmt not in output_formats: output_formats.append(fmt) else: - raise Exception(f"Unknown output format: {code}") + raise click.UsageError(f"Unknown output format: {code}") output_formats = tuple(output_formats) output_formats_str = ( f'[{"|".join(output_formats)}]' @@ -150,7 +150,11 @@ def wireviz( for file in filepaths: if str(file) == "-": yaml_input = prepend_input + sys.stdin.read() - image_paths = set() + # No source-file directory available, so any relative + # `image: src:` paths in the stdin YAML are resolved against + # the current working directory (matching how a typical + # piped invocation would be run from a project root). + image_paths = {Path.cwd()} sys.stderr.write("Input: \n") _output_dir = output_dir if output_dir else "-" _output_name = output_name if output_name else "stdin" From 25b426c9357195306a990731b99f1bf2fd242c27 Mon Sep 17 00:00:00 2001 From: Cole Gentry Date: Mon, 4 May 2026 22:16:56 -0400 Subject: [PATCH 2/5] Add comprehensive pytest suite + fix three real bugs surfaced by it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds 134 pytest tests organized into nine files under ``tests/``, covering the entire WireViz API surface from parse() through CLI to BOM and color logic. Every upstream PR we ported and every gemini review fix gets a pinned regression test in tests/test_regressions.py so future refactors can't silently undo the work. ## Test files (134 tests total) * tests/test_smoke.py (10) — every output format renders * tests/test_parse.py (20) — wireviz.parse() API surface * tests/test_cli.py (19) — CLI flags via Click's CliRunner * tests/test_harness.py (11) — Harness public methods + render * tests/test_dataclasses.py (19) — DataClasses coercion + validation * tests/test_colors.py (12) — color schemes + hex + thickness * tests/test_bom.py (8) — BOM aggregation * tests/test_regressions.py (29) — one per ported PR + review fix * tests/test_round_trip.py (6) — PNG embed + stdin/stdout ## Real bugs surfaced and fixed 1. **wv_html.py:72** — ``re.sub(..., 1)`` triggered Python 3.13+'s "positional 'count' is deprecated" DeprecationWarning. Switched to ``count=1``. Would have started failing CI on Python 3.13 once the warning becomes an error. 2. **wv_cli.py "File does not exist"** — was a bare ``raise Exception(...)`` that produced a Python traceback for users who pointed the CLI at a missing input or prepend file. Promoted both sites to ``click.UsageError`` so users see the canonical "Try 'wireviz -h' for help" footer instead. 3. **wireviz.py:_get_yaml_data_and_path** — when called with a dict input, parse() would mutate the caller's dict in place during connection-set expansion (``[[{X1: [1,2]}, ...]]`` got rewritten to ``[[{X1: 1}, {X1: 2}, ...]``). Added ``copy.deepcopy(inp)`` so programmatic callers (especially the planned wireviz-gui that will keep an in-memory model) get clean read-only semantics. A regression test exists in tests/test_round_trip.py (``test_dict_input_not_mutated``) so this can't quietly come back. ## Infrastructure * pyproject.toml gains ``[tool.pytest.ini_options]`` with ``testpaths = ["tests"]`` and ``filterwarnings = ["error", "ignore::SyntaxWarning"]`` so pytest catches future deprecation drift. * tests/conftest.py centralizes fixtures pointing at small YAMLs in tests/fixtures/ — deliberately distinct from the gallery YAMLs in examples/ so tests don't break when the gallery shifts. * New .github/workflows/tests.yml runs pytest across the same Python 3.7-3.12 matrix as the existing Create Examples workflow. ## Also folded in: cherry-pick of e4f485e (PR #10 review fixes) The PR #10 review-fix commit (data URIs without leading space, click .UsageError on unknown -f code, image_paths default for stdin, better tweak conflict error message, source_path auto-fill in parse()) was on port-321-stdin-stdout but never reached master because PR #10 merged before that commit was pushed. Cherry-picked here so the test suite can validate the fixed behavior. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/tests.yml | 35 ++ CLAUDE.md | 37 +- pyproject.toml | 13 + src/wireviz/wireviz.py | 8 +- src/wireviz/wv_cli.py | 6 +- src/wireviz/wv_html.py | 2 +- tests/__init__.py | 0 tests/conftest.py | 97 +++++ tests/fixtures/custom_template.yml | 18 + tests/fixtures/dpi_192.yml | 17 + tests/fixtures/hex_color.yml | 16 + tests/fixtures/loopback.yml | 11 + tests/fixtures/loopback_template.yml | 21 ++ tests/fixtures/minimal.yml | 14 + tests/fixtures/per_node_tweak.yml | 23 ++ tests/fixtures/revisions.yml | 26 ++ tests/fixtures/templates/branded.html | 9 + tests/test_bom.py | 190 ++++++++++ tests/test_cli.py | 272 ++++++++++++++ tests/test_colors.py | 104 ++++++ tests/test_dataclasses.py | 181 ++++++++++ tests/test_harness.py | 146 ++++++++ tests/test_parse.py | 259 ++++++++++++++ tests/test_regressions.py | 490 ++++++++++++++++++++++++++ tests/test_round_trip.py | 131 +++++++ tests/test_smoke.py | 106 ++++++ 26 files changed, 2224 insertions(+), 8 deletions(-) create mode 100644 .github/workflows/tests.yml create mode 100644 tests/__init__.py create mode 100644 tests/conftest.py create mode 100644 tests/fixtures/custom_template.yml create mode 100644 tests/fixtures/dpi_192.yml create mode 100644 tests/fixtures/hex_color.yml create mode 100644 tests/fixtures/loopback.yml create mode 100644 tests/fixtures/loopback_template.yml create mode 100644 tests/fixtures/minimal.yml create mode 100644 tests/fixtures/per_node_tweak.yml create mode 100644 tests/fixtures/revisions.yml create mode 100644 tests/fixtures/templates/branded.html create mode 100644 tests/test_bom.py create mode 100644 tests/test_cli.py create mode 100644 tests/test_colors.py create mode 100644 tests/test_dataclasses.py create mode 100644 tests/test_harness.py create mode 100644 tests/test_parse.py create mode 100644 tests/test_regressions.py create mode 100644 tests/test_round_trip.py create mode 100644 tests/test_smoke.py diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 00000000..11442378 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,35 @@ +name: Tests + +on: [push, pull_request, workflow_dispatch] + +jobs: + pytest: + strategy: + max-parallel: 6 + matrix: + # Same Python matrix shape as the Create Examples workflow so + # we get coverage from the oldest officially supported runtime + # (3.7) up through the latest released (3.12). + os: [ubuntu-latest] + python-version: ["3.9", "3.10", "3.11", "3.12"] + include: + - os: ubuntu-22.04 + python-version: "3.7" + - os: ubuntu-22.04 + python-version: "3.8" + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v4 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + - name: Setup Graphviz + uses: ts-graphviz/setup-graphviz@v2 + - name: Install package + test dependencies + run: | + python -m pip install --upgrade pip + pip install . + pip install pytest + - name: Run pytest + run: pytest -v diff --git a/CLAUDE.md b/CLAUDE.md index a35b20f7..c3edd06d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,18 +10,33 @@ This repo is the **upstream Python CLI**. A separate GUI front-end is being buil ## Commands -WireViz has **no automated test suite**. The de-facto regression check is rebuilding the examples and diffing the output. +WireViz has both an **automated pytest suite** (`tests/`) and a separate **example-rebuild regression check** (`build_examples.py`). They serve complementary purposes — pytest validates the API and CLI surface; the example sweep validates rendered visual output across the gallery. ```bash # Install for development (from repo root) pip install -e . +pip install pytest + +# Run the unit / integration test suite (~134 tests, ~5s) +pytest + +# Run a single test file or test +pytest tests/test_regressions.py +pytest tests/test_cli.py::test_cli_template_dir # Run the CLI on a YAML file (produces .gv .svg .png .html .bom.tsv next to input) wireviz path/to/file.yml -# Limit output formats: g=gv h=html p=png s=svg t=tsv +# Limit output formats: g=gv h=html p=png s=svg t=tsv P=pdf wireviz -f hps path/to/file.yml +# Stdin → stdout: pipe YAML in, get one rendered format out +cat harness.yml | wireviz -f s -O - - > harness.svg +cat harness.yml | wireviz -f p -O - - > harness.png + +# .png input: extract the embedded YAML and re-render +wireviz harness.png + # Rebuild every demo, example, and tutorial (must cd into src/wireviz) cd src/wireviz && python build_examples.py @@ -40,7 +55,23 @@ cd src/wireviz && python build_examples.py compare -g examples tutorial demos GraphViz must be installed as a system dep (`dot -V`). Code is formatted with `black` + `isort` (`isort` profile is `black`, configured in `pyproject.toml`). -CI (`.github/workflows/`) runs only `build_examples.py` across Python 3.7–3.12 — there is no `pytest`. If you add real tests, also wire them into CI. +CI (`.github/workflows/`) runs both `pytest` (the `Tests` workflow) and `build_examples.py` (the `Create Examples` workflow) across Python 3.7–3.12. Both must pass for a PR to be considered green. + +## Test suite layout (`tests/`) + +- **`tests/test_smoke.py`** — every output format renders without error, has the right magic bytes, and produces the expected basic structure. +- **`tests/test_parse.py`** — the `wireviz.parse()` library API: input shapes (Path / str / dict), output shapes, return_types, source_path auto-fill, embed_yaml flag. +- **`tests/test_cli.py`** — every CLI flag via Click's `CliRunner`. Covers stdin/stdout, `.png` input round-trip, `--no-embed-yaml`, `--template-dir`, `--prepend`, error paths. +- **`tests/test_harness.py`** — `Harness` public methods, the `_render` dict shape contract, file vs stdout dispatch. +- **`tests/test_dataclasses.py`** — `Connector` / `Cable` / `Tweak` / `Options` / `Image` coercion and validation logic. +- **`tests/test_colors.py`** — color schemes (DIN/IEC/T568/TEL), hex parsing, `get_color_hex` padding behavior. +- **`tests/test_bom.py`** — BOM aggregation: identical-component dedup, ignore_in_bom, additional_bom_items, bundle category, part-number columns. +- **`tests/test_regressions.py`** — **one test per upstream-PR port + every gemini review fix.** This is where every bug we ported a fix for gets pinned down so it can never silently regress. If you change behavior touched by any of those PRs, expect tests here to fail and update them deliberately. +- **`tests/test_round_trip.py`** — PNG embed/extract, stdin→stdout pipelines, dict-input no-mutation contract. + +`tests/conftest.py` provides shared fixtures (paths to small targeted YAMLs in `tests/fixtures/`). The fixture YAMLs are deliberately separate from the gallery YAMLs in `examples/` so tests aren't coupled to visual gallery changes. + +`pyproject.toml` configures pytest to treat unexpected warnings as errors (`filterwarnings = ["error", "ignore::SyntaxWarning"]`) so deprecations like the `re.sub(..., 1)` → `re.sub(..., count=1)` migration get caught early. ## Architecture diff --git a/pyproject.toml b/pyproject.toml index 5d7bf33d..f55d39d6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,2 +1,15 @@ [tool.isort] profile = "black" + +[tool.pytest.ini_options] +testpaths = ["tests"] +python_files = ["test_*.py"] +python_classes = ["Test*"] +python_functions = ["test_*"] +# Treat unexpected warnings as errors so dataclass / deprecation drift is caught +filterwarnings = [ + "error", + # graphviz emits a SyntaxWarning on Python 3.12+ for an internal regex — + # not our problem and not worth blocking the suite on + "ignore::SyntaxWarning", +] diff --git a/src/wireviz/wireviz.py b/src/wireviz/wireviz.py index 4a300275..c383a081 100755 --- a/src/wireviz/wireviz.py +++ b/src/wireviz/wireviz.py @@ -510,8 +510,12 @@ def _get_yaml_data_and_path( yaml_data = yaml.safe_load(yaml_str) else: # received a Dict — serialize back to YAML so the caller has a - # text form for round-trip embedding into PNG output. - yaml_data = inp + # text form for round-trip embedding into PNG output, and + # deep-copy so the parsing pipeline's in-place expansion of + # the connections section doesn't leak back to the caller. + import copy + + yaml_data = copy.deepcopy(inp) yaml_path = None yaml_str = yaml.safe_dump(inp, sort_keys=False, allow_unicode=True) return yaml_data, yaml_path, yaml_str diff --git a/src/wireviz/wv_cli.py b/src/wireviz/wv_cli.py index f3871089..7ed949c5 100644 --- a/src/wireviz/wv_cli.py +++ b/src/wireviz/wv_cli.py @@ -136,7 +136,9 @@ def wireviz( for prepend_file in prepend: prepend_file = Path(prepend_file) if not prepend_file.exists(): - raise Exception(f"File does not exist:\n{prepend_file}") + raise click.UsageError( + f"Prepend file does not exist: {prepend_file}" + ) sys.stderr.write(f"Prepend file: {prepend_file}\n") prepend_input += file_read_text(prepend_file) + "\n" @@ -161,7 +163,7 @@ def wireviz( else: file = Path(file) if not file.exists(): - raise Exception(f"File does not exist:\n{file}") + raise click.UsageError(f"Input file does not exist: {file}") if file.suffix.lower() == ".png": # PNG input: try to recover the YAML embedded by an diff --git a/src/wireviz/wv_html.py b/src/wireviz/wv_html.py index 2393317d..71230f43 100644 --- a/src/wireviz/wv_html.py +++ b/src/wireviz/wv_html.py @@ -73,7 +73,7 @@ def svgdata() -> str: "^<[?]xml [^?>]*[?]>[^<]*]*>", "", svg_input or "", - 1, + count=1, ) # generate BOM table diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 00000000..02509a0d --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,97 @@ +# -*- coding: utf-8 -*- +"""Shared pytest fixtures for the WireViz test suite. + +Most fixtures yield Path objects pointing at small targeted YAML files +under ``tests/fixtures/``. These fixtures are intentionally separate +from the larger gallery YAMLs in ``examples/`` so test failures are +about the *thing being tested*, not about an unrelated example feature +shifting underneath us. +""" + +import os +import sys +from pathlib import Path + +import pytest + +# Ensure the in-tree src/wireviz package is importable when running +# pytest from the repo root without a prior `pip install -e .`. +SRC = Path(__file__).resolve().parent.parent / "src" +if str(SRC) not in sys.path: + sys.path.insert(0, str(SRC)) + + +FIXTURES = Path(__file__).resolve().parent / "fixtures" + + +@pytest.fixture +def fixtures_dir() -> Path: + return FIXTURES + + +@pytest.fixture +def minimal_yaml(fixtures_dir: Path) -> Path: + """Two-pin connector + 2-wire cable. The smallest renderable harness.""" + return fixtures_dir / "minimal.yml" + + +@pytest.fixture +def loopback_yaml(fixtures_dir: Path) -> Path: + """Single connector with a ``loops:`` entry, no cables — exercises + the auto-instantiation path added by upstream PR #496.""" + return fixtures_dir / "loopback.yml" + + +@pytest.fixture +def loopback_template_yaml(fixtures_dir: Path) -> Path: + """Connector template with ``loops:`` used via Template.Designator + syntax — exercises the auto-instantiation skip added by the + review fix on PR #1.""" + return fixtures_dir / "loopback_template.yml" + + +@pytest.fixture +def revisions_yaml(fixtures_dir: Path) -> Path: + """metadata.revisions with three entries — exercises the + %revision% placeholder.""" + return fixtures_dir / "revisions.yml" + + +@pytest.fixture +def per_node_tweak_yaml(fixtures_dir: Path) -> Path: + """Per-connector / per-cable tweak with a ``@@`` placeholder.""" + return fixtures_dir / "per_node_tweak.yml" + + +@pytest.fixture +def hex_color_yaml(fixtures_dir: Path) -> Path: + """Cable with a single hex-RGB wire color — exercises the wire- + thickness padding regression fix from upstream PR #495.""" + return fixtures_dir / "hex_color.yml" + + +@pytest.fixture +def custom_template_yaml(fixtures_dir: Path) -> Path: + """Harness referencing a custom HTML template by name.""" + return fixtures_dir / "custom_template.yml" + + +@pytest.fixture +def custom_template_dir(fixtures_dir: Path) -> Path: + """Directory holding the ``branded.html`` template referenced by + ``custom_template.yml``.""" + return fixtures_dir / "templates" + + +@pytest.fixture +def dpi_192_yaml(fixtures_dir: Path) -> Path: + """``options.output_dpi: 192`` — twice the graphviz default.""" + return fixtures_dir / "dpi_192.yml" + + +@pytest.fixture +def workdir(tmp_path: Path, monkeypatch) -> Path: + """Like ``tmp_path`` but also chdir's into it so tests that exercise + relative-path behavior can use the temp dir as cwd.""" + monkeypatch.chdir(tmp_path) + return tmp_path diff --git a/tests/fixtures/custom_template.yml b/tests/fixtures/custom_template.yml new file mode 100644 index 00000000..203c4297 --- /dev/null +++ b/tests/fixtures/custom_template.yml @@ -0,0 +1,18 @@ +metadata: + template: + name: branded + +connectors: + X1: + pinlabels: [A, B] + +cables: + W1: + gauge: 0.25 mm2 + length: 0.1 + color_code: DIN + wirecount: 2 + +connections: + - - X1: [1, 2] + - W1: [1, 2] diff --git a/tests/fixtures/dpi_192.yml b/tests/fixtures/dpi_192.yml new file mode 100644 index 00000000..1621b5b9 --- /dev/null +++ b/tests/fixtures/dpi_192.yml @@ -0,0 +1,17 @@ +options: + output_dpi: 192 + +connectors: + X1: + pinlabels: [A, B] + +cables: + W1: + gauge: 0.25 mm2 + length: 0.1 + color_code: DIN + wirecount: 2 + +connections: + - - X1: [1, 2] + - W1: [1, 2] diff --git a/tests/fixtures/hex_color.yml b/tests/fixtures/hex_color.yml new file mode 100644 index 00000000..bff74987 --- /dev/null +++ b/tests/fixtures/hex_color.yml @@ -0,0 +1,16 @@ +# Single hex-RGB wire color — the upstream PR #495 regression case. +# Before the fix, ``len(colorstr) > 2`` mis-detected a 7-character +# hex string as a multi-colored wire and padded it like one. +connectors: + X1: + pinlabels: [A] + +cables: + W1: + gauge: 0.25 mm2 + length: 0.1 + colors: ["#ff0000"] + +connections: + - - X1: [1] + - W1: [1] diff --git a/tests/fixtures/loopback.yml b/tests/fixtures/loopback.yml new file mode 100644 index 00000000..e5350380 --- /dev/null +++ b/tests/fixtures/loopback.yml @@ -0,0 +1,11 @@ +# Loop-only connector that is NOT referenced in any connection set. +# Upstream PR #496 added auto-instantiation so this still renders; +# without that fix the connector is silently dropped as an unused +# template. +connectors: + X1: + pincount: 4 + pinlabels: [A, B, C, D] + loops: + - [1, 4] + - [2, 3] diff --git a/tests/fixtures/loopback_template.yml b/tests/fixtures/loopback_template.yml new file mode 100644 index 00000000..db782ddc --- /dev/null +++ b/tests/fixtures/loopback_template.yml @@ -0,0 +1,21 @@ +# Connector template (``DSub:``) carrying loops, used via the +# Template.Designator syntax (``DSub.X1`` / ``DSub.X2``). The review +# fix on PR #1 ensures we don't ALSO auto-instantiate ``DSub`` itself +# as a phantom floating connector. +connectors: + DSub: + type: D-Sub + pinlabels: [GND, RX, TX, DTR] + loops: [[1, 4]] + +cables: + W1: + gauge: 0.25 mm2 + length: 0.5 + color_code: DIN + wirecount: 2 + +connections: + - - DSub.X1: [2, 3] + - W1: [1, 2] + - DSub.X2: [3, 2] diff --git a/tests/fixtures/minimal.yml b/tests/fixtures/minimal.yml new file mode 100644 index 00000000..0761ce7f --- /dev/null +++ b/tests/fixtures/minimal.yml @@ -0,0 +1,14 @@ +connectors: + X1: + pinlabels: [A, B] + +cables: + W1: + gauge: 0.25 mm2 + length: 0.1 + color_code: DIN + wirecount: 2 + +connections: + - - X1: [1, 2] + - W1: [1, 2] diff --git a/tests/fixtures/per_node_tweak.yml b/tests/fixtures/per_node_tweak.yml new file mode 100644 index 00000000..ed08e821 --- /dev/null +++ b/tests/fixtures/per_node_tweak.yml @@ -0,0 +1,23 @@ +tweak: + placeholder: "@@" + +connectors: + X1: + pinlabels: [A, B] + tweak: + append: + - "@@_extra [color=red, style=dashed];" + +cables: + W1: + gauge: 0.25 mm2 + length: 0.1 + color_code: DIN + wirecount: 2 + tweak: + append: + - "@@_label [label=\"cable @@\"];" + +connections: + - - X1: [1, 2] + - W1: [1, 2] diff --git a/tests/fixtures/revisions.yml b/tests/fixtures/revisions.yml new file mode 100644 index 00000000..09fba566 --- /dev/null +++ b/tests/fixtures/revisions.yml @@ -0,0 +1,26 @@ +metadata: + revisions: + A: + date: 2024-01-01 + name: Initial draft + B: + date: 2024-06-15 + name: Pin labels added + C: + date: 2026-04-10 + name: Latest tweak + +connectors: + X1: + pinlabels: [A, B] + +cables: + W1: + gauge: 0.25 mm2 + length: 0.1 + color_code: DIN + wirecount: 2 + +connections: + - - X1: [1, 2] + - W1: [1, 2] diff --git a/tests/fixtures/templates/branded.html b/tests/fixtures/templates/branded.html new file mode 100644 index 00000000..ab02fb0a --- /dev/null +++ b/tests/fixtures/templates/branded.html @@ -0,0 +1,9 @@ + + +BRANDED-<!-- %filename_stem% --> + +

CMDIY HARNESS

+ + + + diff --git a/tests/test_bom.py b/tests/test_bom.py new file mode 100644 index 00000000..42ac0018 --- /dev/null +++ b/tests/test_bom.py @@ -0,0 +1,190 @@ +# -*- coding: utf-8 -*- +"""Coverage of BOM aggregation logic.""" + +from pathlib import Path + +import pytest + +from wireviz.wireviz import parse + + +def _parse_to_tsv(yaml_str: str, workdir: Path, name: str = "bom") -> str: + parse( + yaml_str, + output_formats=("tsv",), + output_dir=workdir, + output_name=name, + ) + return (workdir / f"{name}.bom.tsv").read_text() + + +def test_bom_lists_connector(workdir: Path): + tsv = _parse_to_tsv( + """ +connectors: + X1: + type: D-Sub + pinlabels: [A, B] +cables: + W1: + gauge: 0.25 mm2 + length: 0.1 + color_code: DIN + wirecount: 2 +connections: + - [{X1: [1, 2]}, {W1: [1, 2]}] +""", + workdir, + ) + assert "Connector" in tsv + assert "X1" in tsv + assert "D-Sub" in tsv + + +def test_bom_lists_cable_with_gauge_and_length(workdir: Path): + tsv = _parse_to_tsv( + """ +connectors: + X1: {pinlabels: [A]} +cables: + W1: + gauge: 0.5 mm2 + length: 1.5 + color_code: DIN + wirecount: 1 +connections: + - [{X1: [1]}, {W1: [1]}] +""", + workdir, + ) + assert "0.5" in tsv # gauge token preserved + assert "1.5" in tsv # length appears + assert "W1" in tsv + + +def test_bom_aggregates_identical_components(workdir: Path): + """Two identical D-Sub connectors aggregate into one BOM row with + qty=2 and both designators listed.""" + tsv = _parse_to_tsv( + """ +connectors: + X1: {type: D-Sub, pinlabels: [A]} + X2: {type: D-Sub, pinlabels: [A]} +cables: + W1: {gauge: 0.25 mm2, length: 0.1, color_code: DIN, wirecount: 1} +connections: + - [{X1: [1]}, {W1: [1]}, {X2: [1]}] +""", + workdir, + ) + rows = tsv.splitlines() + dsub_rows = [r for r in rows[1:] if "D-Sub" in r] + assert len(dsub_rows) == 1, "identical connectors should aggregate to one row" + # Both designators should appear in the same row + assert "X1" in dsub_rows[0] and "X2" in dsub_rows[0] + + +def test_bom_separates_connectors_with_different_types(workdir: Path): + """A D-Sub and a Molex don't aggregate; they're different components.""" + tsv = _parse_to_tsv( + """ +connectors: + X1: {type: D-Sub, pinlabels: [A]} + X2: {type: Molex KK, pinlabels: [A]} +cables: + W1: {gauge: 0.25 mm2, length: 0.1, color_code: DIN, wirecount: 1} +connections: + - [{X1: [1]}, {W1: [1]}, {X2: [1]}] +""", + workdir, + ) + assert "D-Sub" in tsv and "Molex" in tsv + rows = tsv.splitlines() + assert len([r for r in rows[1:] if "D-Sub" in r]) == 1 + assert len([r for r in rows[1:] if "Molex" in r]) == 1 + + +def test_bom_includes_additional_bom_items(workdir: Path): + tsv = _parse_to_tsv( + """ +connectors: + X1: {pinlabels: [A]} +cables: + W1: {gauge: 0.25 mm2, length: 0.1, color_code: DIN, wirecount: 1} +connections: + - [{X1: [1]}, {W1: [1]}] +additional_bom_items: + - description: Heat shrink tubing 3mm + qty: 4 + unit: cm + - description: Cable tie + qty: 6 +""", + workdir, + ) + assert "Heat shrink tubing 3mm" in tsv + assert "Cable tie" in tsv + + +def test_bom_ignores_components_marked_ignore_in_bom(workdir: Path): + """``ignore_in_bom: true`` keeps a connector out of the BOM but + still in the diagram.""" + tsv = _parse_to_tsv( + """ +connectors: + X1: {type: TestPin, pinlabels: [A], ignore_in_bom: true} +cables: + W1: {gauge: 0.25 mm2, length: 0.1, color_code: DIN, wirecount: 1} +connections: + - [{X1: [1]}, {W1: [1]}] +""", + workdir, + ) + assert "TestPin" not in tsv + + +def test_bom_part_numbers_appear(workdir: Path): + """``pn``, ``mpn``, ``spn`` fields land in the BOM.""" + tsv = _parse_to_tsv( + """ +connectors: + X1: + type: D-Sub + pinlabels: [A] + pn: INTERNAL-001 + mpn: AMP-12345 + manufacturer: Amphenol +cables: + W1: {gauge: 0.25 mm2, length: 0.1, color_code: DIN, wirecount: 1} +connections: + - [{X1: [1]}, {W1: [1]}] +""", + workdir, + ) + assert "INTERNAL-001" in tsv + assert "AMP-12345" in tsv + assert "Amphenol" in tsv + + +def test_bom_bundle_category_emits_one_row_per_wire(workdir: Path): + """A cable with ``category: bundle`` produces one BOM row per + wire instead of a single cable row.""" + tsv = _parse_to_tsv( + """ +connectors: + X1: {pinlabels: [A, B]} +cables: + W1: + category: bundle + gauge: 0.25 mm2 + length: 0.1 + color_code: DIN + wirecount: 2 +connections: + - [{X1: [1, 2]}, {W1: [1, 2]}] +""", + workdir, + ) + rows = tsv.splitlines() + wire_rows = [r for r in rows[1:] if "Wire" in r] + assert len(wire_rows) == 2, "bundle should emit one row per wire" diff --git a/tests/test_cli.py b/tests/test_cli.py new file mode 100644 index 00000000..003aa53b --- /dev/null +++ b/tests/test_cli.py @@ -0,0 +1,272 @@ +# -*- coding: utf-8 -*- +"""CLI behavior tests via Click's ``CliRunner``. + +Every flag, every error path, every input mode (file / stdin / .png). +The CLI is the user-visible surface — if anything regresses here, +people notice. +""" + +from pathlib import Path + +import pytest +from click.testing import CliRunner + +from wireviz.Harness import read_yaml_from_png +from wireviz.wv_cli import wireviz as cli + + +@pytest.fixture +def runner(): + # mix_stderr was removed in Click 8.3; modern CliRunner always + # exposes stderr separately via ``result.stderr``. + return CliRunner() + + +# --- Basic file-mode invocations -------------------------------------------- + + +def test_cli_file_input_default_formats(runner, minimal_yaml: Path, workdir: Path): + """No flags → defaults render html/png/svg/tsv next to the input.""" + target = workdir / "h.yml" + target.write_text(minimal_yaml.read_text()) + result = runner.invoke(cli, [str(target)]) + assert result.exit_code == 0, result.stderr + for ext in ("html", "png", "svg", "bom.tsv"): + assert (workdir / f"h.{ext}").exists(), f"{ext} missing" + + +def test_cli_format_flag_subset(runner, minimal_yaml: Path, workdir: Path): + """``-f s`` only renders SVG.""" + target = workdir / "s.yml" + target.write_text(minimal_yaml.read_text()) + result = runner.invoke(cli, ["-f", "s", str(target)]) + assert result.exit_code == 0 + assert (workdir / "s.svg").exists() + assert not (workdir / "s.png").exists() + assert not (workdir / "s.html").exists() + + +def test_cli_pdf_format_flag(runner, minimal_yaml: Path, workdir: Path): + """``-f P`` (capital P, distinct from lowercase p for PNG) produces PDF.""" + target = workdir / "p.yml" + target.write_text(minimal_yaml.read_text()) + result = runner.invoke(cli, ["-f", "P", str(target)]) + assert result.exit_code == 0 + pdf = workdir / "p.pdf" + assert pdf.exists() + assert pdf.read_bytes()[:5] == b"%PDF-" + + +def test_cli_unknown_format_uses_click_usage_error(runner, minimal_yaml: Path): + """An unknown format flag raises ``click.UsageError`` → exit 2 plus + the canonical 'Try \\'wireviz -h\\' for help.' footer.""" + result = runner.invoke(cli, ["-f", "X", str(minimal_yaml)]) + assert result.exit_code == 2 + assert "Unknown output format" in result.stderr + assert "Try 'wireviz -h' for help" in result.stderr + + +def test_cli_missing_input_file_errors(runner, workdir: Path): + """A non-existent input file produces a clean error.""" + result = runner.invoke(cli, [str(workdir / "does-not-exist.yml")]) + assert result.exit_code != 0 + assert "does not exist" in result.stderr.lower() + + +# --- output-dir / output-name ----------------------------------------------- + + +def test_cli_output_dir(runner, minimal_yaml: Path, tmp_path: Path): + """``-o`` redirects output to a different directory.""" + out = tmp_path / "out" + out.mkdir() + result = runner.invoke( + cli, ["-f", "s", "-o", str(out), str(minimal_yaml)] + ) + assert result.exit_code == 0 + assert (out / f"{minimal_yaml.stem}.svg").exists() + + +def test_cli_output_name(runner, minimal_yaml: Path, workdir: Path): + """``-O`` overrides the output filename stem.""" + target = workdir / "input.yml" + target.write_text(minimal_yaml.read_text()) + result = runner.invoke(cli, ["-f", "s", "-O", "renamed", str(target)]) + assert result.exit_code == 0 + assert (workdir / "renamed.svg").exists() + assert not (workdir / "input.svg").exists() + + +# --- stdin / stdout ---------------------------------------------------------- + + +def test_cli_stdin_input(runner, minimal_yaml: Path, workdir: Path): + """``wireviz -`` reads YAML from stdin.""" + yaml_text = minimal_yaml.read_text() + result = runner.invoke( + cli, + ["-f", "s", "-O", "stdin_out", "-o", str(workdir), "-"], + input=yaml_text, + ) + assert result.exit_code == 0, result.stderr + assert (workdir / "stdin_out.svg").exists() + + +def test_cli_stdout_svg(runner, minimal_yaml: Path): + """``-O -`` (or ``-o -``) writes the single requested format to stdout.""" + result = runner.invoke( + cli, + ["-f", "s", "-O", "-", str(minimal_yaml)], + ) + assert result.exit_code == 0, result.stderr + assert result.stdout.lstrip().startswith(" Harness: + return Harness( + metadata=Metadata(), + options=Options(), + tweak=Tweak(), + ) + + +def test_empty_harness_constructs(): + """A bare ``Harness()`` builds with empty containers.""" + h = _new_harness() + assert h.connectors == {} + assert h.cables == {} + assert h.mates == [] + assert h.additional_bom_items == [] + + +def test_add_connector_appends_to_connectors(): + h = _new_harness() + h.add_connector("X1", pinlabels=["A", "B"]) + assert "X1" in h.connectors + assert h.connectors["X1"].pinlabels == ["A", "B"] + + +def test_add_cable_appends_to_cables(): + h = _new_harness() + h.add_cable("W1", gauge="0.25 mm2", length=0.1, color_code="DIN", wirecount=2) + assert "W1" in h.cables + assert h.cables["W1"].wirecount == 2 + + +def test_add_connector_rejects_old_attr_names(): + """Renamed-since-v0.2 attributes raise a clear error rather than + silently being ignored.""" + h = _new_harness() + with pytest.raises(ValueError, match="pinout"): + h.add_connector("X1", pinout=["A", "B"]) + with pytest.raises(ValueError, match="pinnumbers"): + h.add_connector("X2", pinnumbers=[1, 2]) + + +def test_render_dict_shapes(minimal_yaml: Path): + """``Harness._render`` returns a dict with the right key/value + shapes for each format. This is the contract the stdout dispatch + relies on.""" + h = parse(minimal_yaml, return_types="harness") + outputs = h._render(("svg", "png", "gv", "tsv")) + assert isinstance(outputs["svg"], str) + assert isinstance(outputs["png"], bytes) + assert isinstance(outputs["gv"], str) + assert isinstance(outputs["tsv"], str) + assert outputs["svg"].lstrip().startswith(" placeholder +# =========================================================================== + + +def test_pr492_latest_revision_returns_last_dict_key(): + """Upstream PR #492. ``_latest_revision`` returns the most-recently + declared revision name (which Python preserves via dict insertion + order).""" + md = {"revisions": {"A": {}, "B": {}, "C": {}}} + assert _latest_revision(md) == "C" + + +def test_pr492_review_handles_scalar_revisions(): + """PR #6 review fix. A scalar ``revisions: v1.0`` value used to + return ``'0'`` (last char of str). Now returns the whole scalar.""" + assert _latest_revision({"revisions": "v1.0"}) == "v1.0" + assert _latest_revision({"revisions": 42}) == "42" + + +def test_pr492_review_handles_missing_or_empty_revisions(): + """PR #6 review fix. None / missing / empty containers all return + empty string instead of raising.""" + assert _latest_revision({}) == "" + assert _latest_revision({"revisions": None}) == "" + assert _latest_revision({"revisions": {}}) == "" + assert _latest_revision({"revisions": []}) == "" + + +# =========================================================================== +# Upstream PR #357 — per-connector / per-cable tweak with placeholder +# =========================================================================== + + +def test_pr357_per_node_tweak_substitutes_placeholder( + workdir: Path, per_node_tweak_yaml: Path +): + """Upstream PR #357. A per-connector ``tweak.append`` line with + a placeholder ``@@`` gets the actual connector designator + substituted in the rendered .gv.""" + parse( + per_node_tweak_yaml, + output_formats=("gv",), + output_dir=workdir, + output_name="t", + ) + gv = (workdir / "t.gv").read_text() + assert "X1_extra" in gv, "per-connector @@_extra didn't substitute" + assert "W1_label" in gv, "per-cable @@_label didn't substitute" + assert "cable W1" in gv, "@@ inside string value didn't substitute" + + +def test_pr357_review_none_override_value_no_crash(workdir: Path): + """PR #7 review fix. An override value of ``null`` (YAML deletion + sentinel) used to crash the rph lambda with AttributeError.""" + yaml_str = """ +tweak: + placeholder: "@@" +connectors: + X1: + pinlabels: [A] + tweak: + override: + "@@": + color: null +cables: + W1: {gauge: 0.25 mm2, length: 0.1, color_code: DIN, wirecount: 1} +connections: + - [{X1: [1]}, {W1: [1]}] +""" + # Should not raise + parse(yaml_str, output_formats=("gv",), output_dir=workdir, output_name="ndiv") + + +def test_pr357_review_conflict_error_includes_values(workdir: Path): + """PR #10 review fix. The conflict error reports both the existing + and the new value, so users can resolve without spelunking.""" + yaml_str = """ +tweak: + placeholder: "@@" + override: + X1: + color: "red" +connectors: + X1: + pincount: 1 + tweak: + override: + "@@": + color: "blue" +cables: + W1: {gauge: 0.25 mm2, length: 0.1, color_code: DIN, wirecount: 1} +connections: + - [{X1: [1]}, {W1: [1]}] +""" + with pytest.raises(ValueError, match="'blue'.*conflicts.*'red'"): + parse( + yaml_str, + output_formats=("gv",), + output_dir=workdir, + output_name="conf", + ) + + +# =========================================================================== +# Upstream PR #367 — PDF output +# =========================================================================== + + +def test_pr367_pdf_format_produces_valid_pdf(workdir: Path, minimal_yaml: Path): + """Upstream PR #367. ``-f P`` (capital P) renders a valid PDF.""" + parse( + minimal_yaml, + output_formats=("pdf",), + output_dir=workdir, + output_name="p", + embed_yaml=False, + ) + pdf = workdir / "p.pdf" + assert pdf.exists() + assert pdf.read_bytes()[:5] == b"%PDF-" + + +def test_pr367_review_pdf_docstring_says_diagram_only(): + """PR #8 review fix. The parse() docstring's PDF entry was updated + to be honest about the implementation: PDF is diagram-only, + no BOM (HTML covers that).""" + docstring = parse.__doc__ or "" + pdf_line = next( + (line for line in docstring.splitlines() if '"pdf"' in line), "" + ) + assert "no BOM" in pdf_line or "diagram, as a PDF" in pdf_line + + +# =========================================================================== +# PR #10 review fixes (data URI, image_paths cwd, click.UsageError) +# =========================================================================== + + +def test_pr10_review_data_uri_no_leading_space(): + """PR #10 review fix. RFC 2397 says no whitespace after the + ``base64,`` separator in data URIs.""" + from wireviz.svgembed import data_URI_base64 + import tempfile, os + + # Write a tiny PNG to disk and base64-URI it + with tempfile.NamedTemporaryFile(suffix=".png", delete=False) as f: + Image.new("RGB", (4, 4), "blue").save(f, format="PNG") + tmp = f.name + try: + uri = data_URI_base64(tmp) + # Right after `base64,` should be a non-space character (the + # first base64 byte is alphabetic for any non-trivial PNG). + idx = uri.index(";base64,") + char_after_comma = uri[idx + len(";base64,")] + assert char_after_comma != " ", "data URI must not have leading space" + finally: + os.unlink(tmp) + + +def test_pr10_review_unknown_format_uses_click_usage_error(minimal_yaml: Path): + """PR #10 review fix. Unknown -f code raises click.UsageError, + not a generic Exception.""" + runner = CliRunner() + result = runner.invoke(cli, ["-f", "X", str(minimal_yaml)]) + assert result.exit_code == 2 + # click.UsageError adds the canonical "Try 'wireviz -h' for help." + assert "Try 'wireviz -h' for help" in result.stderr + + +def test_pr10_review_source_path_autofills(minimal_yaml: Path): + """PR #10 review fix. ``source_path`` was documented to auto-fill + when ``inp`` is a Path; the code now actually does it.""" + h = parse(minimal_yaml, return_types="harness") + assert h.source_path is not None + assert Path(h.source_path).resolve() == minimal_yaml.resolve() + + +def test_pr10_review_missing_input_clean_error(workdir: Path): + """PR #10 review fix (extension). 'File does not exist' on input + was a generic Exception too; now click.UsageError.""" + runner = CliRunner() + result = runner.invoke(cli, [str(workdir / "missing.yml")]) + assert result.exit_code == 2 + assert "does not exist" in result.stderr.lower() diff --git a/tests/test_round_trip.py b/tests/test_round_trip.py new file mode 100644 index 00000000..fb2b7305 --- /dev/null +++ b/tests/test_round_trip.py @@ -0,0 +1,131 @@ +# -*- coding: utf-8 -*- +"""Round-trip tests — render → output bytes → re-parse → identical +result. The high-leverage scenarios are PNG-with-embedded-YAML and +the stdin/stdout pipe.""" + +from pathlib import Path + +import pytest +from click.testing import CliRunner + +from wireviz.Harness import read_yaml_from_png +from wireviz.wireviz import parse +from wireviz.wv_cli import wireviz as cli + + +def test_png_yaml_round_trip_via_cli(workdir: Path, minimal_yaml: Path): + """Render YAML → PNG-with-embedded-source → feed PNG back to CLI → + rendered output uses the same harness model. This is the + load-bearing workflow for the planned wireviz-gui.""" + runner = CliRunner() + + target = workdir / "orig.yml" + target.write_text(minimal_yaml.read_text()) + + # First render: produces orig.png with embedded YAML + result = runner.invoke(cli, ["-f", "p", str(target)]) + assert result.exit_code == 0, result.stderr + + # Second render: feed orig.png back, produce a fresh SVG + result2 = runner.invoke( + cli, ["-f", "s", "-O", "round2", str(workdir / "orig.png")] + ) + assert result2.exit_code == 0, result2.stderr + assert "(extracted YAML)" in result2.stderr + + # The re-rendered SVG should be a valid SVG referencing X1 and W1 + svg = (workdir / "round2.svg").read_text() + assert ""), + ("pdf", "pdf", b"%PDF-"), + ], +) +def test_each_format_renders(minimal_yaml: Path, workdir: Path, fmt, ext, signature): + """Every supported output format produces a file with the right + magic-bytes signature. Catches: missing format codes, render-path + regressions, broken graphviz piping.""" + parse( + minimal_yaml, + output_formats=(fmt,), + output_dir=workdir, + output_name="out", + embed_yaml=False, + ) + artifact = workdir / f"out.{ext}" + assert artifact.exists(), f"{ext} not produced" + head = artifact.read_bytes()[: len(signature)] + assert head == signature, f"unexpected {ext} signature: {head!r}" + + +def test_multi_format_one_call(minimal_yaml: Path, workdir: Path): + """One ``parse()`` call writes every requested format atomically.""" + parse( + minimal_yaml, + output_formats=("svg", "png", "gv", "tsv", "html", "pdf"), + output_dir=workdir, + output_name="multi", + embed_yaml=False, + ) + for ext in ("svg", "png", "gv", "bom.tsv", "html", "pdf"): + assert (workdir / f"multi.{ext}").exists(), f"{ext} missing" + + +def test_minimal_gv_is_valid_graphviz(minimal_yaml: Path, workdir: Path): + """The .gv source must parse as valid Graphviz — i.e. balanced + braces, well-formed graph header. We don't shell out to ``dot -c``; + a structural check on the source string is enough.""" + parse( + minimal_yaml, + output_formats=("gv",), + output_dir=workdir, + output_name="m", + embed_yaml=False, + ) + gv = (workdir / "m.gv").read_text() + assert gv.startswith("graph {") + assert gv.rstrip().endswith("}") + # Nodes for X1 and W1 must both appear + assert "X1 [label=" in gv + assert "W1 [label=" in gv + + +def test_html_embeds_svg_inline(minimal_yaml: Path, workdir: Path): + """HTML output embeds the SVG inline (not a sibling-file ), + so a single .html is self-contained.""" + parse( + minimal_yaml, + output_formats=("html",), + output_dir=workdir, + output_name="h", + embed_yaml=False, + ) + html = (workdir / "h.html").read_text() + assert "" not in html, "diagram placeholder unresolved" + + +def test_tsv_has_header_and_one_data_row(minimal_yaml: Path, workdir: Path): + """The BOM TSV always starts with a header row and contains at + least one component row for the cable in this minimal harness.""" + parse( + minimal_yaml, + output_formats=("tsv",), + output_dir=workdir, + output_name="b", + embed_yaml=False, + ) + rows = (workdir / "b.bom.tsv").read_text().splitlines() + assert len(rows) >= 2, "expected header + at least one data row" + # Header line: tab-separated column names starting with "Id" + assert rows[0].split("\t")[0] == "Id", "first row should be the column header" + # Cable W1 should appear in some data row + assert any("W1" in r or "Cable" in r for r in rows[1:]) From c277a2f2ad0c595c48a358fd2a117fe38498f4a1 Mon Sep 17 00:00:00 2001 From: Cole Gentry Date: Mon, 4 May 2026 22:21:43 -0400 Subject: [PATCH 3/5] =?UTF-8?q?Import=20``Optional``=20in=20wireviz.py=20?= =?UTF-8?q?=E2=80=94=20fixes=20CI=20on=20Python=203.7-3.12?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ``_get_yaml_data_and_path`` return-type annotation ``Tuple[Dict, Optional[Path], Optional[str]]`` references ``Optional`` which wasn't in the typing import line. This worked locally on Python 3.14 because of PEP 649 deferred annotations, but every CI runner (3.7-3.12) evaluates annotations at module-import time and crashes with:: NameError: name 'Optional' is not defined Both the Tests workflow (pytest) and the Create Examples workflow (build_examples.py) failed for this reason — both import the wireviz package at startup. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/wireviz/wireviz.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wireviz/wireviz.py b/src/wireviz/wireviz.py index c383a081..271fd079 100755 --- a/src/wireviz/wireviz.py +++ b/src/wireviz/wireviz.py @@ -4,7 +4,7 @@ import platform import sys from pathlib import Path -from typing import Any, Dict, List, Tuple, Union +from typing import Any, Dict, List, Optional, Tuple, Union import yaml From c17060dd7a9c8fa091018ff17e4069b7cdd96ef1 Mon Sep 17 00:00:00 2001 From: Cole Gentry Date: Mon, 4 May 2026 22:28:19 -0400 Subject: [PATCH 4/5] Fix CI on Click 8.0-8.2 + move import copy to module top (PR review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## CI fix: Click version drift The Click ``CliRunner`` API changed between 8.2 and 8.3: * 8.0-8.2 needs ``CliRunner(mix_stderr=False)`` to expose stderr separately on ``result.stderr``. * 8.3+ removed the ``mix_stderr`` parameter entirely and made separate-capture the always-on default — passing it now raises ``TypeError``. The local dev venv had Click 8.3+, so ``CliRunner()`` worked. CI's older Pythons (3.7-3.11) resolved to Click 8.0-8.2 where the bare ``CliRunner()`` mixed stderr into stdout, so all 16 tests that asserted on ``result.stderr`` failed with ``ValueError: stderr not separately captured``, and tests that asserted ``result.stdout.startswith(" --- src/wireviz/wireviz.py | 3 +-- tests/conftest.py | 20 ++++++++++++++++++++ tests/test_cli.py | 9 +++------ tests/test_regressions.py | 22 +++++++--------------- tests/test_round_trip.py | 11 +++-------- 5 files changed, 34 insertions(+), 31 deletions(-) diff --git a/src/wireviz/wireviz.py b/src/wireviz/wireviz.py index 271fd079..5454b366 100755 --- a/src/wireviz/wireviz.py +++ b/src/wireviz/wireviz.py @@ -1,6 +1,7 @@ #!/usr/bin/env python3 # -*- coding: utf-8 -*- +import copy import platform import sys from pathlib import Path @@ -513,8 +514,6 @@ def _get_yaml_data_and_path( # text form for round-trip embedding into PNG output, and # deep-copy so the parsing pipeline's in-place expansion of # the connections section doesn't leak back to the caller. - import copy - yaml_data = copy.deepcopy(inp) yaml_path = None yaml_str = yaml.safe_dump(inp, sort_keys=False, allow_unicode=True) diff --git a/tests/conftest.py b/tests/conftest.py index 02509a0d..c7cb2814 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -95,3 +95,23 @@ def workdir(tmp_path: Path, monkeypatch) -> Path: relative-path behavior can use the temp dir as cwd.""" monkeypatch.chdir(tmp_path) return tmp_path + + +@pytest.fixture +def runner(): + """Click ``CliRunner`` with stderr captured separately, regardless + of which Click version is installed. + + * Click 8.0-8.2 needs ``mix_stderr=False`` explicitly. + * Click 8.3+ removed the parameter and made separate-capture the + always-on default — passing it raises ``TypeError``. + + Tests need separate stderr to assert on banner / error-footer + messages, so the fallback ladder isolates that from Click drift. + """ + from click.testing import CliRunner + + try: + return CliRunner(mix_stderr=False) + except TypeError: + return CliRunner() diff --git a/tests/test_cli.py b/tests/test_cli.py index 003aa53b..a3dee00c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -9,17 +9,14 @@ from pathlib import Path import pytest -from click.testing import CliRunner from wireviz.Harness import read_yaml_from_png from wireviz.wv_cli import wireviz as cli -@pytest.fixture -def runner(): - # mix_stderr was removed in Click 8.3; modern CliRunner always - # exposes stderr separately via ``result.stderr``. - return CliRunner() +# Tests in this file use the ``cli_runner`` fixture from conftest.py +# rather than instantiating CliRunner directly, so the Click 8.0-8.2 +# vs 8.3+ ``mix_stderr`` API drift is handled in one place. # --- Basic file-mode invocations -------------------------------------------- diff --git a/tests/test_regressions.py b/tests/test_regressions.py index 9888b8c4..23815a5b 100644 --- a/tests/test_regressions.py +++ b/tests/test_regressions.py @@ -11,7 +11,6 @@ import pytest import yaml -from click.testing import CliRunner from PIL import Image from wireviz.DataClasses import Cable, Options @@ -125,17 +124,15 @@ def test_pr1_review_loop_template_with_designator_no_phantom( # =========================================================================== -def test_pr321_stdout_writes_svg_text(minimal_yaml: Path): +def test_pr321_stdout_writes_svg_text(runner, minimal_yaml: Path): """Single-format stdout writes the rendered text to ``sys.stdout``.""" - runner = CliRunner() result = runner.invoke(cli, ["-f", "s", "-O", "-", str(minimal_yaml)]) assert result.exit_code == 0 assert result.stdout.lstrip().startswith(" Date: Mon, 4 May 2026 22:33:46 -0400 Subject: [PATCH 5/5] Switch wv_cli.py status writes to click.echo for Click 8.1 capture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reproducing the remaining CI failures locally with ``pip install 'click<8.2'`` showed Click 8.1.x's CliRunner with ``mix_stderr=False`` captures ``click.echo(..., err=True)`` only — direct ``sys.stderr.write()`` calls bypass its capture and never reach ``result.stderr``. Click 8.2+ closed that gap and now captures both, which is why the local dev venv (Click 8.2.1) passed. Tests asserting on the WireViz banner ("WireViz 0.4.1") and per-file status lines ("Input file: ...", "Output: .svg", "(extracted YAML)") all read empty stderr on the three CI matrix Pythons (3.7/3.8/3.9) where pip resolved Click 8.1.x. Switched all eight ``sys.stderr.write`` sites in wv_cli.py to ``click.echo(message, err=True)``. That's the idiomatic Click pattern and works uniformly on every Click 8.x release. Library-level prints in DataClasses.py / Harness.py / wv_helper.py / wv_colors.py intentionally *keep* using ``sys.stderr.write`` — those modules mustn't import Click since they're consumable by non-CLI library callers (and will be by the planned wireviz-gui). Verified: 134/134 passing locally on Click 8.1.8 (matches the failing CI matrix entries) and on Click 8.3+. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/wireviz/wv_cli.py | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/wireviz/wv_cli.py b/src/wireviz/wv_cli.py index 7ed949c5..ca1c06e2 100644 --- a/src/wireviz/wv_cli.py +++ b/src/wireviz/wv_cli.py @@ -96,7 +96,12 @@ def wireviz( --output-dir or --output-name to write a single rendered format to stdout (e.g. ``cat harness.yml | wireviz -f s -O - -``). """ - sys.stderr.write(f"\n{APP_NAME} {__version__}\n") + # Use ``click.echo(..., err=True)`` instead of ``sys.stderr.write`` + # for status/log lines so they reach Click's captured-stderr stream + # uniformly across Click 8.1 (only captures click.echo) and 8.2+ + # (captures sys.stderr.write too). The end-state is the same — log + # lines on stderr, render output on stdout. + click.echo(f"\n{APP_NAME} {__version__}", err=True) if version: return # print version number only and exit @@ -139,7 +144,7 @@ def wireviz( raise click.UsageError( f"Prepend file does not exist: {prepend_file}" ) - sys.stderr.write(f"Prepend file: {prepend_file}\n") + click.echo(f"Prepend file: {prepend_file}", err=True) prepend_input += file_read_text(prepend_file) + "\n" else: @@ -157,7 +162,7 @@ def wireviz( # the current working directory (matching how a typical # piped invocation would be run from a project root). image_paths = {Path.cwd()} - sys.stderr.write("Input: \n") + click.echo("Input: ", err=True) _output_dir = output_dir if output_dir else "-" _output_name = output_name if output_name else "stdin" else: @@ -182,10 +187,10 @@ def wireviz( f"'wireviz:yaml' iTXt chunk found)." ) yaml_input = prepend_input + embedded - sys.stderr.write(f"Input file: {file} (extracted YAML)\n") + click.echo(f"Input file: {file} (extracted YAML)", err=True) else: yaml_input = prepend_input + file_read_text(file) - sys.stderr.write(f"Input file: {file}\n") + click.echo(f"Input file: {file}", err=True) image_paths = {file.parent} _output_dir = output_dir if output_dir else file.parent _output_name = output_name if output_name else file.stem @@ -194,10 +199,13 @@ def wireviz( image_paths.add(Path(p).parent) if write_to_stdout: - sys.stderr.write(f"Output: .{output_formats_str}\n") + click.echo( + f"Output: .{output_formats_str}", err=True + ) else: - sys.stderr.write( - f"Output file: {Path(_output_dir) / _output_name}.{output_formats_str}\n" + click.echo( + f"Output file: {Path(_output_dir) / _output_name}.{output_formats_str}", + err=True, ) wv.parse( @@ -211,7 +219,7 @@ def wireviz( embed_yaml=embed_yaml, ) - sys.stderr.write("\n") + click.echo("", err=True) if __name__ == "__main__":