Skip to content

fix(test): stop asserting concurrency via a wall-clock threshold - #566

Merged
githubrobbi merged 1 commit into
mainfrom
fix/flaky-concurrency-timing-tests
Jul 20, 2026
Merged

fix(test): stop asserting concurrency via a wall-clock threshold#566
githubrobbi merged 1 commit into
mainfrom
fix/flaky-concurrency-timing-tests

Conversation

@githubrobbi

Copy link
Copy Markdown
Collaborator

Summary

Two `uffs-content` tests inferred concurrency from
`elapsed < sequential_estimate * 3 / 4` against 100ms-per-operation
fixtures. That threshold is only meaningful relative to how fast the
test machine happens to be that run — on a loaded/throttled
GitHub-hosted Windows runner, `thread::sleep(100ms)` itself can take
several hundred milliseconds of wall-clock time, pushing both the
sequential and concurrent paths past any fixed absolute threshold.

Hit for real in release PR #564's merge-queue run: elapsed 889ms vs. a
600ms "fully sequential" estimate — worse than fully sequential, which
is only possible under uniform scheduling overhead, not an actual
concurrency regression. I independently re-verified the dispatch code:
`emit::read_and_emit_all_candidates` and
`workflow::enumerate_all_roots_concurrently` both still spawn one
thread per lease/root via `std::thread::scope`.

Fix

Replace the wall-clock-ratio assertion with direct proof: record every
operation's (start, end) `Instant` and assert at least two intervals
overlap. Two things overlapping in time is true or false independent
of how slow the machine is.

Test plan

  • 15/15 consecutive local runs of both tests
  • `cargo xwin test --no-run` — cross-compiles clean
  • `just lint-tests` (pedantic + nursery) — clean
  • `cargo fmt --check` — clean

Two tests inferred concurrency from `elapsed < sequential_estimate * 3
/ 4` (100ms-per-operation fixtures). That's only ever true relative to
how fast the machine happens to be that run: on a loaded/throttled
GitHub-hosted Windows runner, thread::sleep(100ms) itself can take
several hundred milliseconds of wall-clock time, which pushes *both*
the sequential and concurrent paths past any fixed absolute threshold.

Hit for real in release PR #564's merge-queue run: elapsed 889ms vs.
a 600ms "fully sequential" estimate -- worse than fully sequential,
which is only possible under uniform scheduling overhead, not an
actual concurrency regression (the dispatch code was independently
re-verified: `emit::read_and_emit_all_candidates` and
`workflow::enumerate_all_roots_concurrently` both still spawn one
thread per lease/root via `std::thread::scope`).

Replace the wall-clock-ratio assertion with direct proof: record every
operation's (start, end) Instant and assert at least two intervals
overlap. Two things overlapping in time is true or false independent
of how slow the machine is -- it only asks whether they ran at the
same time, which is what "concurrent" actually means.

Verified 15/15 consecutive local runs; xwin cross-compile clean;
lint-tests (pedantic + nursery) clean.
@githubrobbi
githubrobbi enabled auto-merge July 20, 2026 02:21
@githubrobbi
githubrobbi added this pull request to the merge queue Jul 20, 2026
Merged via the queue into main with commit 1191e40 Jul 20, 2026
21 checks passed
@githubrobbi
githubrobbi deleted the fix/flaky-concurrency-timing-tests branch July 20, 2026 02:43
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