test(web-search): drive the connect deadline on a virtual clock - #4867
Conversation
`fast headers plus raw byte progress can outlive connectTimeoutMs` timed out at 1002.37ms against its own 1000ms ceiling on macOS 2/2 of run 35186268515. Its siblings in the same describe run in 2.58, 3.48, 2.57, 3.06 and 14.65ms, so this case was three orders of magnitude slower than everything around it and sitting 2.37ms outside a bound it could not reliably clear. It was spending that second on five real 12ms waits used to walk a real deadline forward. Real sleeps and a real ceiling race the same scheduler, so runner load decides the outcome - which is how a case about timeout SEMANTICS became a case about how busy the machine was. It now drives the existing clearableDeadline seam with a virtual elapsed counter. The first byte lands at a virtual 26ms against a 25ms deadline, which is the exact condition the case exists to describe: headers that arrive fast must clear the deadline before the body is consumed, and continuing byte progress must not re-arm it. Body, cancellation, status and completion assertions are unchanged. The 1000ms ceiling stays exactly where it was. Nothing was widened; the wall-clock wait was removed instead. Ablation: leave the deadline armed, or move its clear to first-byte progress, and the abort lands before "a" is enqueued, so the body and completion assertions fail. No local suite, focused test, typecheck, build, or install was run.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
added
the
chore
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe web-search timeout test now uses virtual time and a mocked deadline. It validates deadline creation and cleanup, prevents synthetic timeout abortion during body progress, confirms the body is not cancelled, and restores the spy. ChangesWeb-search timeout test
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~12 minutes Change: Other Merge Risk: ⚪ Minimal · up to The timeout test is stabilized while retaining coverage that response headers clear the connect deadline before body consumption. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
리뷰 · 우선순위 77 / 80이 PR은 예전에 이 테스트는 본문 청크마다 진짜 지금 바꾼 점은 네 가지입니다. (1) 진짜 sleep 제거. (2) 현재 라인 961-1020 - 메인테이너의 판단이 필요한 지점
너의 추천
이 댓글은 grok-bot이 작성했습니다 |
The rewrite took this file to 2857 lines against a 2823 baseline, so the repository file-size ratchet failed and took test 1/4 and macos 2/2 with it. Compacted to exactly 2823. Every assertion, the virtual deadline semantics, the ablation and the 1000ms ceiling are unchanged; only the expression is tighter. The baseline itself is untouched. Editing the cap to fit a change is the same move as widening a timeout to fit a slow test. No local suite, focused test, typecheck, build, or install was run.
…e-jun#4867) * test(web-search): drive the connect deadline on a virtual clock `fast headers plus raw byte progress can outlive connectTimeoutMs` timed out at 1002.37ms against its own 1000ms ceiling on macOS 2/2 of run 35186268515. Its siblings in the same describe run in 2.58, 3.48, 2.57, 3.06 and 14.65ms, so this case was three orders of magnitude slower than everything around it and sitting 2.37ms outside a bound it could not reliably clear. It was spending that second on five real 12ms waits used to walk a real deadline forward. Real sleeps and a real ceiling race the same scheduler, so runner load decides the outcome - which is how a case about timeout SEMANTICS became a case about how busy the machine was. It now drives the existing clearableDeadline seam with a virtual elapsed counter. The first byte lands at a virtual 26ms against a 25ms deadline, which is the exact condition the case exists to describe: headers that arrive fast must clear the deadline before the body is consumed, and continuing byte progress must not re-arm it. Body, cancellation, status and completion assertions are unchanged. The 1000ms ceiling stays exactly where it was. Nothing was widened; the wall-clock wait was removed instead. Ablation: leave the deadline armed, or move its clear to first-byte progress, and the abort lands before "a" is enqueued, so the body and completion assertions fail. No local suite, focused test, typecheck, build, or install was run. * test(web-search): fit the virtual-clock rewrite inside the ratchet cap The rewrite took this file to 2857 lines against a 2823 baseline, so the repository file-size ratchet failed and took test 1/4 and macos 2/2 with it. Compacted to exactly 2823. Every assertion, the virtual deadline semantics, the ablation and the 1000ms ceiling are unchanged; only the expression is tighter. The baseline itself is untouched. Editing the cap to fit a change is the same move as widening a timeout to fit a slow test. No local suite, focused test, typecheck, build, or install was run.
1 participant
Summary
BUG-R86 routed web-search timeout semantics > fast headers plus raw byte progress can outlive connectTimeoutMstimed out at 1002.37ms against its own 1000ms ceiling onmacos 2/2of run 35186268515.Its siblings in the same describe run in 2.58, 3.48, 2.57, 3.06 and 14.65ms. This case was three orders of magnitude slower than everything around it and failing by 2.37ms — the signature of a case racing the scheduler rather than testing anything about the scheduler.
It was spending that second on five real 12ms waits used to walk a real deadline forward. Real sleeps under a real ceiling race the same scheduler that runner load perturbs, so a case about timeout semantics had quietly become a case about how busy the machine was.
It now drives the existing
clearableDeadlineseam with a virtual elapsed counter. The first byte lands at a virtual 26ms against a 25ms deadline, which is exactly the condition the case exists to describe: headers that arrive fast must clear the deadline before the body is consumed, and continuing byte progress must not re-arm it. Body, cancellation, status and completion assertions are unchanged.The 1000ms ceiling is unchanged. Nothing was widened — the wall-clock wait was removed instead.
Verification
No local suite, focused test, typecheck, build, or install was run; this lane is hosted-CI-only by task contract. Verification is static plus exact-head hosted CI.
Ablation, which is what makes this a real test rather than one that merely stopped failing: leave the deadline armed, or move its clear to first-byte progress, and the abort lands before
"a"is enqueued, so the body and completion assertions fail.The seam used already exists in this subsystem; no new pattern was introduced and no production code changed.
git diff --checkclean.Checklist
devSummary by CodeRabbit