Add comprehensive pytest suite (134 tests) + fix 3 bugs it found#11
Merged
Conversation
Five gemini-code-assist comments on #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, <b64>`` 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,<b64>``. * 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive automated test suite using pytest, covering the CLI, API, BOM aggregation, and various internal logic components. It also includes several bug fixes and improvements, such as preventing the mutation of input dictionaries through deep copying, refining data URI formatting by removing illegal whitespace, and enhancing CLI error reporting with specific usage errors. Documentation and CI configurations have been updated to reflect the transition to automated testing. Feedback is provided regarding the placement of an import statement in src/wireviz/wireviz.py to better align with PEP 8 standards.
| # 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 |
There was a problem hiding this comment.
## 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("<?xml")`` failed because the WireViz banner
("WireViz 0.4.1") was now part of stdout.
Promoted the runner factory to a shared ``runner`` fixture in
``tests/conftest.py`` that tries the 8.0-8.2 form first and falls
back to 8.3+ on TypeError. Replaced the eight inline
``CliRunner()`` constructions in ``test_regressions.py`` and
``test_round_trip.py`` with the fixture.
## Gemini PR review
Moved ``import copy`` from inside ``_get_yaml_data_and_path`` to the
module-level imports (PEP 8). Function-local imports are reserved
for genuinely circular import situations; that wasn't the case here.
Verified: 134/134 still green locally; the suite no longer depends on
which Click version pip resolves to.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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: <stdout>.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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an automated pytest test suite covering the entire WireViz API surface. 134 tests across 9 files, completing in ~5 seconds. Every upstream PR we ported and every gemini review fix gets a pinned regression test, so future refactors can't silently undo any of the work that landed in PRs #1-#10.
The suite already paid for itself by surfacing three real bugs, all fixed in this same PR.
Test files
tests/test_smoke.pytests/test_parse.pywireviz.parse()API: input shapes, return_types, embed_yaml, source_path auto-filltests/test_cli.pyCliRunner— stdin/stdout,.pnginput,--no-embed-yaml,-t,-p, error pathstests/test_harness.pyHarnesspublic methods +_renderdict shapetests/test_dataclasses.pyConnector/Cable/Tweak/Options/Imagecoercion + validationtests/test_colors.pytests/test_bom.pytests/test_regressions.pytests/test_round_trip.pyBugs found and fixed
1. Python 3.13+
re.subpositional-countdeprecationwv_html.py:72was callingre.sub(pattern, repl, string, 1)— passingcountpositionally. As of Python 3.13 that emits aDeprecationWarning; pytest'sfilterwarnings = "error"config turns it into a test failure. Fixed by switching tocount=1keyword form. Would have broken CI on Python 3.13 once that DeprecationWarning becomes a real error.2. CLI "File does not exist" raised generic
Exceptionwv_cli.pyhad tworaise Exception(f"File does not exist:\n{file}")sites — one for the input file, one for--prepend. Both produced a Python stack trace for users instead of the clean Click error footer. Promoted both toclick.UsageError, matching the pattern we'd already applied to the unknown-format check earlier.3.
parse(dict)silently mutating caller's dictThis is the most consequential one for the upcoming GUI work. When
parse()was called with a pre-parsed dict, it rewrote the connections section in-place during processing —[[{X1: [1, 2]}, {W1: [1, 2]}]]became[[{X1: 1}, {X1: 2}], [{W1: 1}, {W1: 2}]]on the caller's reference. The GUI will likely keep an in-memory model of the harness and callparse()repeatedly on it; without this fix every render would corrupt the model.Fixed by
copy.deepcopy(inp)in_get_yaml_data_and_path. Pinned withtests/test_round_trip.py::test_dict_input_not_mutated.Also: cherry-pick of e4f485e (PR #10 review fixes)
The PR #10 review-fix commit was on
port-321-stdin-stdoutbut never reachedmasterbecause PR #10 merged before that commit was pushed. Cherry-picked here so the test suite can validate:-f X(unknown format) raisesclick.UsageError, not genericExceptionimage_paths = {Path.cwd()}so relativeimage: src:paths can resolvesource_pathauto-fills from aPathinput (matching the docstring)Infrastructure
pyproject.tomlgains[tool.pytest.ini_options]withtestpaths = ["tests"]andfilterwarnings = ["error", "ignore::SyntaxWarning"]so future deprecation drift gets caught.tests/conftest.pycentralizes fixtures pointing at small YAMLs intests/fixtures/— deliberately distinct from the gallery YAMLs inexamples/so tests don't break when the gallery shifts..github/workflows/tests.ymlruns pytest across Python 3.7-3.12 (same matrix as the existing Create Examples workflow).CLAUDE.mdupdated to document the test suite layout and how to run it.Verification
```
$ pytest -v
============================= 134 passed in 5.08s ==============================
```
Both
pytestandbuild_examples.pypass green. The two are complementary — pytest validates the API/CLI/data-model surface, the example sweep validates rendered visual output across the gallery.Test plan
Testsworkflow passes across Python 3.7-3.12Create Examplesworkflow still passes (no rendered output changes)🤖 Generated with Claude Code