nigig-org/REVIEWS/CAD_COVERAGE_100_PLAN.md
andodeki 211ce31e9f
Some checks failed
email.yml / docs(cad): a phased plan to 100% coverage, written after measuring (push) Failing after 0s
repo hygiene / hygiene (push) Has been cancelled
docs(cad): a phased plan to 100% coverage, written after measuring
Asked for a plan to 100%. Four things had to be established first,
because each one changes the plan's shape, and three of them contradict
what I would have assumed.

**There are already 148 widget tests and they have never run.**
tests/ui.rs is 1,889 lines of #[makepad_test] tests driving the real app
through TestApp/Selector. Line 1 imports a crate that is not a
dependency, so the file has never compiled, and no CI job names it. Same
defect the spreadsheet-ui suite documents about itself; same class as the
nigig-email binary that had never been built.

**They compile with a one-line manifest change, and they run.** Adding
makepad-test as a dev-dependency produces a binary; under xvfb-run all
148 execute in 59 seconds without hanging.

**All 148 fail at a known point.** The harness's child build of the app
exits non-zero before startup: code 127 with no cargo on the child PATH,
code 101 after fixing that — while `cargo check -p nigig-build --bins`
passes. So the blocker is in how the harness invokes the child, not in
the app, and spreadsheet-ui already documents the workaround.

**#[coverage(off)] is unstable on 1.97.1.** There is no way to annotate
a line as legitimately unreachable, so anything genuinely uncoverable
has to be covered, moved to an excluded file, or subtracted openly.

The plan puts unblocking those 148 tests first, because it is the
cheapest large prize and because its outcome resizes everything after
it. Engine cleanup runs in parallel since it is independent. Extraction
work is explicitly held until Phase 0 reports, so nobody extracts logic
the app-level tests already cover.

It also argues against 100% as a target for the widget half. The 148
tests are mostly wait_visible(); they will move the number a long way
while proving that widgets exist. Of the four real defects this work has
found, three came from reading uncovered regions and asking why they
were unreachable, not from driving a percentage. The plan targets 100%
of what is worth executing and names the ~48 subtracted lines.
2026-08-17 12:40:35 +00:00

9.8 KiB
Raw Permalink Blame History

CAD test coverage: a phased plan to 100%

Written after measuring, not before. Every number here came from a run in this repository; the commands that produced them are named so you can reproduce or refute any of it.

Where we actually are

Two halves, measured by two tools, because one tool cannot see both.

Half Files Lines Covered Tool
Engine 15 13,752 97.16% tools/test-cad-coverage.sh (gated in CI)
Widget 10 10,637 13.25% tools/test-cad-widget-coverage.sh (not gated)

9,618 lines are uncovered. 96% of them are in the widget half, so that is where the plan spends its effort, and any plan that opens with engine polishing is optimising the wrong term.

The widget half, per file

File Lines Covered
viewport.rs 3,548 14.37%
workspace.rs 2,462 14.18%
viewport_render.rs 2,083 0.00%
script_bindings.rs 724 71.27%
workspace_actions.rs 690 0.00%
viewport_input.rs 672 0.00%
cad_editor_sheet.rs 224 0.00%
viewport_2d.rs 138 0.00%
code_editor.rs 67 0.00%
mod.rs 42 80.95%

Four findings that decide the plan's shape

1. There are already 148 widget tests, and they have never run. tests/ui.rs is 1,889 lines of #[makepad_test] tests driving the real app through TestApp/Selector. Line 1 is use makepad_test::{...} — a crate that is not a dependency — so the file has never compiled, and no CI job references it. This is the same defect the spreadsheet-ui suite documents about itself, and the same class as the nigig-email binary that had never been built.

2. They compile with a one-line manifest change, and they run. Adding to crates/apps/nigig-build/Cargo.toml:

[dev-dependencies]
makepad-test = { git = "https://gitdab.com/andodeki/makepad", rev = "ecf5a572ab62a1c1598909971f602f99083671cc", package = "makepad-test" }

produces a test binary. Under xvfb-run -a, all 148 execute in 59 seconds. They do not hang and they do not crash the harness.

3. All 148 currently fail, at a known point. The harness spawns a child build of the app under test, and that child exits non-zero "before startup". With no cargo on the child's PATH the code is 127; after symlinking cargo into /usr/local/bin it becomes 101 — a build failure — even though cargo check -p nigig-build --bins passes cleanly. So the remaining blocker is in how the harness invokes the child build, not in the app. spreadsheet-ui hit the same wall and worked around it by abandoning the #[makepad_test] attribute for run_with_config, explicitly injecting PATH, CARGO, CARGO_HOME and RUSTUP_HOME into the child environment. That workaround is the first thing to try.

4. #[coverage(off)] is not available. It is still unstable on the pinned 1.97.1 toolchain (error[E0658]). There is therefore no way to annotate a line as legitimately unreachable. Anything genuinely uncoverable must either be covered anyway, moved into a file that the report excludes wholesale, or subtracted honestly from the target.

On the target itself

100% line coverage is reachable for the engine and is not, by itself, a good goal for the widget half. Two reasons, both concrete:

  • Coverage counts lines executed, not assertions made. A test that clicks every button and asserts nothing scores identically to one that checks the result. The 148 existing tests are mostly wait_visible() — they will move the number a long way while proving only that widgets exist.
  • Six sessions of engine work found four real defects. Three were found by reading uncovered regions and asking why they were unreachable (mat4_inverse, the exporter dispatch arms, the nav-pad pixel), not by driving the percentage up. The number is a search strategy, not the product.

So the plan targets 100% of what is worth executing, and states plainly what is subtracted and why. The subtractions total ~50 lines.

Phase 0 — Unblock the existing suite (12 days)

The highest-leverage work in this document. 148 tests already written.

  1. Add the makepad-test dev-dependency (diff above). Commit the lockfile change with it.
  2. Fix the import on line 1 of tests/ui.rs.
  3. Diagnose the child build failure. Capture the child's stderr from makepad_test's runtime; try the spreadsheet-ui approach of run_with_config with PATH/CARGO/CARGO_HOME/RUSTUP_HOME injected, in place of the bare #[makepad_test] attribute.
  4. Triage the failures that remain once the app launches. Expect two populations: tests whose selectors have drifted from the current UI, and tests that need NIGIG_TEST_MODE and a loaded project (the file already has require_project_loaded() for this).

Exit criteria: xvfb-run -a cargo test -p nigig-build --test ui green, and a CI job running it. Record the pass count and the widget coverage delta — both are the input to every later estimate here.

Expected yield: unknown, and deliberately not guessed. These tests touch app launch, workspace open/close, tab switching, the spreadsheet and solar panels. If they lift the widget half from 13% to 40% the rest of this plan is much shorter; if they lift it to 18%, Phase 2 grows. Measure before committing to Phases 24.

Phase 1 — Close the engine's remaining 390 lines (35 days)

Small, well-understood, and it keeps the gated number honest while Phase 0 is in flight.

File Uncovered What it is Verdict
arch_pdf.rs 152 printpdf emitter: page furniture, dimension strings, title block Cover with golden-byte tests over a fixed scene
commands.rs 56 error arms and one #[ignore]d release-only test Cover, except the ignored one
cad_scene.rs 44 defensive arms on builder invariants Cover or delete the unreachable ones
exporters.rs 28 ExportTarget::Prompt — raises a native save dialog Subtract (see below)
construction_geometry.rs 20 other => panic!(...) arms inside tests Subtract: they execute only when a test fails
arch_gltf.rs 14 GLB byte-layout edges Cover
tools.rs 11 tool-state edges Cover
persistence.rs 10 the two real-data-dir wrappers Cover by pointing HOME/XDG_DATA_HOME at a temp dir
others 55 scattered Cover

Exit criteria: engine ≥ 99.5% with per-file floors raised to match, and the ~48 subtracted lines isolated so the number is honest — see Phase 3.

Phase 2 — Extract logic the harness can reach (23 weeks)

For widget code that Phase 0's app-level tests exercise shallowly or not at all. The pattern is proven: nav_pad.rs moved 64 lines of hit-testing out of a 630-line event handler into a pure module at 100%, and found a one-pixel bug on the way.

Targets, in order of expected defect yield rather than size:

  1. viewport_input.rs (672 lines) — snap resolution, direct-distance entry parsing, marquee window-vs-crossing, modifier decisions. All pure given a small state struct.
  2. workspace_actions.rs (690) — action dispatch and validation.
  3. viewport_2d.rs (138) — screen↔world projection is already pure maths wearing an impl CadViewport coat.
  4. viewport_render.rs (2,083) — geometry generation is separable from the draw calls; the draw calls themselves are not worth extracting.

Calibration from the one data point we have: ~60 lines per extraction, roughly half a day each including tests. The handler does not get covered — it shrinks. Expect the widget percentage to move slowly; expect the defect yield to be the real return.

Exit criteria: the four files above under 200 lines of unextracted imperative glue each, every extracted module ≥ 95% with a floor.

Phase 3 — Make the subtractions explicit (2 days)

Because #[coverage(off)] is unavailable, the ~48 genuinely unreachable lines have to be handled structurally:

  • Move ExportTarget::Prompt's dialog arm into exporters_dialog.rs, excluded from the report by filename, with the reason in the file header. The arm becomes three lines that call into it.
  • Leave the in-test panic! arms where they are and subtract them in the report generator, which already knows which files are test-only.

Exit criteria: tools/test-cad-coverage.sh prints both a raw and an adjusted figure, and the adjusted one can legitimately be 100%. Anything subtracted is listed by file and line count in TEST_BASELINE.md.

Phase 4 — Gate it (2 days)

  1. Add the widget coverage run to CI, report-only for one week, then with floors at measured-minus-one.
  2. Raise the engine floor to 99.
  3. Add the --test ui suite to full-crate-check under xvfb.
  4. Fix --test cost_estimator and --test cost_estimator_ui, which still do not compile and are noted as such in nigig-build.yml. Another 1,873 lines of tests nobody runs.

What this does not cover

  • The 3D render path. viewport_render.rs's draw calls can be executed but not meaningfully asserted without golden images, and golden-image testing over a GL context in CI is its own project.
  • script_bindings.rs's VM integration beyond its current 71%. The remaining lines are the makepad script VM boundary.
  • Assertion quality. Nothing here prevents a test that executes a line and checks nothing. The --test ui suite in particular is visibility-heavy. Coverage will not tell you; review has to.

Order of work, and why

Phase 0 first because 148 written tests that have never run is a larger and cheaper prize than anything else on the list, and because its result resizes Phases 24. Phase 1 in parallel — it is independent, and it keeps the one gated number moving while Phase 0's outcome is unknown. Do not start Phase 2 before Phase 0 reports: extracting logic that the app-level tests already cover would be effort spent for no measurement.