fix(rerun): emit partial stdout on TimeoutError in single-FE rerun --wait#219
fix(rerun): emit partial stdout on TimeoutError in single-FE rerun --wait#219Awad-de wants to merge 2 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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughModified 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/commands/test.ts (1)
6102-6113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate partial-timeout-print logic between
TimeoutErrorandRequestTimeoutErrorbranches.The new
TimeoutErrorblock (Lines 6102-6113) and the existingRequestTimeoutErrorblock (Lines 6126-6142) build near-identical{ runId, status: 'running' }objects and near-identical text formatters, differing only in the wording of the timeout reason and hint line. Consider extracting a small shared helper (e.g.printTimeoutPartial(out, runId, reasonText)) to keep both branches — and any future timeout branches — in sync.♻️ Proposed refactor sketch
+function printRerunTimeoutPartial( + out: ReturnType<typeof makeOutput>, + runId: string, + reasonText: string, +): void { + const partial = { runId, status: 'running' as const }; + out.print(partial, data => { + const p = data as typeof partial; + return [ + `runId ${p.runId}`, + `status ${p.status} (${reasonText})`, + `hint Re-attach with: testsprite test wait ${p.runId}`, + ].join('\n'); + }); +}Then call it from both catch branches with the appropriate
reasonText.Also applies to: 6126-6142
🤖 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 6102 - 6113, The TimeoutError and RequestTimeoutError branches in test.ts duplicate the same partial-run printing logic. Extract the shared `{ runId, status: 'running' }` construction and `out.print` formatter into a small helper (for example, a timeout-partial printer) and call it from both catch paths, passing only the branch-specific timeout reason/hint text so the two cases stay consistent.src/commands/test.rerun.spec.ts (1)
4699-4703: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider also asserting
nextActionon the thrown error.Per DOCUMENTATION.md,
--wait --timeoutexceedance is expected to include anextActionpointing callers totest wait <run-id>. Assertingerr.details?.nextAction(or equivalent) here would guard that documented contract, not just the exit code and stdout partial.As per path instructions: "Exit-code mapping: error paths must map to the documented exit code (see DOCUMENTATION.md / the exit-code table)".
🤖 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.rerun.spec.ts` around lines 4699 - 4703, The rerun timeout assertion currently checks only the exit code and stdout, but it should also verify the documented `nextAction` on the thrown error. Update the `test.rerun.spec.ts` timeout case to assert `err.details?.nextAction` (or the equivalent error field) in the same block where `err` is matched, using the rerun/wait flow around the `rerunResp.runId` scenario, so the contract that callers are directed to `test wait <run-id>` is covered.Source: Path instructions
🤖 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.rerun.spec.ts`:
- Around line 4699-4703: The rerun timeout assertion currently checks only the
exit code and stdout, but it should also verify the documented `nextAction` on
the thrown error. Update the `test.rerun.spec.ts` timeout case to assert
`err.details?.nextAction` (or the equivalent error field) in the same block
where `err` is matched, using the rerun/wait flow around the `rerunResp.runId`
scenario, so the contract that callers are directed to `test wait <run-id>` is
covered.
In `@src/commands/test.ts`:
- Around line 6102-6113: The TimeoutError and RequestTimeoutError branches in
test.ts duplicate the same partial-run printing logic. Extract the shared `{
runId, status: 'running' }` construction and `out.print` formatter into a small
helper (for example, a timeout-partial printer) and call it from both catch
paths, passing only the branch-specific timeout reason/hint text so the two
cases stay consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a45e95fc-68a8-4092-b342-b64f27518f03
📒 Files selected for processing (2)
src/commands/test.rerun.spec.tssrc/commands/test.ts
|
Discord Username / User ID |
1 participant
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
Hackathon CLI Improvement — agent recovery on polling timeout (complements
the
test run --wait/test waitTimeoutError fix; this covers theremaining single-FE
test rerun --waitpath).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.README.md/DOCUMENTATION.mdwhere relevant. (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
npm test → 1571 passed
npm run typecheck → clean
npm run lint:fix → clean
Summary by CodeRabbit
runningstatus) before exiting.--waittimeout behavior, including partial output validation.