test: stop pinning settle capture counts against a wall-clock loop#1307
Conversation
…e count settle-observation's "contention flakes" were a zero-margin comparison meeting a 1ms clock skew, not generic load. runStableCaptureLoop derives pollMs = min(300, max(25, quietMs)), so the test's settleQuietMs: 25 made pollMs === quietMs, and the settle check (one sleep(25) plus capture time, against >= 25) a 0ms margin. Node's setTimeout(25) advances Date.now() by only 24ms in 0.13% of calls idle and 0.63% under load, because libuv's timers and Date.now() read different clocks. On a 24 the loop takes a third capture that the transcript never scripted, and settle's best-effort catch reports the resulting throw as settled: false. The fixtures now model the surface rather than the runner's speed: a quiet UI serves the settled tree to every capture, a busy one a fresh tree per capture, via new transcript `repeat` entries and result factories. The snapshot-floor economy guard survives as a bound (2-3 captures), and the follow-up's "no fresh capture" cost — previously implied by the exact transcript — is now asserted directly. Production is untouched: the same zero margin only costs a wasted extra capture and poll there, filed separately as #1306.
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
|
Exact-head review ( One P2 helper-contract finding remains. |
…rio too A sweep for the same bug class found direct-ios-selector's settleObservation scenario carrying the identical 0ms margin: settleQuietMs: 25 (so pollMs === quietMs) against a consume-once transcript scripting exactly two settle captures, with no injected clock. It has not lost the coin flip in CI yet, but it fails the same way when it does — a third capture finds no entry and settle's best-effort catch reports settled: false. Same fix: the fixture models a quiet UI (every settle capture sees the same tree) instead of scripting how many captures fit in a wall-clock window. The rest of the sweep was clean. The other contract scenarios and the interaction runtime tests already use clamped mocks that repeat the last snapshot, so any capture count is tolerated; the fake-clock tests are correct to pin exact counts.
thymikee
changed the title
The review is right: `one-shot entries still outrank a repeat entry` asserted a guarantee the lookup did not provide. It passed only because the one-shot happened to be declared first — unordered lookup took the first match, so a repeat declared ahead of a matching one-shot shadowed it forever and left it permanently unconsumed. Both failure modes reproduce; the reverse-order test added here fails on the previous implementation. Unordered lookup now searches matching one-shots before repeats, so outranking holds whatever the declaration order. A repeat is documented as its command's fallback. Ordered transcripts now reject repeats at construction: ordered lookup only ever reads the head, so a repeat there never advances and strands every entry behind it. Refusing the combination beats failing later as a confusing "Provider command mismatch". Coverage added for both: reverse declaration order, and ordered + repeat.
|
Fixed in I wrote both failure modes as tests before touching the implementation, and both reproduce on Unordered — took option (b), explicit outranking. Ordered — took option (a), rejection. Ordered lookup only ever reads the head, so a repeat there can never advance and strands everything behind it. There's no coherent meaning to give it, so Coverage added for both gaps: reverse declaration order, and ordered + repeat. No fixture behaviour changes — every current |
|
Exact-head re-review ( |
thymikee
added
the
ready-for-human
|
1 participant
Summary
settle-observation.test.tshas been failing on loaded CI runners and passing on rerun (#1260, #1288, #1289). It wasn't generic contention — it was a zero-margin comparison meeting a 1ms clock skew.runStableCaptureLoopderivespollMs = min(300, max(25, quietMs)), so the test'ssettleQuietMs: 25made pollMs === quietMs. Settling needsnowMs - quietSinceMs >= quietMs, and that gap is onesleep(pollMs)plus capture time — i.e. "~25 >= 25", a 0ms margin. Instrumented,sinceQuietlands at 25, 26, 27, 28, 29 against a threshold of 25.setTimeout(25)advancesDate.now()by only 24ms in 0.13% of calls idle and 0.63% under load (5x) — libuv's timers fire off cached monotonic loop time whilenow()reads the wall clock (runtime-common.ts:16-23). On a 24, capture #2 misses the window, the loop takes a third capture, the transcript scripted only two, and the throw is swallowed by settle's best-effort catch (settle.ts:113) intosettled: false.Signature:
false !== trueat thesettledassert in ~600ms — far too fast for the 10s budget, which rules out timeout pressure.What changed (tests only — no production code)
The fixtures scripted an exact capture count, which asserts how many captures fit in a wall-clock window — the runner's speed, not the contract. They now model the surface instead:
repeatentry)repeat+ result factory)Both make the loop's capture count irrelevant to the verdict. New
repeatentries and result factories in the provider-scenario transcript helper, with unit coverage.Guards kept rather than dropped:
captures >= 2 && captures <= 3) instead of=== 2. Under any legitimate timing a quiet UI settles at capture 2, or 3 on skew — an unbounded assertion would let a regression to 5 captures per settle pass unnoticed.transcript.callsacross the call. Arepeatentry would otherwise happily serve a stray capture.Also fixed the never-settled test, which was pinned to exactly 3 captures from both directions (transcript size above,
assertComplete()below) on a ~10ms budget margin, and whose 60ms budget a loaded capture could outlast — reporting the stalled hint instead of the kept-changing one.Validation
Forced-undershoot A/B (CI's exact condition, made deterministic via
sleep(pollMs - 6)):[[2,1],[3,59]]The histogram is why the bound is
[2,3]: 59 of 60 runs settle at capture 3 under skew, so=== 2would fail every one.Also 150 loaded iterations, 0 failures — though noting honestly that the control also passed at that sample size (~39% chance of zero failures at a 0.63% rate), so the A/B above is the load-bearing evidence, not the volume run.
Full
provider-integrationsuite: 33 files / 130 tests green. Typecheck, lint, format clean.Not in this PR
The same zero margin exists in production for every
--settle-quietin [25, 300] —pollMs === quietMsacross that whole range. Production doesn't fail on it; it pays a wasted extra capture + poll on ~0.13–0.63% of settles. A deadline-aware sleep deletes it at the source, but that changes production timing, so it's filed separately as #1306 with the full diagnosis.Follow-up in this PR: the same bug class, swept
A sweep of every test driving
runStableCaptureLoop(the--settleflag andwait stable) found one more instance, now fixed here:test/integration/interaction-contract/direct-ios-selector.contract.test.ts— thesettleObservationscenario carries the identical 0ms margin (settleQuietMs: 25→pollMs === quietMs) against a consume-once transcript scripting exactly two settle captures, with no injected clock. It hasn't lost the coin flip in CI yet, but it fails the same way when it does.Everything else came back clean, for reasons worth recording:
captures === 1 ? before : after) that repeat the last snapshot forever, so any capture count is tolerated.runtime-selector.contract.test.tsalready bounds withcaptures >= 2.interaction-settle.test.tshas the same acute margin but its mock clamps rather than throws, and it never asserts an exact count.settle.test.ts,selector-read-stable.test.ts) are correct to pin exact counts — with an injected clock, time only advances viasleep().wait stableis never exercised against a real-timed transcript anywhere.