Skip to content

Commit 8fc8884

Browse files
Prithvi-Webclaude
andcommitted
test(walk): the cancel test holds a listing open and counts the listings begun after the cancel, instead of racing the clock twice
tm-walk's cancel_returns_within_200ms_while_directories_are_being_listed slept 50 ms and asserted that the one-second scripted walk was still running, then timed the cancel against 200 ms. Both read this thread's scheduling: a thread descheduled for a second saw the walk done, and on a busy runner every wall bound eventually fails. The two tests of the same shape were fixed in 24df6c4 and eb45e4e. Now cancel_while_directories_are_being_listed_stops_every_worker_at_its_next_folder: d0's listing is held open (Special::Held) until the cancel is asked, so the walk cannot end first; two fixed workers; and after the cancel at most one listing begins, the one the free worker was about to begin. The clock is only the hang guard for take(), which joins every worker. A cancel reaches the workers three ways: it closes the queue, each worker checks at its next folder, and the driver closes the queue when it sees it. The mutant with all three gone is red ("98 listings began after the cancel"); the old test caught that mutant only through its 200 ms clock. REACT_WITHIN is now used only by macOS's live module, so it is compiled there only (clippy on the Linux and Windows targets called it unused). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent bbb93a8 commit 8fc8884

1 file changed

Lines changed: 43 additions & 15 deletions

File tree

  • native/treemap-core/crates/tm-walk/tests

‎native/treemap-core/crates/tm-walk/tests/walk.rs‎

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ use tm_walk::{
2323
WalkOptions, WalkOutput, start_with,
2424
};
2525

26-
/// What the contract allows a cancel or a pause to take.
26+
/// What the contract allows a cancel or a pause to take: the live-disk tests'
27+
/// wall-clock bound (macOS's `live` module). The scripted tests count instead.
28+
#[cfg(target_os = "macos")]
2729
const REACT_WITHIN: Duration = Duration::from_millis(200);
2830
/// How long a test waits for a walk to report `done` before giving up.
2931
const SETTLE: Duration = Duration::from_secs(10);
@@ -1192,23 +1194,49 @@ fn a_root_that_is_not_a_directory_is_refused_at_start() -> TestResult {
11921194
}
11931195

11941196
#[test]
1195-
fn cancel_returns_within_200ms_while_directories_are_being_listed() -> TestResult {
1196-
let tree = Arc::new(wide_tree(100, 10, Duration::from_millis(20)));
1197-
let handle = start_with(options(Path::new("/fake")), FakePacer::new(2), tree)
1198-
.map_err(|e| e.to_string())?;
1199-
thread::sleep(Duration::from_millis(50));
1197+
fn cancel_while_directories_are_being_listed_stops_every_worker_at_its_next_folder() -> TestResult {
1198+
// `d0`'s listing is held open until the cancel is asked, so the walk
1199+
// cannot end first however late this thread runs. (This test once slept
1200+
// 50 ms and asserted the one-second walk was still running, then timed the
1201+
// cancel against 200 ms: both raced this thread's scheduling.) Counted,
1202+
// not timed: a worker sees the cancel at its next folder, so after it at
1203+
// most one listing begins per worker not holding `d0` — the one it was
1204+
// about to begin — and the clock is only the hang guard for `take`.
1205+
let workers: u32 = 2;
1206+
let gate = Arc::new(StatGate::default());
1207+
let mut tree = wide_tree(100, 10, Duration::from_millis(20));
1208+
tree.special("d0", Special::Held(Arc::clone(&gate)));
1209+
let tree = Arc::new(tree);
1210+
// A fixed count: the hill-climber is off.
1211+
let mut opts = options(Path::new("/fake"));
1212+
opts.max_workers = 2;
1213+
let handle =
1214+
start_with(opts, FakePacer::new(workers), tree.clone()).map_err(|e| e.to_string())?;
1215+
if !wait_until(SETTLE, || gate.reached.load(Ordering::SeqCst)) {
1216+
return Err(format!("d0's listing did not begin within {SETTLE:?}"));
1217+
}
1218+
let calls_before_the_cancel = tree.calls();
1219+
handle.cancel();
1220+
gate.release.store(true, Ordering::SeqCst);
1221+
let (tx, rx) = std::sync::mpsc::channel();
1222+
thread::spawn(move || {
1223+
let _ = tx.send(handle.take());
1224+
});
1225+
let outcome = rx.recv_timeout(SETTLE).map_err(|_| {
1226+
format!("take() did not return within {SETTLE:?}: the cancel never reached the walk")
1227+
})?;
1228+
let begun = tree.calls() - calls_before_the_cancel;
12001229
assert!(
1201-
!handle.progress().done,
1202-
"a two-second walk is still running at 50 ms"
1230+
begun < u64::from(workers),
1231+
"{begun} listings began after the cancel"
12031232
);
1204-
let asked = Instant::now();
1205-
handle.cancel();
1206-
let result = handle.take();
1207-
let took = asked.elapsed();
1208-
assert!(took <= REACT_WITHIN, "cancel took {took:?}");
1209-
match result {
1233+
match outcome {
12101234
Err(WalkError::Cancelled) => Ok(()),
1211-
other => Err(format!("expected Cancelled, got {other:?}")),
1235+
Ok(out) => Err(format!(
1236+
"the walk completed ({} entries) though it was cancelled",
1237+
out.stats.entries
1238+
)),
1239+
Err(other) => Err(format!("expected Cancelled, got {other:?}")),
12121240
}
12131241
}
12141242

0 commit comments

Comments
 (0)