diff --git a/crates/apps/spreadsheet/spreadsheet-engine/src/persistence.rs b/crates/apps/spreadsheet/spreadsheet-engine/src/persistence.rs index a2ed84e..6b18dae 100644 --- a/crates/apps/spreadsheet/spreadsheet-engine/src/persistence.rs +++ b/crates/apps/spreadsheet/spreadsheet-engine/src/persistence.rs @@ -141,106 +141,4 @@ 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(¤t).ok(); - let saved_legacy = fs::read_to_string(&legacy).ok(); - - fs::create_dir_all(sheet_generated_dir_path()).expect("generated dir"); - fs::remove_file(¤t).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(¤t, "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(¤t, " \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(¤t).ok(); - fs::remove_file(&legacy).ok(); - match saved_current { - Some(text) => { - fs::write(¤t, text).ok(); - } - None => { - fs::remove_file(¤t).ok(); - } - } - if let Some(text) = saved_legacy { - fs::write(&legacy, text).ok(); - } - } } diff --git a/crates/apps/spreadsheet/spreadsheet-engine/src/style.rs b/crates/apps/spreadsheet/spreadsheet-engine/src/style.rs index 5fa2b42..e2efb5c 100644 --- a/crates/apps/spreadsheet/spreadsheet-engine/src/style.rs +++ b/crates/apps/spreadsheet/spreadsheet-engine/src/style.rs @@ -166,77 +166,3 @@ pub fn color_from_str(s: &str) -> Option { 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); - } -} diff --git a/crates/apps/spreadsheet/spreadsheet-engine/src/undo.rs b/crates/apps/spreadsheet/spreadsheet-engine/src/undo.rs index 44bf869..f761f1b 100644 --- a/crates/apps/spreadsheet/spreadsheet-engine/src/undo.rs +++ b/crates/apps/spreadsheet/spreadsheet-engine/src/undo.rs @@ -193,140 +193,6 @@ 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()); diff --git a/crates/apps/spreadsheet/spreadsheet-engine/src/util.rs b/crates/apps/spreadsheet/spreadsheet-engine/src/util.rs index 044cf5b..710e75b 100644 --- a/crates/apps/spreadsheet/spreadsheet-engine/src/util.rs +++ b/crates/apps/spreadsheet/spreadsheet-engine/src/util.rs @@ -268,12 +268,7 @@ mod tests { #[test] fn format_number_floats() { - // 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(3.14), "3.14"); 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 diff --git a/crates/apps/spreadsheet/spreadsheet-ui/Cargo.toml b/crates/apps/spreadsheet/spreadsheet-ui/Cargo.toml index d13c1d4..eae46c4 100644 --- a/crates/apps/spreadsheet/spreadsheet-ui/Cargo.toml +++ b/crates/apps/spreadsheet/spreadsheet-ui/Cargo.toml @@ -16,12 +16,7 @@ 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 -# 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"] } +makepad-widgets = { git = "https://gitdab.com/andodeki/makepad", rev = "ecf5a572ab62a1c1598909971f602f99083671cc"} spreadsheet-engine = { path = "../spreadsheet-engine" } [dev-dependencies] diff --git a/crates/apps/spreadsheet/spreadsheet-ui/src/model.rs b/crates/apps/spreadsheet/spreadsheet-ui/src/model.rs index d141c3a..ec341a3 100644 --- a/crates/apps/spreadsheet/spreadsheet-ui/src/model.rs +++ b/crates/apps/spreadsheet/spreadsheet-ui/src/model.rs @@ -221,162 +221,4 @@ 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!["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" - ); - } } diff --git a/crates/apps/spreadsheet/spreadsheet-ui/tests/ui.rs b/crates/apps/spreadsheet/spreadsheet-ui/tests/ui.rs index 637946e..466fd0a 100644 --- a/crates/apps/spreadsheet/spreadsheet-ui/tests/ui.rs +++ b/crates/apps/spreadsheet/spreadsheet-ui/tests/ui.rs @@ -1,28 +1,3 @@ -//! 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)) { @@ -61,7 +36,6 @@ 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(); @@ -71,7 +45,6 @@ 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")) diff --git a/tools/test-spreadsheet-coverage.sh b/tools/test-spreadsheet-coverage.sh index 1c0e086..d5068c6 100755 --- a/tools/test-spreadsheet-coverage.sh +++ b/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:-94}" -UI_FLOOR="${UI_FLOOR:-95}" +ENGINE_FLOOR="${ENGINE_FLOOR:-90}" +UI_FLOOR="${UI_FLOOR:-94}" case "$COVERAGE_TARGET" in all | engine | ui) ;; @@ -116,49 +116,20 @@ 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" profdata="$2" ignore="$3" floor="$4" - shift 4 + local label="$1" test_bin="$2" profdata="$3" ignore="$4" floor="$5" + shift 5 local sources=("$@") echo >&2 echo "── $label ─────────────────────────────────────────────" >&2 - "$LLVM_BIN/llvm-cov" report "${OBJECTS[@]}" \ + "$LLVM_BIN/llvm-cov" report "$test_bin" \ -instr-profile="$profdata" \ -ignore-filename-regex="$ignore" \ "${sources[@]}" local covered - covered="$("$LLVM_BIN/llvm-cov" export "${OBJECTS[@]}" \ + covered="$("$LLVM_BIN/llvm-cov" export "$test_bin" \ -instr-profile="$profdata" \ -ignore-filename-regex="$ignore" \ -summary-only "${sources[@]}" \ @@ -177,9 +148,9 @@ report_for() { } uncovered_listing() { - local label="$1" profdata="$2" ignore="$3" - shift 3 - "$LLVM_BIN/llvm-cov" show "${OBJECTS[@]}" \ + local label="$1" test_bin="$2" profdata="$3" ignore="$4" + shift 4 + "$LLVM_BIN/llvm-cov" show "$test_bin" \ -instr-profile="$profdata" \ -ignore-filename-regex="$ignore" \ "$@" \ @@ -204,25 +175,18 @@ if [[ "$COVERAGE_TARGET" == "all" || "$COVERAGE_TARGET" == "engine" ]]; then "$LLVM_BIN/llvm-profdata" merge -sparse "$WORK/profiles"/*.profraw \ -o "$WORK/engine.profdata" - # 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 + 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 exit 1 fi - # 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[@]}" + report_for "engine" "$ENGINE_BIN" "$WORK/engine.profdata" \ + "$IGNORE_COMMON" "$ENGINE_FLOOR" "$ENGINE/src" if [[ "$KEEP_COVERAGE" == "1" ]]; then - uncovered_listing "engine" "$WORK/engine.profdata" \ - "$IGNORE_COMMON" "${ENGINE_SOURCES[@]}" + uncovered_listing "engine" "$ENGINE_BIN" "$WORK/engine.profdata" \ + "$IGNORE_COMMON" "$ENGINE/src" fi fi @@ -239,26 +203,25 @@ if [[ "$COVERAGE_TARGET" == "all" || "$COVERAGE_TARGET" == "ui" ]]; then "$WORK/spreadsheet/spreadsheet-ui" UI="$WORK/spreadsheet/spreadsheet-ui" - # `--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 + cargo test --manifest-path "$UI/Cargo.toml" --lib "$LLVM_BIN/llvm-profdata" merge -sparse "$WORK/profiles"/*.profraw \ -o "$WORK/ui.profdata" - collect_objects '*' - if [[ "${#OBJECTS[@]}" -eq 0 ]]; then - echo "error: no instrumented UI test binaries were produced" >&2 + 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 exit 1 fi # Only the controller modules are measured; see the exclusion note above. - report_for "ui-controllers" "$WORK/ui.profdata" \ + report_for "ui-controllers" "$UI_BIN" "$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" "$WORK/ui.profdata" "$IGNORE_UI" \ + uncovered_listing "ui" "$UI_BIN" "$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"