fix(rerun): emit partial stdout on TimeoutError in single-FE rerun --wait#155
fix(rerun): emit partial stdout on TimeoutError in single-FE rerun --wait#155Awad-de wants to merge 3 commits into
Conversation
…wait
Apply the fix in src/commands/test.ts. When the overall --timeout polling
deadline is exceeded on a single FE rerun, emit {runId, status:"running"}
to stdout before exit 7.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds partial JSON stdout output when a single FE rerun with ChangesRerun timeout partial output
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Discord: |
|
Approved — this redo is exactly what we asked for: only the claimed fix, mirroring the existing RequestTimeoutError partial-stdout contract. Its sibling (#153) merged today, which is also what made this one go CONFLICTING (same regions of |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/commands/test.ts (1)
6638-6684: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider consolidating the repeated timeout-partial-print pattern.
The
{ runId, status: 'running' as const }+out.print(..., renderer)+ reattach-hint block is now duplicated 6 times acrossrunTestRun,runTestWait, andrunTestRerun(this PR adds the 6th instance). A small shared helper (e.g.printTimeoutPartial(out, { runId, extra?, timeoutSeconds, reason })) would reduce drift risk across these near-identical call sites, though the per-site field differences (enqueuedAt/codeVersion/targetUrl inrunTestRunvs bare runId/status here) mean some parameterization is needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/test.ts` around lines 6638 - 6684, The timeout-partial rendering logic is duplicated again in the error handling around the rerun polling path, so factor the repeated `{ runId, status: 'running' as const }` plus `out.print(..., renderer)` and reattach-hint behavior into a shared helper. Update the catch block in `runTestRerun` to call that helper, and design it so existing call sites in `runTestRun` and `runTestWait` can share it while still allowing their extra fields and message variations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/commands/test.ts`:
- Around line 6638-6684: The timeout-partial rendering logic is duplicated again
in the error handling around the rerun polling path, so factor the repeated `{
runId, status: 'running' as const }` plus `out.print(..., renderer)` and
reattach-hint behavior into a shared helper. Update the catch block in
`runTestRerun` to call that helper, and design it so existing call sites in
`runTestRun` and `runTestWait` can share it while still allowing their extra
fields and message variations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b7883e5f-dbf7-41f8-8ed5-4bc9148977db
📒 Files selected for processing (2)
src/commands/test.rerun.spec.tssrc/commands/test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/commands/test.rerun.spec.ts
|
Approved on content — #154 landed just ahead of this and took the adjacent |
|
@zeshi-du Hii Thank you, and sorry for the inconvenience. Everything should be ready. |
2 participants
What does this PR do?
Fixes an asymmetry in
testsprite test rerun --wait(single FE rerun path):when the overall
--timeoutpolling deadline is exceeded, the CLI now emits apartial
{ runId, status: "running" }object to stdout before throwingexit 7 — matching the behavior already present for
RequestTimeoutErrorandthe recently fixed
test run --wait/test waitpaths.Before:
TimeoutError→ throwApiErrorimmediately → stdout empty in--output jsonmode → AI agents must scrape stderr to recover the runId.After:
TimeoutError→out.print({ runId, status: "running" })→ throw→ caller can programmatically chain into
testsprite test wait <runId>.Related issue
Fixes #157
Type of change
Checklist
mainbranch.(
feat(...),fix(...),docs(...), …).npm run lintandnpm run format:checkpass.npm run typecheckpasses.npm testpasses and coverage stays at or above the 80% gate.credentials required).
README.md/DOCUMENTATION.mdwhererelevant. (N/A — internal error-handling parity fix, no CLI surface change.)
Notes for reviewers
Root cause
In
runTestRerun(single FE rerun, no BE closure), theTimeoutErrorcatchblock at ~line 6132 called
ticker.finalize()then threwApiErrorwithoutany
out.print(). The adjacentRequestTimeoutErrorbranch already emitteda partial run to stdout — this was simply a missed mirror.
Scope
test rerun --waitTimeoutErrorpath only (~12 lines).into the batch result object on timeout;
test run --wait/test waitwere fixed in a prior PR.
Test added
[finding-4]intest.rerun.spec.ts:TimeoutErrorviatimeoutSeconds: 0(immediate deadline){ runId, status: "running" }Verification
Summary by CodeRabbit
--waittimeouts by emitting a partial run update to standard output before the failure is returned.runIdandstatus: running, making it easier to capture progress while a rerun is still in progress.