nigig-org/TEST_BASELINE.md

nigig-build test baseline

cargo test --locked -p nigig-build --lib
test result: ok. 1133 passed; 0 failed; 21 ignored

The suite is green. CI asserts a plain pass — any failing test fails the build. Do not reintroduce a "no worse than N failures" threshold.

The 21 ignored are the CAD profile_benchmarks and release-only merge tests, deliberately #[ignore]-d because they are timing measurements rather than assertions. Run them with cargo test -- --ignored --nocapture.

---

History

Stage	Result
Before any work	Crate did not compile (44 errors); no test had ever run
After Phase 0	634 passed / 17 failed
After Phase 1	679 passed / 17 failed
After baseline cleanup	702 passed / 0 failed
After Phase 2 (security)	720 passed / 0 failed
After Phase 3 (performance)	728 passed / 0 failed
After Phase 4 (deletion)	741 passed / 0 failed
After Phase 5.1 (generation-tracked store)	750 passed / 0 failed
After the CAD engine coverage work	1044 passed / 0 failed

Phase 4 removed 29 tests along with the dead Legacy* command system they exercised. That is a reduction in duplicated coverage, not a regression: every one of them tested code that no longer exists, and the equivalent Command/UndoRedoStack paths keep their own tests. Verified by diffing the full test-name list before and after.

The 17 failures were not regressions — they were pre-existing logic defects that had been invisible because the test binary never built. Each is described below, because the diagnoses are the useful part: roughly half turned out to be wrong tests rather than wrong code, and telling those apart required reading the production callers in every case.

---

What the 17 were

Genuine product bugs (10)

Area	Defect
cad::construction_geometry	next_polar_increment used position(..).unwrap_or(0) then advanced, so an unrecognised increment returned 10 instead of restarting the cycle at 5.
cad::construction_geometry	snap_to_polar_angle returned its result unnormalised, so 370° snapped to 360° instead of 0°. Now wraps into (-π, π], matching the atan2 output the viewport feeds it.
cad::construction_geometry	parse_coord_input accepted "3." as 3.0. Rust parses it, but the user is mid-way through typing 3.5, so the live preview committed a wrong-length segment on every keystroke. Trailing ./e/+/- are now treated as incomplete, and "nan"/"inf" are rejected.
cad::viewport	The viewport carried inline copies of both polar helpers, so neither fix above would have reached users. Both now call the shared functions.
project_management::logic	would_create_cycle walked the graph from the wrong end and tested the seed node before expanding, so it missed both direct (1→2, then link 2→1) and indirect (1→2→3, then link 3→1) cycles. Dependency cycles could be created through the UI.
project_management::logic	get_over_allocated_assignees keyed unassigned tasks on the empty string, so two overlapping unassigned tasks were reported as one over-booked person.
project_management::persistence	Serialization escaped " but not \, and deserialization used trim_matches(.. '"'), which stripped escaped quotes and left the escaping backslash behind. Phase "Alpha" round-tripped as Phase "Alpha\. Replaced with a proper escape/unescape pair covering \ " \n \r \t.
cost_estimator::data_raw	The default-rooms index used HashMap::remove, which consumes the entry. The same property type appears once per region, so the first region got its default rooms and the other two got none.
cost_estimator::estimate_store	Selections were normalised only in response to a user command, so a freshly constructed store kept Estimate::default()'s empty region — which matches nothing in the lookup index, leaving the property dropdown blank until the user touched a control.
cost_estimator::screen_state	is_blocked omitted Initializing, so the screen reported itself interactive while cost data was still loading. Now the exact complement of is_ready.
doc-engine::crdt	Text atoms sorted ascending by id. RGA requires siblings ordered counter descending, actor ascending: newest-nearest-the-anchor for correct sequential typing, actor tiebreak for deterministic convergence. Typing ! after the h of hi produced hi! instead of h!i. Block ordering legitimately stays ascending and was left alone.

Also fixed here: CrdtDocument::to_json, added in Phase 0, had never been executed. operations is a BTreeMap<OpId, _> and JSON object keys must be strings, so it failed at runtime with "key must be a string". It now serialises the log as an array of [key, value] pairs.

Wrong tests (7)

Each was verified against the production caller before being changed.

Test	Why it was wrong
dde_buffer_incremental_parse	Asserted "5<4" is "partial". It is a valid polar entry (5 units at 4°) and indistinguishable from "5<0", which a neighbouring test asserts is valid. Rewritten; a separate test now covers genuinely incomplete input.
logic::no_cycle	Asserted the opposite of cycle_direct on a structurally identical graph — relabel the ids and they are the same call with contradictory expectations. Duplicate-link suppression is the caller's job (dependencies.contains), not this function's.
history::undo_redo_roundtrip, history::multiple_undo_redo_cycle	Modelled snapshots taken after each mutation; every production caller snapshots before (self.snapshot(); self.tasks.push(..)). Under the real protocol the old sequences pushed the current state, so undo popped the state you were already in.
solar::calculate_lead_acid_battery	Asserted 3.125 while the engine rounds every output to 2 dp. The test's own comment computed the unrounded intermediate.
services::export_json_empty_rooms	Wanted compact "rooms": []; the writer emitted a valid but awkward multi-line empty array. Fixed the writer — the test's preference was reasonable.
types::*_inequal_to_* (2, fixed in Phase 0)	Asserted assert_ne! between distinct newtypes — a comparison the type system now rejects at compile time, which is strictly stronger.

---

CAD engine line coverage

The pass/fail count above says the suite is green. It does not say what the suite touches, and until this work nothing measured that.

A correction worth recording, because it cost six sessions. The reason given for not measuring — including by me, repeatedly, in commit messages and in this file — was that cargo test -p nigig-build needs wayland, X11, GL, alsa and polkit and therefore "does not build in this environment". That was never tested. It builds fine: in a plain container, bash tools/makepad-native-libs.sh --install followed by cargo check --locked -p nigig-build --lib takes 2m52s, and cargo test --locked -p nigig-build --lib takes about five minutes cold and under a second warm for 1133 tests. --test cad_integration adds 154 more. cargo fmt -p nigig-build --check, the lockfile gate and the mistyped-enum-variant gate all pass.

The install script existed the whole time, written for exactly this, with a comment explaining that libasound and libpulse are needed to link a test binary rather than merely to check it. Both this file's "Reproducing" section and the review's "Required next commands" pointed at it. An assumption made once and never retested is indistinguishable from a fact, which is the same failure this file criticises the CAD review for.

tools/test-cad-coverage.sh measures it, and is still the right tool for the job: it needs no apt, no root and no desktop packages, it runs in about 20 seconds warm against five minutes for the full crate, and it is what the CI gate uses. The full-crate build is the thing to reach for when a change touches the widget layer, which the harness cannot see. Nineteen of the CAD module's thirty files are pure -- geometry, the scene graph, undo/redo, the four exporters, file I/O -- and their only Makepad imports are the math types, the CSG library and two log macros, all dependency-free Rust. The script copies those into a host-only crate with the same module path, runs them plus tests/cad_integration.rs under -C instrument-coverage, and enforces a total floor and a per-file floor. The cad-engine-coverage job in .forgejo/workflows/nigig-build.yml runs it on every push.

File	Lines	Floor	Note
batching.rs	100.00%	99	new — Phase 3 instanced batching
cull.rs	100.00%	99	Phase 1 viewport culling
nav_pad.rs	100.00%	99
render_budget.rs	99.75%	99	Phase 4 batching + Phase 5 2-D/3-D LOD budget
lod.rs	99.68%	99	new — Phase 5 2-D/3-D sub-pixel policy
scene_holder.rs	100.00%	99
section_shape.rs	100.00%	99
send_sync_audit.rs	100.00%	99
math.rs	99.60%	97	was 20.40%, no tests at all
arch_svg.rs	99.02%	97
arch_stl.rs	98.88%	98
arch_gltf.rs	98.65%	97
cad_scene.rs	98.56%	96
constants.rs	98.51%	95
tools.rs	97.07%	95	was 0.00% — see below
commands.rs	96.23%	94
construction_geometry.rs	95.88%	95	rest is panic! arms inside tests
persistence.rs	94.51%	90	rest is the two real-data-dir wrappers
exporters.rs	91.17%	88	rest is the save-dialog branch
arch_pdf.rs	88.99%	86	rest is the printpdf emitter
Total	97.39%	96	653 engine-coverage tests (499 unit + 154 integration); 1,133 full-crate unit tests

Measured against the state of the tree when this table was written; the floors, not the table, are what CI enforces.

The other half: the widget layer, measured at last

The engine table above is 97% of the smaller half. Here is the rest, from ./tools/test-cad-widget-coverage.sh — the real crate, real tests, -C instrument-coverage, no desktop required:

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

9,228 lines have never been executed by a test. Six files have never had a single line run. Among them is viewport_input.rs — every click, drag, modifier and keystroke the CAD editor handles — and viewport_render.rs, every draw call.

Worth sitting with, because it reframes the engine work above: 1133 green tests and a 97% engine coexist with an input layer that no test has ever touched. The two facts were never in tension; they were just never on the same page.

No floors here yet, deliberately. A floor at 13% reads as a blessing. The first person to write a real input test should set one behind them.

What moving the number actually costs

The first attempt is worth recording, because it calibrates the rest. nav_pad.rs extracted the navigation pad's hit-testing out of viewport_input.rs into a pure module: 64 lines moved, now at 100% with 7 tests, and a one-pixel misalignment fixed on the way. The widget total moved 13.15% -> 13.25%.

viewport_input.rs is still at 0.00%, and will stay there under this strategy — extraction makes logic testable by moving it out, so the handler shrinks rather than getting covered. At roughly 60 lines per extraction, the remaining 672 lines of input handling are ten more of these. That is a reasonable way to spend the effort, and the pixel bug suggests the yield is real, but it will not produce a big number quickly. Genuinely covering the handlers themselves means driving them with an event loop — makepad-test, which tests/cad_ui.rs and spreadsheet-ui already use — and that is a different piece of work.

Which is the right tool

	test-cad-coverage.sh	test-cad-widget-coverage.sh
Measures	the 15 pure engine files	the 10 widget files
Needs	nothing but network	the Makepad Linux packages, so root
Cold / warm	~3 min / ~20 s	~6 min / ~4 s
Gated in CI	yes, with per-file floors	not yet

./tools/test-cad-coverage.sh                  # measure and enforce
KEEP_COVERAGE=1 ./tools/test-cad-coverage.sh  # keep the uncovered-line list

# local loop: seed the caches once, then each run takes ~20s
export CAD_COV_TOOLCHAIN_HOME=~/.cache/cad-cov \
       CAD_COV_TARGET_DIR=~/.cache/cad-cov/target

---

Reproducing

sudo apt-get install -y pkg-config libwayland-dev libxcursor-dev \
  libxrandr-dev libxi-dev libx11-dev libgl1-mesa-dev libasound2-dev \
  libpolkit-gobject-1-dev libpolkit-agent-1-dev libglib2.0-dev \
  libssl-dev libsqlite3-dev libudev-dev libpulse-dev libxkbcommon-dev

cargo test --locked -p nigig-build --lib
cargo test --locked -p doc-engine

libpulse-dev and libxkbcommon-dev are link-time-only requirements — cargo check passes without them, cargo test does not.