From 8eda32ec64c7b48c7b8f9195e3abc8c10efa8f00 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 30 Jan 2025 09:34:22 +0100 Subject: [PATCH] egui_kittest: succeed and keep going when `UPDATE_SNAPSHOTS` is set (#5649) It used to be that `UPDATE_SNAPSHOTS=true cargo test --all-features` would stop on the first crate with a failure, requiring you to run it multiple times, which is annoying, and a waste of time. --- crates/egui_kittest/README.md | 13 ++--- crates/egui_kittest/src/snapshot.rs | 79 +++++++++++++++++------------ 2 files changed, 54 insertions(+), 38 deletions(-) diff --git a/crates/egui_kittest/README.md b/crates/egui_kittest/README.md index 99fe9be6..a9c1286b 100644 --- a/crates/egui_kittest/README.md +++ b/crates/egui_kittest/README.md @@ -1,4 +1,4 @@ -# egui_kittest +# egui_kittest Ui testing library for egui, based on [kittest](https://github.com/rerun-io/kittest) (an [AccessKit](https://github.com/AccessKit/accesskit) based testing library). @@ -14,16 +14,16 @@ fn main() { }; let mut harness = Harness::new_ui(app); - + let checkbox = harness.get_by_label("Check me!"); assert_eq!(checkbox.toggled(), Some(Toggled::False)); checkbox.click(); - + harness.run(); let checkbox = harness.get_by_label("Check me!"); assert_eq!(checkbox.toggled(), Some(Toggled::True)); - + // Shrink the window size to the smallest size possible harness.fit_contents(); @@ -38,9 +38,10 @@ There is a snapshot testing feature. To create snapshot tests, enable the `snaps Once enabled, you can call `Harness::snapshot` to render the ui and save the image to the `tests/snapshots` directory. To update the snapshots, run your tests with `UPDATE_SNAPSHOTS=true`, so e.g. `UPDATE_SNAPSHOTS=true cargo test`. -Running with `UPDATE_SNAPSHOTS=true` will still cause the tests to fail, but on the next run, the tests should pass. +Running with `UPDATE_SNAPSHOTS=true` will cause the tests to succeed. +This is so that you can set `UPDATE_SNAPSHOTS=true` and update _all_ tests, without `cargo test` failing on the first failing crate. -If you want to have multiple snapshots in the same test, it makes sense to collect the results in a `Vec` +If you want to have multiple snapshots in the same test, it makes sense to collect the results in a `Vec` ([look here](https://github.com/emilk/egui/blob/70a01138b77f9c5724a35a6ef750b9ae1ab9f2dc/crates/egui_demo_lib/src/demo/demo_app_windows.rs#L388-L427) for an example). This way they can all be updated at the same time. diff --git a/crates/egui_kittest/src/snapshot.rs b/crates/egui_kittest/src/snapshot.rs index be4f68a0..409c287c 100644 --- a/crates/egui_kittest/src/snapshot.rs +++ b/crates/egui_kittest/src/snapshot.rs @@ -155,8 +155,15 @@ impl Display for SnapshotError { } } +/// If this is set, we update the snapshots (if different), +/// and _succeed_ the test. +/// This is so that you can set `UPDATE_SNAPSHOTS=true` and update _all_ tests, +/// without `cargo test` failing on the first failing crate. fn should_update_snapshots() -> bool { - std::env::var("UPDATE_SNAPSHOTS").is_ok() + match std::env::var("UPDATE_SNAPSHOTS") { + Ok(value) => !matches!(value.as_str(), "false" | "0" | "no" | "off"), + Err(_) => false, + } } /// Image snapshot test with custom options. @@ -203,23 +210,22 @@ pub fn try_image_snapshot_options( 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(); + let update_snapshot = || { + // 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(), - })?; + // 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(); + // No need for an explicit `.new` file: + std::fs::remove_file(&new_path).ok(); + + println!("Updated snapshot: {snapshot_path:?}"); - println!("Updated snapshot: {snapshot_path:?}"); - } Ok(()) }; @@ -234,21 +240,27 @@ pub fn try_image_snapshot_options( Ok(image) => image.to_rgba8(), Err(err) => { // No previous snapshot - probablye a new test. - maybe_update_snapshot()?; - return Err(SnapshotError::OpenSnapshot { - path: snapshot_path.clone(), - err, - }); + if should_update_snapshots() { + return update_snapshot(); + } else { + return Err(SnapshotError::OpenSnapshot { + path: snapshot_path.clone(), + err, + }); + } } }; if previous.dimensions() != new.dimensions() { - maybe_update_snapshot()?; - return Err(SnapshotError::SizeMismatch { - name: name.to_owned(), - expected: previous.dimensions(), - actual: new.dimensions(), - }); + if should_update_snapshots() { + return update_snapshot(); + } else { + return Err(SnapshotError::SizeMismatch { + name: name.to_owned(), + expected: previous.dimensions(), + actual: new.dimensions(), + }); + } } // Compare existing image to the new one: @@ -262,12 +274,15 @@ pub fn try_image_snapshot_options( path: diff_path.clone(), err, })?; - maybe_update_snapshot()?; - Err(SnapshotError::Diff { - name: name.to_owned(), - diff, - diff_path, - }) + if should_update_snapshots() { + update_snapshot() + } else { + Err(SnapshotError::Diff { + name: name.to_owned(), + diff, + diff_path, + }) + } } else { Ok(()) }