From c7224aab261fa1808faf31631dcc93ff3a1ea2da Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 3 Dec 2024 13:40:51 +0100 Subject: [PATCH] Improve error message when kittest fails (#5427) * Closes https://github.com/emilk/egui/issues/5423 New output is actionable ``` failures: ---- demo::demo_app_windows::tests::demos_should_match_snapshot stdout ---- thread 'demo::demo_app_windows::tests::demos_should_match_snapshot' panicked at crates/egui_demo_lib/src/demo/demo_app_windows.rs:433:9: Errors: [ "'demos/Code Example' Image size did not match snapshot. Expected: (402, 574), Actual: (415, 574). Run `UPDATE_SNAPSHOTS=1 cargo test` to update the snapshots.", ] note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace failures: demo::demo_app_windows::tests::demos_should_match_snapshot ``` --- CODEOWNERS | 1 + .../src/demo/demo_app_windows.rs | 2 +- crates/egui_kittest/src/snapshot.rs | 41 ++++++++++++++----- 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/CODEOWNERS b/CODEOWNERS index 40b72a3140a..8b71b22c10e 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1 +1,2 @@ +/crates/egui_kittest @lucasmerlin /crates/egui-wgpu @Wumpf 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 8729d148f11..5b57c675b5a 100644 --- a/crates/egui_demo_lib/src/demo/demo_app_windows.rs +++ b/crates/egui_demo_lib/src/demo/demo_app_windows.rs @@ -426,7 +426,7 @@ mod tests { let result = harness.try_wgpu_snapshot_options(&format!("demos/{name}"), &options); if let Err(err) = result { - errors.push(err); + errors.push(err.to_string()); } } diff --git a/crates/egui_kittest/src/snapshot.rs b/crates/egui_kittest/src/snapshot.rs index 40e02027b4b..5be6c275419 100644 --- a/crates/egui_kittest/src/snapshot.rs +++ b/crates/egui_kittest/src/snapshot.rs @@ -53,6 +53,9 @@ impl SnapshotOptions { pub enum SnapshotError { /// Image did not match snapshot Diff { + /// Name of the test + name: String, + /// Count of pixels that were different diff: i32, @@ -72,6 +75,9 @@ pub enum SnapshotError { /// The size of the image did not match the snapshot SizeMismatch { + /// Name of the test + name: String, + /// Expected size expected: (u32, u32), @@ -89,32 +95,43 @@ pub enum SnapshotError { }, } +const HOW_TO_UPDATE_SCREENSHOTS: &str = + "Run `UPDATE_SNAPSHOTS=1 cargo test` to update the snapshots."; + impl Display for SnapshotError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Diff { diff, diff_path } => { + Self::Diff { + name, + diff, + diff_path, + } => { write!( f, - "Image did not match snapshot. Diff: {diff}, {diff_path:?}" + "'{name}' Image did not match snapshot. Diff: {diff}, {diff_path:?}. {HOW_TO_UPDATE_SCREENSHOTS}" ) } Self::OpenSnapshot { path, err } => match err { ImageError::IoError(io) => match io.kind() { ErrorKind::NotFound => { - write!(f, "Missing snapshot: {path:?}") + write!(f, "Missing snapshot: {path:?}. {HOW_TO_UPDATE_SCREENSHOTS}") } err => { - write!(f, "Error reading snapshot: {err:?}\nAt: {path:?}") + write!(f, "Error reading snapshot: {err:?}\nAt: {path:?}. {HOW_TO_UPDATE_SCREENSHOTS}") } }, err => { - write!(f, "Error decoding snapshot: {err:?}\nAt: {path:?}") + write!(f, "Error decoding snapshot: {err:?}\nAt: {path:?}. Make sure git-lfs is setup correctly. Read the instructions here: https://github.com/emilk/egui/blob/master/CONTRIBUTING.md#making-a-pr") } }, - Self::SizeMismatch { expected, actual } => { + Self::SizeMismatch { + name, + expected, + actual, + } => { write!( f, - "Image size did not match snapshot. Expected: {expected:?}, Actual: {actual:?}" + "'{name}' Image size did not match snapshot. Expected: {expected:?}, Actual: {actual:?}. {HOW_TO_UPDATE_SCREENSHOTS}" ) } Self::WriteSnapshot { path, err } => { @@ -194,6 +211,7 @@ pub fn try_image_snapshot_options( if previous.dimensions() != current.dimensions() { maybe_update_snapshot(&path, current)?; return Err(SnapshotError::SizeMismatch { + name: name.to_owned(), expected: previous.dimensions(), actual: current.dimensions(), }); @@ -217,13 +235,16 @@ pub fn try_image_snapshot_options( err, })?; maybe_update_snapshot(&path, current)?; - return Err(SnapshotError::Diff { diff, diff_path }); + Err(SnapshotError::Diff { + name: name.to_owned(), + diff, + diff_path, + }) } else { // Delete old diff if it exists std::fs::remove_file(diff_path).ok(); + Ok(()) } - - Ok(()) } /// Image snapshot test.