Repository navigation
test(e2e): don't count harness stalls against the quit timeout - #719
Conversation
dash-09 and dash-34 fail "When the user quits the dashboard" on main's hosted E2E lane. The TUI has opened the quit-confirm prompt and is idle waiting for an answer, but the driver never sends "y". cucumber polls every concurrent scenario from one task, and many steps run `rocm` synchronously, so while one of them runs no other scenario is polled. Instrumented on a 3-CPU container, the quit loop's first 500 ms wait came back after 102 s, while the therock-next and comfyui steps ran their dry-run installs back to back. By then the 30 s deadline had passed, so the loop gave up before it read the screen again and answered the prompt it found there. #630 added four of those therock scenarios, which is why main went red at that commit. Add any time a wait runs past its budget to the deadline, so the timeout counts only time the loop was being polled. A TUI that never exits still times out, and the timeout message now reports how long the step was stalled. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
tomastola
left a comment
There was a problem hiding this comment.
Review — needs work
Full review of the whole change (#719, 32834b29…5fd8053b).
The description says this fixes main's red hosted E2E lane (dash-09, dash-34), with the async run_rocm root fix left for later. No issue is linked.
Blocking
-
The new "never exits" test claims a check it cannot fail on —
tests/e2e-cucumber/src/quit_and_wait.rs:376-408
The assertion message says "the stall must extend the deadline once, not indefinitely", and the description repeats that claim. The test checks onlyelapsed < 2s, and its timeout is 60 ms. Mutations run on a scratch copy:- Removing
deadline += overrun(:98): the test still passes. - A conditional second extension when
overrun > 50ms: all 8 tests pass, in about 460 ms. - An unconditional double extension: three tests hang until the 120 s outer
timeoutkills them. They hang because nothing bounds them.
The extension itself is covered only by
a_stall_longer_than_the_timeout_still_answers_the_prompt.
Confidence 100 · logic · Fix: bound the elapsed time tightly (abouttimeout + stall + slack, e.g.< 60 + 200 + 150 ms) and assert it is at leaststall + timeout, so the test proves the deadline moved once. Wrap the call intokio::time::timeoutso an over-extension fails the test instead of hanging it. - Removing
-
The docs, the error message and the description overstate what the timeout now excludes —
tests/e2e-cucumber/src/quit_and_wait.rs:34-36,:52,tests/e2e-cucumber/tests/e2e/tui_driver.rs:899-900,:932-934; the description's "The timeout now means 30 s of time the loop was actually polled".
The code (:96-98) credits onlyelapsed - budget, the part of a wait that ran past its own budget. The code governs, and:60already describes it correctly. The other claims are wrong in both directions, and a probe with the real wait's shape confirmed each:- Stalls inside a budget are still counted. One 300 ms stall inside a 400 ms budget, with a 1 s timeout, gave
TimedOut { stalled: 9ms }after 1.009 s of wall time, so only about 700 ms was actually polled. The budgets double toward about 14.5 s at a 30 s timeout, so later stalls increasingly count against the timeout, which is the failure this PR targets. - Ordinary polling slack is reported as a harness stall.
wait_for_exit_code_within(tui_driver.rs:963-976) sleepsPOLL_INTERVAL(20 ms) past its deadline. With no stall and a 3 s timeout, the run reportedstalled: 77.5ms, and the message would blame "the harness did not poll this step" for a step that was never starved.
DEFAULT_TIMEOUT_SECS(tui_driver.rs:76) is still documented as a "wall-clock budget", whichquit_and_waitno longer treats it as.
Confidence 100 · mechanical · Fix: reword the doc comments, the driver doc and the description to say "time a wait runs past its budget is added to the deadline", and either subtract onePOLL_INTERVALof slack before reportingstalled, or word the message as "waits overran their budget by X". - Stalls inside a budget are still counted. One 300 ms stall inside a 400 ms budget, with a 1 s timeout, gave
Non-blocking
send_untilkeeps the defect class, and the description says it does not —tests/e2e-cucumber/src/send_until.rs:53-67
The description says thatsend_until"already re-check[s] [its] condition after a stall, so a stall only delays them". That holds when the first key took effect.send_untilexists to resend a key that did not take effect. If that key was lost and a stall pushes the wait past the deadline,:67returns before the resend, which is the same "deadline checked before acting again" shape this PR fixes inquit_and_wait. It is rarer because it needs a lost key. Juho's review note 2 on #713 already flagged that these two loops duplicate the same deadline logic, and this PR makes them diverge further.
Confidence 70 · logic · Fix: apply the same overrun credit there, or factor the deadline/overrun handling into one helper both loops use. At minimum, correct the sentence in the description.
Decisions for the author
-
Patch the symptom here, or make
run_rocmasync — tradeoff
The description already says this is the minimal fix. For the record, it is at least the fifth workaround for the same root cause:chat_request.rsmoved the request onto its own task (tokio::spawn);paced_download.rsrunstarviaspawn_blocking;- the comfyui update-check disable (#661);
launch_releasetiming (e2e.rs:88-90);- this PR.
The root fix is mechanical: a
spawn_blocking-backedrun_rocmfuture, about 186.awaits and about 29 helpers. It needs no new tokio feature. Shipping this unblocks main now. The cost is that every new deadline loop has to remember the same credit. -
Wall time on a starved lane is now bounded only by the CI job — tradeoff
Each of the roughly 6-8 doubling iterations can absorb a full round of the other scenarios' blocking calls. There is no per-step or per-scenario timeout. On the 15-minute mock job, a pathological stall now ends in a job kill, which loses every scenario's report, instead of one failed step. A hung TUI still fails aftertimeoutof polled time, so nothing real is masked. Confirm that trade is acceptable, or add an outer wall-clock cap (e.g. 4×timeout).
Positive signals
- The overrun is measured against the budget the loop itself granted, so the existing clamp still guarantees termination, and a hung TUI still times out.
stalls_onceisolates one stall cleanly, anda_stall_longer_than_the_timeout_still_answers_the_promptfails without the fix (TimedOut, keys["q"]) and passes with it.- Surfacing
stalledin the timeout message makes the next harness stall recognisable from the log. Keep it once the wording is accurate.
Deployment notes
None. The change is internal to the test harness.
juhovainio
left a comment
There was a problem hiding this comment.
Reviewed the quit_and_wait deadline-extension logic (tests/e2e-cucumber/src/quit_and_wait.rs, tests/e2e-cucumber/tests/e2e/tui_driver.rs) plus the new unit tests.
No blocking issues. The core claim checks out: a_stall_longer_than_the_timeout_still_answers_the_prompt genuinely fails without the deadline += overrun line and passes with it, and a_stalled_child_that_never_exits_still_times_out confirms a single stall doesn't extend the deadline indefinitely. saturating_sub/saturating_duration_since are used consistently, so no Duration underflow risk. Grepped for other QuitError::TimedOut consumers — none outside the two changed files, so the added stalled field doesn't break anything silently.
Two non-blocking notes:
overrun = started.elapsed().saturating_sub(budget)can't distinguish a genuine harness stall from ordinary polling-loop jitter (e.g.exit_poll_within's own poll interval occasionally overshooting). Harmless at the current scale (tens of ms vs a 30s timeout) and consistent with the stated design, so just worth knowingstalledis an upper bound rather than an exact measurement.- The deadline extension is unbounded if a wait kept overrunning on every iteration rather than once — only the single-stall case is tested. Given the PR explicitly frames this as the minimal fix ahead of the deeper async rework, that's probably fine as-is, but flagging in case it's worth a cap later.
On CI: the two scenarios this PR exists to fix, dash-09 and dash-34, pass cleanly on the Strix Halo WSL2 lane. That same job still shows overall failure from an unrelated scenario (comfyui-source-download-shows-live-progress timing out) that this diff doesn't touch — looks like the same contention-driven shape as other known flakes on that runner, but it's not one I've seen characterized before, so worth a rerun/second look rather than assuming it's safe to ignore. The other Strix Halo lanes show cancelled (not failure), consistent with known self-hosted runner contention.
…_until The previous fix credited only the time a wait ran past its own budget. A stall that lands inside a budget (which doubles toward ~15 s at a 30 s timeout) still counted, and the one-tick overshoot of every wait's own poll loop was reported as a harness stall. Move the deadline into a PolledDeadline that measures the gaps between polls of each wait instead: a gap longer than twice the wait's poll interval is a stall, and all but one poll interval of it is added to the deadline. Cap the loop at 4x the timeout of wall time, so a pathological stall fails one step rather than the whole job and its reports. send_until uses the same deadline: if its first key was lost, a stall counted against the timeout would give up before the resend. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
|
@tomastola Thanks for the review. Each point is addressed in 1. The "never exits" test could not fail on its claim (blocking). Fixed. It is now
None of the three runs hung; each finished in under 1 s. 2. The docs, the message and the description overstated what is excluded (blocking). I fixed the accounting rather than only the wording. Your in-budget probe was right, and budgets near 15 s make that the common case.
3. 4. Wall-clock cap (decision). Added. 5. Async Verification on |
Please re-review.
juhovainio
left a comment
There was a problem hiding this comment.
This change fixes real CI flakiness (dash-09/dash-34) with a well-reasoned shared PolledDeadline abstraction. The gap-crediting and wall-clock-cap arithmetic matches the description's stated semantics, and I didn't find a correctness bug in the deadline logic itself (no double-counting, no lost stall credit, the wall-clock cap correctly takes precedence once it's exceeded).
Blocking
The PR's own new timing-sensitive unit tests currently fail on windows-build-and-test (confirmed on this PR's CI run), in both polled_deadline.rs::a_stall_inside_the_budget_moves_the_deadline and quit_and_wait.rs::a_stalled_child_that_never_exits_moves_the_deadline_by_the_stall_once. Windows is a supported platform, so this needs fixing (a wider tolerance, or a platform-scaled TICK) rather than merging red — see the two inline comments.
The target E2E tests job (containing dash-09/dash-34) currently passes, so the actual fix looks effective; this is specifically about the new tests' own tolerance being too tight for Windows' coarser timer granularity, not about the harness fix itself.
Non-blocking
send_until.rsdoesn't have a "stall absorbed inside the wait's own budget" test the wayquit_and_wait.rsdoes (a_stall_inside_a_long_budget_still_answers_the_prompt). The mechanism is covered elsewhere, so this is a minor coverage gap rather than untested behavior.wait_for_screen/wait_for_screen_where/wait_for_popup_body_whereintui_driver.rsare left on wall-clock deadlines, per the PR description's reasoning (they only re-check a condition rather than acting on a stale read). Reads as a deliberate scope boundary — worth a one-line confirmation that it's intentional rather than deferred.quit_and_wait.rsandsend_until.rsstill share a near-identical retry-loop skeleton on top of the now-deduplicatedPolledDeadline. Low-signal, not asking for action here.
CI
windows-build-and-test: FAIL (see above). E2E tests: PASS. Everything else that's completed is green; several self-hosted hardware E2E lanes (Strix Halo, MI300X, W7900) were still pending/queued at review time.
| .run(ticking_wait(Duration::from_millis(200), stall, || false)) | ||
| .await; | ||
| assert!( | ||
| deadline.stalled >= stall.saturating_sub(TICK) && deadline.stalled < stall + 4 * TICK, |
There was a problem hiding this comment.
This bound (stalled >= stall - TICK && stalled < stall + 4*TICK, with TICK = 5ms) is tight enough to fail on Windows' coarser timer granularity — confirmed failing right now on windows-build-and-test (stalled 181.8125ms for a 150ms stall, well outside this window). Windows is a documented supported platform, so this needs a wider tolerance (or a platform-scaled poll interval) rather than being left red.
There was a problem hiding this comment.
Fixed in 08856d6. I did not widen the tolerance. These tests now run on tokio's paused clock (start_paused = true), and PolledDeadline reads tokio::time::Instant, which is the host clock in the harness. Under the paused clock a sleep advances time by exactly what it asks for, so this test now asserts the exact credit, stalled == credited(stall), and that the deadline moved by exactly that amount. It no longer depends on how precisely the host's timer fires.
The 181 ms was an over-count, not only a tight bound. A gap between polls counted as a stall once it exceeded 2 * poll_interval, and these tests polled every 5 ms. On Windows each 5 ms sleep wakes on a 15.6 ms tick, so every tick crossed the 10 ms threshold and about 10 ms of it was credited as a stall. The stall rule now credits only what a gap runs past poll_interval + TIMER_SLACK, where TIMER_SLACK is 50 ms, about three Windows ticks. The new a_coarse_timer_is_not_a_stall test pins this: a 20 ms poll loop whose sleeps each last 46.9 ms is credited zero.
Linux reproduction: I set a 16 ms timer slack with prctl(PR_SET_TIMERSLACK). Under it the old head failed this test (stalled 189ms for a 150ms stall) along with three others. The new head passed 30 of 30 runs pinned to one core with that slack, and 30 of 30 more with a CPU hog on the same core.
Mutations that fail this test: the stall not added to the deadline; a conditional second extension; a double extension; every gap counted as a stall; and the old 2 * poll_interval threshold. None of them hangs, and every run finishes in under 10 ms.
| let QuitError::TimedOut(Expiry::Polled { stalled, .. }) = error else { | ||
| panic!("expected a polled-time timeout, got {error:?}"); | ||
| }; | ||
| assert!( |
There was a problem hiding this comment.
Same issue as polled_deadline.rs's a_stall_inside_the_budget_moves_the_deadline: this bound is too tight for Windows' timer granularity. CI shows this failing with reported stall 368.6094ms for a 150ms stall, outside [stall - TICK, stall + slack). Needs widening before merge since windows-build-and-test is currently red.
There was a problem hiding this comment.
Fixed in 08856d6. This one was a real over-count. A 368 ms report for a 150 ms stall means about 220 ms of ordinary 5 ms polling was credited as stall, one ~10 ms overshoot per Windows timer tick. Each credit also moved the deadline, which kept the loop running longer and added more credit. On Linux, with the timer slack set to 16 ms, the old code went as far as the wall-clock cap: WallClock { stalled: 631ms } for the same 150 ms stall.
In production the harness polls every 20 ms. A Windows wake lands up to about 36 ms after the last poll, just under the old 40 ms threshold, so one extra tick on a loaded runner would have reported sleep overshoot as "the harness left this step unpolled" and extended the deadline by it. Stalls now count only past poll_interval + TIMER_SLACK (20 + 50 ms in the harness). The credit is now a slight underestimate, never an overestimate, and real stalls (a synchronous rocm run) are hundreds of milliseconds or more.
The test runs on tokio's paused clock and asserts exact values. The reported stall is credited(stall), and the loop ends in [timeout + credited(stall), timeout + credited(stall) + TICK]. Missing the credit, crediting it twice (conditionally or always), and counting every gap each fail this test, with no hang: the HANG timeout is virtual too.
…se timers The new timing tests failed on Windows: a 150 ms stall was reported as 181 ms and 368 ms. A gap between polls counted as a stall once it passed twice the poll interval, and the tests polled every 5 ms. A Windows sleep ends on a 15.6 ms timer tick, so every 5 ms sleep overran that threshold and its overshoot was credited as a stall. The harness polls every 20 ms, whose two-tick wake (up to ~36 ms) sat just under the 40 ms threshold, so a loaded Windows runner could report sleep overshoot as the harness stalling, too. Count as a stall only what a gap runs past the poll interval plus a fixed 50 ms timer slack (about three Windows ticks). Real harness stalls are hundreds of milliseconds or more, so they still register. Read time from tokio::time::Instant, which is the host clock in the harness, and run the retry loops' unit tests on tokio's paused clock. Sleeps then advance time exactly, so the tests assert exact stall credits and deadlines instead of tolerances, and do not depend on host timers. Add a send_until test where a stall is absorbed inside the wait's own budget, and a test that a coarse timer is not reported as a stall. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
|
@juhovainio thanks for the review. The two blocking timing failures are answered inline. The fix is 08856d6: the tests now run on tokio's paused clock, and the stall rule allows for coarse timers. Here are the non-blocking items:
Verification on 08856d6:
|
Addressed on 08856d6 (reviewed 8e4cf24). The timing tests now run on tokio's paused clock with exact assertions. The 368 ms was a real over-count from timer slack, now fixed with TIMER_SLACK and pinned by a_coarse_timer_is_not_a_stall. windows-build-and-test passes. The send_until test was added, and the inline replies cover each point.
volen-silo
left a comment
There was a problem hiding this comment.
Thanks for the fix. The shared PolledDeadline reads right to me, and moving the timing tests onto tokio's paused clock makes them exact on any host. CI is green on this head. Two small things before merge, plus a few notes.
Before merge
- The description does not match the code for the stall threshold. The code credits
gap - (poll_interval + TIMER_SLACK)withTIMER_SLACKat 50 ms (stall_ininpolled_deadline.rs), so at the driver's 20 ms poll interval up to 70 ms counts as polling. The description says gaps up to two poll intervals are not stalls and longer gaps lose one poll interval, and it never mentionsTIMER_SLACK. The unit tests assert the code's behaviour, so the description is the side to correct. It also says 164 tests passed;cargo test -p e2e-cucumber --libon this head gives 166. - The
.min(self.wall_deadline...)clamp inPolledDeadline::budgetis not covered. I removed it locally and all 166 lib tests still passed. A test where stalls pushdeadlinepast the cap while the resend interval is large, asserting the wait ends at the cap, would pin it.
Smaller
the_wall_clock_cap_ends_a_loop_that_keeps_stallingloops onwhile deadline.expired().is_none()with no outer bound, unlike the guards inquit_and_wait.rsandsend_until.rs. If the cap branch ofexpired()ever breaks, this test would spin instead of failing. Wrapping it intokio::time::timeoutwould match the others. This also bears on the description's "no run hung" line in the mutation section.- In
send_until.rs,deadline.budget(timing.resend_interval)has no test that each attempt is cut to the time left.quit_and_waithas one for the same logic. - The doc comment on
TuiSession::quit_and_waitintui_driver.rsends with "gives the reasons for both", and "both" no longer has anything to point at.
To confirm it is deliberate
- A 30 s step can now take up to 120 s of wall time (cap factor 4), and up to 360 s with
E2E_TUI_TIMEOUT_SECS=90. A hung TUI still fails after the normal polled time, so this only costs time when the host stalls. - With tokio's
test-utilas a dev-dependency, the timing unit tests no longer exercise the real clock on Windows, which is where the slack matters. The slack constant is covered only through the virtualcoarsetest.
Not covered by this review: a full cargo xtask e2e run, and the reproduction claims (3-CPU containers, the 102 s stall).
Cover the `.min(wall_deadline)` clamp in `PolledDeadline::budget`: a stall pushes the deadline past the cap, and a wait that asks for 10 s must end at the cap, both on `PolledDeadline` directly and through `quit_and_wait`. Add the `send_until` counterpart of `quit_and_wait`'s test that a wait is cut to the time left. Bound `the_wall_clock_cap_ends_a_loop_that_keeps_stalling` with `tokio::time::timeout` like the other guards, and have `ticking_wait` sleep one tick before giving up on an already-spent budget, so a broken cap fails the tests instead of spinning them on the paused clock. Fix the dangling "both" in `TuiSession::quit_and_wait`'s doc comment. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
|
Thanks for the review, @volen-silo. All points are addressed in 2bd7a0c. Before merge
Smaller
I redid the full mutation table on this head. Every mutation fails at least one test, and none hangs. To confirm
|
|
Thanks, @rominf. I checked the new head (2bd7a0c) and the points hold.
Your reasoning on the real-clock test and the Windows coverage gap is clear and I have no further points. I did not re-run a full |
Summary
Main's CI is red. The hosted
E2E testsjob fails on two scenarios,dash-gen-tps-expiry-boundary(dash-09) anddash-esc-menu-quit-row-confirms-when-serving(dash-34). The failing step is "When the user quits the dashboard":Every merge-queue run is ejected for the same reason. #713 did not fix this.
Cause
The runner image is not the cause, and neither is the dashboard. The quit driver stops waiting before it answers the prompt.
rocm dashprocess, taken at the timeout, shows it idle: the event loop is parked and crossterm's reader is waiting for input. The dashboard was waiting for an answer, and none arrived.rocmsynchronously (run_rocmcallsCommand::output()). While one of those runs, no other scenario is polled. I instrumented the run in anubuntu:26.04container limited to 3 CPUs. In the failing scenario,quit_and_waitsentqand started a 500 ms wait. That wait came back after 102 s, while the therock-next and comfyui steps ran their installs back to back. Each blocked for 5 to 16 s; therock-next-08 took 11.3 s and comfyui-03 15.7 s. By then the 30 s deadline had passed, soquit_and_waitreturnedTimedOutwithout reading the screen again, and without answering the prompt it would have found there.5dfa9a85(run 39560032665). The first red one is3067a103, which is fix(therock): pin torchaudio to the requested ROCm 10 build #630 (run 39563117268). Both ran onubuntu-26.04, so the image change is not the trigger. Later failures: main push run 39604606023 (6186f159, after test(e2e): stop quit_and_wait deadlocking on a slow quit prompt #713), and merge groups for test: check that every advised rocm command parses; fix dashboard labels that don't #535 (39604610199) and fix(therock): order runtime versions by PEP 440 sorocm updatenever offers a downgrade #566 (39599121754). ci: run Linux CI on both Ubuntu LTS images; fork-free reclaim decoys with liveness checks (ROCMAI-670) #718's run 39741048934 fails the same two scenarios on both 24.04 and 26.04.qstill in flight when the screen is read. Here the driver itself is frozen, so test(e2e): stop quit_and_wait deadlocking on a slow quit prompt #713's doubling never got a second iteration.Fix
The retry loops that have to act on what they read,
quit_and_waitandsend_until, now share aPolledDeadline(newtests/e2e-cucumber/src/polled_deadline.rs). It measures the gaps between polls of each wait. Each wait re-checks its condition every poll interval (20 ms in the driver), and its timer may fire up toTIMER_SLACK(50 ms) late, as a sleep on Windows ends on a 15.6 ms timer tick.stall_intreats whatever a gap runs pastpoll_interval + TIMER_SLACKas a stall and addsgap - (poll_interval + TIMER_SLACK)to the deadline. At the driver's 20 ms poll interval, gaps up to 70 ms count as polling: they move nothing and are not reported as a stall. So the timeout is the time the loop's waits were actually polled, whether a stall lands inside a wait's budget or runs past it.timeoutof polled time.WALL_CLOCK_CAP_FACTOR) bounds how far stalls can push the deadline, and each wait's budget is cut to whichever of the deadline and the cap comes first. A 30 s step can therefore take up to 120 s of wall time (360 s withE2E_TUI_TIMEOUT_SECS=90), but only when the host stalls it. A pathological stall therefore fails one step with its own message, instead of running into the CI job timeout and losing every scenario's report.timed out after 30s of polling, not counting 72s the harness left this step unpolled waiting for ...timed out after the 120s wall-clock cap (4x the 30s timeout); the harness left this step unpolled for 100s of it waiting for ...timed out after 30s waiting for ...).send_untilhad the same defect. It exists to resend a key that did not take effect, and when that key was lost and a stall pushed the wait past the deadline, it returned before the resend. Sharing the deadline also removes the duplicated deadline logic between the two loops.Other waits (
wait_for_screen,wait_for_screen_where, ...) still spend their timeout in wall-clock time. They only re-check a condition and do not act on it, so a stall delays them but cannot make them give up before acting.This is the minimal harness fix. The deeper cause is that synchronous subprocess calls in async steps freeze every concurrent scenario. Making
run_rocmand its siblings async fixes that at the root, and is tracked separately in #720.Tests
polled_deadline.rs:polling_slack_is_not_a_stall: at a 20 ms poll interval, gaps up to 70 ms (poll_interval + TIMER_SLACK) are not stalls; a 71 ms gap credits 1 ms.a_coarse_timer_is_not_a_stall: a wait whose 20 ms sleeps each last three 15.625 ms ticks moves nothing.a_stall_inside_the_budget_moves_the_deadline: a stall that ends inside the wait's budget is still credited, exactly once.a_budget_is_cut_to_the_wall_clock_cap_once_stalls_pass_it: a stall pushes the deadline to 425 ms, past the 400 ms cap; a wait that asks for 10 s gets the 20 ms left before the cap and ends at exactly 400 ms with a wall-clock expiry.the_wall_clock_cap_ends_a_loop_that_keeps_stalling(under a 5 stokio::time::timeout),the_report_names_only_time_the_step_went_unpolled.quit_and_wait.rs:a_stall_longer_than_the_timeout_still_answers_the_promptanda_stall_inside_a_long_budget_still_answers_the_prompt: the prompt is answered (q, theny) whether the stall runs past the wait's budget or lands inside it.a_stalled_child_that_never_exits_moves_the_deadline_by_the_stall_once: with a 200 ms timeout and a 150 ms stall, the reported stall is the credited part (150 ms - (poll_interval + TIMER_SLACK)), and the loop ends within one tick aftertimeoutplus that credit.a_wait_longer_than_the_time_left_is_cut_to_the_deadline,a_child_that_never_exits_times_out,a_stall_past_the_wall_clock_cap_fails_the_step.a_wait_is_cut_to_the_wall_clock_cap_when_stalls_push_the_deadline_past_it: the same clamp as above, through the loop: after a 380 ms stall the doubled 10 s interval is cut to the cap, and the loop ends at exactly 400 ms on the cap.tokio::time::timeout, so a deadline that keeps moving fails the test instead of hanging it.send_until.rs:a_lost_key_is_resent_after_a_stall_longer_than_the_timeout,a_lost_key_is_resent_after_a_stall_inside_a_long_budget,a_wait_longer_than_the_time_left_is_cut_to_the_deadline(each attempt's budget is cut to the time left, mirroringquit_and_wait), anda_marker_that_never_appears_times_out_and_names_the_stall.The test double
ticking_waitsleeps one tick before giving up on a budget that is already spent, so a broken deadline that keeps handing out empty budgets still advances the paused clock and the outer timeout fails the test instead of spinning.Mutation check. Each mutation was applied on the current head and
cargo test -p e2e-cucumber --libwas run. Every mutation failed at least one test, and no run hung (the test binary finished in about 2 s each time):.min(self.wall_deadline...)clamp removed frombudgeta_budget_is_cut_to_the_wall_clock_cap_once_stalls_pass_it,a_wait_is_cut_to_the_wall_clock_cap_when_stalls_push_the_deadline_past_itexpired()never firesthe_wall_clock_cap_ends_a_loop_that_keeps_stalling(fails on its timeout instead of spinning)send_untilwaits the fullresend_interval(nobudget)send_until::a_wait_longer_than_the_time_left_is_cut_to_the_deadlinesend_untilresend testsa_stall_inside_the_budget_moves_the_deadline,a_stalled_child_..._by_the_stall_onceTIMER_SLACKpolling_slack_is_not_a_stallanda_coarse_timer_is_not_a_stallThe timing tests were run 30 times pinned to one CPU (
taskset -c 0), and the full--libsuite 10 times pinned to one CPU, with no failure.Reproduction with the CI command (
cargo xtask e2e) in containers limited to 3 CPUs, on the first revision of this PR:On
08856d68(the later commit changes only unit tests, their test double, and a doc comment), the full mock suite (cargo xtask e2e) pinned to 3 CPUs (taskset -c 0-2) on the dev host: 219/221 passed. dash-09 and dash-34 passed, and the 2 failures are the expected serve-01/02 xfails ("0 unexpected failure(s)", exit 0). No step reported a stall.Local gates:
cargo fmt --all --check,cargo test -p e2e-cucumber --lib(169 passed),--test feature_naming,cargo check -p e2e-cucumber --test e2e, both clippy invocations (--workspace --all-targetsand-p e2e-cucumber --test e2e, with-D warnings), and a Windows-target (x86_64-pc-windows-gnu)cargo check -p e2e-cucumber --testsand--test e2ewithRUSTFLAGS=-D warnings.There is no new Gherkin scenario. This change is internal to the test harness, and the affected scenarios already exist; the E2E lane on this PR runs dash-09 and dash-34.
tests/e2e-cucumber/expectations.tomlfor the fixed ticket ID and removed/narrowed any now-stale xfail rows. (No xfail rows exist for dash-09 or dash-34.)docs/architecture.md(the clap declaration and dispatch wiring staying inmain.rs/lib.rsis expected, not a violation). (N/A)the_report_names_only_time_the_step_went_unpolledpins the wording;a_stalled_child_that_never_exits_moves_the_deadline_by_the_stall_onceanda_marker_that_never_appears_times_out_and_names_the_stallassert the reported stall together with the loop's timing and outcome, andpolling_slack_is_not_a_stallproves ordinary polling is not reported.)