From 1c8a472354f1a340eca8a9aae4c02f9283f4ac1a Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Wed, 12 Aug 2026 00:37:42 +0800 Subject: [PATCH] test(render-idle): settle the pane's git pipeline before counting frames MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Separate from the watch loop above, and the reason it was found: these tests count frames across a window they advance a virtual clock over, but the pane runs its own git pipeline off a 300ms timer on that same clock, and nothing advanced it while a test waited for the panel to settle. So the pane's first `git` run was set off by the measurement itself, and the repaint it lands with fell inside the counted window or just after it depending on how fast git ran. That is what made #523 a flake rather than a failure — five red runs across both runners, none reproducible on an idle machine. `test_window::quiesce` pumps the virtual clock until a round draws nothing, the pane's probe has answered for the directory, and no debounce window is still open — and then requires that quiet to hold for 400ms of real time, because the kernel is a third clock: watch events for a test's own setup writes are still being delivered long after every future it can wait on has resolved. There is deliberately no real-time exposure left in the counted windows. Real time is a channel input arrives on, and a test that spends it inside a frame count is asking to be handed some. `ui::file_tree`'s five render-idle tests had the same latent race and had simply not been unlucky yet; they are fixed here rather than left to be filed again. `ScmData::is_debouncing` is `cfg(test)`. --- src/terminal/git_data.rs | 17 +++++++++ src/ui/app.rs | 82 ++++++++++++++++++++++++++++++++++++++++ src/ui/file_tree.rs | 7 ++++ src/ui/scm/graph.rs | 7 ++++ src/ui/scm/panel.rs | 8 +++- 5 files changed, 120 insertions(+), 1 deletion(-) diff --git a/src/terminal/git_data.rs b/src/terminal/git_data.rs index 3b7e8793..53f241ed 100644 --- a/src/terminal/git_data.rs +++ b/src/terminal/git_data.rs @@ -166,6 +166,12 @@ impl Debounce { self.seq } + /// Is an event still waiting for its probe? + #[cfg(test)] + pub fn is_open(&self) -> bool { + self.opened.is_some() + } + /// What the timer should do now. `Fire` closes the burst, so it is /// returned exactly once however many events went into it. pub fn poll(&mut self, now: Instant) -> DebounceStep { @@ -489,6 +495,17 @@ impl ScmData { self.subs.is_empty() && self.watches.is_empty() } + /// Is any repository still inside a debounce window? + /// + /// The window is measured on the real clock, and closing it costs a + /// frame. A test driving a virtual clock has no other way to tell a panel + /// that has gone quiet from one whose next repaint is merely still owed — + /// see `ui::app::test_window::quiesce`. + #[cfg(test)] + pub fn is_debouncing(&self) -> bool { + self.watches.values().any(|watch| watch.debounce.is_open()) + } + /// Repositories that have a holder but no watch, nothing on the way, and /// no failed attempt still resting. fn unwatched(&self, now: Instant) -> Vec<(HostId, PathBuf)> { diff --git a/src/ui/app.rs b/src/ui/app.rs index 668aa9f4..58409f33 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -7842,6 +7842,88 @@ pub(crate) mod test_window { vcx.background_executor.run_until_parked(); (app, vcx, stream) } + + /// Wait until the window has actually stopped drawing — which is not the + /// same as having reached the state a test was waiting for. + /// + /// Called both after a settle and at the top of `draws_while_idle`: a test + /// that reaches its state, asserts a few things about it and only then + /// measures has given the setup more time to land, but not necessarily + /// enough, and the measurement is the place that cannot afford to be + /// wrong. + /// + /// Both of a `render_idle` test's clocks have to be pumped here, and they + /// are pumped differently. + /// + /// The pane runs its own git pipeline, separate from whatever panel is on + /// screen, and it hangs off a 300ms timer on the *virtual* clock — so it + /// never starts at all unless a test advances that clock. It used to be + /// `draws_while_idle`'s own `advance_clock` that started it, which put the + /// pane's first real `git` run, and the repaint it lands with, inside the + /// window being counted. Whether that repaint arrived before or after the + /// count then came down to how fast git ran, which is why these tests were + /// green here and red on a loaded CI runner (issue #523). + /// + /// What that repaint sets off in turn is timed on the *real* clock: the + /// landing opens a `GIT_WATCH_DEBOUNCE` burst, and closing the burst costs + /// another frame 250ms later. So the sleep below is load-bearing too, and + /// a round that drew nothing is not on its own enough to stop on — a burst + /// still open is a frame already owed. + #[cfg(unix)] + pub(crate) fn quiesce(vcx: &mut VisualTestContext, cwd: Option<&std::path::Path>) { + use crate::terminal::git_data::ScmData; + use crate::terminal::git_status::GitStatusCache; + use crate::ui::app::render_probe; + use crate::ui::host_ops::HostId; + + /// How long quiet has to hold before it counts as quiet. + /// + /// Real time, and the only defence against the third clock in play: + /// the kernel's. The file tree keeps a real `inotify`/`FSEvents` + /// watch, and the writes a test makes while setting up its repository + /// are still being delivered long after every future the test can wait + /// on has resolved. They arrive on the channel, sit in a 200ms debounce + /// on the virtual clock, and are released by the next `advance_clock` + /// — which, without this, was the measurement's own. Any delivery + /// restarts the hold, so the wait is as long as the runner needs and + /// no longer. + const QUIET_HOLD: std::time::Duration = std::time::Duration::from_millis(400); + + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); + let mut quiet_since: Option = None; + loop { + render_probe::arm(u64::MAX); + vcx.executor() + .advance_clock(std::time::Duration::from_millis(300)); + vcx.background_executor.run_until_parked(); + let quiet = render_probe::draws() == 0 + && vcx.update(|_, cx| { + let owed = cx + .try_global::() + .is_some_and(ScmData::is_debouncing); + let answered = cwd.is_none_or(|cwd| { + cx.try_global::() + .and_then(|cache| cache.known_repo_for(HostId::LOCAL, cwd)) + .is_some() + }); + !owed && answered + }); + match quiet { + false => quiet_since = None, + true => { + let since = *quiet_since.get_or_insert_with(std::time::Instant::now); + if since.elapsed() >= QUIET_HOLD { + return; + } + } + } + assert!( + std::time::Instant::now() < deadline, + "the window never stopped drawing" + ); + std::thread::sleep(std::time::Duration::from_millis(20)); + } + } } #[cfg(all(test, unix))] diff --git a/src/ui/file_tree.rs b/src/ui/file_tree.rs index 86b26d23..eaa7beeb 100644 --- a/src/ui/file_tree.rs +++ b/src/ui/file_tree.rs @@ -2801,6 +2801,7 @@ mod render_idle_gpui_tests { std::thread::sleep(std::time::Duration::from_millis(20)); } vcx.background_executor.run_until_parked(); + test_window::quiesce(&mut vcx, Some(root)); (app, vcx, pane) } @@ -2887,12 +2888,18 @@ mod render_idle_gpui_tests { } fn draws_while_idle(vcx: &mut VisualTestContext) -> u64 { + test_window::quiesce(vcx, None); render_probe::arm(BUDGET); vcx.background_executor.run_until_parked(); vcx.executor() .advance_clock(std::time::Duration::from_secs(3)); vcx.background_executor.run_until_parked(); render_probe::arm(BUDGET); + // No real-time exposure in the counted window, deliberately. The file + // tree holds a real filesystem watch, so real time is a channel input + // arrives on — and a test that spends it here is asking to be handed + // some. What has to be waited out is waited out in `quiesce` above, + // where a frame costs nothing. vcx.executor() .advance_clock(std::time::Duration::from_secs(9)); vcx.background_executor.run_until_parked(); diff --git a/src/ui/scm/graph.rs b/src/ui/scm/graph.rs index 52e3c097..bea86035 100644 --- a/src/ui/scm/graph.rs +++ b/src/ui/scm/graph.rs @@ -2212,12 +2212,18 @@ mod render_idle_gpui_tests { } fn draws_while_idle(vcx: &mut VisualTestContext) -> u64 { + test_window::quiesce(vcx, None); render_probe::arm(BUDGET); vcx.background_executor.run_until_parked(); vcx.executor() .advance_clock(std::time::Duration::from_secs(3)); vcx.background_executor.run_until_parked(); render_probe::arm(BUDGET); + // No real-time exposure in the counted window, deliberately. The file + // tree holds a real filesystem watch, so real time is a channel input + // arrives on — and a test that spends it here is asking to be handed + // some. What has to be waited out is waited out in `quiesce` above, + // where a frame costs nothing. vcx.executor() .advance_clock(std::time::Duration::from_secs(9)); vcx.background_executor.run_until_parked(); @@ -2234,6 +2240,7 @@ mod render_idle_gpui_tests { let page = app.update_in(vcx, |app, _, _| app.scm.graph.page.clone()); if page.is_some() { vcx.background_executor.run_until_parked(); + test_window::quiesce(vcx, None); return page; } if std::time::Instant::now() >= deadline { diff --git a/src/ui/scm/panel.rs b/src/ui/scm/panel.rs index adbcbe1d..309580c7 100644 --- a/src/ui/scm/panel.rs +++ b/src/ui/scm/panel.rs @@ -2712,17 +2712,23 @@ mod render_idle_gpui_tests { ); std::thread::sleep(std::time::Duration::from_millis(20)); } - vcx.background_executor.run_until_parked(); + test_window::quiesce(&mut vcx, Some(root)); (app, vcx, pane) } fn draws_while_idle(vcx: &mut VisualTestContext) -> u64 { + test_window::quiesce(vcx, None); render_probe::arm(BUDGET); vcx.background_executor.run_until_parked(); vcx.executor() .advance_clock(std::time::Duration::from_secs(3)); vcx.background_executor.run_until_parked(); render_probe::arm(BUDGET); + // No real-time exposure in the counted window, deliberately. The file + // tree holds a real filesystem watch, so real time is a channel input + // arrives on — and a test that spends it here is asking to be handed + // some. What has to be waited out is waited out in `quiesce` above, + // where a frame costs nothing. vcx.executor() .advance_clock(std::time::Duration::from_secs(9)); vcx.background_executor.run_until_parked();