From a07bdabef0683065f8b7184b363d042cc343fc49 Mon Sep 17 00:00:00 2001 From: lucasmerlin Date: Wed, 23 Oct 2024 08:54:43 +0200 Subject: [PATCH] Improve egui_kittest documentation and update CONTRIBUTING.md (#5296) This should hopefully make it easier for egui contributors and egui_kittest users to understand the snapshot tests * [X] I have followed the instructions in the PR template --- ARCHITECTURE.md | 3 +++ CONTRIBUTING.md | 8 ++++++++ crates/egui_kittest/README.md | 12 ++++++++++++ crates/egui_kittest/src/snapshot.rs | 12 ++++++++++++ crates/egui_kittest/src/wgpu.rs | 10 ++++++++++ 5 files changed, 45 insertions(+) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index cf2c2d60a31..000f2b7ac9e 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -50,6 +50,9 @@ This contains a bunch of uses of `egui` and looks like the ui code you would wri Thin wrapper around `egui_demo_lib` so we can compile it to a web site or a native app executable. Depends on `egui_demo_lib` + `eframe`. +### `egui_kittest` +A test harness for egui based on [kittest](https://github.com/rerun/kittest) and [AccessKit](https://github.com/AccessKit/accesskit/). + ### Other integrations There are also many great integrations for game engines such as `bevy` and `miniquad` which you can find at . diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index be55d133c22..f7215529544 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -32,6 +32,14 @@ For small things, just go ahead an open a PR. For bigger things, please file an Browse through [`ARCHITECTURE.md`](ARCHITECTURE.md) to get a sense of how all pieces connects. You can test your code locally by running `./scripts/check.sh`. +There are snapshots test that might need to be updated. Run the tests with `UPDATE_SNAPSHOTS=true` to update them. +For more info about the tests see [egui_kittest](./crates/egui_kittest/README.md). + +We use [git-lfs](https://git-lfs.com/) to store big files in the repository. +Make sure you have it installed (running `git lfs ls-files` from the repository root should list some files). +Don't forget to run `git lfs install` after installing the git-lfs binary. + +If you see an `InvalidSignature` error when running snapshot tests, it's probably a problem related to git-lfs. When you have something that works, open a draft PR. You may get some helpful feedback early! When you feel the PR is ready to go, do a self-review of the code, and then open it for review. diff --git a/crates/egui_kittest/README.md b/crates/egui_kittest/README.md index 86ccf551506..d4d50845a71 100644 --- a/crates/egui_kittest/README.md +++ b/crates/egui_kittest/README.md @@ -2,6 +2,7 @@ Ui testing library for egui, based on [kittest](https://github.com/rerun-io/kittest) (an [AccessKit](https://github.com/AccessKit/accesskit) based testing library). +## Example usage ```rust use egui::accesskit::{Role, Toggled}; use egui::{CentralPanel, Context, TextEdit, Vec2}; @@ -33,3 +34,14 @@ fn main() { harness.wgpu_snapshot("readme_example"); } ``` + +## Snapshot testing +There is a snapshot testing feature. To create snapshot tests, enable the `snapshot` and `wgpu` features. +Once enabled, you can call `Harness::wgpu_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. + +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 a2c31a44130..9a64fb42abd 100644 --- a/crates/egui_kittest/src/snapshot.rs +++ b/crates/egui_kittest/src/snapshot.rs @@ -80,6 +80,9 @@ impl Display for SnapshotError { } /// Image snapshot test. +/// The snapshot will be saved under `tests/snapshots/{name}.png`. +/// The new image from the last test run will be saved under `tests/snapshots/{name}.new.png`. +/// If new image didn't match the snapshot, a diff image will be saved under `tests/snapshots/{name}.diff.png`. /// /// # Errors /// Returns a [`SnapshotError`] if the image does not match the snapshot or if there was an error @@ -170,6 +173,9 @@ fn maybe_update_snapshot( } /// Image snapshot test. +/// The snapshot will be saved under `tests/snapshots/{name}.png`. +/// The new image from the last test run will be saved under `tests/snapshots/{name}.new.png`. +/// If new image didn't match the snapshot, a diff image will be saved under `tests/snapshots/{name}.diff.png`. /// /// # Panics /// Panics if the image does not match the snapshot or if there was an error reading or writing the @@ -187,6 +193,9 @@ pub fn image_snapshot(current: &image::RgbaImage, name: &str) { #[cfg(feature = "wgpu")] impl Harness<'_> { /// Render a image using a default [`crate::wgpu::TestRenderer`] and compare it to the snapshot. + /// The snapshot will be saved under `tests/snapshots/{name}.png`. + /// The new image from the last test run will be saved under `tests/snapshots/{name}.new.png`. + /// If new image didn't match the snapshot, a diff image will be saved under `tests/snapshots/{name}.diff.png`. /// /// # Errors /// Returns a [`SnapshotError`] if the image does not match the snapshot or if there was an error @@ -198,6 +207,9 @@ impl Harness<'_> { } /// Render a image using a default [`crate::wgpu::TestRenderer`] and compare it to the snapshot. + /// The snapshot will be saved under `tests/snapshots/{name}.png`. + /// The new image from the last test run will be saved under `tests/snapshots/{name}.new.png`. + /// If new image didn't match the snapshot, a diff image will be saved under `tests/snapshots/{name}.diff.png`. /// /// # Panics /// Panics if the image does not match the snapshot or if there was an error reading or writing the diff --git a/crates/egui_kittest/src/wgpu.rs b/crates/egui_kittest/src/wgpu.rs index c79f411433a..a979226ebc2 100644 --- a/crates/egui_kittest/src/wgpu.rs +++ b/crates/egui_kittest/src/wgpu.rs @@ -6,6 +6,7 @@ use image::RgbaImage; use std::iter::once; use wgpu::Maintain; +/// Utility to render snapshots from a [`Harness`] using [`egui_wgpu`]. pub struct TestRenderer { device: wgpu::Device, queue: wgpu::Queue, @@ -19,6 +20,7 @@ impl Default for TestRenderer { } impl TestRenderer { + /// Create a new [`TestRenderer`] using a default [`wgpu::Instance`]. pub fn new() -> Self { let instance = wgpu::Instance::new(InstanceDescriptor::default()); @@ -39,6 +41,7 @@ impl TestRenderer { Self::create(device, queue) } + /// Create a new [`TestRenderer`] using the provided [`wgpu::Device`] and [`wgpu::Queue`]. pub fn create(device: wgpu::Device, queue: wgpu::Queue) -> Self { Self { device, @@ -47,13 +50,20 @@ impl TestRenderer { } } + /// Enable or disable dithering. + /// + /// Disabled by default. #[inline] pub fn with_dithering(mut self, dithering: bool) -> Self { self.dithering = dithering; self } + /// Render the [`Harness`] and return the resulting image. pub fn render(&mut self, harness: &Harness<'_>) -> RgbaImage { + // We need to create a new renderer each time we render, since the renderer stores + // textures related to the Harnesses' egui Context. + // Calling the renderer from different Harnesses would cause problems if we store the renderer. let mut renderer = egui_wgpu::Renderer::new( &self.device, TextureFormat::Rgba8Unorm,