From 5e07917fff259d26e12aec7b3a1342a5576f2592 Mon Sep 17 00:00:00 2001 From: matt wilkie Date: Tue, 6 Oct 2026 23:01:21 -0700 Subject: [PATCH] Deepen watch mode into a debounce module C2 / Beads mdo-6w5.3 (epic mdo-6w5, PR order 3 of 6; stacked on the C6 branch). Watch mode's mechanics were interleaved with main.rs's CLI plumbing: parent-dir registration rationale, event relevance, the trailing-edge debounce drain, channel death, and the render-failure continuation all lived inline around ~180 lines with only two spawned-binary e2e tests as coverage, both unix-gated. New src/watch.rs in the lib crate owns all three mechanics behind one interface: - watch::run(input, render) - one deep watch entry point. Registers the watch BEFORE rendering (a save landing during the initial render is queued, not lost), runs the render callback once immediately, then once per settled burst. A render callback Err (transient filesystem state mid-rename) never ends watch mode; channel death ends it gracefully with Ok(()). - The debounce timing and the loop clock are injectable at pump level: unit tests script a fake event channel (relative arrival times) against a virtual clock, so the semantics are covered cross-platform by construction: burst collapse, bursts running longer than the window, events after the window, sibling noise, watcher error storms, channel death during a drain, callback errors, and event-relevance gates. - watch-mode UX lines (Watching / File changed, re-rendering / Watcher error) are the module's; rendering and its reporting stay with the CLI via the callback. main.rs keeps the CLI-local helpers (render_and_report, launch_open_- output) and its exit-code policy; the -watch -open launch hook stays a one-time duty consumed exactly on the first callback, as before the module existed. tests/cli.rs keeps one spawned-binary e2e smoke test (watch_survives_atomic_rename); the watch_ignores_sibling_changes e2e moves into the module unit tests. Gates: fmt --check, clippy --all-targets --all-features -D warnings, and cargo test --all-targets --all-features --locked all green (12 new watch unit tests; byte-identity harness against the pre-C1 binary still passes for the once-mode CLI surface). Beads-Ref: mdo-6w5.3 Agent-Signature: kilo-kilo-auto-efficient on behalf of matt --- src/lib.rs | 4 +- src/main.rs | 304 ++++++++++------------------ src/watch.rs | 558 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/cli.rs | 58 +----- 4 files changed, 666 insertions(+), 258 deletions(-) create mode 100644 src/watch.rs diff --git a/src/lib.rs b/src/lib.rs index 26a262e..c962aed 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,5 @@ -//! Shared mdo rendering, temp-output, browser launch, and integration helpers. +//! Shared mdo rendering, watch-mode, temp-output, browser launch, and +//! integration helpers. use std::collections::hash_map::DefaultHasher; use std::collections::BTreeSet; @@ -23,6 +24,7 @@ use url::Url; mod android; pub mod file_manager; mod highlight; +pub mod watch; #[cfg(target_os = "windows")] pub mod windows_setup; diff --git a/src/main.rs b/src/main.rs index bb60f10..c98e10a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -33,15 +33,13 @@ use std::io::{self, IsTerminal, Write}; use std::path::{Path, PathBuf}; -use std::sync::mpsc::channel; -use std::time::Duration; +use std::time::{Duration, Instant}; use clap::Parser; use mdo_cli::{ - convert, file_manager, launch_browser, open_setup_sample, temp_output_for, ConvertOutcome, - ConvertRequest, RenderError, StageTimings, + convert, file_manager, launch_browser, open_setup_sample, temp_output_for, watch, + ConvertOutcome, ConvertRequest, RenderError, StageTimings, }; -use notify::{recommended_watcher, EventKind, RecursiveMode, Watcher}; /// Markdown to HTML converter. Converts once by default; pass --watch to keep watching. #[derive(Parser)] @@ -398,190 +396,122 @@ fn main() -> notify::Result<()> { (None, false) => (input.with_extension("html"), false), }; - // Register the watch BEFORE the initial render so a save that lands - // while that render runs is queued by the watcher rather than silently - // lost; the watch loop below drains it and re-renders. Registration - // failure aborts before any rendering, which is fine — the user asked - // for a watch we cannot deliver. - // - // Watch the parent DIRECTORY rather than the file itself. Editors that - // save atomically (write a temp file, then rename it over the target) - // replace the target's inode; a watch on the file itself goes dead the - // moment that happens because the inode/handle notify was watching is - // gone. Watching the directory survives renames, deletes, and recreates - // — we just have to filter directory events down to ones that touch our - // target file. - // - // Resolve the target once, up front. Comparing later events against - // this fixed absolute path — rather than re-canonicalizing each event's - // path — matters because the target may momentarily not exist - // mid-rename, which would make canonicalize fail and misclassify a - // perfectly relevant event. - // - // Known limitation: canonicalizing a symlinked input means we watch the - // TARGET's parent, so edits made through the link are seen, but an - // editor atomically replacing the symlink itself is not. Watching both - // parents isn't worth the complexity until someone actually hits this. - let watch_setup = if args.watch { - let input_absolute = std::fs::canonicalize(&input).unwrap_or_else(|_| { - std::env::current_dir() - .map(|cwd| cwd.join(&input)) - .unwrap_or_else(|_| input.clone()) + if args.watch { + // Watch's contract: register before rendering (a save that lands + // while the initial render runs is queued by the watcher rather + // than silently lost), run the render callback once immediately, + // then pump. In watch mode we deliberately do NOT apply the one-shot + // exit-code check below: a one-time --open failure must not end + // watch mode, since the whole point is to keep re-rendering on + // future edits. + let mut pending_open_launch = args.open; + return watch::run(&input, &mut || { + let result = render_and_report( + &input, + &output, + args.bare, + args.unsafe_html, + args.css.as_deref(), + private_output, + args.verbose, + ); + if pending_open_launch { + // The launch hook is a one-time post-initial-render duty + // (the pump runs the callback immediately): consume the + // pending flag on the first callback whether or not that + // render succeeded, exactly as the pre-module flow did. + if result.is_ok() { + launch_open_output(&output, args.verbose); + } + pending_open_launch = false; + } + result.map(|_| ()) }); - let watch_dir = input_absolute - .parent() - .map(PathBuf::from) - .unwrap_or_else(|| PathBuf::from(".")); - let target_file_name = input_absolute.file_name().map(|n| n.to_os_string()); - - let (tx, rx) = channel(); - let mut watcher = recommended_watcher(tx)?; - watcher.watch(&watch_dir, RecursiveMode::NonRecursive)?; - Some((watcher, rx, watch_dir, target_file_name)) - } else { - None - }; + } - let converted = report_render( + let converted = render_and_report( &input, &output, - &convert(ConvertRequest { - input: &input, - output: &output, - bare: args.bare, - unsafe_html: args.unsafe_html, - private_output, - css_override: args.css.as_deref(), - }), + args.bare, + args.unsafe_html, + args.css.as_deref(), + private_output, args.verbose, - ); + ) + .is_ok(); // Track whether --open promised a browser launch and failed to deliver // one, so the one-shot exit code below can be honest about it. let mut open_launch_failed = false; if args.open && converted { - // Browser launch is reported separately from the render workflow: - // launch_browser only initiates the launch (fire-and-forget), so - // this measures initiation, not the browser starting up. - let launch_start = std::time::Instant::now(); - match launch_browser(&output) { - Ok(()) => { - if args.verbose { - eprintln!( - "⏱ Browser launch initiated in {:.3} ms", - launch_start.elapsed().as_secs_f64() * 1000.0 - ); - } - println!("🌐 Opened {:?} in default browser", output); - } - Err(e) => { - // The render already succeeded and nothing removes the - // rendered file on a launch failure, so point the user at it - // instead of silently downgrading this to a warning. - eprintln!("❌ Failed to launch browser: {}", e); - eprintln!(" Rendered output is available at {:?}", output); - open_launch_failed = true; - } - } + open_launch_failed = launch_open_output(&output, args.verbose); } - if !args.watch { - // Exit non-zero on a failed one-shot render so scripts and the docs - // pipeline can detect errors. A failed --open browser launch is the - // same kind of broken promise even though the render itself - // succeeded: `mdo --open` only fully succeeds when the file is both - // rendered AND opened, so it exits non-zero here too. In watch mode - // we deliberately do NOT apply this exit-code check: watch's - // contract is to keep running regardless of a one-time --open - // failure, since the whole point is to keep re-rendering on future - // edits. The stderr message above still fires so the user learns - // about the failed launch either way. - if converted && !open_launch_failed { - return Ok(()); - } - std::process::exit(1); + // Exit non-zero on a failed one-shot render so scripts and the docs + // pipeline can detect errors. A failed --open browser launch is the + // same kind of broken promise even though the render itself succeeded: + // `mdo --open` only fully succeeds when the file is both rendered AND + // opened, so it exits non-zero here too. + if converted && !open_launch_failed { + return Ok(()); } + std::process::exit(1); +} - let (_watcher, rx, watch_dir, target_file_name) = - watch_setup.expect("watch setup was built above whenever --watch is set"); - - // Still named after the file, not the directory: that's what the user - // asked to watch, even though the underlying notify watch is scoped one - // level up. - println!("👀 Watching {:?} for changes... (Ctrl+C to stop)", input); - - // Trailing-edge debounce window: once a relevant event arrives we keep - // draining/absorbing further relevant events for this long before - // rendering, so a burst (e.g. truncate + write, or temp-file write + - // rename) collapses into exactly one render of the final content. This - // is unlike a leading-edge "ignore anything for N ms after the last - // render" debounce, which can drop the trailing event of a burst if the - // burst runs longer than the window. - const DEBOUNCE: Duration = Duration::from_millis(200); - - loop { - // Block for the next event. Anything that isn't a relevant, - // content-changing event (wrong file, Access events, watcher - // errors) is skipped without starting a debounce window. - let event = match rx.recv() { - Ok(Ok(event)) => event, - Ok(Err(e)) => { - eprintln!("⚠️ Watcher error: {}", e); - continue; - } - Err(_) => return Ok(()), // watcher/channel gone; nothing left to watch - }; - - if !is_relevant_event(&event, target_file_name.as_deref(), &watch_dir) { - continue; - } +/// Run one render request and report it. All rendering output lives behind +/// `report_render`, so the one-shot path and watch-mode re-renders report +/// identically. The typed outcome still comes back: the watch callback +/// feeds its `Err` into `watch::run`, which must not end watch mode on a +/// transient filesystem state mid-rename. +fn render_and_report( + input: &Path, + output: &Path, + bare: bool, + unsafe_html: bool, + css_override: Option<&Path>, + private_output: bool, + verbose: bool, +) -> Result<(), RenderError> { + let outcome = convert(ConvertRequest { + input, + output, + bare, + unsafe_html, + private_output, + css_override, + }); + report_render(input, output, &outcome, verbose); + outcome.map(|_| ()) +} - // Drain/absorb further relevant events for a quiet window, then - // render once. This covers direct writes, truncate+rewrite, and - // atomic saves (temp file write followed by a rename onto the - // target) uniformly: whichever event kinds the editor and platform - // happen to emit (Create, Modify, Rename-as-Modify(Name), Remove - // followed by a Create), each just extends the quiet window until - // the burst settles. Only RELEVANT events extend the deadline: - // sibling-file noise merely waits out the remaining window, so a - // busy directory (a build churning artifacts next to the watched - // file) cannot postpone the render indefinitely. - let mut deadline = std::time::Instant::now() + DEBOUNCE; - loop { - let remaining = deadline.saturating_duration_since(std::time::Instant::now()); - if remaining.is_zero() { - break; - } - match rx.recv_timeout(remaining) { - Ok(Ok(event)) => { - if is_relevant_event(&event, target_file_name.as_deref(), &watch_dir) { - deadline = std::time::Instant::now() + DEBOUNCE; - } - // Irrelevant events neither extend nor cut the window. - } - Ok(Err(e)) => eprintln!("⚠️ Watcher error: {}", e), - Err(_) => break, // quiet window elapsed (or channel closed) +/// `--open`'s post-render launch hook: initiate the browser launch and +/// report it. Returns whether the launch failed to deliver on the promise — +/// the one-shot exit code owes this honesty; watch mode ignores failures +/// and keeps watching to re-render future edits. +fn launch_open_output(output: &Path, verbose: bool) -> bool { + // Browser launch is reported separately from the render workflow: + // launch_browser only initiates the launch (fire-and-forget), so + // this measures initiation, not the browser starting up. + let launch_start = Instant::now(); + match launch_browser(output) { + Ok(()) => { + if verbose { + eprintln!( + "⏱ Browser launch initiated in {:.3} ms", + launch_start.elapsed().as_secs_f64() * 1000.0 + ); } + println!("🌐 Opened {:?} in default browser", output); + false + } + Err(e) => { + // The render already succeeded and nothing removes the + // rendered file on a launch failure, so point the user at it + // instead of silently downgrading this to a warning. + eprintln!("❌ Failed to launch browser: {}", e); + eprintln!(" Rendered output is available at {:?}", output); + true } - - println!("🔁 File changed, re-rendering..."); - // A failed render (e.g. convert returned Err while the file was - // momentarily absent mid-rename) must not end watch mode; the ❌ - // error is reported by `report_render`, so we just loop and wait - // for the next event. - let _ = report_render( - &input, - &output, - &convert(ConvertRequest { - input: &input, - output: &output, - bare: args.bare, - unsafe_html: args.unsafe_html, - private_output, - css_override: args.css.as_deref(), - }), - args.verbose, - ); } } @@ -643,36 +573,6 @@ fn report_render_timings(input: &Path, timings: &StageTimings) { eprintln!(" total {:>10.3} ms", ms(timings.total)); } -/// True if `event` plausibly changed the content of the file named -/// `target_file_name` inside `watch_dir`. We match by file name (and, when -/// notify reports one, by parent directory) rather than canonicalizing the -/// event's path, since the target can momentarily not exist mid-rename. -/// Access events (reads, permission-bit-only changes) are excluded; every -/// other kind (Create, Modify, Remove, Any, Other) is treated as a possible -/// content change so recreation-after-remove and atomic renames are all -/// covered by the same check. -fn is_relevant_event( - event: ¬ify::Event, - target_file_name: Option<&std::ffi::OsStr>, - watch_dir: &std::path::Path, -) -> bool { - if matches!(event.kind, EventKind::Access(_)) { - return false; - } - - let Some(target_file_name) = target_file_name else { - return false; - }; - - event.paths.iter().any(|p| { - p.file_name() == Some(target_file_name) - && match p.parent() { - Some(parent) => parent == watch_dir, - None => true, - } - }) -} - #[cfg(test)] mod tests { use super::*; diff --git a/src/watch.rs b/src/watch.rs new file mode 100644 index 0000000..6ed723a --- /dev/null +++ b/src/watch.rs @@ -0,0 +1,558 @@ +//! Watch-mode mechanics, behind one interface: folder watching, event +//! relevance, and the trailing-edge debounce. +//! +//! The module owns exactly the three mechanics `mdo --watch` needs; the +//! render itself and how the CLI reports it belong to the caller, passed as +//! a render callback. `watch::run` registers the watch, runs the callback +//! once immediately, and then once per settled quiet window whenever the +//! watched file plausibly changed content. A callback `Err` (e.g. a +//! transient filesystem state mid-rename) is ignored and never ends watch +//! mode, matching the CLI's contract; channel death ends the watch +//! gracefully. +//! +//! Both the debounce timing and the loop clock are injectable at the pump +//! level, so unit tests script fake event channels with a virtual clock and +//! cover burst collapse, late events, sibling noise, watcher error storms, +//! and channel death cross-platform by construction. + +use std::ffi::OsStr; +use std::path::{Path, PathBuf}; +use std::sync::mpsc::{self, channel, RecvError, RecvTimeoutError}; +use std::time::{Duration, Instant}; + +use notify::{recommended_watcher, Event, EventKind, RecursiveMode, Watcher}; + +use crate::RenderError; + +/// Trailing-edge debounce window: once a relevant event arrives the pump +/// keeps draining/absorbing further relevant events for this long before +/// rendering, so a burst (e.g. truncate + write, or temp-file write + +/// rename) collapses into exactly one render of the final content. This is +/// unlike a leading-edge "ignore anything for N ms after the last render" +/// debounce, which can drop the trailing event of a burst if the burst runs +/// longer than the window. +const DEFAULT_DEBOUNCE: Duration = Duration::from_millis(200); + +/// Run watch mode for `input`: register a parent-directory watch, run +/// `render` once immediately, then re-run it once per settled burst of +/// events that plausibly changed the file's content. +/// +/// `render` runs once immediately — before any event is seen — so callers +/// that must do a one-time thing after the initial render (e.g. the +/// `--open` browser launch) guard that in their own callback state. Its +/// `Err` is ignored: a failed render must never end watch mode. +/// +/// Errors propagate only from registration itself, before any render: a +/// watch we cannot deliver aborts up front rather than rendering once and +/// failing later. Once pumping, the loop returns `Ok(())` only when the +/// watcher channel dies; it otherwise runs until killed. +pub fn run Result<(), RenderError>>( + input: &Path, + render: &mut F, +) -> notify::Result<()> { + // Watch the parent DIRECTORY rather than the file itself. Editors that + // save atomically (write a temp file, then rename it over the target) + // replace the target's inode; a watch on the file itself goes dead the + // moment that happens because the inode/handle notify was watching is + // gone. Watching the directory survives renames, deletes, and recreates + // — we just have to filter directory events down to ones that touch our + // target file. + // + // Resolve the target once, up front. Comparing later events against + // this fixed absolute path — rather than re-canonicalizing each event's + // path — matters because the target may momentarily not exist + // mid-rename, which would make canonicalize fail and misclassify a + // perfectly relevant event. + // + // Known limitation: canonicalizing a symlinked input means we watch the + // TARGET's parent, so edits made through the link are seen, but an + // editor atomically replacing the symlink itself is not. Watching both + // parents isn't worth the complexity until someone actually hits this. + let input_absolute = std::fs::canonicalize(input).unwrap_or_else(|_| { + std::env::current_dir() + .map(|cwd| cwd.join(input)) + .unwrap_or_else(|_| input.to_path_buf()) + }); + let watch_dir = input_absolute + .parent() + .map(PathBuf::from) + .unwrap_or_else(|| PathBuf::from(".")); + let target_file_name = input_absolute.file_name().map(|n| n.to_os_string()); + + // Register the watch BEFORE the initial render so a save that lands + // while that render runs is queued by the watcher rather than silently + // lost; the pump below drains it and re-renders. Registration + // failure aborts before any rendering, which is fine — the user asked + // for a watch we cannot deliver. + + let (tx, rx) = channel(); + let mut watcher = recommended_watcher(tx)?; + watcher.watch(&watch_dir, RecursiveMode::NonRecursive)?; + + let _ = render(); + + // Still named after the file, not the directory: that's what the user + // asked to watch, even though the underlying notify watch is scoped one + // level up. + println!("👀 Watching {:?} for changes... (Ctrl+C to stop)", input); + + pump( + &rx, + &watch_dir, + target_file_name.as_deref(), + render, + DEFAULT_DEBOUNCE, + &SystemClock, + ); + Ok(()) +} + +/// The event pump: block for the next event, skip everything that isn't a +/// relevant change to the target file, drain/absorb bursts through the +/// trailing-edge debounce window, then render once per settled burst. +/// +/// `debounce` and `clock` are the injectable timing seams (unit tests +/// script relative arrival times against a virtual clock; production uses +/// the system monotonic clock). +fn pump Result<(), RenderError>, C: Clock>( + rx: &R, + watch_dir: &Path, + target_file_name: Option<&OsStr>, + render: &mut F, + debounce: Duration, + clock: &C, +) { + loop { + // Block for the next event. Anything that isn't a relevant, + // content-changing event (wrong file, Access events, watcher + // errors) is skipped without starting a debounce window. + let event = match rx.recv() { + Ok(Ok(event)) => event, + Ok(Err(e)) => { + eprintln!("⚠️ Watcher error: {}", e); + continue; + } + Err(_) => return, // watcher/channel gone; nothing left to watch + }; + + if !is_relevant_event(&event, target_file_name, watch_dir) { + continue; + } + + // Drain/absorb further relevant events for a quiet window, then + // render once. This covers direct writes, truncate+rewrite, and + // atomic saves (temp file write followed by a rename onto the + // target) uniformly: whichever event kinds the editor and platform + // happen to emit (Create, Modify, Rename-as-Modify(Name), Remove + // followed by a Create), each just extends the quiet window until + // the burst settles. Only RELEVANT events extend the deadline: + // sibling-file noise merely waits out the remaining window, so a + // busy directory (a build churning artifacts next to the watched + // file) cannot postpone the render indefinitely. + let mut deadline = clock.now() + debounce; + loop { + let remaining = deadline.saturating_duration_since(clock.now()); + if remaining.is_zero() { + break; + } + match rx.recv_timeout(remaining) { + Ok(Ok(event)) => { + if is_relevant_event(&event, target_file_name, watch_dir) { + deadline = clock.now() + debounce; + } + // Irrelevant events neither extend nor cut the window. + } + Ok(Err(e)) => eprintln!("⚠️ Watcher error: {}", e), + Err(_) => break, // quiet window elapsed (or channel closed) + } + } + + println!("🔁 File changed, re-rendering..."); + // A failed render (e.g. one that ran while the file was momentarily + // absent mid-rename) must not end watch mode; its reporting already + // happened inside the callback, so just loop and wait for the next + // event. + let _ = render(); + } +} + +/// True if `event` plausibly changed the content of the file named +/// `target_file_name` inside `watch_dir`. We match by file name (and, when +/// notify reports one, by parent directory) rather than canonicalizing the +/// event's path, since the target can momentarily not exist mid-rename. +/// Access events (reads, permission-bit-only changes) are excluded; every +/// other kind (Create, Modify, Remove, Any, Other) is treated as a possible +/// content change so recreation-after-remove and atomic renames are all +/// covered by the same check. +fn is_relevant_event(event: &Event, target_file_name: Option<&OsStr>, watch_dir: &Path) -> bool { + if matches!(event.kind, EventKind::Access(_)) { + return false; + } + + let Some(target_file_name) = target_file_name else { + return false; + }; + + event.paths.iter().any(|p| { + p.file_name() == Some(target_file_name) + && match p.parent() { + Some(parent) => parent == watch_dir, + None => true, + } + }) +} + +/// One item on the watch event channel: a real event, or a watcher-level +/// error delivered as a channel payload (the queue keeps flowing either +/// way). +type EventItem = Result; + +/// The receiving half of the watch event channel narrowed to the exact +/// shape the pump uses. Abstracted so unit tests script fake sequences. +trait EventRx { + fn recv(&self) -> Result; + fn recv_timeout(&self, timeout: Duration) -> Result; +} + +impl EventRx for mpsc::Receiver { + fn recv(&self) -> Result { + mpsc::Receiver::recv(self) + } + + fn recv_timeout(&self, timeout: Duration) -> Result { + mpsc::Receiver::recv_timeout(self, timeout) + } +} + +/// Where the pump reads its clock. Production uses the system monotonic +/// clock; tests inject a virtual one. +trait Clock { + fn now(&self) -> Instant; +} + +struct SystemClock; + +impl Clock for SystemClock { + fn now(&self) -> Instant { + Instant::now() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::cell::Cell; + use std::cell::RefCell; + use std::collections::VecDeque; + use std::rc::Rc; + + use notify::event::{AccessKind, ModifyKind}; + + /// Virtual monotonic clock shared with the fake channel: entries in a + /// script declare how long after "now" their payload is ready. + #[derive(Clone)] + struct VirtualClock(Rc>); + + impl VirtualClock { + fn new() -> Self { + Self(Rc::new(Cell::new(Instant::now()))) + } + + fn advance(&self, by: Duration) { + self.0.set(self.0.get() + by); + } + } + + impl Clock for VirtualClock { + fn now(&self) -> Instant { + self.0.get() + } + } + + /// One scripted channel entry: a payload plus how long after the + /// current virtual time it becomes available. + struct Scripted { + ready_after: Duration, + payload: EventItem, + } + + /// Fake channel half driving `pump` from a script. Events ready within + /// the polled timeout are delivered on that poll; anything later goes + /// past the window (timed out), exactly as a real channel would. + struct FakeRx { + clock: VirtualClock, + script: RefCell>, + } + + impl FakeRx { + fn now_or_elapsed(&self, timeout: Duration) -> Result { + let next = self.script.borrow().front().map(|s| s.ready_after); + match next { + Some(ready_after) if ready_after <= timeout => { + self.clock.advance(ready_after); + let scripted = self + .script + .borrow_mut() + .pop_front() + .expect("front checked above"); + Ok(scripted.payload) + } + _ => { + self.clock.advance(timeout); + Err(RecvTimeoutError::Timeout) + } + } + } + } + + impl EventRx for FakeRx { + fn recv(&self) -> Result { + match self.script.borrow_mut().pop_front() { + Some(scripted) => { + self.clock.advance(scripted.ready_after); + Ok(scripted.payload) + } + None => Err(RecvError), + } + } + + fn recv_timeout(&self, timeout: Duration) -> Result { + self.now_or_elapsed(timeout) + } + } + + fn event_ready_after(ready_after: u64, kind: EventKind, path: PathBuf) -> Scripted { + Scripted { + ready_after: Duration::from_millis(ready_after), + payload: Ok(Event::new(kind).add_path(path)), + } + } + + fn watcher_error_after(ready_after: u64) -> Scripted { + Scripted { + ready_after: Duration::from_millis(ready_after), + payload: Err(notify::Error::io(std::io::Error::other("watcher failed"))), + } + } + + /// A content-changing event for `name` inside `watch_dir`. + fn modify(name: &str, watch_dir: &Path, ready_after: u64) -> Scripted { + event_ready_after( + ready_after, + EventKind::Modify(ModifyKind::Any), + watch_dir.join(name), + ) + } + + /// A change to a file that is not the watched target. + fn sibling(name: &str, watch_dir: &Path, ready_after: u64) -> Scripted { + event_ready_after( + ready_after, + EventKind::Modify(ModifyKind::Any), + watch_dir.join(name), + ) + } + + fn access(watch_dir: &Path, ready_after: u64) -> Scripted { + event_ready_after( + ready_after, + EventKind::Access(AccessKind::Any), + watch_dir.join("sample.md"), + ) + } + + /// Pump over a script, counting renders in the callback. + fn pump_scripted(script: Vec) -> (usize, VirtualClock) { + let clock = VirtualClock::new(); + let renders = Rc::new(Cell::new(0_usize)); + let rx = FakeRx { + clock: clock.clone(), + script: RefCell::new(script.into()), + }; + pump( + &rx, + Path::new("/watch"), + Some(OsStr::new("sample.md")), + &mut || { + renders.set(renders.get() + 1); + Ok(()) + }, + Duration::from_millis(200), + &clock, + ); + (renders.get(), clock) + } + + #[test] + fn burst_collapses_into_one_render() { + // Three events inside one window: one render of the final content. + let (renders, _clock) = pump_scripted(vec![ + modify("sample.md", Path::new("/watch"), 50), + modify("sample.md", Path::new("/watch"), 70), + modify("sample.md", Path::new("/watch"), 60), + ]); + assert_eq!(renders, 1, "a settled burst renders exactly once"); + } + + #[test] + fn burst_running_longer_than_window_still_renders_the_trailing_event() { + // Events every 80ms outrun a 200ms quiet window for a while; no + // render fires mid-burst — the trailing event is never dropped. + let (renders, _clock) = pump_scripted(vec![ + modify("sample.md", Path::new("/watch"), 50), + modify("sample.md", Path::new("/watch"), 80), + modify("sample.md", Path::new("/watch"), 80), + modify("sample.md", Path::new("/watch"), 80), + modify("sample.md", Path::new("/watch"), 80), + ]); + assert_eq!(renders, 1, "a long burst settles into exactly one render"); + } + + #[test] + fn event_after_the_window_renders_again() { + // One burst, then quiet past the window, then another event: the + // first event settles into render one; the late event is NOT + // dropped — it becomes the next outer event and renders again. + let (renders, _clock) = pump_scripted(vec![ + modify("sample.md", Path::new("/watch"), 50), + modify("sample.md", Path::new("/watch"), 700), + ]); + assert_eq!(renders, 2, "events after the window are fresh triggers"); + } + + #[test] + fn sibling_noise_renders_nothing() { + // Sibling-file churn alone never triggers or extends a window. + let (renders, _clock) = pump_scripted(vec![ + sibling("other.md", Path::new("/watch"), 50), + sibling("build.log", Path::new("/watch"), 80), + ]); + assert_eq!(renders, 0, "no target file in any event, no render"); + } + + #[test] + fn sibling_noise_waits_out_but_does_not_extend_the_window() { + // A sibling event arriving well past the current window cannot + // extend the deadline: the pump waits out only the remaining window + // (bounded by one debounce) and renders; the sibling is then seen + // as the next outer event, classified irrelevant, and skipped. + let (renders, _clock) = pump_scripted(vec![ + modify("sample.md", Path::new("/watch"), 50), + sibling("other.md", Path::new("/watch"), 1000), + ]); + assert_eq!(renders, 1, "the sibling burst never postponed the render"); + } + + #[test] + fn access_events_are_never_relevant() { + let (renders, _clock) = pump_scripted(vec![access(Path::new("/watch"), 50)]); + assert_eq!(renders, 0, "Access events are excluded"); + } + + #[test] + fn channel_death_before_any_event_ends_the_watch_gracefully() { + let (renders, _clock) = pump_scripted(vec![]); + assert_eq!(renders, 0); + } + + #[test] + fn channel_death_during_drain_renders_and_exits() { + // An event lands, the window is open, the channel dies: the + // pending burst still renders, then the watch ends. + let (renders, _clock) = pump_scripted(vec![modify("sample.md", Path::new("/watch"), 50)]); + assert_eq!(renders, 1); + } + + #[test] + fn watcher_error_storm_renders_nothing_and_survives() { + let script: Vec = (0..50u64) + .map(|i| watcher_error_after(i * 10 + 1)) + .collect(); + let (renders, _clock) = pump_scripted(script); + assert_eq!(renders, 0, "watcher errors are not renders"); + } + + #[test] + fn watcher_errors_inside_the_drain_do_not_extend_the_window() { + // Errors arriving inside a window neither extend nor cut it: they + // wait out the remaining window like any other noise. + let script = vec![ + modify("sample.md", Path::new("/watch"), 50), + watcher_error_after(60), + watcher_error_after(60), + ]; + let (renders, _clock) = pump_scripted(script); + assert_eq!(renders, 1); + } + + #[test] + fn callback_errors_never_end_watch_mode() { + // A render callback that reports a transient filesystem failure + // (e.g. the file was absent mid-rename) must not end watch mode. + let clock = VirtualClock::new(); + let renders = Rc::new(Cell::new(0_usize)); + let failures = Rc::new(Cell::new(0_usize)); + let rx = FakeRx { + clock: clock.clone(), + script: RefCell::new( + vec![ + // Two bursts separated past the window: render One and + // render two must BOTH happen for the errors never to + // have ended watch mode. + modify("sample.md", Path::new("/watch"), 50), + modify("sample.md", Path::new("/watch"), 700), + ] + .into(), + ), + }; + let renders_flag = renders.clone(); + let failures_flag = failures.clone(); + pump( + &rx, + Path::new("/watch"), + Some(OsStr::new("sample.md")), + &mut || { + renders_flag.set(renders_flag.get() + 1); + failures_flag.set(failures_flag.get() + 1); + Err(RenderError::Read { + path: PathBuf::from("/watch/sample.md"), + source: std::io::Error::other("transient"), + }) + }, + Duration::from_millis(200), + &clock, + ); + assert_eq!(renders.get(), 2, "every burst was rendered"); + assert_eq!(failures.get(), 2, "every callback Err surfaced to it"); + } + + #[test] + fn relevance_requires_name_and_parent_match() { + let watch_dir = Path::new("/watch"); + let named = Some(OsStr::new("sample.md")); + + let same_dir = + Event::new(EventKind::Modify(ModifyKind::Any)).add_path(watch_dir.join("sample.md")); + assert!(is_relevant_event(&same_dir, named, watch_dir)); + + // A bare relative path has parent Some("") — never equal to a real + // watch dir, so it is not relevant. Real events carry absolute + // paths, so this only matters to the filter's strictness, not to + // production labeling. + let bare_relative = + Event::new(EventKind::Modify(ModifyKind::Any)).add_path(PathBuf::from("sample.md")); + assert!(!is_relevant_event(&bare_relative, named, watch_dir)); + + let other_dir = Event::new(EventKind::Modify(ModifyKind::Any)) + .add_path(PathBuf::from("/elsewhere/sample.md")); + assert!(!is_relevant_event(&other_dir, named, watch_dir)); + + let other_name = + Event::new(EventKind::Modify(ModifyKind::Any)).add_path(watch_dir.join("other.md")); + assert!(!is_relevant_event(&other_name, named, watch_dir)); + + // No resolvable target: nothing is ever relevant. + let none: Option<&OsStr> = None; + assert!(!is_relevant_event(&same_dir, none, watch_dir)); + } +} diff --git a/tests/cli.rs b/tests/cli.rs index f426c92..124b056 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -838,58 +838,6 @@ fn watch_survives_atomic_rename() { fs::remove_dir_all(dir).expect("failed to clean up temp fixture dir"); } -#[cfg(unix)] -#[test] -fn watch_ignores_sibling_changes() { - use std::process::Stdio; - - let dir = fixture_dir("watch-sibling-changes"); - let input = dir.join("sample.md"); - let output_path = dir.join("sample.html"); - let sibling = dir.join("other.md"); - fs::write(&input, "# Initial\n").expect("failed to write markdown fixture"); - - let child = Command::new(env!("CARGO_BIN_EXE_mdo")) - .arg("--watch") - .arg(&input) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn() - .expect("failed to spawn mdo --watch"); - let _guard = WatchGuard(child); - - assert!( - wait_for_file_containing(&output_path, "Initial", Duration::from_secs(10)), - "initial render should appear within the timeout" - ); - let rendered_before = - fs::read_to_string(&output_path).expect("failed to read initial rendered output"); - let mtime_before = fs::metadata(&output_path) - .and_then(|m| m.modified()) - .expect("failed to read initial rendered output mtime"); - - fs::write(&sibling, "# Sibling\n\nUnrelated file.\n") - .expect("failed to write sibling markdown file"); - std::thread::sleep(Duration::from_secs(1)); - - // A sibling-file event must not trigger a re-render. Byte-comparing the - // output alone is not conclusive — the embedded generated date is - // day-granular and fast renders can format to the same duration — so - // also require the file's mtime (nanosecond granularity on Linux) to be - // untouched: any rewrite, even byte-identical, would bump it. - let rendered_after = fs::read_to_string(&output_path) - .expect("failed to read rendered output after sibling change"); - let mtime_after = fs::metadata(&output_path) - .and_then(|m| m.modified()) - .expect("failed to read rendered output mtime after sibling change"); - assert_eq!( - rendered_before, rendered_after, - "watch mode should not re-render sample.html for changes to an unrelated sibling file" - ); - assert_eq!( - mtime_before, mtime_after, - "sample.html should not have been rewritten (mtime changed) for a sibling-file change" - ); - - fs::remove_dir_all(dir).expect("failed to clean up temp fixture dir"); -} +// Sibling-noise and burst semantics moved into src/watch.rs unit tests +// (fake event channel + virtual clock, cross-platform by construction); +// this file keeps only the spawned-binary e2e smoke test.