From 6405b00cf70545eb133f9bbf06a2a99c6cef3923 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Tue, 13 Oct 2026 00:34:01 -0400 Subject: [PATCH] Fix-up: repair test files after merging the conflicting PRs - restore closing braces the line-level merges of side-by-side tests dropped - new ChartData literals carry the palette field - the filter-menu shortcut passes the row with the column - #688's tab tests account for #851's All Sheets button - Serbian margins for the ruler-margin labels Co-Authored-By: Claude Opus 5.5 --- crates/calc/src/tests.rs | 7 +++++++ crates/chart/src/tests.rs | 21 +++++++++++++++------ crates/engine/src/tests.rs | 6 ++++++ crates/formula/src/tests.rs | 3 +++ crates/pdf/src/tests.rs | 2 ++ crates/ui-egui/src/i18n/sr.rs | 6 ++++++ crates/ui-egui/src/keymap.rs | 2 +- crates/ui-egui/tests/clipboard.rs | 3 +++ crates/ui-egui/tests/keyboard_shortcuts.rs | 4 ++-- crates/ui-egui/tests/sheet_tabs.rs | 6 ++++-- crates/xlsx/src/sheet_read.rs | 7 ++----- crates/xlsx/src/tests/roundtrip.rs | 14 +++++++------- crates/xlsx/src/tests/xls_tests.rs | 6 ++++++ 13 files changed, 64 insertions(+), 23 deletions(-) diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index 8b29f631b..a0c9fe26d 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -571,6 +571,9 @@ fn functions_given_by_name() { // A LET variable of that name is still the variable. t.set("J1", "=LET(ABS,LAMBDA(x,x*10),SUM(MAP(B1:B3,ABS)))"); assert_eq!(t.num("J1"), 20.0); +} + +#[test] fn lambda_argument_count() { let mut t = T::new(); t.wb.names.push(gridcraft_model::DefinedName { @@ -1134,6 +1137,8 @@ fn cell_width_is_the_column_width_dialog_value_rounded() { t.set(at, &format!("=CELL(\"width\",{col}1)")); } assert_eq!([t.num("H1"), t.num("H2"), t.num("H3"), t.num("H4"), t.num("H5")], [8.0, 11.0, 12.0, 11.0, 3.0]); +} + /// Expected results are Excel's, from Apache POI's `DGet.xls` (rows 46-49): a criteria cell holding /// a formula under a label that names no field is evaluated for every record. #[test] @@ -1184,6 +1189,8 @@ fn database_functions_with_computed_criteria() { assert_eq!(t.num("B51"), 1.0); assert_eq!(t.num("B52"), 7.0); assert_eq!(t.num("B53"), 4.0); +} + #[test] fn offset_extreme_arguments_return_ref() { let wb = Workbook::new(); diff --git a/crates/chart/src/tests.rs b/crates/chart/src/tests.rs index 1093e9209..01f1aac49 100644 --- a/crates/chart/src/tests.rs +++ b/crates/chart/src/tests.rs @@ -456,11 +456,8 @@ fn pie_starts_at_twelve_clockwise() { #[test] fn negative_values_put_axis_below_zero() { let c = chart(ChartKind::ColumnClustered); - let d = ChartData { - palette: vec![], - categories: vec![], - series: vec![series("N", &[-5.0, 3.0, -2.0], [1, 2, 3, 255], ChartKind::ColumnClustered)], - }; + let d = + ChartData { palette: vec![], categories: vec![], series: vec![series("N", &[-5.0, 3.0, -2.0], [1, 2, 3, 255], ChartKind::ColumnClustered)] }; let prims = render(&c, &d, 400.0, 300.0, &ApproxMeasure); let t = texts(&prims); assert!(t.iter().any(|s| s.starts_with('-') || s.starts_with("($") || s.starts_with("-$")), "{t:?}"); @@ -901,7 +898,7 @@ fn treemap_and_histogram_shapes() { #[test] fn histogram_boundary_values_match_interval_labels() { let heights = |vals: &[f64]| -> Vec { - let d = ChartData { categories: vec![], series: vec![series("h", vals, [10, 20, 30, 255], ChartKind::Histogram)] }; + let d = ChartData { palette: vec![], categories: vec![], series: vec![series("h", vals, [10, 20, 30, 255], ChartKind::Histogram)] }; let prims = render(&chart(ChartKind::Histogram), &d, 600.0, 400.0, &ApproxMeasure); prims.iter().filter_map(|p| if let Prim::Rect { h, fill: Some([10, 20, 30, 255]), .. } = p { Some(*h) } else { None }).collect() }; @@ -964,8 +961,13 @@ fn box_and_stock_charts_honour_the_gridlines_flag() { let off = render(&c, &d, 500.0, 350.0, &ApproxMeasure); assert!(grids(&on) > 0, "{kind:?}: gridlines on must draw lines"); assert_eq!(grids(&off), 0, "{kind:?}: gridlines off must hide them"); + } +} + +#[test] fn funnel_value_labels_follow_the_data_labels_flag() { let data = ChartData { + palette: vec![], categories: vec!["a".into(), "b".into(), "c".into()], series: vec![series("S", &[11111.0, 22222.0, 33333.0], [20, 96, 130, 255], ChartKind::Funnel)], }; @@ -982,6 +984,10 @@ fn funnel_value_labels_follow_the_data_labels_flag() { for cat in ["a", "b", "c"] { assert!(texts(&off).contains(&cat)); assert!(texts(&on).contains(&cat)); + } +} + +#[test] fn pie_points_follow_workbook_theme() { let mut wb = Workbook::new(); wb.theme.colors[4] = 0x4F6D8A; @@ -1024,6 +1030,9 @@ fn pie_points_follow_workbook_theme() { assert_eq!(legend.len(), 40); assert_eq!(legend.get(37), Some(&series_color(&wb.theme, 37))); assert_ne!(series_color(&wb.theme, 37), series_color(&gridcraft_model::Theme::default(), 37)); +} + +#[test] fn area_charts_draw_source_value_labels_only_when_enabled() { for kind in [ChartKind::Area, ChartKind::AreaStacked] { let d = sample(kind); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index de592f5c0..0225b838c 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1086,6 +1086,9 @@ fn structured_refs_to_columns_with_edge_spaces() { s.execute("file.open", json!({"name": "x.xlsx", "base64": b64})).unwrap(); assert_eq!(s.execute("cell.get", json!({"cell": "F3"})).unwrap()["formula"], "=SUBTOTAL(109,Table1[[Qty ]])"); assert_eq!([v(&s, "F1"), v(&s, "F2"), v(&s, "F3")], [n(5.0), n(5.0), n(5.0)]); +} + +#[test] fn table_rename_keeps_a_defined_name_with_the_same_name() { let mut s = s(); s.execute("range.setValues", json!({"range": "A1", "values": [["Qty"], [7]]})).unwrap(); @@ -3054,6 +3057,9 @@ fn subtotal_109_recalculates_when_row_height_or_autofit_reveals_rows() { assert_eq!(v(&s, "C1"), Value::Number(9.0)); s.execute("home.autofitRowHeight", json!({"rows": "2:3"})).unwrap(); assert_eq!(v(&s, "C1"), Value::Number(15.0)); +} + +#[test] fn paste_special_validation_copies_the_rule() { let mut s = s(); let ok = |s: &mut Session, cell: &str, input: &str| s.execute("data.validate", json!({"cell": cell, "input": input})).unwrap()["ok"] == true; diff --git a/crates/formula/src/tests.rs b/crates/formula/src/tests.rs index c8359b19d..103a26bd1 100644 --- a/crates/formula/src/tests.rs +++ b/crates/formula/src/tests.rs @@ -10,6 +10,9 @@ fn quoted_sheet_qualified_name_keeps_quotes() { assert_eq!(rt("'My Sheet'!Total"), "'My Sheet'!Total"); assert_eq!(rt("'Data'!Total"), "Data!Total"); assert_eq!(rt("'It''s'!Total+1"), "'It''s'!Total+1"); +} + +#[test] fn intersection_with_deleted_operand_reparses() { // Deleting the columns of an intersection operand prints `A1:B2 #REF!`; that text must parse back. assert_eq!(rt("A1:B2 #REF!"), "A1:B2 #REF!"); diff --git a/crates/pdf/src/tests.rs b/crates/pdf/src/tests.rs index 6483cc737..7d6a61a45 100644 --- a/crates/pdf/src/tests.rs +++ b/crates/pdf/src/tests.rs @@ -236,6 +236,8 @@ fn every_draw_method_survives_hostile_numbers() { assert!(!c.contains("NaN") && !c.contains("inf")); let pdf = d.finish(); check_xref(&pdf); +} + /// Decodes the `` hex string following `key` in the file bytes. fn utf16_entry(pdf: &[u8], key: &str) -> String { let at = find(pdf, key.as_bytes()).unwrap() + key.len(); diff --git a/crates/ui-egui/src/i18n/sr.rs b/crates/ui-egui/src/i18n/sr.rs index bc6a8ee02..152bddee8 100644 --- a/crates/ui-egui/src/i18n/sr.rs +++ b/crates/ui-egui/src/i18n/sr.rs @@ -391,6 +391,12 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Workbook calculation" => "Izračunavanje radne sveske", "Enable iterative calculation" => "Omogući iterativno izračunavanje", "Calculation settings are saved with the workbook." => "Podešavanja izračunavanja se čuvaju sa radnom sveskom.", + "Left Margin" => "Leva margina", + "Right Margin" => "Desna margina", + "Top Margin" => "Gornja margina", + "Bottom Margin" => "Donja margina", + "Header Margin" => "Margina zaglavlja", + "Footer Margin" => "Margina podnožja", _ => return None, }) } diff --git a/crates/ui-egui/src/keymap.rs b/crates/ui-egui/src/keymap.rs index cb4f7c6fe..032db1a2d 100644 --- a/crates/ui-egui/src/keymap.rs +++ b/crates/ui-egui/src/keymap.rs @@ -223,7 +223,7 @@ pub(crate) fn dispatch(app: &mut SheetApp, ctx: &egui::Context, key: Key, physic app.grid.list_picker = Some(active); } else if sh.has_filter_button(active) { let at = geo.mirror_pos(crate::grid::filter_button_rect(geo, sh, active).center()); - app.grid.filter_menu = Some((active.col, at)); + app.grid.filter_menu = Some((active.col, active.row, at)); } } Shortcut::Zoom(step) => { diff --git a/crates/ui-egui/tests/clipboard.rs b/crates/ui-egui/tests/clipboard.rs index bf835d893..08f83b53c 100644 --- a/crates/ui-egui/tests/clipboard.rs +++ b/crates/ui-egui/tests/clipboard.rs @@ -139,6 +139,9 @@ fn enter_pastes_the_copied_cells_and_ends_copy_mode() { h.key_press(egui::Key::Enter); h.run_steps(2); assert_eq!(h.state().session.active().map(|d| d.selection.active.a1()), Some("D6".to_string())); +} + +#[test] fn clicking_the_ribbon_paste_button_or_its_menu_does_not_freeze() { // #273: a click on Paste deadlocked the UI thread (a click-state query inside the egui context // lock), so the frame never returned. Run the clicks on a worker and fail on a timeout instead of diff --git a/crates/ui-egui/tests/keyboard_shortcuts.rs b/crates/ui-egui/tests/keyboard_shortcuts.rs index ae0ce5c66..c0a1307b3 100644 --- a/crates/ui-egui/tests/keyboard_shortcuts.rs +++ b/crates/ui-egui/tests/keyboard_shortcuts.rs @@ -160,7 +160,7 @@ fn alt_down_opens_autofilter_headers_without_changing_the_workbook() { let mut h = harness(session); let before = h.state().session.active().unwrap().wb.clone(); key(&mut h, Key::ArrowDown, Modifiers::ALT); - let (column, at) = h.state().grid.filter_menu.expect("Alt+Down must open the filter header menu"); + let (column, _row, at) = h.state().grid.filter_menu.expect("Alt+Down must open the filter header menu"); let d = h.state().session.active().unwrap(); let active = CellRef::parse(cell).unwrap(); assert_eq!(column, active.col); @@ -210,7 +210,7 @@ fn alt_down_only_opens_enabled_table_filter_buttons() { session.execute("selection.set", json!({"cell": "B1"})).unwrap(); let mut h = harness(session); key(&mut h, Key::ArrowDown, Modifiers::ALT); - assert_eq!(h.state().grid.filter_menu.map(|(column, _)| column), enabled.then_some(1)); + assert_eq!(h.state().grid.filter_menu.map(|(column, _, _)| column), enabled.then_some(1)); assert_eq!(h.state().session.active().unwrap().selection.active.a1(), "B1"); } } diff --git a/crates/ui-egui/tests/sheet_tabs.rs b/crates/ui-egui/tests/sheet_tabs.rs index 966ea60f0..e799f7a89 100644 --- a/crates/ui-egui/tests/sheet_tabs.rs +++ b/crates/ui-egui/tests/sheet_tabs.rs @@ -71,7 +71,8 @@ fn opening_and_activating_late_sheets_reveals_their_tabs() { fn navigation_arrows_reveal_the_selected_tab() { let mut h = harness(700.0, 0); let (_, clip) = tab(&h, "Tab00").unwrap(); - let next = pos2(clip.left() - 20.0, clip.center().y); + // The Next arrow sits left of the All Sheets button (24 px) and the 8 px gap before the tabs. + let next = pos2(clip.left() - 44.0, clip.center().y); for _ in 0..29 { click(&mut h, next); } @@ -129,7 +130,8 @@ fn hidden_sheets_long_names_and_window_resize_keep_active_tab_visible() { h.state_mut().run("sheet.activate", json!({"sheet":29})).unwrap(); h.run_steps(20); assert_visible(&h, name); - h.set_size(vec2(500.0, 120.0)); + // Narrow, but still wide enough for the long tab next to the arrows and the All Sheets button. + h.set_size(vec2(560.0, 120.0)); h.run_steps(20); assert_visible(&h, name); h.set_size(vec2(1000.0, 120.0)); diff --git a/crates/xlsx/src/sheet_read.rs b/crates/xlsx/src/sheet_read.rs index 61857a7ef..1aa9b407b 100644 --- a/crates/xlsx/src/sheet_read.rs +++ b/crates/xlsx/src/sheet_read.rs @@ -598,11 +598,8 @@ fn read_cf(cx: &mut Ctx<'_>, e: &El, sheet: &mut Sheet) { let f0 = formulas.first().cloned().unwrap_or_default(); let text = r.attr("text").unwrap_or("").to_string(); // The x14 id that links a data bar to its Excel 2010 extension (solid fill lives there). - let ext_id = r - .child("extLst") - .and_then(|l| l.kids("ext").find_map(|x| x.child("id"))) - .map(|i| i.text.trim().to_string()) - .filter(|i| !i.is_empty()); + let ext_id = + r.child("extLst").and_then(|l| l.kids("ext").find_map(|x| x.child("id"))).map(|i| i.text.trim().to_string()).filter(|i| !i.is_empty()); let rule = match r.attr("type").unwrap_or("") { "cellIs" => CfRule::CellIs { op: cf_op(r.attr("operator")), a: f0, b: formulas.get(1).cloned(), style }, "expression" => CfRule::Expression { formula: f0, style }, diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs index d214fd214..e9d72d5ad 100644 --- a/crates/xlsx/src/tests/roundtrip.rs +++ b/crates/xlsx/src/tests/roundtrip.rs @@ -1096,18 +1096,15 @@ fn default_font_survives_a_round_trip() { ); let saved = write_xlsx(&read_xlsx(&file).unwrap().0).unwrap(); assert_eq!(font0(&saved), (Some("12".into()), Some("Calibri".into()), Some("2".into()), Some("minor".into()))); +} + #[test] fn solid_data_bar_survives_with_sparklines_and_gradient_neighbours() { let mut wb = Workbook::new(); let s = wb.sheet_mut(0).unwrap(); s.set_value(at("A1"), Value::Number(1.0)); - let bar = |gradient: bool, max: CfValueKind| CfRule::DataBar { - min: CfValueKind::Min, - max, - color: Color::Rgb(0x638EC6), - gradient, - show_value: true, - }; + let bar = + |gradient: bool, max: CfValueKind| CfRule::DataBar { min: CfValueKind::Min, max, color: Color::Rgb(0x638EC6), gradient, show_value: true }; let rules = [(true, "A1", CfValueKind::Max), (false, "B1", CfValueKind::Formula("$C$1".into())), (false, "D1:D3", CfValueKind::Number(9.0))]; for (i, (gradient, range, max)) in rules.into_iter().enumerate() { s.cond_formats.push(CondFormat { ranges: vec![rr(range)], rule: bar(gradient, max), priority: i as u32 + 1, stop_if_true: false }); @@ -1122,6 +1119,9 @@ fn solid_data_bar_survives_with_sparklines_and_gradient_neighbours() { let b = &back.sheets[0]; assert_eq!(b.cond_formats, wb.sheets[0].cond_formats); assert_eq!(b.sparklines.len(), 1); +} + +#[test] fn doc_props_keep_boundary_whitespace() { let mut wb = Workbook::new(); wb.props = DocProps { diff --git a/crates/xlsx/src/tests/xls_tests.rs b/crates/xlsx/src/tests/xls_tests.rs index 38820a114..91e63c18f 100644 --- a/crates/xlsx/src/tests/xls_tests.rs +++ b/crates/xlsx/src/tests/xls_tests.rs @@ -535,6 +535,9 @@ fn formulas_using_a_skipped_name_keep_their_saved_results() { assert_eq!(v(&wb, 0, "B6"), Value::Number(60.0)); assert_eq!(ftext(&wb, "B7").as_deref(), Some("Local*2")); assert!(report.warnings.iter().any(|w| w.contains("defined names") && w.contains("saved results")), "{:?}", report.warnings); +} + +#[test] fn reads_iterative_calculation_settings() { // CalcMode, CalcCount, CalcIter and CalcDelta repeat in every worksheet substream; here only // the first sheet carries them, so the sheet after it must not reset them to the defaults. @@ -554,6 +557,9 @@ fn reads_iterative_calculation_settings() { // Without the records: Excel's defaults, iteration off. let (wb, _) = read_xls(&compound("Workbook", &sample(false))).unwrap(); assert_eq!(wb.calc, gridcraft_model::CalcSettings::default()); +} + +#[test] fn a_format_shorter_than_its_count_keeps_its_characters() { // A FORMAT whose cch is one more than the characters in the record (seen in real files): // Excel opens them, the record's characters are the format code.