Add diagnostic_max_steps for a betterExceededMaxStepsError (#8421)

This allows you to see how many more steps it would have taken for the
harness to settle.
Useful to see if you're in a infinite repaint loop or just need a
`harness.run_steps(3)` to give it some more frames to settle.
Should make it easier to make the default step count tighter, which
should make tests faster.

crates/egui_kittest/README.md

@@ -52,6 +52,10 @@ threshold = 0.6
 # (an absolute pixel count, not a fraction of the image)
 max_failed_pixels = 0
 
+# how many steps past `max_steps` `Harness::run` keeps stepping to report how many steps the ui
+# would have needed to settle
+diagnostic_max_steps = 100
+
 [windows]
 threshold = 0.6
 max_failed_pixels = 0

crates/egui_kittest/src/config.rs

@@ -1,5 +1,3 @@
-#![cfg(feature = "snapshot")]
-
 use std::io;
 use std::path::PathBuf;
 
@@ -30,6 +28,14 @@ pub struct Config {
     #[serde(alias = "failed_pixel_count_threshold")]
     max_failed_pixels: usize,
 
+    /// How far past `max_steps` [`crate::Harness::try_run`] keeps stepping to find out how many
+    /// steps the ui would have needed.
+    ///
+    /// This tells a budget which is slightly too tight apart from a ui that never stops repainting.
+    ///
+    /// Default is 100.
+    diagnostic_max_steps: u64,
+
     windows: OsConfig,
     mac: OsConfig,
     linux: OsConfig,
@@ -41,6 +47,7 @@ impl Default for Config {
             output_path: PathBuf::from("tests/snapshots"),
             threshold: 0.6,
             max_failed_pixels: 0,
+            diagnostic_max_steps: 100,
             windows: Default::default(),
             mac: Default::default(),
             linux: Default::default(),
@@ -144,16 +151,24 @@ impl Config {
         &INSTANCE
     }
 
+    /// How far past `max_steps` [`crate::Harness::try_run`] keeps stepping to find out how many
+    /// steps the ui would have needed.
+    ///
+    /// Default is 100.
+    pub fn diagnostic_max_steps(&self) -> u64 {
+        self.diagnostic_max_steps
+    }
+}
+
+#[cfg(feature = "snapshot")]
+impl Config {
     /// The output path for image snapshots.
     ///
     /// Default is "tests/snapshots".
     pub fn output_path(&self) -> PathBuf {
         self.output_path.clone()
     }
-}
 
-#[cfg(feature = "snapshot")]
-impl Config {
     pub fn os_threshold(&self) -> crate::OsThreshold<f32> {
         let fallback = self.threshold;
         crate::OsThreshold {

crates/egui_kittest/src/lib.rs

@@ -38,22 +38,43 @@ use egui::{
 };
 use kittest::Queryable;
 
-use crate::app_kind::AppKind;
+use crate::{app_kind::AppKind, config::config};
 
 #[derive(Debug, Clone)]
 pub struct ExceededMaxStepsError {
     pub max_steps: u64,
+
+    /// How many steps the ui would have needed to settle.
+    ///
+    /// `None` if it did not settle within `diagnostic_max_steps` (see `kittest.toml`) further
+    /// steps either, i.e. it just keeps repainting.
+    pub steps_to_settle: Option<u64>,
+
+    /// How far past [`Self::max_steps`] we kept stepping to find [`Self::steps_to_settle`].
+    pub diagnostic_max_steps: u64,
+
     pub repaint_causes: Vec<RepaintCause>,
 }
 
 impl Display for ExceededMaxStepsError {
     fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
+        write!(f, "Harness::run exceeded max_steps ({}). ", self.max_steps)?;
+
+        match self.steps_to_settle {
+            Some(steps) => write!(f, "It would have settled after {steps} steps. ")?,
+            None => write!(
+                f,
+                "It did not settle within {} further steps either. ",
+                self.diagnostic_max_steps
+            )?,
+        }
+
         write!(
             f,
-            "Harness::run exceeded max_steps ({}). If your expect your ui to keep repainting \
+            "If you expect your ui to keep repainting \
             (e.g. when showing a spinner) call Harness::step or Harness::run_steps instead.\
             \nRepaint causes: {:#?}",
-            self.max_steps, self.repaint_causes,
+            self.repaint_causes,
         )
     }
 }
@@ -333,7 +354,23 @@ impl<'a, State> Harness<'a, State> {
         }
     }
 
-    fn try_run_impl(&mut self, sleep: bool) -> Result<u64, ExceededMaxStepsError> {
+    /// When `sleep` is true, each step sleeps for `self.step_dt`.
+    /// When `diagnostic` is true, we run extra steps to find [`ExceededMaxStepsError::steps_to_settle`].
+    fn try_run_impl(
+        &mut self,
+        sleep: bool,
+        diagnostic: bool,
+    ) -> Result<u64, ExceededMaxStepsError> {
+        // Once the budget is blown we keep going for a while, purely to find out how many steps
+        // would have been needed. The repaint causes are the ones from the moment we blew it.
+        let diagnostic_max_steps = if diagnostic {
+            config().diagnostic_max_steps()
+        } else {
+            0
+        };
+        let last_diagnostic_step = self.max_steps.saturating_add(diagnostic_max_steps);
+        let mut repaint_causes_at_max_steps = None;
+
         let mut steps = 0;
         loop {
             steps += 1;
@@ -343,14 +380,28 @@ impl<'a, State> Harness<'a, State> {
 
             // We only care about immediate repaints
             if self.root_viewport_output().repaint_delay != Duration::ZERO && !wait_for_images {
+                if let Some(repaint_causes) = repaint_causes_at_max_steps {
+                    return Err(ExceededMaxStepsError {
+                        max_steps: self.max_steps,
+                        steps_to_settle: Some(steps),
+                        diagnostic_max_steps,
+                        repaint_causes,
+                    });
+                }
                 break;
             } else if sleep || wait_for_images {
                 std::thread::sleep(Duration::from_secs_f32(self.step_dt));
             }
-            if steps > self.max_steps {
+            if steps > self.max_steps && repaint_causes_at_max_steps.is_none() {
+                repaint_causes_at_max_steps = Some(self.ctx.repaint_causes());
+            }
+            if steps > last_diagnostic_step {
                 return Err(ExceededMaxStepsError {
                     max_steps: self.max_steps,
-                    repaint_causes: self.ctx.repaint_causes(),
+                    steps_to_settle: None,
+                    diagnostic_max_steps,
+                    repaint_causes: repaint_causes_at_max_steps
+                        .unwrap_or_else(|| self.ctx.repaint_causes()),
                 });
             }
         }
@@ -374,7 +425,7 @@ impl<'a, State> Harness<'a, State> {
     /// - [`Harness::run_steps`].
     /// - [`Harness::try_run_realtime`].
     pub fn try_run(&mut self) -> Result<u64, ExceededMaxStepsError> {
-        self.try_run_impl(false)
+        self.try_run_impl(false, true)
     }
 
     /// Run until
@@ -384,6 +435,8 @@ impl<'a, State> Harness<'a, State> {
     ///
     /// Returns the number of steps that were run, or None if the maximum number of steps was exceeded.
     ///
+    /// Unlike [`Harness::run`], this never steps past `max_steps`.
+    ///
     /// See also:
     /// - [`Harness::run`].
     /// - [`Harness::try_run`].
@@ -391,7 +444,7 @@ impl<'a, State> Harness<'a, State> {
     /// - [`Harness::run_steps`].
     /// - [`Harness::try_run_realtime`].
     pub fn run_ok(&mut self) -> Option<u64> {
-        self.try_run().ok()
+        self.try_run_impl(false, false).ok()
     }
 
     /// Run multiple frames, sleeping for [`HarnessBuilder::with_step_dt`] between frames.
@@ -414,7 +467,7 @@ impl<'a, State> Harness<'a, State> {
     /// - [`Harness::run_steps`].
     /// - [`Harness::try_run`].
     pub fn try_run_realtime(&mut self) -> Result<u64, ExceededMaxStepsError> {
-        self.try_run_impl(true)
+        self.try_run_impl(true, true)
     }
 
     /// Run a number of steps.