Cap the liquid-test polling interval at 5 seconds#263
Conversation
The delay between result polls grows 5% per poll with no upper bound, so a long test run (~10 minutes server-side) ends up being checked only every 30-60 seconds - a finished run can sit unnoticed for up to a minute, and in CI that lag multiplies across batches. Cap the delay at 5s: at most one poll per 5 seconds per run (reached after ~70s), bounding the detection lag without meaningfully increasing request volume for short runs.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
WalkthroughThis PR caps the exponential polling backoff delay in liquid test result fetching at 5000ms using Math.min, preventing excessively long polling gaps during long-running tests. Includes a corresponding unit test, documentation updates, changelog entry, and package version bump to 1.56.4. ChangesPolling Delay Cap
Estimated code review effort: 1 (Trivial) | ~5 minutes Related PRs: None identified. Suggested labels: bug, tests Suggested reviewers: None identified. 🐰 A hop, a poll, a gentle cap, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
2 participants
Why
fetchResultgrows the poll delay 5% per poll with no cap. For long test runs that means:A liquid test that takes ~10 minutes server-side (observed in be_market CI, e.g. run 28656835005: a single test, 9m51s) is only checked every ~30s by then, so a finished run sits unnoticed for up to a minute. In CI batches that detection lag multiplies.
What
Cap the delay at 5 seconds (reached after ~70s of polling). Detection lag is now bounded at 5s; short runs poll exactly as before, and the steady-state request rate is at most one
readTestRunper 5s per run.Validation
setTimeoutdelay is exactly 5000ms (cap reached, never exceeded).Context: follow-up from silverfin/bso_github_actions#28 (CI-side retries + parallel firms) — this is the CLI-side piece of the latency work.