Add SnapshotResults struct to egui_kittest (#5672)

I got annoyed by all the slightly different variations of "collect
snapshot results and unwrap them at the end of test" I've written, so I
added a struct to make this nice and simple.

One controversial thing: It panics when dropped. I wanted to ensure
people cannot forget to unwrap the results at the end, and this was the
best thing I could come up with. I don't think this is possible via
clippy lint or something like that.

* [x] I have followed the instructions in the PR template

Author: lucasmerlin

.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:

crates/egui/src/painter.rs

@@ -435,7 +435,6 @@ impl Painter {
         self.add(RectShape::filled(rect, corner_radius, fill_color))
     }
 
-    /// The stroke extends _outside_ the [`Rect`].
     pub fn rect_stroke(
         &self,
         rect: Rect,

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,6 @@ 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);
-        }
-    }
-
-    if let Some(error) = results.first() {
-        panic!("{error}");
+        results.add(harness.try_snapshot(&anchor.to_string()));
     }
 }

crates/egui_demo_lib/src/demo/demo_app_windows.rs

@@ -366,13 +366,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
@@ -406,12 +406,7 @@ 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:#?}");
     }
 }

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,18 @@ 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"));
-
-        for result in results {
-            result.unwrap();
-        }
+        results.add(harness.try_snapshot("modals_3"));
     }
 
     // This tests whether the backdrop actually prevents interaction with lower layers.

crates/egui_demo_lib/src/demo/widget_gallery.rs

@@ -23,6 +23,7 @@ pub struct WidgetGallery {
     #[cfg_attr(feature = "serde", serde(skip))]
     date: Option<chrono::NaiveDate>,
 
+    #[cfg(feature = "chrono")]
     with_date_button: bool,
 }
 

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,7 @@ 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:#?}");
     }
 }

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,
@@ -306,7 +308,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())
 }
 
@@ -378,7 +380,7 @@ impl<State> Harness<'_, State> {
         &mut self,
         name: &str,
         options: &SnapshotOptions,
-    ) -> Result<(), SnapshotError> {
+    ) -> SnapshotResult {
         let image = self
             .render()
             .map_err(|err| SnapshotError::RenderError { err })?;
@@ -393,7 +395,7 @@ impl<State> 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 })?;
@@ -460,15 +462,15 @@ impl<State> Harness<'_, State> {
         &mut self,
         name: &str,
         options: &SnapshotOptions,
-    ) -> Result<(), SnapshotError> {
+    ) -> SnapshotResult {
         self.try_snapshot_options(name, options)
     }
 
     #[deprecated(
         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)
     }
 
@@ -488,3 +490,105 @@ impl<State> 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"));
+///
+/// // 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<SnapshotError>,
+}
+
+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<SnapshotError> {
+        std::mem::take(&mut self.errors)
+    }
+
+    /// Panics if there are any errors, displaying each.
+    #[allow(clippy::unused_self)]
+    #[track_caller]
+    pub fn unwrap(self) {
+        // Panic is handled in drop
+    }
+}
+
+impl From<SnapshotResults> for Vec<SnapshotError> {
+    fn from(results: SnapshotResults) -> Self {
+        results.into_inner()
+    }
+}
+
+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;
+        }
+        #[allow(clippy::manual_assert)]
+        if self.has_errors() {
+            panic!("{}", self);
+        }
+    }
+}

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,18 +64,18 @@ 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();
 
     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
@@ -87,10 +87,4 @@ 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);
-        }
-    }
 }

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"));
 }