Skip to content

Make three load-sensitive tests immune to a busy CI runner - #3123

Merged
lahma merged 1 commit into
sebastienros:mainfrom
lahma:fix/flaky-ci-tests
Aug 20, 2026
Merged

lahma merged 1 commit into
sebastienros:mainfrom
lahma:fix/flaky-ci-tests

Conversation

@lahma

@lahma lahma commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Three tests failed one CI leg each this week — each from runner load rather than a defect, and each cost a manual rerun before a green merge.

StackOverflowGuardTests.TheEngineIsUnchangedAfterTheGuardFires asserted exact equality of recovered stack depths across rounds. The JIT may promote an interpreter method in any round — no fixed warm-up count covers that on a loaded runner — and the promoted code's different frame size moves the depth: observed on CI as one-off steps of up to 15% (694/593, 679/671, 607/565). What a genuine unwind leak produces instead is a march: every guard fire leaves the same residue, so depth falls round after round and the loss compounds. The assertion now bounds the total spread at 25% of the best round, which rides out any single recompilation while a compounding leak of meaningful size still trips it. The deterministic leak signals — CallStack.Count == 0, IsEvaluationInProgress == false, fib(20) recovering after every fire — were already asserted per round and are untouched, so the test's real teeth are unchanged.

AsyncTests.ShouldReturnedValueTaskOfTConvertedToPromiseInJS and the AsyncModuleLoaderTests outcome tests run real awaits (a delayed ValueTask, 5 ms loader hops) under the engine's default 10-second PromiseTimeout, which a starved CI thread pool can miss — the failure reads Promise was rejected with value Timeout of 00:00:10 reached, and it hit both a windows and a linux leg this week on unrelated PRs. These tests assert outcomes, never durations, so they now run under a 2-minute budget only a genuine hang can reach (CI's own job timeout still bounds everything). The AsyncModuleLoaderTests that do assert timeout semantics configure their own short explicit budgets and never go through the new helper.

Both suites green on net10.0 and net472.

🤖 Generated with Claude Code

https://claude.ai/code/session_011G7Ud48VAs1JicxRZxLDxg

Three tests failed one CI leg each this week, each from runner load
rather than a defect, and each cost a manual rerun:

- StackOverflowGuardTests.TheEngineIsUnchangedAfterTheGuardFires
  asserted exact equality of recovered stack depths across rounds. The
  JIT may promote an interpreter method in any round, and the promoted
  code's different frame size moves the depth - observed as one-off
  steps of up to 15% (694/593, 679/671, 607/565). A genuine unwind leak
  compounds round after round instead, so the assertion now bounds the
  total spread at 25% of the best round; the deterministic leak signals
  (CallStack.Count == 0, IsEvaluationInProgress false, recovery per
  round) were already asserted and are untouched.

- AsyncTests.ShouldReturnedValueTaskOfTConvertedToPromiseInJS and the
  AsyncModuleLoaderTests outcome tests ran real awaits under the
  default 10-second PromiseTimeout, which a starved thread pool can
  miss ('Promise was rejected with value Timeout of 00:00:10 reached').
  They assert outcomes, never durations, so they now run under a
  2-minute budget only a genuine hang can reach; the tests that assert
  timeout semantics keep their own short explicit budgets.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011G7Ud48VAs1JicxRZxLDxg
@lahma
lahma merged commit 8d8c6e0 into sebastienros:main Aug 20, 2026
5 checks passed
@lahma
lahma deleted the fix/flaky-ci-tests branch August 20, 2026 20:18
lahma added a commit that referenced this pull request Aug 21, 2026
…3154)

Both UnwrapIfPromise(Async)_WithCancellationToken_ThrowsOperationCanceled-
Exception unwrap a promise that never settles, so the only exits are the
50ms token and the engine's PromiseTimeout. The default 10-second budget
is meant to lose that race by four orders of magnitude, and a stalled CI
runner beat it anyway - the windows leg failed one run with the timeout's
PromiseRejectedException where the cancellation's exception was expected.
A two-minute budget makes cancellation the only realistic exit while
still bounding the test if cancellation were genuinely lost, the same
treatment AsyncModuleLoaderTests got in #3123.


Claude-Session: https://claude.ai/code/session_011G7Ud48VAs1JicxRZxLDxg

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
lahma added a commit to lahma/jint that referenced this pull request Aug 22, 2026
Three of these rows failed CI legs on changes that touched nothing near
them (sebastienros#3221, and sebastienros#3139 before it). Each divides a wall-clock budget into
slices and asserts that several of them fit, which is only true while no
single slice is stalled past the whole budget - a statement about the
runner, not about the engine. The engine property each row exists to pin
is separable from that coincidence, and is what they now assert.

HostCallLoopConstraintTests. The deadline row drops `calls > 1` and its
skip: with the stopwatch assertion already proving the budget had really
elapsed, `calls == 1` can only mean the runner stalled the first call,
and every way of mis-arming the budget is still caught without it - one
refunded per entry never fires for calls this short, one armed in the
past fires with the stopwatch reading zero. What that row was reaching
for is now pinned with no clock at all by a new sibling: arm a budget,
spend it on the host's own clock, and a fresh entry must inherit it
rather than be handed a new one. Thread.Sleep only overshoots, so no
machine can change the verdict.

The two built-in-timeout rows keep their load-bearing assertion, which
is the implication a stall cannot falsify - a throw may only come from
an entry that itself outlived the interval - and which a carried-over
deadline falsifies on the first iteration past the interval. Their
`completed > 1` becomes an observation rather than an assertion: the
loop now runs twelve 100 ms calls instead of six 150 ms ones, so six of
them begin after the interval has passed, and a run that sees none of
them completing skips rather than fails. Widening further would not
help, because the stall that produces a zero is systemic.

HostResultLimitsTests. `OperationDeadlineSpansEvaluationAndConversion`
gave a 400 ms budget to a 250 ms pause and required the evaluation to
fit in the remaining 150 ms; it failed the macOS leg of sebastienros#3223 that way.
The budget is wall clock from Begin, so the host can spend it itself,
and a script pause long enough to spend it would have failed the entry
that ran it. The conversion assertion is unchanged and is stall-proof in
the direction that matters. The evaluation entry now provably reaches a
check (400 statements, past the amortized interval), and a stall that
fails it skips only when the stopwatch agrees the budget was gone.

Both files also stop starting blocking engine work with Task.Run: a test
that blocks waiting for a script which itself holds a pool worker makes
the pool's injection rate part of the outcome. DedicatedThread.RunAsync
already exists for exactly this. The asynchronous-loader row gets the
2-minute PromiseTimeout of sebastienros#3123/sebastienros#3154 and waits for the loader to have
been asked before completing it, closing a latent null-completion race.

Verified by mutation: making OperationDeadlineConstraint.Reset re-arm
fails all three deadline rows, and making TimeConstraint.Reset carry its
deadline across entries fails both timeout rows - through the assertion,
never through a skip.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JmK1CzJSgMd1LyTEMX3yiE
lahma added a commit that referenced this pull request Aug 22, 2026
…3231)

Three of these rows failed CI legs on changes that touched nothing near
them (#3221, and #3139 before it). Each divides a wall-clock budget into
slices and asserts that several of them fit, which is only true while no
single slice is stalled past the whole budget - a statement about the
runner, not about the engine. The engine property each row exists to pin
is separable from that coincidence, and is what they now assert.

HostCallLoopConstraintTests. The deadline row drops `calls > 1` and its
skip: with the stopwatch assertion already proving the budget had really
elapsed, `calls == 1` can only mean the runner stalled the first call,
and every way of mis-arming the budget is still caught without it - one
refunded per entry never fires for calls this short, one armed in the
past fires with the stopwatch reading zero. What that row was reaching
for is now pinned with no clock at all by a new sibling: arm a budget,
spend it on the host's own clock, and a fresh entry must inherit it
rather than be handed a new one. Thread.Sleep only overshoots, so no
machine can change the verdict.

The two built-in-timeout rows keep their load-bearing assertion, which
is the implication a stall cannot falsify - a throw may only come from
an entry that itself outlived the interval - and which a carried-over
deadline falsifies on the first iteration past the interval. Their
`completed > 1` becomes an observation rather than an assertion: the
loop now runs twelve 100 ms calls instead of six 150 ms ones, so six of
them begin after the interval has passed, and a run that sees none of
them completing skips rather than fails. Widening further would not
help, because the stall that produces a zero is systemic.

HostResultLimitsTests. `OperationDeadlineSpansEvaluationAndConversion`
gave a 400 ms budget to a 250 ms pause and required the evaluation to
fit in the remaining 150 ms; it failed the macOS leg of #3223 that way.
The budget is wall clock from Begin, so the host can spend it itself,
and a script pause long enough to spend it would have failed the entry
that ran it. The conversion assertion is unchanged and is stall-proof in
the direction that matters. The evaluation entry now provably reaches a
check (400 statements, past the amortized interval), and a stall that
fails it skips only when the stopwatch agrees the budget was gone.

Both files also stop starting blocking engine work with Task.Run: a test
that blocks waiting for a script which itself holds a pool worker makes
the pool's injection rate part of the outcome. DedicatedThread.RunAsync
already exists for exactly this. The asynchronous-loader row gets the
2-minute PromiseTimeout of #3123/#3154 and waits for the loader to have
been asked before completing it, closing a latent null-completion race.

Verified by mutation: making OperationDeadlineConstraint.Reset re-arm
fails all three deadline rows, and making TimeConstraint.Reset carry its
deadline across entries fails both timeout rows - through the assertion,
never through a skip.


Claude-Session: https://claude.ai/code/session_01JmK1CzJSgMd1LyTEMX3yiE

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
lahma added a commit that referenced this pull request Aug 23, 2026
…ider (#3268)

`TimerQueue` has taken its clock from a `TimeProvider` since #3216, which is
what lets its tests drive it with a `ManualClock` and assert exactly.
`TimeConstraint` and `OperationDeadlineConstraint` read `Stopwatch.GetTimestamp()`
directly, so nothing could move their clock and every test of them was a
real-time test — one-sided at best, plus a skip for the run where a contended
machine stalled past the whole budget. That is the shape behind #3123, #3154,
#3221 and the sighting in #3230; each was fixed by widening or by weakening the
claim rather than by removing the cause.

The seam is public, because the problem is not Jint's alone: any embedder that
configures `TimeoutInterval` has the same untestable wall clock. Two entry
points, matching who creates the constraint — `Options.Constraints.TimeProvider`
for the one the engine registers, and an `OperationDeadlineConstraint(TimeProvider)`
overload for the one the host constructs itself. Both are `net8.0`-and-later:
reaching `TimeProvider` from `net462`/`netstandard2.x` would mean a second
runtime dependency on a package that has exactly one, which is not a trade worth
making for a seam whose main consumer is a test.

Cost on the amortized lane, which is where `TimeConstraint.Check` is driven:

  * `ConstraintClock.Resolve` folds `TimeProvider.System` — and no provider — to
    a null field, and a null field means the constraint reads `Stopwatch`
    directly exactly as before. `TimeProvider`'s own base `GetTimestamp` and
    `TimestampFrequency` *are* `Stopwatch`'s, so the fold changes no answer.
  * Default-path codegen gains three instructions: a load from the same cache
    line as `_deadline`, a test, and a never-taken branch, in the shadow of an
    ~18 ns `QueryPerformanceCounter`. Measured per check across six rounds of a
    monomorphic micro-benchmark: |delta| <= 0.26 ns, best-conditioned rounds
    ~0.1 ns.
  * `Check` runs once per 64 statements. On the most check-dense row in the
    suite (`ConstrainedExecutionBenchmark`'s 1M-iteration loop) that is exactly
    15,625 checks per evaluation, so the delta is <= 4.1 us of a ~20.4 ms
    evaluation — <= 0.02% of the row, and <= 0.8% of what the whole amortized
    timeout lane already costs it (~2.4%). Two orders of magnitude below what a
    paired BenchmarkDotNet A/B can resolve, which is why the evidence here is
    codegen plus an isolated per-check measurement rather than a suite A/B.
  * On the downlevel targets the field does not exist and neither does the
    branch: the only change is a static `AggressiveInlining` helper that inlines
    to the instruction it replaced.

No constraint behaviour changes. Same exception types, same messages, same
firing points, same `IsAmortizable`, same no-op `Reset` on the operation
deadline; the interval-to-ticks arithmetic and its overflow clamp move to
`ConstraintClock.ToTimestampTicks` unaltered. A clock reporting a non-positive
`TimestampFrequency` is rejected where it is supplied, since it would otherwise
arm every deadline at its own start and fail the first check of every execution.

The payoff, on `HostResultLimitsTests.OperationDeadlineSpansEvaluationAndConversion`:
it used to spend the budget with a 500 ms `Thread.Sleep` and could then make
only two claims — "the evaluation did not throw", skipped when a stalled runner
made it throw anyway, and "the conversion did throw" — because how much of a
wall-clock budget an entry consumes is a fact about the machine. It now straddles
the budget to the tick: one tick short of it a fresh entry still completes, and
the tick that exhausts it fails the next one. Neither half was expressible
before, there is no skip and no sleep, and everything except the clock is the
shipped path. The downlevel leg keeps the old row. Both halves were confirmed
live by mutation: arming `Begin` off the seam, and arming the deadline one tick
early, each fail it.

Verified: solution builds Release on all five target frameworks; `Jint.Tests`
(10498/7256) and `Jint.Tests.PublicInterface` (2490/1899) green on both target
frameworks, plain and with `JINT_HOST_CONTRACT_VERIFICATION=1`; test262
102495 passed / 189 skipped / 102684 total, identical to an `upstream/main`
baseline run on the same machine.

Fixes #3232


Claude-Session: https://claude.ai/code/session_01FHnbgQGW5QgfatxL6DgGEd

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant