Repository navigation
Wait for the TanStack Start page before its dev server tests run - #386
Merged
Merged
Conversation
Another test file can bind the port that getPort() released before the dev server starts. The readiness check accepted any HTTP reply, so the tests ran against that server. Require the app page, fail fast when the dev server exits, and pass --strictPort so a taken port is reported instead of moved. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🚀 Deploying Preview to Cloudflare 🚀Preview URL: https://fix-tanstack-dev-test-port-void-cloud.voidzero-docs.workers.dev (commit 97e69f3)This URL reflects your latest Preview deploymentPreview Deployments by commit
|
Brooooooklyn
added a commit
that referenced
this pull request
Oct 8, 2026
…#396) ## Problem `solid-pages-playground.test.ts > solid-pages client (dev)` failed once in a full local run and passed on rerun: ``` Error: Timed out waiting for server at http://127.0.0.1:52761 (waited 45000ms) Port 52761 is in use, trying another one... Port 52762 is in use, trying another one... ➜ Local: http://127.0.0.1:52763/ ``` Every dev and preview server test starts the same way. The test picks a free port, frees it, and starts the server. The server binds the port 1–7 s later. Anything can take the port in that gap. ``` solid test another process getPort() → 52761, then frees it spawn `vp dev --port 52761` (Vite loads for 1–7 s) binds 52761 ◄── the gap Vite: 52761 in use → tries 52762 (52762 = the next test's port) Vite: 52762 in use → binds 52763 test polls 52761 for 45 s ─────────► never our server → timeout ``` Two things make it worse: - The OS hands out ports in order, so tests that start together get ports next to each other (seen: 59293, 59294, 59295 in the same 10 ms). When Vite moves to `port + 1`, it lands on another test's port. That test then fails too. - Vite without `--strictPort` never stops. It moves silently, and the test only finds out after the full timeout. ### Who took 52761 The process is gone, so it cannot be named for that run. What I checked: | Check | Result | | --- | --- | | Is our own port hold still open? | No. It is freed right after spawn. Holding it makes Vite print exactly this message, but the trace shows it is freed seconds before Vite binds. | | Can the OS give the freed port to another `listen(0)`? | No on macOS. Ports go out in order (measured 54 ports/s, a full cycle takes about 5 min). Ports held by wildcard listeners are skipped too. | | Who binds explicit port numbers in a run? | Traced every Node `listen()` in 6 full runs (+2 overlapping pairs): only each test server on its own port (and inspector ports near 9229). No collision happened in those runs. | | Who binds explicit ports outside a run? | Long-running dev servers from other sessions: Miniflare restarts workerd on the port it had before (`--socket-addr=entry=127.0.0.1:55332`). | | CI (Linux) | The OS can reuse a freed port right away. #386 saw another test's `listen(0)` get the port. | So the port was taken by a process that binds a port number it chose itself. On Linux CI, another test's `listen(0)` can also take it. The test design turns any of these into a 45 s timeout. The fix does not depend on which process it was. ## What this PR changes ``` startServer(spawnServer, timeout, options) ┌─► pick free port P │ spawn server with --port P --strictPort │ wait until it prints its URL with P ("Local: …:P/", "Listening on …:P") │ then wait for HTTP on P │ ├─ ready ─────────────────────────────► return { child, base, output } │ ├─ server exits "already in use" ──┐ │ ├─ Vite prints "P is in use" ──────┤ (Astro: no strict flag) │ │ ▼ └─────┴──── kill it, try a new port (up to 3 times) └─ any other failure ─────────────► throw, as before ``` - All 30 server starts in 21 test files use `startServer()`. - Vite servers get `--strictPort`, so they never move into another test's port. - An answer on P only counts after our server printed its URL with P. Before that, the answer can come from the process that took P. - `getPort()` and the port hold are gone. The hold only covered the time until spawn, not the seconds until the server binds. - TESTING.md tells new tests to use `startServer()` with `--strictPort`. ## Decisions | Choice | Why | | --- | --- | | Strict port + retry, not "use whatever URL the server prints" | Servers never take another test's port. | | Wait for the server's own URL line before counting answers | Vite, Astro, Node, Bun and Deno print it only once they listen on P. No fixed delay. | | Nuxt: `printsUrl: false` | Nuxt prints no URL under Vitest (`NODE_ENV=test`). Its test checks the page body `Nuxt + Void` instead. | | Retry only when the output says the port is taken | Real start failures still fail at once, with the same output. | | 3 attempts | One retry fixes a single collision. A third covers a second one. More would hide a real problem. | | Astro: watch for Vite's move message | `astro dev` has no `--strictPort`. The message is enough to restart it. | | No longer timeouts | Timeouts are unchanged. A taken port now costs about 1 s instead of 45 s. | ## Known limits - Nuxt has no strict option and moves to a random port without a message. It still times out if its port is taken. It never takes another test's port. It prints no URL under Vitest, so only its page body check shows the answer is ours. - On macOS, the built Node, Bun and Deno servers listen on all addresses and do not fail when another process holds `127.0.0.1` on the same port. They print their URL, so there is nothing to retry. Requests to `127.0.0.1` then reach the other process. ## Testing | Command | Result | | --- | --- | | Solid test on `main` with a process binding its port (and port + 1) right after spawn | Same failure as reported: `Port N is in use` ×2, 45 s timeout | | Same, with this PR (dev, preview, and Astro) | Pass. Log: `Another process took port N; starting the server on a new port.` | | An HTTP server answers on the port right after spawn (real `vp dev`, ssr playground) | Without the URL check: returns in 13 ms, page is the other server, ours exits `Port N is already in use`. With it: retries, returns our page, server still running | | `vp test run packages/void/test/unit/integration-helpers.test.ts` | 9 passed (4 new: taken port, other HTTP server on the port, moved server, real failure not retried). The retry tests fail with retries turned off; the HTTP server test fails with the URL check turned off. | | `vp test run packages/solid/test/integration/solid-pages-playground.test.ts` ×3 | 32/32 each, 6.1–6.4 s | | The 21 files that use `startServer()` ×2 | 21/21 files each, 17.4 s | | Astro, Vite dev and preview, Analog, Node and Bun server tests with `CI=true` | Pass | | `vp test run --project parallel-integration` ×2 | 52/52 files each, 19.1–20.2 s (`main`: 27.6–27.9 s) | | `vp run test --base origin/main` (selects all 691 files) | 690 passed, 1 skipped | | `vp test run` | 690 passed, 1 skipped | | `vp lint`, `vp fmt --check`, `vp run check` | pass | 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
tanstack-start.test.tssometimes fails in CI with the wrong page and a 404 on/smoke. Another test file's server answers on its port.Seen on CI run 37720538898: the returned HTML was the node-static playground page ("This is a statically prerendered React app built with Node.js."), and the TanStack server reported ready after 1.5 s instead of the usual 5–10 s.
Fix
--strictPortmakes Vite exit with "Port N is already in use" instead of moving to another port, so a taken port gives a clear error.Testing
vp test run packages/void/test/integration/tanstack-start.test.tsvp lint,vp fmt --checkon the file🤖 Generated with Claude Code