From 938d8b0d2e2a551ac354d76d6208188e5942b21e Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Fri, 3 Jan 2025 16:23:31 +0100 Subject: [PATCH] egui_kittest: write `.old.png` files when updating images (#5578) After running `UPDATE_SNAPSHOTS=1 cargo test --all-features` I want to compare before/after images before committing them. Now I can! --- .gitignore | 1 + crates/egui_kittest/src/snapshot.rs | 96 ++++++++++++++++------------- 2 files changed, 55 insertions(+), 42 deletions(-) diff --git a/.gitignore b/.gitignore index 887de9da..588826e7 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ **/target_wasm **/tests/snapshots/**/*.diff.png **/tests/snapshots/**/*.new.png +**/tests/snapshots/**/*.old.png /.*.json /.vscode /media/* diff --git a/crates/egui_kittest/src/snapshot.rs b/crates/egui_kittest/src/snapshot.rs index 95632912..be4f68a0 100644 --- a/crates/egui_kittest/src/snapshot.rs +++ b/crates/egui_kittest/src/snapshot.rs @@ -2,7 +2,7 @@ use crate::Harness; use image::ImageError; use std::fmt::Display; use std::io::ErrorKind; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; #[non_exhaustive] pub struct SnapshotOptions { @@ -159,22 +159,6 @@ fn should_update_snapshots() -> bool { std::env::var("UPDATE_SNAPSHOTS").is_ok() } -fn maybe_update_snapshot( - snapshot_path: &Path, - current: &image::RgbaImage, -) -> Result<(), SnapshotError> { - if should_update_snapshots() { - current - .save(snapshot_path) - .map_err(|err| SnapshotError::WriteSnapshot { - err, - path: snapshot_path.into(), - })?; - println!("Updated snapshot: {snapshot_path:?}"); - } - Ok(()) -} - /// Image snapshot test with custom options. /// /// If you want to change the default options for your whole project, it's recommended to create a @@ -188,11 +172,14 @@ fn maybe_update_snapshot( /// The new image from the most recent test run will be saved under `{output_path}/{name}.new.png`. /// If new image didn't match the snapshot, a diff image will be saved under `{output_path}/{name}.diff.png`. /// +/// If the env-var `UPDATE_SNAPSHOTS` is set, then the old image will backed up under `{output_path}/{name}.old.png`. +/// and then new image will be written to `{output_path}/{name}.png` +/// /// # 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_options( - current: &image::RgbaImage, + new: &image::RgbaImage, name: &str, options: &SnapshotOptions, ) -> Result<(), SnapshotError> { @@ -201,45 +188,72 @@ pub fn try_image_snapshot_options( output_path, } = options; - let path = output_path.join(format!("{name}.png")); - std::fs::create_dir_all(path.parent().expect("Could not get snapshot folder")).ok(); + std::fs::create_dir_all(output_path).ok(); + // The one that is checked in to git + let snapshot_path = output_path.join(format!("{name}.png")); + + // These should be in .gitignore: let diff_path = output_path.join(format!("{name}.diff.png")); - let current_path = output_path.join(format!("{name}.new.png")); + let old_backup_path = output_path.join(format!("{name}.old.png")); + let new_path = output_path.join(format!("{name}.new.png")); - current - .save(¤t_path) + // Delete old temporary files if they exist: + std::fs::remove_file(&diff_path).ok(); + std::fs::remove_file(&old_backup_path).ok(); + std::fs::remove_file(&new_path).ok(); + + let maybe_update_snapshot = || { + if should_update_snapshots() { + // Keep the old version so the user can compare it: + std::fs::rename(&snapshot_path, &old_backup_path).ok(); + + // Write the new file to the checked in path: + new.save(&snapshot_path) + .map_err(|err| SnapshotError::WriteSnapshot { + err, + path: snapshot_path.clone(), + })?; + + // No need for an explicit `.new` file: + std::fs::remove_file(&new_path).ok(); + + println!("Updated snapshot: {snapshot_path:?}"); + } + Ok(()) + }; + + // Always write a `.new` file so the user can compare: + new.save(&new_path) .map_err(|err| SnapshotError::WriteSnapshot { err, - path: current_path, + path: new_path.clone(), })?; - let previous = match image::open(&path) { + let previous = match image::open(&snapshot_path) { Ok(image) => image.to_rgba8(), Err(err) => { - maybe_update_snapshot(&path, current)?; - return Err(SnapshotError::OpenSnapshot { path, err }); + // No previous snapshot - probablye a new test. + maybe_update_snapshot()?; + return Err(SnapshotError::OpenSnapshot { + path: snapshot_path.clone(), + err, + }); } }; - if previous.dimensions() != current.dimensions() { - maybe_update_snapshot(&path, current)?; + if previous.dimensions() != new.dimensions() { + maybe_update_snapshot()?; return Err(SnapshotError::SizeMismatch { name: name.to_owned(), expected: previous.dimensions(), - actual: current.dimensions(), + actual: new.dimensions(), }); } - let result = dify::diff::get_results( - previous, - current.clone(), - *threshold, - true, - None, - &None, - &None, - ); + // Compare existing image to the new one: + let result = + dify::diff::get_results(previous, new.clone(), *threshold, true, None, &None, &None); if let Some((diff, result_image)) = result { result_image @@ -248,15 +262,13 @@ pub fn try_image_snapshot_options( path: diff_path.clone(), err, })?; - maybe_update_snapshot(&path, current)?; + maybe_update_snapshot()?; 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(()) } }