test(spreadsheet): raise the engine floor to 94, where the merge landed it

Rebasing onto a859053 and a88fb68 put the engine at 94.55%: their new
data.rs dependency-graph tests, measured through the every-binary fix in
the previous commit. A floor of 91 under a 94.55% measurement protects
nothing — three and a half points of regression would pass silently.

crates/apps/spreadsheet/spreadsheet-engine/src/persistence.rs

@@ -141,4 +141,106 @@ mod tests {
         assert!(validate_filename("backup-2024-01.tsv").is_ok());
         assert!(validate_filename("Book 1.tsv").is_ok());
     }
+
+    /// `save_spreadsheet_state_as` writes where it says it does.
+    ///
+    /// The validation above is tested in isolation, but nothing proved the
+    /// validated name is the name actually used — a function that
+    /// validates and then writes somewhere else would pass every test in
+    /// this module. So this reads the file back from the path it names.
+    ///
+    /// The filename is unique per test to stay safe under `cargo test`'s
+    /// default parallelism; these tests share one generated directory.
+    #[test]
+    fn save_as_writes_the_file_it_names() {
+        let name = "coverage-save-as.tsv";
+        save_spreadsheet_state_as(name, "SHEET\tone\n").expect("save_as failed");
+
+        let path = sheet_generated_dir_path().join(name);
+        let written = fs::read_to_string(&path).expect("the named file must exist");
+        assert_eq!(written, "SHEET\tone\n");
+
+        fs::remove_file(&path).ok();
+    }
+
+    /// A rejected filename must write **nothing**. Validating and then
+    /// writing anyway would be worse than not validating at all, because
+    /// the error return would say the write did not happen.
+    #[test]
+    fn a_rejected_filename_writes_nothing() {
+        let escape = "../coverage-escape.tsv";
+        let outside = sheet_generated_dir_path().join(escape);
+
+        // Remove any leftover before asserting. Without this the test is
+        // not idempotent: verifying it by mutation — making the function
+        // ignore its own validation — really does write the escape file,
+        // and every later run then fails on the *previous* run's debris
+        // rather than on the current code. Verified by doing exactly that.
+        fs::remove_file(&outside).ok();
+
+        assert!(save_spreadsheet_state_as(escape, "should never land").is_err());
+
+        let landed = outside.exists();
+        fs::remove_file(&outside).ok();
+        assert!(
+            !landed,
+            "a rejected filename still wrote a file at {}",
+            outside.display()
+        );
+    }
+
+    /// The legacy `current.sheet.csv` is read when the current-format file
+    /// is absent — the whole point of keeping the fallback.
+    ///
+    /// This runs single-threaded against the shared default filenames, so
+    /// it saves and restores whatever was already there rather than
+    /// clobbering another test's fixture.
+    #[test]
+    fn the_legacy_file_is_read_when_the_current_one_is_missing() {
+        let current = sheet_generated_file_path();
+        let legacy = sheet_generated_legacy_file_path();
+        let saved_current = fs::read_to_string(&current).ok();
+        let saved_legacy = fs::read_to_string(&legacy).ok();
+
+        fs::create_dir_all(sheet_generated_dir_path()).expect("generated dir");
+        fs::remove_file(&current).ok();
+        fs::write(&legacy, "LEGACY CONTENT\n").expect("write legacy");
+
+        assert_eq!(
+            load_saved_spreadsheet_state().as_deref(),
+            Some("LEGACY CONTENT\n"),
+            "with no current file the legacy one must be read"
+        );
+
+        // And the current format wins when both exist.
+        fs::write(&current, "CURRENT CONTENT\n").expect("write current");
+        assert_eq!(
+            load_saved_spreadsheet_state().as_deref(),
+            Some("CURRENT CONTENT\n"),
+            "the current format must take precedence over the legacy one"
+        );
+
+        // An empty current file must fall through to the legacy one rather
+        // than being returned as an empty workbook.
+        fs::write(&current, "   \n").expect("write blank current");
+        assert_eq!(
+            load_saved_spreadsheet_state().as_deref(),
+            Some("LEGACY CONTENT\n"),
+            "a whitespace-only current file must not shadow a real legacy one"
+        );
+
+        fs::remove_file(&current).ok();
+        fs::remove_file(&legacy).ok();
+        match saved_current {
+            Some(text) => {
+                fs::write(&current, text).ok();
+            }
+            None => {
+                fs::remove_file(&current).ok();
+            }
+        }
+        if let Some(text) = saved_legacy {
+            fs::write(&legacy, text).ok();
+        }
+    }
 }

crates/apps/spreadsheet/spreadsheet-engine/src/style.rs

@@ -166,3 +166,77 @@ pub fn color_from_str(s: &str) -> Option<Color> {
     let a: f32 = it.next()?.parse().ok()?;
     Some(Color::new(r, g, b, a))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Every format's code must round-trip through `from_code`.
+    ///
+    /// A table-driven test rather than six assertions, because the failure
+    /// mode being guarded against is a *copy-paste* one: two variants
+    /// sharing a code string, or `from_code` mapping one to another. Both
+    /// survive a spot check of the one variant someone happened to test,
+    /// and both silently rewrite a user's cell formatting on the next save
+    /// and load.
+    #[test]
+    fn every_number_format_code_round_trips() {
+        let all = [
+            NumberFormat::General,
+            NumberFormat::Currency,
+            NumberFormat::Percent,
+            NumberFormat::Decimal1,
+            NumberFormat::Decimal2,
+            NumberFormat::Integer,
+        ];
+        for format in all {
+            assert_eq!(
+                NumberFormat::from_code(format.code()),
+                format,
+                "{format:?} did not survive a code round trip"
+            );
+        }
+
+        // And the codes must all be distinct, or the round trip above
+        // passes while two formats are indistinguishable on disk.
+        let mut codes: Vec<&str> = all.iter().map(|f| f.code()).collect();
+        codes.sort_unstable();
+        let count = codes.len();
+        codes.dedup();
+        assert_eq!(codes.len(), count, "two number formats share a code");
+    }
+
+    #[test]
+    fn every_alignment_code_round_trips() {
+        let all = [CellAlign::Left, CellAlign::Center, CellAlign::Right];
+        for align in all {
+            assert_eq!(
+                CellAlign::from_code(align.code()),
+                align,
+                "{align:?} did not survive a code round trip"
+            );
+        }
+        let mut codes: Vec<&str> = all.iter().map(|a| a.code()).collect();
+        codes.sort_unstable();
+        let count = codes.len();
+        codes.dedup();
+        assert_eq!(codes.len(), count, "two alignments share a code");
+    }
+
+    /// An unknown code must fall back to the default rather than panic.
+    /// These strings come off disk, so they are attacker-influenced in
+    /// exactly the way a corrupt or hand-edited workbook is.
+    #[test]
+    fn unknown_codes_fall_back_to_the_default() {
+        for junk in ["", "ZZZ", "gen", " CUR", "🙂", "L "] {
+            assert_eq!(NumberFormat::from_code(junk), NumberFormat::General);
+            assert_eq!(CellAlign::from_code(junk), CellAlign::Left);
+        }
+    }
+
+    #[test]
+    fn the_defaults_are_the_ones_the_codes_fall_back_to() {
+        assert_eq!(NumberFormat::default(), NumberFormat::General);
+        assert_eq!(CellAlign::default(), CellAlign::Left);
+    }
+}

crates/apps/spreadsheet/spreadsheet-engine/src/undo.rs

@@ -193,6 +193,140 @@ mod tests {
     // `SpreadsheetData` is already in scope via `use crate::*`
     // (undo.rs imports `use crate::data::{CellData, CellId, SpreadsheetData};`).
 
+    /// Undo and redo of a resize must restore *and* re-apply, in both the
+    /// "there was an override" and "there was no override" directions.
+    ///
+    /// The `None` arms are the interesting half: a column that was using
+    /// the default width has no entry in `col_widths` at all, so undoing a
+    /// resize has to **remove** the key rather than write a default into
+    /// it. Writing a default looks identical until someone changes what the
+    /// default is, at which point every previously-resized-then-undone
+    /// column silently stops following it.
+    #[test]
+    fn undoing_a_column_resize_restores_the_previous_state() {
+        let mut data = SpreadsheetData::default();
+
+        // No override before: undo must leave no override.
+        let fresh = Change::ResizeColumn {
+            col: 3,
+            old: None,
+            new: Some(140.0),
+        };
+        fresh.apply_redo(&mut data);
+        assert_eq!(data.col_widths.get(&3), Some(&140.0));
+        fresh.apply_undo(&mut data);
+        assert!(
+            !data.col_widths.contains_key(&3),
+            "undo must remove the override, not store a default in it"
+        );
+
+        // An override before: undo must put the old value back.
+        data.col_widths.insert(3, 80.0);
+        let changed = Change::ResizeColumn {
+            col: 3,
+            old: Some(80.0),
+            new: Some(200.0),
+        };
+        changed.apply_redo(&mut data);
+        assert_eq!(data.col_widths.get(&3), Some(&200.0));
+        changed.apply_undo(&mut data);
+        assert_eq!(data.col_widths.get(&3), Some(&80.0));
+    }
+
+    #[test]
+    fn undoing_a_row_resize_restores_the_previous_state() {
+        let mut data = SpreadsheetData::default();
+
+        let fresh = Change::ResizeRow {
+            row: 7,
+            old: None,
+            new: Some(44.0),
+        };
+        fresh.apply_redo(&mut data);
+        assert_eq!(data.row_heights.get(&7), Some(&44.0));
+        fresh.apply_undo(&mut data);
+        assert!(
+            !data.row_heights.contains_key(&7),
+            "undo must remove the override, not store a default in it"
+        );
+
+        data.row_heights.insert(7, 20.0);
+        let changed = Change::ResizeRow {
+            row: 7,
+            old: Some(20.0),
+            new: Some(60.0),
+        };
+        changed.apply_redo(&mut data);
+        assert_eq!(data.row_heights.get(&7), Some(&60.0));
+        changed.apply_undo(&mut data);
+        assert_eq!(data.row_heights.get(&7), Some(&20.0));
+    }
+
+    /// Redoing a resize whose `new` is `None` — the shape produced by
+    /// resetting a column to its default — must clear the override.
+    #[test]
+    fn redoing_a_reset_to_default_clears_the_override() {
+        let mut data = SpreadsheetData::default();
+        data.col_widths.insert(1, 300.0);
+        data.row_heights.insert(2, 90.0);
+
+        Change::ResizeColumn {
+            col: 1,
+            old: Some(300.0),
+            new: None,
+        }
+        .apply_redo(&mut data);
+        assert!(!data.col_widths.contains_key(&1));
+
+        Change::ResizeRow {
+            row: 2,
+            old: Some(90.0),
+            new: None,
+        }
+        .apply_redo(&mut data);
+        assert!(!data.row_heights.contains_key(&2));
+    }
+
+    /// A whole `ChangeSet` round-trips: undo then redo lands back exactly
+    /// where it started, across all three change kinds at once.
+    #[test]
+    fn a_mixed_changeset_round_trips_through_undo_and_redo() {
+        let mut data = SpreadsheetData::default();
+        data.set_cell(0, 0, "before");
+
+        let old = data.cells.get(&CellId::new(0, 0)).cloned();
+        data.set_cell(0, 0, "after");
+        let new = data.cells.get(&CellId::new(0, 0)).cloned();
+
+        let mut set = ChangeSet::default();
+        set.push(Change::SetCell {
+            row: 0,
+            col: 0,
+            old,
+            new,
+        });
+        set.push(Change::ResizeColumn {
+            col: 5,
+            old: None,
+            new: Some(111.0),
+        });
+        set.push(Change::ResizeRow {
+            row: 6,
+            old: None,
+            new: Some(33.0),
+        });
+
+        set.apply_redo(&mut data);
+        assert_eq!(data.get_raw(0, 0), "after");
+        assert_eq!(data.col_widths.get(&5), Some(&111.0));
+        assert_eq!(data.row_heights.get(&6), Some(&33.0));
+
+        set.apply_undo(&mut data);
+        assert_eq!(data.get_raw(0, 0), "before");
+        assert!(!data.col_widths.contains_key(&5));
+        assert!(!data.row_heights.contains_key(&6));
+    }
+
     #[test]
     fn empty_changeset_is_empty() {
         assert!(ChangeSet::default().is_empty());

crates/apps/spreadsheet/spreadsheet-engine/src/util.rs

@@ -268,7 +268,12 @@ mod tests {
 
     #[test]
     fn format_number_floats() {
-        assert_eq!(format_number(3.14), "3.14");
+        // 3.15 rather than 3.14: clippy denies `approx_constant` because
+        // 3.14 is an approximation of PI, and the lint fires even in a test
+        // where the value is just a two-decimal number. Nothing about this
+        // assertion needs PI, so the number changes rather than the lint
+        // being silenced.
+        assert_eq!(format_number(3.15), "3.15");
         assert_eq!(format_number(0.5), "0.5");
         assert_eq!(format_number(-2.5), "-2.5");
         assert_eq!(format_number(1.0 / 3.0), "0.333333"); // 6 decimals

crates/apps/spreadsheet/spreadsheet-ui/Cargo.toml

@@ -16,7 +16,12 @@ description = "Makepad widget wrappers for the spreadsheet engine."
 #
 # For now, using a relative path from this crate's location:
 # crates/apps/spreadsheet/spreadsheet-ui/ → ../../../../makepad/widgets
-makepad-widgets = { git = "https://gitdab.com/andodeki/makepad", rev = "ecf5a572ab62a1c1598909971f602f99083671cc"}
+# The `test` feature gates makepad-widgets' re-export of makepad-test, which
+# `tests/ui.rs` imports as `makepad_widgets::makepad_test`. Without it that
+# file does not compile at all — it was checked in against a manifest that
+# never enabled the feature, so `cargo test --all-targets` failed outright
+# and `cargo test --lib` skipped it silently. Matches `pdf-makepad`.
+makepad-widgets = { git = "https://gitdab.com/andodeki/makepad", rev = "ecf5a572ab62a1c1598909971f602f99083671cc", features = ["test"] }
 spreadsheet-engine = { path = "../spreadsheet-engine" }
 
 [dev-dependencies]

crates/apps/spreadsheet/spreadsheet-ui/src/model.rs

@@ -221,4 +221,162 @@ mod tests {
             })
         );
     }
+
+    /// `dispatch_input` must actually undo and redo, not merely return
+    /// `true`. The model is a thin adapter, and a thin adapter wired to the
+    /// wrong method still type-checks: `Undo => self.workbook.redo()` is a
+    /// single-word mistake that no signature catches. So the cell value is
+    /// asserted at each step rather than the boolean.
+    #[test]
+    fn undo_and_redo_intents_reach_the_workbook() {
+        let mut model = WorkspaceModel::new(Workbook::new());
+        // `apply_batch`, not `apply`: only the batch entry point calls
+        // `begin_recording`, so a bare `apply` leaves nothing on the undo
+        // stack and `dispatch_input(Undo)` correctly returns false. That
+        // asymmetry is the trap pinned by the engine's
+        // `only_set_cell_records_its_own_undo_step`, and this test walked
+        // straight into it on first run.
+        model.apply_batch([spreadsheet_engine::WorkbookCommand::SetCell {
+            row: 0,
+            col: 0,
+            value: "typed".into(),
+        }]);
+        assert_eq!(model.workbook().get_raw(0, 0), "typed");
+
+        assert!(model.dispatch_input(crate::input::InputIntent::Undo));
+        assert_eq!(
+            model.workbook().get_raw(0, 0),
+            "",
+            "the Undo intent must undo the edit"
+        );
+
+        assert!(model.dispatch_input(crate::input::InputIntent::Redo));
+        assert_eq!(
+            model.workbook().get_raw(0, 0),
+            "typed",
+            "the Redo intent must put it back"
+        );
+    }
+
+    /// With nothing left to undo, the intent must report `false` rather
+    /// than claiming it did something.
+    #[test]
+    fn undo_and_redo_report_false_when_there_is_nothing_to_do() {
+        let mut model = WorkspaceModel::new(Workbook::new());
+        assert!(!model.dispatch_input(crate::input::InputIntent::Undo));
+        assert!(!model.dispatch_input(crate::input::InputIntent::Redo));
+    }
+
+    /// The recording asymmetry, stated from the model's side.
+    ///
+    /// A bare `apply` does not open an undo step, so the Undo intent has
+    /// nothing to pop. This is surprising enough that it is worth an
+    /// explicit test at this layer too: a caller reaching for
+    /// `model.apply(..)` and then offering the user an undo button gets a
+    /// button that does nothing, with no error anywhere.
+    #[test]
+    fn a_bare_apply_leaves_nothing_for_the_undo_intent() {
+        let mut model = WorkspaceModel::new(Workbook::new());
+        model.apply(spreadsheet_engine::WorkbookCommand::SetCell {
+            row: 0,
+            col: 0,
+            value: "unrecorded".into(),
+        });
+        assert_eq!(model.workbook().get_raw(0, 0), "unrecorded");
+        assert!(
+            !model.dispatch_input(crate::input::InputIntent::Undo),
+            "apply() does not call begin_recording, so there is no step to \
+             undo; use apply_batch() when the edit must be undoable"
+        );
+    }
+
+    /// `from_parts` and `into_parts` must be inverses. They are the seam
+    /// the widget uses to hand its sheets to the model and take them back,
+    /// so a mismatch in the active index silently switches the user's
+    /// visible sheet on every round trip.
+    #[test]
+    fn from_parts_and_into_parts_round_trip() {
+        let model = WorkspaceModel::from_parts(
+            vec![
+                Sheet::empty("Alpha"),
+                Sheet::empty("Beta"),
+                Sheet::empty("Gamma"),
+            ],
+            2,
+        );
+        assert_eq!(model.active_sheet(), 2);
+        assert_eq!(model.active_sheet_name(), "Gamma");
+
+        let (sheets, active) = model.into_parts();
+        assert_eq!(active, 2, "the active index must survive the round trip");
+        assert_eq!(
+            sheets.iter().map(|s| s.name.as_str()).collect::<Vec<_>>(),
+            vec!["Alpha", "Beta", "Gamma"]
+        );
+    }
+
+    #[test]
+    fn into_workbook_hands_back_the_same_content() {
+        let mut model = WorkspaceModel::new(Workbook::new());
+        model.apply(spreadsheet_engine::WorkbookCommand::SetCell {
+            row: 1,
+            col: 1,
+            value: "kept".into(),
+        });
+        let workbook = model.into_workbook();
+        assert_eq!(workbook.get_raw(1, 1), "kept");
+    }
+
+    /// The read-only and mutable accessors must address the *same* sheet.
+    /// Two accessors that disagree about which sheet is active is the
+    /// desynchronisation the whole model exists to prevent.
+    #[test]
+    fn the_active_sheet_accessors_agree_with_each_other() {
+        let mut model = WorkspaceModel::new(Workbook::with_sheets_active(
+            vec![Sheet::empty("One"), Sheet::empty("Two")],
+            1,
+        ));
+        model.active_sheet_data_mut().set_cell(3, 4, "written");
+        assert_eq!(
+            model.active_sheet_data().get_raw(3, 4),
+            "written",
+            "the immutable accessor must see the mutable one's write"
+        );
+        // And it must be sheet 1, not sheet 0.
+        assert_eq!(model.sheet_data_mut(1).unwrap().get_raw(3, 4), "written");
+        assert_eq!(model.sheet_data_mut(0).unwrap().get_raw(3, 4), "");
+        assert!(model.sheet_data_mut(99).is_none());
+    }
+
+    /// `save` writes and `load_saved` reads it back.
+    ///
+    /// These are the two functions that touch the user's actual file, and
+    /// they were the only wholly uncovered ones left in this module. The
+    /// test is `#[ignore]`d by default: it writes to the engine crate's
+    /// shared `generated/` directory, so running it concurrently with the
+    /// engine's own persistence tests makes both flaky for reasons that
+    /// have nothing to do with the code. Run it deliberately:
+    ///
+    /// ```bash
+    /// cargo test -p spreadsheet-ui --lib -- --ignored --test-threads=1
+    /// ```
+    #[test]
+    #[ignore = "writes the shared generated/ workbook file; run single-threaded"]
+    fn save_and_load_round_trip_through_disk() {
+        let mut model = WorkspaceModel::new(Workbook::new());
+        model.add_sheet("Persisted");
+        model.apply(spreadsheet_engine::WorkbookCommand::SetCell {
+            row: 0,
+            col: 0,
+            value: "on disk".into(),
+        });
+
+        model.save().expect("save must succeed");
+        let loaded = WorkspaceModel::load_saved().expect("load must find the file");
+        assert_eq!(loaded.workbook().get_raw(0, 0), "on disk");
+        assert!(
+            loaded.sheet_names().any(|n| n == "Persisted"),
+            "the sheet added before saving must come back"
+        );
+    }
 }

crates/apps/spreadsheet/spreadsheet-ui/tests/ui.rs

@@ -1,3 +1,28 @@
+//! UI tests driving the spreadsheet workspace through Makepad event
+//! delivery.
+//!
+//! **Currently `#[ignore]`d, for the same reason as `pdf-makepad`'s.** The
+//! fork's `makepad_test` sets `MAKEPAD=headless` for the app it spawns, but
+//! `platform/src/os/linux/windowing_backend.rs` only knows `X11` and
+//! `Wayland` — there is no headless backend, so the app selects X11, finds
+//! no display and dies. The full diagnosis is in
+//! `REVIEWS/PDF_PARITY_PHASE1_STATUS.md`.
+//!
+//! Until this file was reached by `--all-targets` it had **never compiled**:
+//! `spreadsheet-ui` did not enable `makepad-widgets`' `test` feature, so the
+//! `makepad_widgets::makepad_test` import failed. `cargo test --lib` never
+//! built it and nothing reported the breakage. The manifest now enables the
+//! feature, matching `pdf-makepad`, so the file is compiled on every run and
+//! cannot rot further while appearing to be coverage.
+//!
+//! Ignored rather than deleted or left failing on purpose: a red suite
+//! everyone knows to disregard stops reporting the next real regression.
+//! Remove the markers when the fork grows a headless backend.
+//!
+//! ```bash
+//! cargo test -p spreadsheet-ui --test ui -- --ignored --test-threads=1
+//! ```
+
 use makepad_widgets::makepad_test::{run_with_config, Selector, TestApp, TestConfig};
 
 fn run_ui_test(test_name: &str, body: impl FnOnce(TestApp)) {
@@ -36,6 +61,7 @@ fn run_ui_test(test_name: &str, body: impl FnOnce(TestApp)) {
 }
 
 #[test]
+#[ignore = "needs a Makepad headless backend; see REVIEWS/PDF_PARITY_PHASE1_STATUS.md"]
 fn spreadsheet_workspace_smoke() {
     run_ui_test("spreadsheet_workspace_smoke", |app: TestApp| {
         app.locator(Selector::id("formula_input")).wait_visible();
@@ -45,6 +71,7 @@ fn spreadsheet_workspace_smoke() {
 }
 
 #[test]
+#[ignore = "needs a Makepad headless backend; see REVIEWS/PDF_PARITY_PHASE1_STATUS.md"]
 fn spreadsheet_workspace_can_add_sheet() {
     run_ui_test("spreadsheet_workspace_can_add_sheet", |app: TestApp| {
         app.locator(Selector::id("add_sheet_btn"))

tools/test-spreadsheet-coverage.sh

@@ -22,8 +22,8 @@ IFS=$'\n\t'
 ROOT="$(CDPATH= cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)"
 COVERAGE_TARGET="${COVERAGE_TARGET:-all}"
 KEEP_COVERAGE="${KEEP_COVERAGE:-0}"
-ENGINE_FLOOR="${ENGINE_FLOOR:-90}"
-UI_FLOOR="${UI_FLOOR:-94}"
+ENGINE_FLOOR="${ENGINE_FLOOR:-94}"
+UI_FLOOR="${UI_FLOOR:-95}"
 
 case "$COVERAGE_TARGET" in
     all | engine | ui) ;;
@@ -116,20 +116,49 @@ IGNORE_COMMON='(/cargo/registry|/cargo/git|/rustc/)'
 # grid.rs, ui.rs and workspace.rs are the three files carrying `script_mod!`.
 IGNORE_UI="$IGNORE_COMMON"'|spreadsheet-ui/src/(grid|ui|workspace)\.rs|spreadsheet-ui/src/bin/'
 
+# Collect every instrumented test binary as `-object` arguments.
+#
+# One binary is not enough, and the shortfall is silent. Cargo builds each
+# integration test into its *own* executable, so measuring only the lib-test
+# binary discards everything `tests/` exercised. That is not a rounding
+# error: `persistence.rs` reported 41.77% with 14 of its 17 functions
+# apparently never called, while `tests/sync_flow.rs` was calling
+# `save_spreadsheet_state` and `load_saved_spreadsheet_state` on every run
+# and passing. The functions were covered; the report was reading the wrong
+# object. Same class of defect as the two already fixed in this script.
+collect_objects() {
+    local pattern="$1"
+    OBJECTS=()
+    local bin first=1
+    while IFS= read -r bin; do
+        # llvm-cov takes the first binary positionally and the rest through
+        # `-object`. Passing every one as `-object` makes it read the first
+        # *source* path as the positional binary and fail with "not a valid
+        # object file", so the split matters.
+        if (( first )); then
+            OBJECTS+=("$bin")
+            first=0
+        else
+            OBJECTS+=(-object "$bin")
+        fi
+    done < <(find "$CARGO_TARGET_DIR/debug/deps" -maxdepth 1 -type f \
+        -executable ! -name '*.d' ! -name '*.so' -name "$pattern" | sort)
+}
+
 report_for() {
-    local label="$1" test_bin="$2" profdata="$3" ignore="$4" floor="$5"
-    shift 5
+    local label="$1" profdata="$2" ignore="$3" floor="$4"
+    shift 4
     local sources=("$@")
 
     echo >&2
     echo "── $label ─────────────────────────────────────────────" >&2
-    "$LLVM_BIN/llvm-cov" report "$test_bin" \
+    "$LLVM_BIN/llvm-cov" report "${OBJECTS[@]}" \
         -instr-profile="$profdata" \
         -ignore-filename-regex="$ignore" \
         "${sources[@]}"
 
     local covered
-    covered="$("$LLVM_BIN/llvm-cov" export "$test_bin" \
+    covered="$("$LLVM_BIN/llvm-cov" export "${OBJECTS[@]}" \
         -instr-profile="$profdata" \
         -ignore-filename-regex="$ignore" \
         -summary-only "${sources[@]}" \
@@ -148,9 +177,9 @@ report_for() {
 }
 
 uncovered_listing() {
-    local label="$1" test_bin="$2" profdata="$3" ignore="$4"
-    shift 4
-    "$LLVM_BIN/llvm-cov" show "$test_bin" \
+    local label="$1" profdata="$2" ignore="$3"
+    shift 3
+    "$LLVM_BIN/llvm-cov" show "${OBJECTS[@]}" \
         -instr-profile="$profdata" \
         -ignore-filename-regex="$ignore" \
         "$@" \
@@ -175,18 +204,25 @@ if [[ "$COVERAGE_TARGET" == "all" || "$COVERAGE_TARGET" == "engine" ]]; then
 
     "$LLVM_BIN/llvm-profdata" merge -sparse "$WORK/profiles"/*.profraw \
         -o "$WORK/engine.profdata"
-    ENGINE_BIN="$(find "$CARGO_TARGET_DIR/debug/deps" -maxdepth 1 -type f \
-        -executable -name 'spreadsheet_engine-*' | head -1)"
-    if [[ -z "$ENGINE_BIN" ]]; then
-        echo "error: could not find the engine test binary" >&2
+    # Every engine test binary: the lib tests plus each file in tests/.
+    collect_objects '*'
+    if [[ "${#OBJECTS[@]}" -eq 0 ]]; then
+        echo "error: no instrumented engine test binaries were produced" >&2
         exit 1
     fi
 
-    report_for "engine" "$ENGINE_BIN" "$WORK/engine.profdata" \
-        "$IGNORE_COMMON" "$ENGINE_FLOOR" "$ENGINE/src"
+    # A directory is accepted as a source filter only with a single
+    # `-object`; with several, llvm-cov rejects it. Expand to the files.
+    ENGINE_SOURCES=()
+    while IFS= read -r f; do
+        ENGINE_SOURCES+=("$f")
+    done < <(find "$ENGINE/src" -name '*.rs' | sort)
+
+    report_for "engine" "$WORK/engine.profdata" \
+        "$IGNORE_COMMON" "$ENGINE_FLOOR" "${ENGINE_SOURCES[@]}"
     if [[ "$KEEP_COVERAGE" == "1" ]]; then
-        uncovered_listing "engine" "$ENGINE_BIN" "$WORK/engine.profdata" \
-            "$IGNORE_COMMON" "$ENGINE/src"
+        uncovered_listing "engine" "$WORK/engine.profdata" \
+            "$IGNORE_COMMON" "${ENGINE_SOURCES[@]}"
     fi
 fi
 
@@ -203,25 +239,26 @@ if [[ "$COVERAGE_TARGET" == "all" || "$COVERAGE_TARGET" == "ui" ]]; then
         "$WORK/spreadsheet/spreadsheet-ui"
     UI="$WORK/spreadsheet/spreadsheet-ui"
 
-    cargo test --manifest-path "$UI/Cargo.toml" --lib
+    # `--all-targets` rather than `--lib`, so a future tests/ file is
+    # measured the day it is added instead of being silently ignored.
+    cargo test --manifest-path "$UI/Cargo.toml" --all-targets
 
     "$LLVM_BIN/llvm-profdata" merge -sparse "$WORK/profiles"/*.profraw \
         -o "$WORK/ui.profdata"
-    UI_BIN="$(find "$CARGO_TARGET_DIR/debug/deps" -maxdepth 1 -type f \
-        -executable -name 'spreadsheet_ui-*' | head -1)"
-    if [[ -z "$UI_BIN" ]]; then
-        echo "error: could not find the UI test binary" >&2
+    collect_objects '*'
+    if [[ "${#OBJECTS[@]}" -eq 0 ]]; then
+        echo "error: no instrumented UI test binaries were produced" >&2
         exit 1
     fi
 
     # Only the controller modules are measured; see the exclusion note above.
-    report_for "ui-controllers" "$UI_BIN" "$WORK/ui.profdata" \
+    report_for "ui-controllers" "$WORK/ui.profdata" \
         "$IGNORE_UI" "$UI_FLOOR" \
         "$UI/src/clipboard.rs" "$UI/src/edit.rs" "$UI/src/event_router.rs" \
         "$UI/src/input.rs" "$UI/src/model.rs" "$UI/src/render_cache.rs" \
         "$UI/src/selection.rs" "$UI/src/text_measure.rs"
     if [[ "$KEEP_COVERAGE" == "1" ]]; then
-        uncovered_listing "ui" "$UI_BIN" "$WORK/ui.profdata" "$IGNORE_UI" \
+        uncovered_listing "ui" "$WORK/ui.profdata" "$IGNORE_UI" \
             "$UI/src/clipboard.rs" "$UI/src/edit.rs" "$UI/src/event_router.rs" \
             "$UI/src/input.rs" "$UI/src/model.rs" "$UI/src/render_cache.rs" \
             "$UI/src/selection.rs" "$UI/src/text_measure.rs"