From 77e5820e2c26ad943f8040ba0f9677bebe7a1974 Mon Sep 17 00:00:00 2001 From: Lucas Meurer Date: Tue, 4 Feb 2025 11:31:19 +0100 Subject: [PATCH 1/5] Add SnapshotResults struct to egui_kittest --- crates/egui_demo_app/tests/test_demo_app.rs | 11 +- .../src/demo/demo_app_windows.rs | 11 +- crates/egui_demo_lib/src/demo/modals.rs | 14 +-- .../egui_demo_lib/src/demo/widget_gallery.rs | 1 + crates/egui_demo_lib/src/rendering_test.rs | 10 +- crates/egui_kittest/src/snapshot.rs | 110 +++++++++++++++++- crates/egui_kittest/tests/regression_tests.rs | 14 +-- crates/egui_kittest/tests/tests.rs | 6 +- 8 files changed, 132 insertions(+), 45 deletions(-) diff --git a/crates/egui_demo_app/tests/test_demo_app.rs b/crates/egui_demo_app/tests/test_demo_app.rs index fc4940fb9049..92dc73e5c495 100644 --- a/crates/egui_demo_app/tests/test_demo_app.rs +++ b/crates/egui_demo_app/tests/test_demo_app.rs @@ -2,6 +2,7 @@ use egui::accesskit::Role; use egui::Vec2; use egui_demo_app::{Anchor, WrapApp}; use egui_kittest::kittest::Queryable; +use egui_kittest::SnapshotResults; #[test] fn test_demo_app() { @@ -27,7 +28,7 @@ fn test_demo_app() { "Expected to find the Custom3d app.", ); - let mut results = vec![]; + let mut results = SnapshotResults::new(); for (name, anchor) in apps { harness.get_by_role_and_label(Role::Button, name).click(); @@ -68,12 +69,8 @@ fn test_demo_app() { // Can't use Harness::run because fractal clock keeps requesting repaints harness.run_steps(2); - if let Err(e) = harness.try_snapshot(&anchor.to_string()) { - results.push(e); - } + results.add(harness.try_snapshot(&anchor.to_string())); } - if let Some(error) = results.first() { - panic!("{error}"); - } + results.unwrap(); } diff --git a/crates/egui_demo_lib/src/demo/demo_app_windows.rs b/crates/egui_demo_lib/src/demo/demo_app_windows.rs index c8b142cacd5a..3e4c165d5b82 100644 --- a/crates/egui_demo_lib/src/demo/demo_app_windows.rs +++ b/crates/egui_demo_lib/src/demo/demo_app_windows.rs @@ -365,13 +365,13 @@ mod tests { use crate::{demo::demo_app_windows::DemoGroups, Demo}; use egui::Vec2; use egui_kittest::kittest::Queryable; - use egui_kittest::{Harness, SnapshotOptions}; + use egui_kittest::{Harness, SnapshotOptions, SnapshotResults}; #[test] fn demos_should_match_snapshot() { let demos = DemoGroups::default().demos; - let mut errors = Vec::new(); + let mut results = SnapshotResults::new(); for mut demo in demos.demos { // Widget Gallery needs to be customized (to set a specific date) and has its own test @@ -405,12 +405,9 @@ mod tests { options.threshold = 2.1; } - let result = harness.try_snapshot_options(&format!("demos/{name}"), &options); - if let Err(err) = result { - errors.push(err.to_string()); - } + results.add(harness.try_snapshot_options(&format!("demos/{name}"), &options)); } - assert!(errors.is_empty(), "Errors: {errors:#?}"); + results.unwrap(); } } diff --git a/crates/egui_demo_lib/src/demo/modals.rs b/crates/egui_demo_lib/src/demo/modals.rs index 833e07a28ed1..461747ec8f62 100644 --- a/crates/egui_demo_lib/src/demo/modals.rs +++ b/crates/egui_demo_lib/src/demo/modals.rs @@ -165,7 +165,7 @@ mod tests { use egui::accesskit::Role; use egui::Key; use egui_kittest::kittest::Queryable; - use egui_kittest::Harness; + use egui_kittest::{Harness, SnapshotResults}; #[test] fn clicking_escape_when_popup_open_should_not_close_modal() { @@ -233,22 +233,20 @@ mod tests { initial_state, ); - let mut results = Vec::new(); + let mut results = SnapshotResults::new(); harness.run(); - results.push(harness.try_snapshot("modals_1")); + results.add(harness.try_snapshot("modals_1")); harness.get_by_label("Save").click(); harness.run_ok(); - results.push(harness.try_snapshot("modals_2")); + results.add(harness.try_snapshot("modals_2")); harness.get_by_label("Yes Please").click(); harness.run_ok(); - results.push(harness.try_snapshot("modals_3")); + results.add(harness.try_snapshot("modals_3")); - for result in results { - result.unwrap(); - } + results.unwrap(); } // This tests whether the backdrop actually prevents interaction with lower layers. diff --git a/crates/egui_demo_lib/src/demo/widget_gallery.rs b/crates/egui_demo_lib/src/demo/widget_gallery.rs index c51a9efd884c..eae07ccd56a1 100644 --- a/crates/egui_demo_lib/src/demo/widget_gallery.rs +++ b/crates/egui_demo_lib/src/demo/widget_gallery.rs @@ -23,6 +23,7 @@ pub struct WidgetGallery { #[cfg_attr(feature = "serde", serde(skip))] date: Option, + #[cfg(feature = "chrono")] with_date_button: bool, } diff --git a/crates/egui_demo_lib/src/rendering_test.rs b/crates/egui_demo_lib/src/rendering_test.rs index d43116e0b3f5..79520a6be890 100644 --- a/crates/egui_demo_lib/src/rendering_test.rs +++ b/crates/egui_demo_lib/src/rendering_test.rs @@ -688,10 +688,11 @@ fn mul_color_gamma(left: Color32, right: Color32) -> Color32 { mod tests { use crate::ColorTest; use egui_kittest::kittest::Queryable as _; + use egui_kittest::SnapshotResults; #[test] pub fn rendering_test() { - let mut errors = vec![]; + let mut results = SnapshotResults::new(); for dpi in [1.0, 1.25, 1.5, 1.75, 1.6666667, 2.0] { let mut color_test = ColorTest::default(); let mut harness = egui_kittest::Harness::builder() @@ -708,12 +709,9 @@ mod tests { harness.fit_contents(); - let result = harness.try_snapshot(&format!("rendering_test/dpi_{dpi:.2}")); - if let Err(err) = result { - errors.push(err); - } + results.add(harness.try_snapshot(&format!("rendering_test/dpi_{dpi:.2}"))); } - assert!(errors.is_empty(), "Errors: {errors:#?}"); + results.unwrap(); } } diff --git a/crates/egui_kittest/src/snapshot.rs b/crates/egui_kittest/src/snapshot.rs index 0b68677b06b2..5295bf7e07ca 100644 --- a/crates/egui_kittest/src/snapshot.rs +++ b/crates/egui_kittest/src/snapshot.rs @@ -4,6 +4,8 @@ use std::fmt::Display; use std::io::ErrorKind; use std::path::PathBuf; +pub type SnapshotResult = Result<(), SnapshotError>; + #[non_exhaustive] pub struct SnapshotOptions { /// The threshold for the image comparison. @@ -189,7 +191,7 @@ pub fn try_image_snapshot_options( new: &image::RgbaImage, name: &str, options: &SnapshotOptions, -) -> Result<(), SnapshotError> { +) -> SnapshotResult { let SnapshotOptions { threshold, output_path, @@ -301,7 +303,7 @@ pub fn try_image_snapshot_options( /// # Errors /// Returns a [`SnapshotError`] if the image does not match the snapshot or if there was an error /// reading or writing the snapshot. -pub fn try_image_snapshot(current: &image::RgbaImage, name: &str) -> Result<(), SnapshotError> { +pub fn try_image_snapshot(current: &image::RgbaImage, name: &str) -> SnapshotResult { try_image_snapshot_options(current, name, &SnapshotOptions::default()) } @@ -373,7 +375,7 @@ impl Harness<'_, State> { &mut self, name: &str, options: &SnapshotOptions, - ) -> Result<(), SnapshotError> { + ) -> SnapshotResult { let image = self .render() .map_err(|err| SnapshotError::RenderError { err })?; @@ -388,7 +390,7 @@ impl Harness<'_, State> { /// # Errors /// Returns a [`SnapshotError`] if the image does not match the snapshot, if there was an /// error reading or writing the snapshot, if the rendering fails or if no default renderer is available. - pub fn try_snapshot(&mut self, name: &str) -> Result<(), SnapshotError> { + pub fn try_snapshot(&mut self, name: &str) -> SnapshotResult { let image = self .render() .map_err(|err| SnapshotError::RenderError { err })?; @@ -455,7 +457,7 @@ impl Harness<'_, State> { &mut self, name: &str, options: &SnapshotOptions, - ) -> Result<(), SnapshotError> { + ) -> SnapshotResult { self.try_snapshot_options(name, options) } @@ -463,7 +465,7 @@ impl Harness<'_, State> { since = "0.31.0", note = "Use `try_snapshot` instead. This function will be removed in 0.32" )] - pub fn try_wgpu_snapshot(&mut self, name: &str) -> Result<(), SnapshotError> { + pub fn try_wgpu_snapshot(&mut self, name: &str) -> SnapshotResult { self.try_snapshot(name) } @@ -483,3 +485,99 @@ impl Harness<'_, State> { self.snapshot(name); } } + +/// Utility to collect snapshot errors and display them at the end of the test. +/// +/// # Example +/// ``` +/// # let harness = MockHarness; +/// # struct MockHarness; +/// # impl MockHarness { +/// # fn try_snapshot(&self, _: &str) -> Result<(), egui_kittest::SnapshotError> { Ok(()) } +/// # } +/// +/// // [...] Construct a Harness +/// +/// let mut results = egui_kittest::SnapshotResults::new(); +/// +/// // Call add for each snapshot in your test +/// results.add(harness.try_snapshot("my_test")); +/// +/// // At the end of the test, fail if there are any errors +/// results.unwrap(); +/// ``` +/// +/// # Panics +/// Panics if there are any errors when dropped (this way it is impossible to forget to call `unwrap`). +#[derive(Debug, Default)] +pub struct SnapshotResults { + errors: Vec, +} + +impl Display for SnapshotResults { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if self.errors.is_empty() { + write!(f, "All snapshots passed") + } else { + writeln!(f, "Snapshot errors:")?; + for error in &self.errors { + writeln!(f, " {error}")?; + } + Ok(()) + } + } +} + +impl SnapshotResults { + pub fn new() -> Self { + Default::default() + } + + /// Check if the result is an error and add it to the list of errors. + pub fn add(&mut self, result: SnapshotResult) { + if let Err(err) = result { + self.errors.push(err); + } + } + + /// Check if there are any errors. + pub fn has_errors(&self) -> bool { + !self.errors.is_empty() + } + + /// Convert this into a `Result<(), Self>`. + #[allow(clippy::missing_errors_doc)] + pub fn into_result(self) -> Result<(), Self> { + if self.has_errors() { + Err(self) + } else { + Ok(()) + } + } + + pub fn into_inner(mut self) -> Vec { + std::mem::take(&mut self.errors) + } + + /// Panics if there are any errors, displaying each. + #[allow(clippy::unused_self)] + pub fn unwrap(self) { + // Panic is handled in drop + } +} + +impl From for Vec { + fn from(results: SnapshotResults) -> Self { + results.into_inner() + } +} + +impl Drop for SnapshotResults { + fn drop(&mut self) { + // Don't panic if we are already panicking (the test probably failed for another reason) + if std::thread::panicking() { + return; + } + assert!(!self.has_errors(), "{}", self); + } +} diff --git a/crates/egui_kittest/tests/regression_tests.rs b/crates/egui_kittest/tests/regression_tests.rs index 8567e10ea90b..bf7538b8ae72 100644 --- a/crates/egui_kittest/tests/regression_tests.rs +++ b/crates/egui_kittest/tests/regression_tests.rs @@ -1,6 +1,6 @@ use egui::accesskit::Role; use egui::{Button, ComboBox, Image, Vec2, Widget}; -use egui_kittest::{kittest::Queryable, Harness}; +use egui_kittest::{kittest::Queryable, Harness, SnapshotResults}; #[test] pub fn focus_should_skip_over_disabled_buttons() { @@ -64,10 +64,10 @@ fn test_combobox() { harness.run(); - let mut results = vec![]; + let mut results = SnapshotResults::new(); #[cfg(all(feature = "wgpu", feature = "snapshot"))] - results.push(harness.try_snapshot("combobox_closed")); + results.add(harness.try_snapshot("combobox_closed")); let combobox = harness.get_by_role_and_label(Role::ComboBox, "Select Something"); combobox.click(); @@ -75,7 +75,7 @@ fn test_combobox() { harness.run(); #[cfg(all(feature = "wgpu", feature = "snapshot"))] - results.push(harness.try_snapshot("combobox_opened")); + results.add(harness.try_snapshot("combobox_opened")); let item_2 = harness.get_by_role_and_label(Role::Button, "Item 2"); // Node::click doesn't close the popup, so we use simulate_click @@ -88,9 +88,5 @@ fn test_combobox() { // Popup should be closed now assert!(harness.query_by_label("Item 2").is_none()); - for result in results { - if let Err(err) = result { - panic!("{}", err); - } - } + results.unwrap(); } diff --git a/crates/egui_kittest/tests/tests.rs b/crates/egui_kittest/tests/tests.rs index bf7d0bdb2206..29b4c7b114a6 100644 --- a/crates/egui_kittest/tests/tests.rs +++ b/crates/egui_kittest/tests/tests.rs @@ -1,4 +1,4 @@ -use egui_kittest::Harness; +use egui_kittest::{Harness, SnapshotResults}; #[test] fn test_shrink() { @@ -10,6 +10,8 @@ fn test_shrink() { harness.fit_contents(); + let mut results = SnapshotResults::new(); + #[cfg(all(feature = "snapshot", feature = "wgpu"))] - harness.snapshot("test_shrink"); + results.add(harness.try_snapshot("test_shrink")); } From dde36dd888508f38ef0c1478c0a84fe00fdae736 Mon Sep 17 00:00:00 2001 From: Lucas Meurer Date: Tue, 4 Feb 2025 11:54:15 +0100 Subject: [PATCH 2/5] Remove unwraps --- crates/egui_demo_app/tests/test_demo_app.rs | 2 -- crates/egui_demo_lib/src/demo/demo_app_windows.rs | 2 -- crates/egui_demo_lib/src/demo/modals.rs | 2 -- crates/egui_demo_lib/src/rendering_test.rs | 2 -- crates/egui_kittest/tests/regression_tests.rs | 2 -- 5 files changed, 10 deletions(-) diff --git a/crates/egui_demo_app/tests/test_demo_app.rs b/crates/egui_demo_app/tests/test_demo_app.rs index 92dc73e5c495..0247b9fc292f 100644 --- a/crates/egui_demo_app/tests/test_demo_app.rs +++ b/crates/egui_demo_app/tests/test_demo_app.rs @@ -71,6 +71,4 @@ fn test_demo_app() { results.add(harness.try_snapshot(&anchor.to_string())); } - - results.unwrap(); } diff --git a/crates/egui_demo_lib/src/demo/demo_app_windows.rs b/crates/egui_demo_lib/src/demo/demo_app_windows.rs index 3e4c165d5b82..97038e9a229a 100644 --- a/crates/egui_demo_lib/src/demo/demo_app_windows.rs +++ b/crates/egui_demo_lib/src/demo/demo_app_windows.rs @@ -407,7 +407,5 @@ mod tests { results.add(harness.try_snapshot_options(&format!("demos/{name}"), &options)); } - - results.unwrap(); } } diff --git a/crates/egui_demo_lib/src/demo/modals.rs b/crates/egui_demo_lib/src/demo/modals.rs index 461747ec8f62..d344d99c010f 100644 --- a/crates/egui_demo_lib/src/demo/modals.rs +++ b/crates/egui_demo_lib/src/demo/modals.rs @@ -245,8 +245,6 @@ mod tests { harness.get_by_label("Yes Please").click(); harness.run_ok(); results.add(harness.try_snapshot("modals_3")); - - results.unwrap(); } // This tests whether the backdrop actually prevents interaction with lower layers. diff --git a/crates/egui_demo_lib/src/rendering_test.rs b/crates/egui_demo_lib/src/rendering_test.rs index 79520a6be890..e83e643bb83d 100644 --- a/crates/egui_demo_lib/src/rendering_test.rs +++ b/crates/egui_demo_lib/src/rendering_test.rs @@ -711,7 +711,5 @@ mod tests { results.add(harness.try_snapshot(&format!("rendering_test/dpi_{dpi:.2}"))); } - - results.unwrap(); } } diff --git a/crates/egui_kittest/tests/regression_tests.rs b/crates/egui_kittest/tests/regression_tests.rs index bf7538b8ae72..e7186dcac601 100644 --- a/crates/egui_kittest/tests/regression_tests.rs +++ b/crates/egui_kittest/tests/regression_tests.rs @@ -87,6 +87,4 @@ fn test_combobox() { // Popup should be closed now assert!(harness.query_by_label("Item 2").is_none()); - - results.unwrap(); } From b1fd8790a519bb85386bbec14c8332caa4c771cf Mon Sep 17 00:00:00 2001 From: Lucas Meurer Date: Tue, 4 Feb 2025 11:54:28 +0100 Subject: [PATCH 3/5] Documentation improvements --- crates/egui_kittest/src/snapshot.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/egui_kittest/src/snapshot.rs b/crates/egui_kittest/src/snapshot.rs index 5295bf7e07ca..3f7c92c4e1c8 100644 --- a/crates/egui_kittest/src/snapshot.rs +++ b/crates/egui_kittest/src/snapshot.rs @@ -503,12 +503,13 @@ impl Harness<'_, State> { /// // Call add for each snapshot in your test /// results.add(harness.try_snapshot("my_test")); /// -/// // At the end of the test, fail if there are any errors -/// results.unwrap(); +/// // If there are any errors, SnapshotResults will panic once dropped. /// ``` /// /// # Panics /// Panics if there are any errors when dropped (this way it is impossible to forget to call `unwrap`). +/// If you don't want to panic, you can use [`SnapshotResults::into_result`] or [`SnapshotResults::into_inner`]. +/// If you want to panic early, you can use [`SnapshotResults::unwrap`]. #[derive(Debug, Default)] pub struct SnapshotResults { errors: Vec, From 17e1ba177af4782ea42b359dfd58c670c3cb2572 Mon Sep 17 00:00:00 2001 From: Lucas Meurer Date: Tue, 4 Feb 2025 11:54:45 +0100 Subject: [PATCH 4/5] General doc fixes --- .github/workflows/rust.yml | 4 ++-- crates/egui/src/painter.rs | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index f0e3b3a7282c..eadce83bec25 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -9,7 +9,7 @@ env: jobs: fmt-crank-check-test: - name: Format + check + test + name: Format + check runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v4 @@ -223,7 +223,7 @@ jobs: tests: name: Run tests - # We run the tests on macOS because it will run with a actual GPU + # We run the tests on macOS because it will run with an actual GPU runs-on: macos-latest steps: diff --git a/crates/egui/src/painter.rs b/crates/egui/src/painter.rs index 4ed74cddba06..a2bc7585d4e1 100644 --- a/crates/egui/src/painter.rs +++ b/crates/egui/src/painter.rs @@ -435,7 +435,6 @@ impl Painter { self.add(RectShape::filled(rect, rounding, fill_color)) } - /// The stroke extends _outside_ the [`Rect`]. pub fn rect_stroke( &self, rect: Rect, From 128236dfc0a0ab8bfe3ac48e156219d47dc4ddab Mon Sep 17 00:00:00 2001 From: Lucas Meurer Date: Tue, 4 Feb 2025 13:52:23 +0100 Subject: [PATCH 5/5] Improve error logging --- crates/egui_kittest/src/snapshot.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/egui_kittest/src/snapshot.rs b/crates/egui_kittest/src/snapshot.rs index 3f7c92c4e1c8..4a3443b0fa25 100644 --- a/crates/egui_kittest/src/snapshot.rs +++ b/crates/egui_kittest/src/snapshot.rs @@ -562,6 +562,7 @@ impl SnapshotResults { /// Panics if there are any errors, displaying each. #[allow(clippy::unused_self)] + #[track_caller] pub fn unwrap(self) { // Panic is handled in drop } @@ -574,11 +575,15 @@ impl From for Vec { } impl Drop for SnapshotResults { + #[track_caller] fn drop(&mut self) { // Don't panic if we are already panicking (the test probably failed for another reason) if std::thread::panicking() { return; } - assert!(!self.has_errors(), "{}", self); + #[allow(clippy::manual_assert)] + if self.has_errors() { + panic!("{}", self); + } } }