fix(e2e): add explanation if server exited before being ready - #1761
fix(e2e): add explanation if server exited before being ready#1761TheAlexLichter wants to merge 4 commits into
Conversation
commit: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI 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)
📝 WalkthroughWalkthroughThe E2E server module imports the Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@src/e2e/server.ts`:
- Around line 133-136: Before the final stopServer() and timeout/lastError throw
path in the server startup wait flow, re-check ctx.serverProcess using the same
killed-or-non-null-exitCode predicate used in the loop. If the process has
exited, throw earlyExitError(ctx, { dev, elapsed: Date.now() - startedAt }) so
process diagnostics take precedence over the fallback error.
- Around line 113-121: Update the early-exit error construction around
ctx.serverLogs and waitForPort() to await the detached log collector before
formatting output. Preserve any collector error, and report it separately from
the server-exit message instead of classifying failed or incomplete capture as
no output; only use the existing no-output branches when collection completed
successfully.
- Around line 105-110: Update the early server-exit handling around
ctx.serverProcess to detect signal termination via
ctx.serverProcess.process?.signalCode ?? null, alongside the existing exit
conditions. Include the signal code in the details diagnostic, while preserving
the current exitCode and killed information.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a5ef7326-eacc-4e7d-b5b2-9af97e15b7d7
📒 Files selected for processing (1)
src/e2e/server.ts
|
@TheAlexLichter what do you make of the coderabbit suggestions? |
|
@danielroe resolved them! |
| const globalFetch = globalThis.fetch || _fetch | ||
|
|
||
| /** Resolves once the server's output has been fully collected into `ctx.serverLogs`. */ | ||
| let serverLogsCollected: Promise<void> | undefined |
There was a problem hiding this comment.
should this be on the test context instead? it's possible to start multiple servers in a single test
2 participants
🔗 Linked issue
None!
📚 Description
As the titles says. Adds more debug information if the server dies unexpectedly before being used.