Conversation
📝 WalkthroughWalkthroughCentralizes Vite dev-server warmup into shared global setup: adds Changes
Sequence Diagram(s)sequenceDiagram
participant Setup as Test Global Setup
participant Dev as Dev Server
participant Browser as Headless Chromium
participant Tests as E2E Test Runner
Setup->>Dev: waitForServer(baseURL) (poll until 2xx/3xx)
alt server reachable
Setup->>Browser: preOptimizeDevServer(baseURL) (launch headless)
Browser->>Dev: GET /, /modules, /sass-mixin, /quotes
Dev-->>Browser: HTML + assets (triggers Vite dep optimization)
Browser->>Browser: waitFor selectors & network idle
Browser->>Setup: confirm stable URL/state
Browser-->>Browser: close context
Setup->>Tests: proceed with tests (no per-test warmup)
else timeout
Dev-->>Setup: no response -> throw timeout
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
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 unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit edd68d9
☁️ 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:
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.
schiller-manuel
force-pushed
the
stabilize-tests
branch
from
385277c to
a667f4d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/vue-start/css-modules/tests/setup/global.setup.ts`:
- Around line 14-19: In waitForServer, when fetching with redirect: 'manual' the
code currently only accepts res.ok (2xx) which rejects valid 3xx redirect
responses; update the success check in the try block (the fetch result handling
near the fetch call using controller.signal and redirect: 'manual') to accept
any 2xx or 3xx by changing the condition to check res.status >= 200 &&
res.status < 400 so redirects are treated as successful server readiness.
| const res = await fetch(url, { | ||
| redirect: 'manual', | ||
| signal: controller.signal, | ||
| }) | ||
| if (res.ok) return | ||
| } catch { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check vue css-modules config for redirects (basePath/trailing slash)
rg -n "basepath|basePath|redirect|trailingSlash" e2e/vue-start/css-modules -A 3
# Compare waitForServer implementations across css-modules suites
echo "=== Comparing waitForServer across css-modules setups ==="
rg -n "waitForServer" e2e/*/css-modules/tests/setup/global.setup.ts -A 10 -B 2Repository: TanStack/router
Length of output: 5278
🏁 Script executed:
#!/bin/bash
# Find config files in vue css-modules
fd -e "ts" -e "tsx" -e "json" -e "config" "e2e/vue-start/css-modules" | head -20
# Check for basePath/trailingSlash in config files
rg -n "basePath|trailingSlash" e2e/vue-start/css-modules -t ts -t json -t tsxRepository: TanStack/router
Length of output: 492
🏁 Script executed:
#!/bin/bash
# Find config files in vue css-modules
fd . 'e2e/vue-start/css-modules' --type f | head -20
# Check for basePath/trailingSlash in config files
rg -n "basePath|trailingSlash" 'e2e/vue-start/css-modules' -t ts -t jsonRepository: TanStack/router
Length of output: 876
🏁 Script executed:
#!/bin/bash
# Check vite.config.ts for base path config
cat -n 'e2e/vue-start/css-modules/vite.config.ts'
# Check router.tsx for any redirect-related config
cat -n 'e2e/vue-start/css-modules/src/router.tsx'Repository: TanStack/router
Length of output: 892
🏁 Script executed:
#!/bin/bash
# Check if Solid css-modules also has the issue
cat -n 'e2e/solid-start/css-modules/vite.config.ts'
# Check the full waitForServer function in all three to see any differences
echo "=== Vue waitForServer full function ==="
sed -n '8,25p' 'e2e/vue-start/css-modules/tests/setup/global.setup.ts'
echo "=== React waitForServer full function ==="
sed -n '8,25p' 'e2e/react-start/css-modules/tests/setup/global.setup.ts'Repository: TanStack/router
Length of output: 1715
Accept 2xx/3xx redirect responses for consistent server readiness checks.
The waitForServer function uses redirect: 'manual' to prevent automatic redirect following, but then only accepts 2xx responses with res.ok. A 3xx response won't satisfy this condition, causing needless retries even when the server is healthy. The React css-modules setup already uses if (res.status >= 200 && res.status < 400) return to handle this correctly—apply the same pattern for consistency and robustness across all frameworks.
🛠️ Suggested fix
- if (res.ok) return
+ if (res.status >= 200 && res.status < 400) return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/vue-start/css-modules/tests/setup/global.setup.ts` around lines 14 - 19,
In waitForServer, when fetching with redirect: 'manual' the code currently only
accepts res.ok (2xx) which rejects valid 3xx redirect responses; update the
success check in the try block (the fetch result handling near the fetch call
using controller.signal and redirect: 'manual') to accept any 2xx or 3xx by
changing the condition to check res.status >= 200 && res.status < 400 so
redirects are treated as successful server readiness.
1 participant
Summary by CodeRabbit