Skip to content

Commit

Permalink
Improve error message when kittest fails (#5427)
Browse files Browse the repository at this point in the history
* Closes #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
```
  • Loading branch information
emilk authored Dec 3, 2024
1 parent 3411aba commit c7224aa
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 11 deletions.
1 change: 1 addition & 0 deletions CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
/crates/egui_kittest @lucasmerlin
/crates/egui-wgpu @Wumpf
2 changes: 1 addition & 1 deletion crates/egui_demo_lib/src/demo/demo_app_windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}

Expand Down
41 changes: 31 additions & 10 deletions crates/egui_kittest/src/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand All @@ -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),

Expand All @@ -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 } => {
Expand Down Expand Up @@ -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(),
});
Expand All @@ -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.
Expand Down

0 comments on commit c7224aa

Please sign in to comment.