# `nigig-build` test baseline ``` cargo test --locked -p nigig-build --lib test result: ok. 750 passed; 0 failed; 10 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 10 ignored are the CAD `profile_benchmarks`, 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** | 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` 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. | --- ## Reproducing ```bash 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.