MoonshotAI / MoonshotAI/kimi-code
test(server): flaky startServer port-retry assertion under concurrent test runs
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 7.5k
- Forks
- 1.2k
- Avg merge
- 11h 53m
- Merged PRs (30d)
- 350
Description
Summary
packages/server/test/start.test.ts has a flaky test:
"retries on port+1 and updates the lock when the requested port is held by a third party"
It assumes that port + 1 remains free between the helper releasing it and startServer binding it. Under vitest's concurrent workers, another test can grab port + 1 in that window, causing startServer to bind to port + 2 or higher and the assertion to fail.
Reproduction
Run the packages/server suite repeatedly:
for i in {1..10}; do pnpm vitest run packages/server; done
In my environment this fails ~80% of the time with an error like:
Expected: "http://127.0.0.1:55398"
Received: "http://127.0.0.1:55399"
❯ test/start.test.ts:183:25
expect(r.address).toBe(`http://127.0.0.1:${String(next)}`);
Running start.test.ts in isolation always passes, which confirms the failure is caused by concurrent port allocation from other vitest workers.
Root cause
allocateAdjacentFreePair() finds port and port + 1, then immediately releases both. The test occupies port to simulate a third-party listener, but leaves port + 1 unprotected. While the test prepares to start the server, another worker can bind port + 1, so startServer advances to port + 2 and the strict assertion fails.
Possible fixes
-
Relax the integration assertion (recommended)
Verify thatstartServerbinds to a port>= port + 1and that the lock file records the actual bound port. Keep the exactport + 1retry strategy covered by the existinglistenWithPortRetryunit tests, which already use a fake gateway to assert5000 -> 5001 -> 5002. -
Hold
port + 1until right before binding
HaveallocateAdjacentFreePair()return areleaseNext()callback that is awaited immediately beforestartServer()runs. This shrinks the race window but does not fully eliminate it; in my testing it reduced the failure rate from ~80% to ~20%. -
Run the test sequentially
Mark the test ordescribeblock as sequential so it does not race with other workers. This avoids the issue but slows down the suite and does not address the underlying fragility. -
Introduce cross-process locking
Use a file lock or similar mechanism so only one process allocates ports in this helper at a time. This is heavier and adds complexity.
I am happy to open a PR for whichever approach the maintainers prefer. I have already verified option 1 locally: after relaxing the assertion, 10 consecutive full packages/server runs passed for start.test.ts.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with packages/server/test/start.test.ts, especially the flaky retry case around line 183, and run the repeated packages/server Vitest command to reproduce it. Compare the integration assertion with the existing listenWithPortRetry unit tests; done means the test accepts a concurrent-worker-selected retry port while verifying that the lock records the actual bound port.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- testing-qa
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100