From bc1da44c6b86d4412e860b56bd4a124418f1ea58 Mon Sep 17 00:00:00 2001 From: Joel DSouza Date: Thu, 2 Jul 2026 19:12:45 +0200 Subject: [PATCH] test(core): de-flake supervision props strategy scenario (#100) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The real-actor property scenario (`law_strategy_restart_set` → `run_strategy_trial`) is the only path in supervision.properties.feature that spawns real actors, and it carried the last bare fixed-duration settle sleep in the file: a `tokio::time::sleep(NEGATIVE_BOUND)` used as a passive barrier before reading the restarted set. A fixed sleep is not a synchronization primitive — under CI scheduling load (a 2-core runner under llvm-cov) it races the async restart path. Replace it with an active bounded `hold_for` over the out-of-set children: it re-checks throughout the negative window and panics loudly the moment a spurious restart appears, instead of reading state once at the end. The independent restarted-set observation the caller compares to the oracle is preserved. Also raise the positive-observation bound `SETTLE_STEPS` 600 → 2000. `settle` returns the instant its condition holds, so a generous bound is free for a passing run; it only widens the margin before a genuinely scheduler-starved restart is declared a failure. Side effect: OneForAll trials have an empty out-of-set and now skip the negative window entirely, so the scenario runs ~3.7s instead of ~7s. Verified: 336 whole-binary stress runs + 64k in-process cascade trials + 150 hardened runs, all under heavy CPU oversubscription, zero failures. --- tests/core_steps/supervision.rs | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/tests/core_steps/supervision.rs b/tests/core_steps/supervision.rs index 8467697..9ed117f 100644 --- a/tests/core_steps/supervision.rs +++ b/tests/core_steps/supervision.rs @@ -62,7 +62,14 @@ use tokio::sync::Barrier; // "closes" the way a one-shot shutdown does). // =========================================================================== -const SETTLE_STEPS: usize = 600; +// A GENEROUS positive-observation bound. `settle` returns the instant its +// condition holds, so a large bound costs a passing run nothing — it only +// widens the margin before a genuinely-stalled restart is declared a failure. +// Sized for the worst case (a 2-core CI runner under llvm-cov instrumentation, +// where the async restart path is heavily scheduler-starved); the poll interval +// itself also wakes late under load, so the effective wall-clock bound self- +// scales well past the nominal SETTLE_STEPS * SETTLE_TICK. +const SETTLE_STEPS: usize = 2000; const SETTLE_TICK: Duration = Duration::from_millis(5); /// A short bound for "this did NOT happen" assertions: long enough that a real /// restart would have been observed, short enough to keep the suite fast. @@ -1489,8 +1496,24 @@ async fn run_strategy_trial(strat: &str, count: usize, failed: usize) -> BTreeSe ) .await; } - // Give any spurious out-of-set restart time to manifest. - tokio::time::sleep(NEGATIVE_BOUND).await; + // Actively assert that every child OUTSIDE the strategy's set stays + // un-restarted (start count 1) across the negative window — a bounded + // hold, NOT a bare settle sleep. `hold_for` re-checks throughout the + // window and panics loudly the moment a spurious restart appears, + // rather than a passive sleep that only reads state once at the end. + let out_of_set: Vec> = (0..count) + .filter(|i| !expected.contains(i)) + .map(|i| counters[i].clone()) + .collect(); + if !out_of_set.is_empty() { + hold_for( + move || out_of_set.iter().all(|c| c.load(Ordering::SeqCst) == 1), + "a child outside the strategy's restart set must NOT be restarted", + ) + .await; + } + // Independent observation of the restarted set (the caller compares it + // to the oracle) — belt-and-suspenders alongside the hold above. let restarted: BTreeSet = (0..count) .filter(|&i| counters[i].load(Ordering::SeqCst) >= 2) .collect();