test(elysia): mock node http server in provider spec#1358
Conversation
|
📝 WalkthroughWalkthroughThe test file ChangesElysia Server Provider Test Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/server-elysia/src/elysia-server-provider.spec.ts`:
- Line 29: The test setup currently uses an untyped mockServer variable, which
removes the safety of the node:http contract. Update the mock in
elysia-server-provider.spec.ts to use a typed shape such as Pick<Server,
"listen" | "close" | "once" | "listening">, and keep the mock object aligned
with the methods/properties exercised by the Elysia server provider tests.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6775c905-933f-4ab0-9a48-0ec403459a51
📒 Files selected for processing (1)
packages/server-elysia/src/elysia-server-provider.spec.ts
|
|
||
| describe("ElysiaServerProvider", () => { | ||
| let provider: ElysiaServerProvider; | ||
| let mockServer: any; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the spec file and nearby related implementation to check whether `any` is necessary.
git ls-files 'packages/server-elysia/src/*' | sed -n '1,200p'
printf '\n--- spec outline ---\n'
ast-grep outline packages/server-elysia/src/elysia-server-provider.spec.ts --view expanded || true
printf '\n--- spec excerpt ---\n'
cat -n packages/server-elysia/src/elysia-server-provider.spec.ts | sed -n '1,220p'
printf '\n--- provider outline ---\n'
ast-grep outline packages/server-elysia/src/elysia-server-provider.ts --view expanded || true
printf '\n--- provider excerpt ---\n'
cat -n packages/server-elysia/src/elysia-server-provider.ts | sed -n '1,260p'Repository: VoltAgent/voltagent
Length of output: 14398
Replace mockServer: any with a typed server mock. any drops type safety in this test file; use a Pick<Server, "listen" | "close" | "once" | "listening"> (or similar) so the mock stays aligned with node:http.
🤖 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 `@packages/server-elysia/src/elysia-server-provider.spec.ts` at line 29, The
test setup currently uses an untyped mockServer variable, which removes the
safety of the node:http contract. Update the mock in
elysia-server-provider.spec.ts to use a typed shape such as Pick<Server,
"listen" | "close" | "once" | "listening">, and keep the mock object aligned
with the methods/properties exercised by the Elysia server provider tests.
Source: Coding guidelines
Summary
node:http.createServer()instead of the oldapp.listen()pathlisten()/close()lifecycleProblem
startServer()now creates a Node HTTP server directly, but the spec file still mocked and asserted onapp.listen(). That leaves the real HTTP server path unmocked during tests and causes the suite to hitEADDRINUSEinstead of exercising the provider logic.Notes
This PR only updates the test suite to match the current Node-based startup flow.
Fixes #1204
Summary by cubic
Updated Elysia provider tests to mock
node:http.createServer()and assert the Node serverlisten/closelifecycle, aligning with the current startup path and preventing EADDRINUSE. Fixes #1204.Written for commit d9a42db. Summary will update on new commits.
Summary by CodeRabbit