From 0fa3ee68b6f6521e11d14b69f59fbb546c6ad09e Mon Sep 17 00:00:00 2001 From: clover caruso Date: Fri, 2 Oct 2026 13:49:32 -0700 Subject: [PATCH] fix: replay tests wait for the app to settle instead of fixed waits `settle` and `accessibility` now also wait for loads, pages opening, work started with spawn (template art, version copies, renames) and the search index jobs sent but not answered, seen quiet on two frames apart so whatever a finishing thread sent has been handled. Search counts pending jobs in place of its busy flag, which was false until the index thread picked a job up. Every replay test now settles in place of its fixed waits, except after a resize, which the window system does. Assisted-by: claude-opus-5.5 --- crates/snowbound/src/main.rs | 44 ++++++++++++++++------ crates/snowbound/src/pane.rs | 4 +- crates/snowbound/src/search.rs | 28 ++++++++------ crates/snowbound/tests/replay.rs | 64 ++++++++++++++++---------------- 4 files changed, 85 insertions(+), 55 deletions(-) diff --git a/crates/snowbound/src/main.rs b/crates/snowbound/src/main.rs index ffb1f9a43de6ed908ec47194764a56a53fc44d17..487abcc2ef21fdd7af1f116534d00a73247be39d 100644 --- a/crates/snowbound/src/main.rs +++ b/crates/snowbound/src/main.rs @@ -111,7 +111,11 @@ use std::{ collections::{HashMap, HashSet}, error::Error, path::{Path, PathBuf}, - sync::{Arc, Mutex, mpsc}, + sync::{ + Arc, Mutex, + atomic::{AtomicUsize, Ordering}, + mpsc, + }, }; use ui::{Axis, Flags, Id, Spec, Theme, Ui, children, fill, fit, px}; use web_time::Instant; @@ -886,7 +890,8 @@ struct State { snapshot: Option, /// A replay waiting for nothing to be on its way, and where it wants the accessibility /// tree written then. - replay_settle: Option<(Option, std::sync::mpsc::Sender<()>)>, + /// Whether the last frame found the window quiet too, which the tree waits for. + replay_settle: Option<(Option, std::sync::mpsc::Sender<()>, bool)>, /// `SNOWBOUND_FRAMES`: a directory every frame drawn is also written to, named by /// milliseconds since the window opened. frames: Option<(PathBuf, Instant)>, @@ -5629,7 +5634,15 @@ fn page_text(editor: &CanvasEditor) -> String { /// Runs `work` beside the frame: on a thread of its own, or in the browser, which gives the /// page one thread, once the frame under way is done. +/// Work [`spawn`] started that has not ended. +static RUNNING: AtomicUsize = AtomicUsize::new(0); + fn spawn(work: impl FnOnce() + Send + 'static) { + RUNNING.fetch_add(1, Ordering::Relaxed); + let work = move || { + work(); + RUNNING.fetch_sub(1, Ordering::Relaxed); + }; #[cfg(not(target_arch = "wasm32"))] std::thread::spawn(work); #[cfg(target_arch = "wasm32")] @@ -5714,7 +5727,9 @@ impl State { } } Replay::Snapshot(path) => self.snapshot = Some(path), - Replay::Settle(path, settled) => self.replay_settle = Some((path, settled)), + Replay::Settle(path, settled) => { + self.replay_settle = Some((path, settled, false)); + } Replay::Tick | Replay::Quit => {} Replay::Appearance(appearance) => { self.window.set_theme(Some(appearance)); @@ -5730,15 +5745,22 @@ impl State { if let Err(error) = self.frame() { eprintln!("{error}"); } - if self.settled() - && let Some((path, settled)) = self.replay_settle.take() - { - if let Some(path) = path - && let Err(error) = self.write_accessibility(&path) - { - eprintln!("{error}"); + // Quiet on two frames apart, what a thread sent as it ended has been taken. + let quiet = self.settled() + && self.opening.is_none() + && RUNNING.load(Ordering::Relaxed) == 0 + && self.search.pending.load(Ordering::Relaxed) == 0; + if let Some((path, settled, was)) = self.replay_settle.take() { + if !(quiet && was) { + self.replay_settle = Some((path, settled, quiet)); + } else { + if let Some(path) = path + && let Err(error) = self.write_accessibility(&path) + { + eprintln!("{error}"); + } + let _ = settled.send(()); } - let _ = settled.send(()); } } #[cfg(not(target_arch = "wasm32"))] diff --git a/crates/snowbound/src/pane.rs b/crates/snowbound/src/pane.rs index 6a9989eb51d90369388bcb7fffcf777a05329c83..1bedb5be3165617fb2324d084337549ffb0eaa46 100644 --- a/crates/snowbound/src/pane.rs +++ b/crates/snowbound/src/pane.rs @@ -754,7 +754,7 @@ impl State { if found.is_empty() { let status = if self.search.query.trim().is_empty() { "" - } else if self.search.busy.load(Ordering::Relaxed) { + } else if self.search.pending.load(Ordering::Relaxed) > 0 { "Searching…" } else { "No matches" @@ -837,7 +837,7 @@ impl State { mut unchecked: bool, mut scope: TagScope, ) { - let status = if self.search.busy.load(Ordering::Relaxed) { + let status = if self.search.pending.load(Ordering::Relaxed) > 0 { "Searching…" } else { "Search completed" diff --git a/crates/snowbound/src/search.rs b/crates/snowbound/src/search.rs index 97e813e5fde4f8d7ac0a72f442cbe9607ce5533f..fe34337b60d4e7039974e9782815461e04b62ead 100644 --- a/crates/snowbound/src/search.rs +++ b/crates/snowbound/src/search.rs @@ -14,7 +14,7 @@ use std::{ path::{Path, PathBuf}, sync::{ Arc, Mutex, Weak, - atomic::{AtomicBool, AtomicU64, Ordering}, + atomic::{AtomicU64, AtomicUsize, Ordering}, mpsc, }, time::{Duration, SystemTime}, @@ -115,7 +115,8 @@ pub struct Search { pub(crate) index: Arc>, /// Counts the index's changes, so results are made again when it changes. version: Arc, - pub(crate) busy: Arc, + /// Jobs sent the index and not yet answered. + pub(crate) pending: Arc, jobs: mpsc::Sender, /// The index's own state, which the browser, having no thread for it, keeps here to run /// the jobs sent after each frame. @@ -140,10 +141,14 @@ impl Search { let (jobs, receiver) = mpsc::channel(); let index = Arc::new(Mutex::new(Index::default())); let version = Arc::new(AtomicU64::new(0)); - let busy = Arc::new(AtomicBool::new(false)); + let pending = Arc::new(AtomicUsize::new(0)); let indexing = Indexing { jobs: receiver, - shared: (Arc::clone(&index), Arc::clone(&version), Arc::clone(&busy)), + shared: ( + Arc::clone(&index), + Arc::clone(&version), + Arc::clone(&pending), + ), redraw, stamps: HashMap::new(), }; @@ -163,7 +168,7 @@ impl Search { reveal: None, index, version, - busy, + pending, jobs, #[cfg(target_arch = "wasm32")] indexing, @@ -189,6 +194,7 @@ impl Search { } fn send(&self, job: Job) { + self.pending.fetch_add(1, Ordering::Relaxed); let _ = self.jobs.send(job); #[cfg(target_arch = "wasm32")] { @@ -239,7 +245,7 @@ pub(crate) fn now() -> u64 { /// The index thread's jobs and what it keeps between them. struct Indexing { jobs: mpsc::Receiver, - shared: (Arc>, Arc, Arc), + shared: (Arc>, Arc, Arc), redraw: std::task::Waker, stamps: HashMap, } @@ -249,7 +255,6 @@ impl Indexing { #[cfg(not(target_arch = "wasm32"))] fn run(mut self) { while let Ok(first) = self.jobs.recv() { - self.shared.2.store(true, Ordering::Relaxed); // Typing sends a job a keystroke; a pause gathers them into one read. if matches!(first, Job::Pages { .. }) { std::thread::sleep(SETTLE); @@ -261,13 +266,14 @@ impl Indexing { /// Answers `first` and the jobs waiting behind it, the newest notebooks first, then page /// changes, and wakes the window once the index changed. fn answer(&mut self, first: Job) { - let (index, version, busy) = &self.shared; + let (index, version, pending) = &self.shared; let (jobs, stamps) = (&self.jobs, &mut self.stamps); - busy.store(true, Ordering::Relaxed); let mut notebooks = None; let mut changed = HashSet::new(); let mut pages: HashMap, HashSet)> = HashMap::new(); + let mut answered = 0; for job in std::iter::once(first).chain(jobs.try_iter()) { + answered += 1; match job { Job::Notebooks { libraries, @@ -300,7 +306,7 @@ impl Indexing { version.fetch_add(1, Ordering::Relaxed); } lap("index", start); - busy.store(false, Ordering::Relaxed); + pending.fetch_sub(answered, Ordering::Relaxed); self.redraw.wake_by_ref(); } } @@ -933,7 +939,7 @@ impl State { // OneNote's status line: whether the search is done, and where it looked. let status = if query.is_empty() { "Search In:" - } else if self.search.busy.load(Ordering::Relaxed) { + } else if self.search.pending.load(Ordering::Relaxed) > 0 { "Searching:" } else if found.is_empty() { "No matches:" diff --git a/crates/snowbound/tests/replay.rs b/crates/snowbound/tests/replay.rs index e9cb503f650da9a7c1416b594c2c7518d02fa61e..4029f0c3009722df3736766b41e950d0dbf629ba 100644 --- a/crates/snowbound/tests/replay.rs +++ b/crates/snowbound/tests/replay.rs @@ -38,13 +38,13 @@ fn replay(scratch: &Scratch, notebook: Option<&Path>, steps: &[&str]) -> Vec { let path = dir.join(format!("{name}.txt")); - script += &format!("accessibility {}\nwait 100\n", path.display()); + script += &format!("accessibility {}\n", path.display()); trees.push(path); } None => script += &format!("{step}\n"), @@ -115,6 +115,7 @@ fn each_toolbar_control_has_a_name_of_its_own_at_every_width() { let mut steps = Vec::new(); for width in ["1600", "1180", "900", "600"] { steps.push(format!("resize {width} 760")); + // The window system resizes the window, which settling does not wait for. steps.push("wait 300".into()); steps.push(format!("accessibility {width}")); } @@ -140,17 +141,17 @@ fn the_palette_lists_recent_pages_and_the_actions_a_context_menu_offers() { let scratch = Scratch::new("palette-actions"); let notebook = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../corpus/cross-container/candidate"); - let palette = ["modifiers command", "key p", "modifiers", "wait 400"]; - let chord = ["modifiers command", "key k", "modifiers", "wait 400"]; + let palette = ["modifiers command", "key p", "modifiers", "settle"]; + let chord = ["modifiers command", "key k", "modifiers", "settle"]; let mut steps = Vec::from(palette); - steps.extend(["type type over", "wait 200", "key Enter", "wait 800"]); + steps.extend(["type type over", "settle", "key Enter", "settle"]); steps.extend(palette); steps.push("accessibility recent"); steps.extend(chord); steps.push("accessibility actions"); - steps.extend(["key Escape", "wait 400", "accessibility back"]); - steps.extend(["key Escape", "wait 400", "move 1000 87", "press right"]); - steps.extend(["release right", "wait 400", "accessibility context"]); + steps.extend(["key Escape", "accessibility back"]); + steps.extend(["key Escape", "settle", "move 1000 87", "press right"]); + steps.extend(["release right", "accessibility context"]); let [recent, actions, back, context] = replay(&scratch, Some(¬ebook), &steps) .try_into() .unwrap(); @@ -201,11 +202,11 @@ fn undo_walks_back_through_new_pages_and_their_titles() { let notebook = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../corpus/cross-container/candidate"); let chord = - |modifiers: &str, key: &str| [modifiers, key, "modifiers", "wait 800"].map(String::from); + |modifiers: &str, key: &str| [modifiers, key, "modifiers", "settle"].map(String::from); let mut steps = Vec::new(); for title in ["type One", "type Two"] { steps.extend(chord("modifiers command", "key n")); - steps.extend(["settle", title, "wait 300"].map(String::from)); + steps.push(title.into()); } steps.extend(chord("modifiers command control", "key Left")); steps.push("accessibility back".into()); @@ -267,11 +268,7 @@ fn search(query: &str, name: &str) -> Vec { ["modifiers command", "key e", "modifiers"] .into_iter() .map(String::from) - .chain([ - format!("type {query}"), - "wait 2500".into(), - format!("accessibility {name}"), - ]) + .chain([format!("type {query}"), format!("accessibility {name}")]) .collect() } @@ -279,13 +276,13 @@ fn search(query: &str, name: &str) -> Vec { fn version_menu() -> Vec { let mut steps: Vec = ["modifiers command shift", "key p", "modifiers"] .into_iter() - .chain(["type Page Versions", "wait 300", "key Enter", "wait 1000"]) + .chain(["type Page Versions", "settle", "key Enter", "settle"]) .map(String::from) .collect(); // The version row under the page, then the yellow bar above the version. for point in ["1003 114", "177 94"] { steps.extend([format!("move {point}"), "press".into(), "release".into()]); - steps.push("wait 1200".into()); + steps.push("settle".into()); } steps } @@ -301,10 +298,10 @@ fn search_finds_what_a_template_put_on_the_page() { let scratch = Scratch::new("template-search"); let notebook = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../corpus/cross-container/candidate"); - let mut steps: Vec = ["modifiers command", "key n", "modifiers", "wait 1200"] + let mut steps: Vec = ["modifiers command", "key n", "modifiers", "settle"] .into_iter() // The Meeting tile of the template strip over the new page. - .chain(["move 615 287", "press", "release", "wait 3000"]) + .chain(["move 615 287", "press", "release", "settle"]) .map(String::from) .collect(); steps.extend(search("Attendees", "found")); @@ -325,7 +322,7 @@ fn search_finds_a_restored_version() { Path::new(env!("CARGO_MANIFEST_DIR")).join("../../corpus/page-versions/candidate-restore"); let mut steps = version_menu(); // Restore Version. - steps.extend(["key Down", "key Enter", "wait 2000"].map(String::from)); + steps.extend(["key Down", "key Enter", "settle"].map(String::from)); steps.extend(search("Second author", "found")); let [found] = run(&scratch, ¬ebook, &steps).try_into().unwrap(); assert!(!search_results(&found).is_empty(), "{found}"); @@ -339,8 +336,8 @@ fn a_version_copied_into_its_section_is_listed_and_found() { Path::new(env!("CARGO_MANIFEST_DIR")).join("../../corpus/page-versions/candidate-restore"); let mut steps = version_menu(); // Copy Page To…, then the section itself. - steps.extend(["key Down", "key Down", "key Down", "key Enter", "wait 600"].map(String::from)); - steps.extend(["key Down", "key Enter", "wait 2000", "accessibility copied"].map(String::from)); + steps.extend(["key Down", "key Down", "key Down", "key Enter", "settle"].map(String::from)); + steps.extend(["key Down", "key Enter", "accessibility copied"].map(String::from)); steps.extend(search("Second author", "found")); let [copied, found] = run(&scratch, ¬ebook, &steps).try_into().unwrap(); let pages = page_tabs(&copied); @@ -359,9 +356,9 @@ fn renaming_a_section_from_its_tab_types_into_the_sidebar_at_once() { let scratch = Scratch::new("rename-shut-sidebar"); let notebook = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../corpus/cross-container/candidate"); - let mut steps = vec!["move 53 53", "press right", "release right", "wait 400"]; - steps.extend(["key Down", "key Enter", "wait 400"]); - steps.extend(["type Renamed", "wait 100", "accessibility typed"]); + let mut steps = vec!["move 53 53", "press right", "release right", "settle"]; + steps.extend(["key Down", "key Enter", "settle"]); + steps.extend(["type Renamed", "accessibility typed"]); let [typed] = replay(&scratch, Some(¬ebook), &steps) .try_into() .unwrap(); @@ -378,10 +375,10 @@ fn the_find_bar_keeps_room_for_the_query() { let scratch = Scratch::new("find-room"); let notebook = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../corpus/cross-container/candidate"); - let mut steps = vec!["modifiers command", "key f", "modifiers", "wait 200"]; - steps.extend(["type Alpha", "wait 400", "move 600 600", "press", "release"]); - steps.extend(["wait 200", "move 1000 52", "press", "release", "wait 200"]); - steps.extend(["type zz", "wait 200", "accessibility found"]); + let mut steps = vec!["modifiers command", "key f", "modifiers", "settle"]; + steps.extend(["type Alpha", "settle", "move 600 600", "press", "release"]); + steps.extend(["settle", "move 1000 52", "press", "release", "settle"]); + steps.extend(["type zz", "accessibility found"]); let [found] = replay(&scratch, Some(¬ebook), &steps) .try_into() .unwrap(); @@ -417,8 +414,13 @@ fn a_launch_returns_to_the_page_each_notebook_was_left_on() { }); std::fs::write(scratch.0.join("settings.json"), settings.to_string()).unwrap(); // The other notebook's section in the sidebar. - let mut steps = vec!["accessibility launched", "move 63 104", "press", "release"]; - steps.extend(["wait 4000", "accessibility other"]); + let steps = [ + "accessibility launched", + "move 63 104", + "press", + "release", + "accessibility other", + ]; let shown = |tree: &str| page_tabs(tree).into_iter().find(|tab| tab.starts_with('*')); let [launched, other] = replay(&scratch, Some(&source), &steps).try_into().unwrap(); assert_eq!( -- 2.54.0