test(server): relax port retry integration assertion#941
Conversation
|
tomsen-ai
left a comment
There was a problem hiding this comment.
This matches the root cause I described in #937. The relaxed integration assertion plus the lock-file check is exactly the fix I had in mind, and the lint cleanups in the helpers are a nice addition.
One tiny suggestion: the variable is only used for hostname/port, so you could inline to avoid the extra binding, but the current version is perfectly readable.
Thanks for picking this up quickly.
tomsen-ai
left a comment
There was a problem hiding this comment.
This matches the root cause I described in #937. The relaxed integration assertion plus the lock-file check is exactly the fix I had in mind, and the lint cleanups in the helpers are a nice addition.
One tiny suggestion: the boundUrl variable is only used for hostname/port, so you could inline new URL(r.address) to avoid the extra binding, but the current version is perfectly readable.
Thanks for picking this up quickly.
Summary
startServerport-retry integration assertion so it uses the actual bound retry portlistenWithPortRetryunit testRelated Issue
Fixes #937.
Tests
pnpm --filter @moonshot-ai/server exec vitest run test/start.test.tspnpm --filter @moonshot-ai/server exec vitest run test/start.test.tspnpm --filter @moonshot-ai/server run typecheckpnpm --filter @moonshot-ai/server run buildpnpm exec oxlint --type-aware packages/server/test/start.test.ts --quietgit diff --checkAlso ran
pnpm --filter @moonshot-ai/server run test;start.test.tspassed, but the package run failed in unrelatedtest/fs-watch.e2e.test.tswith a 5000ms timeout inAC #2: burst > 500 changes inside 200ms window -> truncated:true.AI Assistance Disclosure
I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.