chore: stabilize tests #6492
chore: stabilize tests #6492
Conversation
📝 WalkthroughWalkthroughThe PR consolidates Vite dev server warmup logic by removing per-test warmup blocks from three framework test suites (React, Solid, Vue CSS modules) and centralizing pre-optimization into shared global setup files with dedicated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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
Comment |
|
View your CI Pipeline Execution ↗ for commit 385277c
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@e2e/react-start/css-modules/tests/setup/global.setup.ts`:
- Around line 8-19: The waitForServer function currently only treats 2xx
responses as success and has no per-attempt timeout; update waitForServer to
create an AbortController for each fetch attempt, set a short per-attempt
timeout (e.g. 5s) that calls controller.abort(), and clear that timer after the
fetch completes; change the success condition from res.ok to accept any 2xx or
3xx status (status >= 200 && status < 400) so redirects count as a successful
server response, and ensure aborted/other fetch errors are caught and ignored so
the retry loop continues.
In `@e2e/solid-start/css-modules/tests/setup/global.setup.ts`:
- Around line 8-19: The waitForServer function can hang because fetch calls may
never resolve; modify waitForServer to create an AbortController for each
iteration, pass controller.signal into fetch(url, { redirect: 'manual', signal
}), and start a per-attempt timer (e.g., 2000–3000ms) that calls
controller.abort() when expired; ensure you clear the timer after fetch
completes and keep the existing try/catch so aborted or network errors are
ignored and the loop retries, then preserve the final timeout error behavior.
In `@e2e/vue-start/css-modules/tests/setup/global.setup.ts`:
- Around line 8-19: In waitForServer, add a per-attempt AbortController and a
short timeout (e.g., 2–5s) so each fetch call cannot hang indefinitely: for each
loop iteration create an AbortController, pass controller.signal to fetch(url, {
redirect: 'manual', signal }), set a timer to call controller.abort() after the
short interval, and clear that timer after the fetch finishes or throws; keep
the existing retry loop behavior and ensure aborted fetches are caught and
ignored as before before retrying.
| async function waitForServer(url: string) { | ||
| const start = Date.now() | ||
| while (Date.now() - start < 30_000) { | ||
| try { | ||
| const res = await fetch(url, { redirect: 'manual' }) | ||
| if (res.ok) return | ||
| } catch { | ||
| // ignore | ||
| } | ||
| await new Promise((r) => setTimeout(r, 250)) | ||
| } | ||
| throw new Error(`Timed out waiting for dev server at ${url}`) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "global.setup.ts" | head -20Repository: TanStack/router
Length of output: 1451
🏁 Script executed:
cat -n ./e2e/react-start/css-modules/tests/setup/global.setup.tsRepository: TanStack/router
Length of output: 3295
🏁 Script executed:
# Check other setup files to see if there are similar patterns
head -30 ./e2e/react-router/basic/tests/setup/global.setup.tsRepository: TanStack/router
Length of output: 279
🏁 Script executed:
# Check if there are other implementations of waitForServer or similar patterns
rg "waitForServer|redirect.*manual" --type ts --type tsx -A 5 -B 2 | head -100Repository: TanStack/router
Length of output: 86
🏁 Script executed:
# Search for waitForServer in the codebase
rg "waitForServer" --type ts -A 5 -B 2Repository: TanStack/router
Length of output: 3412
🏁 Script executed:
# Let me check the TypeScript/ESLint configuration for this file
head -20 ./e2e/react-start/css-modules/tsconfig.json 2>/dev/null || echo "No tsconfig in css-modules"
cat ./tsconfig.json 2>/dev/null | head -30Repository: TanStack/router
Length of output: 1350
🏁 Script executed:
# Check the Node.js version requirement and Playwright setup
cat package.json | grep -A 10 '"engines"' || echo "No engines specified"
cat package.json | grep -A 2 '"node"' || echo "No node version in scripts"
rg "playwright" package.jsonRepository: TanStack/router
Length of output: 188
🏁 Script executed:
# Check how the dev server is started and what port/behavior it has
rg "e2eStartDummyServer|getTestServerPort" --type ts -A 3 -B 1 | head -50Repository: TanStack/router
Length of output: 4428
🏁 Script executed:
# Check if there's any documentation or context about why redirects should be accepted
rg "redirect.*basepath|basePath|3xx|redirect.*accept" --type ts | head -30Repository: TanStack/router
Length of output: 1858
Handle redirects and per-attempt timeouts in waitForServer.
res.ok only returns true for 2xx status codes. With redirect: 'manual', 3xx responses (including base-path redirects) cause the check to fail. Additionally, the fetch call lacks a per-attempt timeout, allowing network hangs to stall the loop. Add an abort timeout per attempt and accept 3xx responses.
Suggested fix
async function waitForServer(url: string) {
const start = Date.now()
while (Date.now() - start < 30_000) {
+ const controller = new AbortController()
+ const timeout = setTimeout(() => controller.abort(), 2_000)
try {
- const res = await fetch(url, { redirect: 'manual' })
- if (res.ok) return
+ const res = await fetch(url, {
+ redirect: 'manual',
+ signal: controller.signal,
+ })
+ if (res.ok || (res.status >= 300 && res.status < 400)) return
} catch {
// ignore
- }
+ } finally {
+ clearTimeout(timeout)
+ }
await new Promise((r) => setTimeout(r, 250))
}
throw new Error(`Timed out waiting for dev server at ${url}`)
}🤖 Prompt for AI Agents
In `@e2e/react-start/css-modules/tests/setup/global.setup.ts` around lines 8 - 19,
The waitForServer function currently only treats 2xx responses as success and
has no per-attempt timeout; update waitForServer to create an AbortController
for each fetch attempt, set a short per-attempt timeout (e.g. 5s) that calls
controller.abort(), and clear that timer after the fetch completes; change the
success condition from res.ok to accept any 2xx or 3xx status (status >= 200 &&
status < 400) so redirects count as a successful server response, and ensure
aborted/other fetch errors are caught and ignored so the retry loop continues.
| async function waitForServer(url: string) { | ||
| const start = Date.now() | ||
| while (Date.now() - start < 30_000) { | ||
| try { | ||
| const res = await fetch(url, { redirect: 'manual' }) | ||
| if (res.ok) return | ||
| } catch { | ||
| // ignore | ||
| } | ||
| await new Promise((r) => setTimeout(r, 250)) | ||
| } | ||
| throw new Error(`Timed out waiting for dev server at ${url}`) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n e2e/solid-start/css-modules/tests/setup/global.setup.ts | sed -n '1,30p'Repository: TanStack/router
Length of output: 1211
🌐 Web query:
MDN fetch API res.ok status codes 3xx redirects
💡 Result:
Response.okis onlytruefor HTTP 200–299; 3xx (redirect) status codes are not “ok” (sook === falsefor a 3xx response). [1]- In browsers,
fetch()usesredirect: "follow"by default, so if the server returns a 3xx,fetch()typically follows the redirect and resolves with the final response (often 200), whereresponse.okmay betrue. You can detect that a redirect happened withresponse.redirected === true(andresponse.urlwill be the final URL). [2] [3] - You can control redirect handling with the request’s
redirectmode:"follow"(default),"error", or"manual". Settingredirect: "error"is the recommended way to disallow redirects. [3] [4]
Sources:
[1] MDN: Response.ok — status in the range 200–299 https://developer.mozilla.org/en-US/docs/Web/API/Response/ok
[2] MDN: Response.redirected (detect redirects, final response.url) https://developer.mozilla.org/en-US/docs/Web/API/Response/redirected
[3] MDN: Request.redirect (follow/error/manual; default follow) https://developer.mozilla.org/en-US/docs/Web/API/Request/redirect
[4] MDN: fetch() resolves on HTTP error statuses; check ok/status https://developer.mozilla.org/en-US/docs/Web/API/Window/fetch
Add per-attempt timeout to prevent hanging fetch calls in waitForServer.
With redirect: 'manual' in effect, a 3xx response is returned as-is and will fail the res.ok check, simply retrying rather than hanging. However, the fetch call itself can hang indefinitely if the server is unresponsive, stalling the loop. Add an abort timeout (e.g., 2–3 seconds) per fetch attempt to ensure robustness.
🛠️ Suggested fix
async function waitForServer(url: string) {
const start = Date.now()
while (Date.now() - start < 30_000) {
+ const controller = new AbortController()
+ const timeout = setTimeout(() => controller.abort(), 2_000)
try {
- const res = await fetch(url, { redirect: 'manual' })
+ const res = await fetch(url, {
+ redirect: 'manual',
+ signal: controller.signal,
+ })
if (res.ok) return
} catch {
// ignore
+ } finally {
+ clearTimeout(timeout)
}
await new Promise((r) => setTimeout(r, 250))
}
throw new Error(`Timed out waiting for dev server at ${url}`)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function waitForServer(url: string) { | |
| const start = Date.now() | |
| while (Date.now() - start < 30_000) { | |
| try { | |
| const res = await fetch(url, { redirect: 'manual' }) | |
| if (res.ok) return | |
| } catch { | |
| // ignore | |
| } | |
| await new Promise((r) => setTimeout(r, 250)) | |
| } | |
| throw new Error(`Timed out waiting for dev server at ${url}`) | |
| async function waitForServer(url: string) { | |
| const start = Date.now() | |
| while (Date.now() - start < 30_000) { | |
| const controller = new AbortController() | |
| const timeout = setTimeout(() => controller.abort(), 2_000) | |
| try { | |
| const res = await fetch(url, { | |
| redirect: 'manual', | |
| signal: controller.signal, | |
| }) | |
| if (res.ok) return | |
| } catch { | |
| // ignore | |
| } finally { | |
| clearTimeout(timeout) | |
| } | |
| await new Promise((r) => setTimeout(r, 250)) | |
| } | |
| throw new Error(`Timed out waiting for dev server at ${url}`) | |
| } |
🤖 Prompt for AI Agents
In `@e2e/solid-start/css-modules/tests/setup/global.setup.ts` around lines 8 - 19,
The waitForServer function can hang because fetch calls may never resolve;
modify waitForServer to create an AbortController for each iteration, pass
controller.signal into fetch(url, { redirect: 'manual', signal }), and start a
per-attempt timer (e.g., 2000–3000ms) that calls controller.abort() when
expired; ensure you clear the timer after fetch completes and keep the existing
try/catch so aborted or network errors are ignored and the loop retries, then
preserve the final timeout error behavior.
| async function waitForServer(url: string) { | ||
| const start = Date.now() | ||
| while (Date.now() - start < 30_000) { | ||
| try { | ||
| const res = await fetch(url, { redirect: 'manual' }) | ||
| if (res.ok) return | ||
| } catch { | ||
| // ignore | ||
| } | ||
| await new Promise((r) => setTimeout(r, 250)) | ||
| } | ||
| throw new Error(`Timed out waiting for dev server at ${url}`) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the actual file and its contents
cat -n e2e/vue-start/css-modules/tests/setup/global.setup.tsRepository: TanStack/router
Length of output: 2727
🏁 Script executed:
# Also check if there are TypeScript errors or configuration requirements
if [ -f e2e/vue-start/css-modules/tests/setup/tsconfig.json ]; then
cat e2e/vue-start/css-modules/tests/setup/tsconfig.json
fi
if [ -f e2e/vue-start/css-modules/tsconfig.json ]; then
cat e2e/vue-start/css-modules/tsconfig.json
fi
if [ -f tsconfig.json ]; then
head -50 tsconfig.json
fiRepository: TanStack/router
Length of output: 1415
🏁 Script executed:
# Search for usages of waitForServer to understand the context
rg "waitForServer" --type ts --type tsx -B 2 -A 2Repository: TanStack/router
Length of output: 86
🏁 Script executed:
# Search for waitForServer usage without tsx type
rg "waitForServer" -B 2 -A 2Repository: TanStack/router
Length of output: 2632
🏁 Script executed:
# Check if there are any comments or documentation about server startup
rg "dev server|localhost|startup" e2e/vue-start/css-modules/tests/setup/ -A 2 -B 2Repository: TanStack/router
Length of output: 856
🏁 Script executed:
# Check the e2eStartDummyServer to understand what server is being waited for
rg "e2eStartDummyServer" --type ts -B 3 -A 3Repository: TanStack/router
Length of output: 43381
🏁 Script executed:
# Check the dummy server implementation to see what status it returns
cat -n e2e/e2e-utils/src/localDummyServer.ts | head -100Repository: TanStack/router
Length of output: 3567
🏁 Script executed:
# Check for any 3xx redirect scenarios in the setup files
rg "basePath|redirect|301|302|3[0-9]{2}" e2e/*/css-modules/tests/setup/Repository: TanStack/router
Length of output: 631
🏁 Script executed:
# Check if react-start has custom basepath
cat -n e2e/react-start/css-modules/tests/setup/global.setup.ts | head -30Repository: TanStack/router
Length of output: 1211
Add per-attempt timeout to prevent hanging requests in waitForServer.
Without an AbortController, a stalled socket connection on any individual fetch attempt can hang indefinitely between retries, causing test setup delays. Add a short abort timeout per attempt to ensure each fetch times out quickly if the server is unresponsive.
🛠️ Suggested fix
async function waitForServer(url: string) {
const start = Date.now()
while (Date.now() - start < 30_000) {
+ const controller = new AbortController()
+ const timeout = setTimeout(() => controller.abort(), 2_000)
try {
- const res = await fetch(url, { redirect: 'manual' })
- if (res.ok) return
+ const res = await fetch(url, {
+ redirect: 'manual',
+ signal: controller.signal,
+ })
+ if (res.ok) return
} catch {
// ignore
+ } finally {
+ clearTimeout(timeout)
}
await new Promise((r) => setTimeout(r, 250))
}
throw new Error(`Timed out waiting for dev server at ${url}`)
}🤖 Prompt for AI Agents
In `@e2e/vue-start/css-modules/tests/setup/global.setup.ts` around lines 8 - 19,
In waitForServer, add a per-attempt AbortController and a short timeout (e.g.,
2–5s) so each fetch call cannot hang indefinitely: for each loop iteration
create an AbortController, pass controller.signal to fetch(url, { redirect:
'manual', signal }), set a timer to call controller.abort() after the short
interval, and clear that timer after the fetch finishes or throws; keep the
existing retry loop behavior and ensure aborted fetches are caught and ignored
as before before retrying.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.