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 |
|
@jangjos-128 @zeshi-du Hello. Is everything okay? This is the previous PR #155. Is there a problem? |
|
Hi @Awad-de — yes, everything's okay! There's no problem with your work at all. We'll review it through the normal process from here. You don't need to do anything else. Apologies for the confusion, and thanks again for contributing to TestSprite! |
|
Hi @jangjos-128 thanks but I'm participating in the hackathon-s3 competition. Do I have to win every PR that's merged? |
|
Not at all @Awad-de! In our community, it's definitely quality over quantity. It really comes down to how impactful and useful your PR is for the folks managing the CLI. Even a single, solid merge that brings real value can absolutely get rewarded! 🙌 |
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
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.