Skip to content

test(cloudflare): stop comparing call-order timing halves in perf regression test - #266

Merged
gfargo merged 1 commit into
mainfrom
fix/flaky-cloudflare-perf-regression-test
Aug 24, 2026
Merged

test(cloudflare): stop comparing call-order timing halves in perf regression test#266
gfargo merged 1 commit into
mainfrom
fix/flaky-cloudflare-perf-regression-test

Conversation

@gfargo

@gfargo gfargo commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Summary

  • The "should maintain consistent performance across multiple operations" test compared the average duration of the first 5 vs. last 5 of 10 consecutive fetchConfig() calls, failing if the second half was >3x slower.
  • This was flaky in a way that plain re-tuning the threshold wouldn't fix: repeated local runs (8 isolated runs, full durations logged) show the first 2-3 calls are consistently ~100x faster than the rest — interpreter/GC warm-up in the benchmark loop itself, not fetchConfig(). Whichever half absorbed that warm-up boundary swung its average unpredictably, so the ratio assertion passed or failed close to at random depending on system load.
  • Tried switching to a median-based comparison (the natural "make it statistical" fix) — it's worse, not better: 10-27x ratio on every single run, because the warm-up split confounds any first-half/second-half comparison regardless of which statistic is used. No threshold or statistic fixes a structural confound in the benchmark design.
  • fetchConfig() has no accumulating per-call state that could cause a real slowdown across repeated calls (confirmed by reading CloudflareFirewallService/CloudflareClient — its internal ApiCache would make repeat calls faster, not slower).
  • Replaced the relative comparison with a per-call absolute ceiling (reusing the existing 10s fetch SLA constant already used elsewhere in this file), which still catches a real regression (a hang, or an accidental O(n²) over repeated calls) without being sensitive to warm-up noise.

Test plan

  • Target test run standalone 10x in a row — 10/10 pass
  • Full test file run 8x in a row — 19/19 tests pass each time
  • Full cloudflare/__tests__/ directory run 3x in a row — 506/506 tests pass each time
  • --ci mode run once — 19/19 pass
  • eslint on the changed file — 0 errors (pre-existing no-console warnings only, not introduced by this change)
  • tsc --noEmit — clean

…ression test

The first-half-vs-second-half average comparison was flaky: repeated
local runs show the first 2-3 calls in the loop are consistently
~100x faster than the rest (interpreter/GC warm-up in the benchmark
loop itself, not fetchConfig()), so whichever half absorbed that
warm-up boundary swung its average unpredictably and the 3x ratio
assertion passed or failed essentially at random. Switching to a
median-based comparison doesn't fix it either (verified empirically:
10-27x ratio every run) since the warm-up split confounds any
first-half/second-half comparison regardless of statistic used.

Replaced with a per-call absolute ceiling, which still catches a
real regression (a hang, or an accidental O(n^2) over repeated
calls) without being sensitive to that noise. Verified stable across
21 consecutive runs (isolated, full-file, and full-directory).
@gfargo
gfargo merged commit 8ad165b into main Aug 24, 2026
2 checks passed
@gfargo
gfargo deleted the fix/flaky-cloudflare-perf-regression-test branch August 24, 2026 14:49
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 3.14.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant