Skip to content

Commit b8051cc

Browse files
authored
Add SnapshotResults struct to egui_kittest (emilk#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
1 parent 23ed493 commit b8051cc

File tree

10 files changed

+135
-53
lines changed

10 files changed

+135
-53
lines changed

.github/workflows/rust.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ env:
99

1010
jobs:
1111
fmt-crank-check-test:
12-
name: Format + check + test
12+
name: Format + check
1313
runs-on: ubuntu-22.04
1414
steps:
1515
- uses: actions/checkout@v4
@@ -223,7 +223,7 @@ jobs:
223223

224224
tests:
225225
name: Run tests
226-
# We run the tests on macOS because it will run with a actual GPU
226+
# We run the tests on macOS because it will run with an actual GPU
227227
runs-on: macos-latest
228228

229229
steps:

crates/egui/src/painter.rs

-1
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,6 @@ impl Painter {
435435
self.add(RectShape::filled(rect, corner_radius, fill_color))
436436
}
437437

438-
/// The stroke extends _outside_ the [`Rect`].
439438
pub fn rect_stroke(
440439
&self,
441440
rect: Rect,

crates/egui_demo_app/tests/test_demo_app.rs

+3-8
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use egui::accesskit::Role;
22
use egui::Vec2;
33
use egui_demo_app::{Anchor, WrapApp};
44
use egui_kittest::kittest::Queryable;
5+
use egui_kittest::SnapshotResults;
56

67
#[test]
78
fn test_demo_app() {
@@ -27,7 +28,7 @@ fn test_demo_app() {
2728
"Expected to find the Custom3d app.",
2829
);
2930

30-
let mut results = vec![];
31+
let mut results = SnapshotResults::new();
3132

3233
for (name, anchor) in apps {
3334
harness.get_by_role_and_label(Role::Button, name).click();
@@ -68,12 +69,6 @@ fn test_demo_app() {
6869
// Can't use Harness::run because fractal clock keeps requesting repaints
6970
harness.run_steps(2);
7071

71-
if let Err(e) = harness.try_snapshot(&anchor.to_string()) {
72-
results.push(e);
73-
}
74-
}
75-
76-
if let Some(error) = results.first() {
77-
panic!("{error}");
72+
results.add(harness.try_snapshot(&anchor.to_string()));
7873
}
7974
}

crates/egui_demo_lib/src/demo/demo_app_windows.rs

+3-8
Original file line numberDiff line numberDiff line change
@@ -366,13 +366,13 @@ mod tests {
366366
use crate::{demo::demo_app_windows::DemoGroups, Demo};
367367
use egui::Vec2;
368368
use egui_kittest::kittest::Queryable;
369-
use egui_kittest::{Harness, SnapshotOptions};
369+
use egui_kittest::{Harness, SnapshotOptions, SnapshotResults};
370370

371371
#[test]
372372
fn demos_should_match_snapshot() {
373373
let demos = DemoGroups::default().demos;
374374

375-
let mut errors = Vec::new();
375+
let mut results = SnapshotResults::new();
376376

377377
for mut demo in demos.demos {
378378
// Widget Gallery needs to be customized (to set a specific date) and has its own test
@@ -406,12 +406,7 @@ mod tests {
406406
options.threshold = 2.1;
407407
}
408408

409-
let result = harness.try_snapshot_options(&format!("demos/{name}"), &options);
410-
if let Err(err) = result {
411-
errors.push(err.to_string());
412-
}
409+
results.add(harness.try_snapshot_options(&format!("demos/{name}"), &options));
413410
}
414-
415-
assert!(errors.is_empty(), "Errors: {errors:#?}");
416411
}
417412
}

crates/egui_demo_lib/src/demo/modals.rs

+5-9
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ mod tests {
165165
use egui::accesskit::Role;
166166
use egui::Key;
167167
use egui_kittest::kittest::Queryable;
168-
use egui_kittest::Harness;
168+
use egui_kittest::{Harness, SnapshotResults};
169169

170170
#[test]
171171
fn clicking_escape_when_popup_open_should_not_close_modal() {
@@ -233,22 +233,18 @@ mod tests {
233233
initial_state,
234234
);
235235

236-
let mut results = Vec::new();
236+
let mut results = SnapshotResults::new();
237237

238238
harness.run();
239-
results.push(harness.try_snapshot("modals_1"));
239+
results.add(harness.try_snapshot("modals_1"));
240240

241241
harness.get_by_label("Save").click();
242242
harness.run_ok();
243-
results.push(harness.try_snapshot("modals_2"));
243+
results.add(harness.try_snapshot("modals_2"));
244244

245245
harness.get_by_label("Yes Please").click();
246246
harness.run_ok();
247-
results.push(harness.try_snapshot("modals_3"));
248-
249-
for result in results {
250-
result.unwrap();
251-
}
247+
results.add(harness.try_snapshot("modals_3"));
252248
}
253249

254250
// This tests whether the backdrop actually prevents interaction with lower layers.

crates/egui_demo_lib/src/demo/widget_gallery.rs

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ pub struct WidgetGallery {
2323
#[cfg_attr(feature = "serde", serde(skip))]
2424
date: Option<chrono::NaiveDate>,
2525

26+
#[cfg(feature = "chrono")]
2627
with_date_button: bool,
2728
}
2829

crates/egui_demo_lib/src/rendering_test.rs

+3-7
Original file line numberDiff line numberDiff line change
@@ -688,10 +688,11 @@ fn mul_color_gamma(left: Color32, right: Color32) -> Color32 {
688688
mod tests {
689689
use crate::ColorTest;
690690
use egui_kittest::kittest::Queryable as _;
691+
use egui_kittest::SnapshotResults;
691692

692693
#[test]
693694
pub fn rendering_test() {
694-
let mut errors = vec![];
695+
let mut results = SnapshotResults::new();
695696
for dpi in [1.0, 1.25, 1.5, 1.75, 1.6666667, 2.0] {
696697
let mut color_test = ColorTest::default();
697698
let mut harness = egui_kittest::Harness::builder()
@@ -708,12 +709,7 @@ mod tests {
708709

709710
harness.fit_contents();
710711

711-
let result = harness.try_snapshot(&format!("rendering_test/dpi_{dpi:.2}"));
712-
if let Err(err) = result {
713-
errors.push(err);
714-
}
712+
results.add(harness.try_snapshot(&format!("rendering_test/dpi_{dpi:.2}")));
715713
}
716-
717-
assert!(errors.is_empty(), "Errors: {errors:#?}");
718714
}
719715
}

crates/egui_kittest/src/snapshot.rs

+110-6
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ use std::fmt::Display;
44
use std::io::ErrorKind;
55
use std::path::PathBuf;
66

7+
pub type SnapshotResult = Result<(), SnapshotError>;
8+
79
#[non_exhaustive]
810
pub struct SnapshotOptions {
911
/// The threshold for the image comparison.
@@ -189,7 +191,7 @@ pub fn try_image_snapshot_options(
189191
new: &image::RgbaImage,
190192
name: &str,
191193
options: &SnapshotOptions,
192-
) -> Result<(), SnapshotError> {
194+
) -> SnapshotResult {
193195
let SnapshotOptions {
194196
threshold,
195197
output_path,
@@ -306,7 +308,7 @@ pub fn try_image_snapshot_options(
306308
/// # Errors
307309
/// Returns a [`SnapshotError`] if the image does not match the snapshot or if there was an error
308310
/// reading or writing the snapshot.
309-
pub fn try_image_snapshot(current: &image::RgbaImage, name: &str) -> Result<(), SnapshotError> {
311+
pub fn try_image_snapshot(current: &image::RgbaImage, name: &str) -> SnapshotResult {
310312
try_image_snapshot_options(current, name, &SnapshotOptions::default())
311313
}
312314

@@ -378,7 +380,7 @@ impl<State> Harness<'_, State> {
378380
&mut self,
379381
name: &str,
380382
options: &SnapshotOptions,
381-
) -> Result<(), SnapshotError> {
383+
) -> SnapshotResult {
382384
let image = self
383385
.render()
384386
.map_err(|err| SnapshotError::RenderError { err })?;
@@ -393,7 +395,7 @@ impl<State> Harness<'_, State> {
393395
/// # Errors
394396
/// Returns a [`SnapshotError`] if the image does not match the snapshot, if there was an
395397
/// error reading or writing the snapshot, if the rendering fails or if no default renderer is available.
396-
pub fn try_snapshot(&mut self, name: &str) -> Result<(), SnapshotError> {
398+
pub fn try_snapshot(&mut self, name: &str) -> SnapshotResult {
397399
let image = self
398400
.render()
399401
.map_err(|err| SnapshotError::RenderError { err })?;
@@ -460,15 +462,15 @@ impl<State> Harness<'_, State> {
460462
&mut self,
461463
name: &str,
462464
options: &SnapshotOptions,
463-
) -> Result<(), SnapshotError> {
465+
) -> SnapshotResult {
464466
self.try_snapshot_options(name, options)
465467
}
466468

467469
#[deprecated(
468470
since = "0.31.0",
469471
note = "Use `try_snapshot` instead. This function will be removed in 0.32"
470472
)]
471-
pub fn try_wgpu_snapshot(&mut self, name: &str) -> Result<(), SnapshotError> {
473+
pub fn try_wgpu_snapshot(&mut self, name: &str) -> SnapshotResult {
472474
self.try_snapshot(name)
473475
}
474476

@@ -488,3 +490,105 @@ impl<State> Harness<'_, State> {
488490
self.snapshot(name);
489491
}
490492
}
493+
494+
/// Utility to collect snapshot errors and display them at the end of the test.
495+
///
496+
/// # Example
497+
/// ```
498+
/// # let harness = MockHarness;
499+
/// # struct MockHarness;
500+
/// # impl MockHarness {
501+
/// # fn try_snapshot(&self, _: &str) -> Result<(), egui_kittest::SnapshotError> { Ok(()) }
502+
/// # }
503+
///
504+
/// // [...] Construct a Harness
505+
///
506+
/// let mut results = egui_kittest::SnapshotResults::new();
507+
///
508+
/// // Call add for each snapshot in your test
509+
/// results.add(harness.try_snapshot("my_test"));
510+
///
511+
/// // If there are any errors, SnapshotResults will panic once dropped.
512+
/// ```
513+
///
514+
/// # Panics
515+
/// Panics if there are any errors when dropped (this way it is impossible to forget to call `unwrap`).
516+
/// If you don't want to panic, you can use [`SnapshotResults::into_result`] or [`SnapshotResults::into_inner`].
517+
/// If you want to panic early, you can use [`SnapshotResults::unwrap`].
518+
#[derive(Debug, Default)]
519+
pub struct SnapshotResults {
520+
errors: Vec<SnapshotError>,
521+
}
522+
523+
impl Display for SnapshotResults {
524+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
525+
if self.errors.is_empty() {
526+
write!(f, "All snapshots passed")
527+
} else {
528+
writeln!(f, "Snapshot errors:")?;
529+
for error in &self.errors {
530+
writeln!(f, " {error}")?;
531+
}
532+
Ok(())
533+
}
534+
}
535+
}
536+
537+
impl SnapshotResults {
538+
pub fn new() -> Self {
539+
Default::default()
540+
}
541+
542+
/// Check if the result is an error and add it to the list of errors.
543+
pub fn add(&mut self, result: SnapshotResult) {
544+
if let Err(err) = result {
545+
self.errors.push(err);
546+
}
547+
}
548+
549+
/// Check if there are any errors.
550+
pub fn has_errors(&self) -> bool {
551+
!self.errors.is_empty()
552+
}
553+
554+
/// Convert this into a `Result<(), Self>`.
555+
#[allow(clippy::missing_errors_doc)]
556+
pub fn into_result(self) -> Result<(), Self> {
557+
if self.has_errors() {
558+
Err(self)
559+
} else {
560+
Ok(())
561+
}
562+
}
563+
564+
pub fn into_inner(mut self) -> Vec<SnapshotError> {
565+
std::mem::take(&mut self.errors)
566+
}
567+
568+
/// Panics if there are any errors, displaying each.
569+
#[allow(clippy::unused_self)]
570+
#[track_caller]
571+
pub fn unwrap(self) {
572+
// Panic is handled in drop
573+
}
574+
}
575+
576+
impl From<SnapshotResults> for Vec<SnapshotError> {
577+
fn from(results: SnapshotResults) -> Self {
578+
results.into_inner()
579+
}
580+
}
581+
582+
impl Drop for SnapshotResults {
583+
#[track_caller]
584+
fn drop(&mut self) {
585+
// Don't panic if we are already panicking (the test probably failed for another reason)
586+
if std::thread::panicking() {
587+
return;
588+
}
589+
#[allow(clippy::manual_assert)]
590+
if self.has_errors() {
591+
panic!("{}", self);
592+
}
593+
}
594+
}

crates/egui_kittest/tests/regression_tests.rs

+4-10
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use egui::accesskit::Role;
22
use egui::{Button, ComboBox, Image, Vec2, Widget};
3-
use egui_kittest::{kittest::Queryable, Harness};
3+
use egui_kittest::{kittest::Queryable, Harness, SnapshotResults};
44

55
#[test]
66
pub fn focus_should_skip_over_disabled_buttons() {
@@ -64,18 +64,18 @@ fn test_combobox() {
6464

6565
harness.run();
6666

67-
let mut results = vec![];
67+
let mut results = SnapshotResults::new();
6868

6969
#[cfg(all(feature = "wgpu", feature = "snapshot"))]
70-
results.push(harness.try_snapshot("combobox_closed"));
70+
results.add(harness.try_snapshot("combobox_closed"));
7171

7272
let combobox = harness.get_by_role_and_label(Role::ComboBox, "Select Something");
7373
combobox.click();
7474

7575
harness.run();
7676

7777
#[cfg(all(feature = "wgpu", feature = "snapshot"))]
78-
results.push(harness.try_snapshot("combobox_opened"));
78+
results.add(harness.try_snapshot("combobox_opened"));
7979

8080
let item_2 = harness.get_by_role_and_label(Role::Button, "Item 2");
8181
// Node::click doesn't close the popup, so we use simulate_click
@@ -87,10 +87,4 @@ fn test_combobox() {
8787

8888
// Popup should be closed now
8989
assert!(harness.query_by_label("Item 2").is_none());
90-
91-
for result in results {
92-
if let Err(err) = result {
93-
panic!("{}", err);
94-
}
95-
}
9690
}

crates/egui_kittest/tests/tests.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use egui_kittest::Harness;
1+
use egui_kittest::{Harness, SnapshotResults};
22

33
#[test]
44
fn test_shrink() {
@@ -10,6 +10,8 @@ fn test_shrink() {
1010

1111
harness.fit_contents();
1212

13+
let mut results = SnapshotResults::new();
14+
1315
#[cfg(all(feature = "snapshot", feature = "wgpu"))]
14-
harness.snapshot("test_shrink");
16+
results.add(harness.try_snapshot("test_shrink"));
1517
}

0 commit comments

Comments
 (0)