andodeki
211ce31e9f
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.
209 lines
9.8 KiB
Markdown
209 lines
9.8 KiB
Markdown
# 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`:
|
||
|
||
```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 (1–2 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 2–4.
|
||
|
||
## Phase 1 — Close the engine's remaining 390 lines (3–5 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 (2–3 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 2–4. 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.
|