chore(ipv6): only run ipv6 e2e test in ipv6 environment#1207
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an async runtime check that detects IPv6 availability by attempting to bind a temporary TCP server to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
commit: |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/test-kit.ts (1)
28-34: Optional: guard against lingering error events and add a safety timeout.The implementation is correct in practice (
Promise.resolveis idempotent, so a lateerrorafter a successfulcloseis harmless). Two minor hardening ideas worth considering:
- Remove the
errorlistener oncelistensucceeds (and vice versa) to avoid holding references to the server after resolution.- Add a short timeout fallback so the detection cannot hang indefinitely on pathological network stacks.
♻️ Suggested hardening
export async function isIPv6Available(): Promise<boolean> { return new Promise((resolve) => { const server = createServer(); - server.listen(0, '::1', () => server.close(() => resolve(true))); - server.on('error', () => resolve(false)); + const done = (result: boolean) => { + server.removeAllListeners('error'); + resolve(result); + }; + server.once('error', () => done(false)); + server.listen(0, '::1', () => server.close(() => done(true))); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/test-kit.ts` around lines 28 - 34, In isIPv6Available, avoid lingering listeners and potential hangs by removing the 'error' listener when listen succeeds and removing the 'listening'/'listening callback' handlers when an error fires, and add a short timeout (e.g., 1–2s) that closes the server and resolves false if neither event occurs; update the Promise logic to clear the timeout and remove the opposite listener on resolution so server references are released and the check cannot hang.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/e2e/test-kit.ts`:
- Around line 28-34: In isIPv6Available, avoid lingering listeners and potential
hangs by removing the 'error' listener when listen succeeds and removing the
'listening'/'listening callback' handlers when an error fires, and add a short
timeout (e.g., 1–2s) that closes the server and resolves false if neither event
occurs; update the Promise logic to clear the timeout and remove the opposite
listener on resolution so server references are released and the check cannot
hang.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5369a6b8-fb9c-470b-929d-86fcde24759c
📒 Files selected for processing (2)
test/e2e/ipv6.spec.tstest/e2e/test-kit.ts
ff09eaa to
31f0775
Compare
31f0775 to
6ad063e
Compare
Summary by CodeRabbit