Skip to content

chore: stabilize tests #6492

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
schiller-manuel wants to merge 1 commit into
base: main
Choose a base branch
from
+252 −150
Open

chore: stabilize tests #6492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 18 additions & 49 deletions e2e/react-start/css-modules/tests/css.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,6 @@ const whitelistErrors = [
test.describe('CSS styles in SSR (dev mode)', () => {
test.use({ whitelistErrors })

// Warmup: trigger Vite's dependency optimization before running tests
// This prevents "optimized dependencies changed. reloading" during actual tests
// We use a real browser context since dep optimization happens on JS load, not HTTP requests
test.beforeAll(async ({ browser, baseURL }) => {
const context = await browser.newContext()
const page = await context.newPage()
try {
// Load both pages to trigger dependency optimization
await page.goto(baseURL!)
await page.waitForTimeout(2000) // Wait for deps to optimize
await page.goto(`${baseURL}/modules`)
await page.waitForTimeout(2000)
// Load again after optimization completes
await page.goto(baseURL!)
await page.waitForTimeout(1000)
} catch {
// Ignore errors during warmup
} finally {
await context.close()
}
})

// Helper to build full URL from baseURL and path
// Playwright's goto with absolute paths (like '/modules') ignores baseURL's path portion
// So we need to manually construct the full URL
Expand Down Expand Up @@ -223,44 +201,35 @@ test.describe('CSS styles in SSR (dev mode)', () => {
// Start from home
await page.goto(buildUrl(baseURL!, '/'))

// Verify initial styles with retry to handle potential Vite dep optimization reload
// Verify initial styles
const globalElement = page.getByTestId('global-styled')
await expect(async () => {
await expect(globalElement).toBeVisible()
const backgroundColor = await globalElement.evaluate(
(el) => getComputedStyle(el).backgroundColor,
)
expect(backgroundColor).toBe('rgb(59, 130, 246)')
}).toPass({ timeout: 10000 })
await expect(globalElement).toBeVisible()
let backgroundColor = await globalElement.evaluate(
(el) => getComputedStyle(el).backgroundColor,
)
expect(backgroundColor).toBe('rgb(59, 130, 246)')

// Navigate to modules page
await page.getByTestId('nav-modules').click()
// Use glob pattern to match with or without basepath
await page.waitForURL('**/modules')

// Verify CSS modules styles with retry to handle potential Vite dep optimization reload
// Verify CSS modules styles
const card = page.getByTestId('module-card')
await expect(async () => {
await expect(card).toBeVisible()
const backgroundColor = await card.evaluate(
(el) => getComputedStyle(el).backgroundColor,
)
expect(backgroundColor).toBe('rgb(240, 253, 244)')
}).toPass({ timeout: 10000 })
await expect(card).toBeVisible()
backgroundColor = await card.evaluate(
(el) => getComputedStyle(el).backgroundColor,
)
expect(backgroundColor).toBe('rgb(240, 253, 244)')

// Navigate back to home
await page.getByTestId('nav-home').click()
// Match home URL with or without trailing slash and optional query string
// Matches: /, /?, /my-app, /my-app/, /my-app?foo=bar
await page.waitForURL(/\/([^/]*)(\/)?($|\?)/)

// Verify global styles still work with retry
await expect(async () => {
await expect(globalElement).toBeVisible()
const backgroundColor = await globalElement.evaluate(
(el) => getComputedStyle(el).backgroundColor,
)
expect(backgroundColor).toBe('rgb(59, 130, 246)')
}).toPass({ timeout: 10000 })
// Verify global styles still work
await expect(globalElement).toBeVisible()
backgroundColor = await globalElement.evaluate(
(el) => getComputedStyle(el).backgroundColor,
)
expect(backgroundColor).toBe('rgb(59, 130, 246)')
})
})
73 changes: 72 additions & 1 deletion e2e/react-start/css-modules/tests/setup/global.setup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,77 @@
import { e2eStartDummyServer } from '@tanstack/router-e2e-utils'
import { chromium } from '@playwright/test'
import {
e2eStartDummyServer,
getTestServerPort,
} from '@tanstack/router-e2e-utils'
import packageJson from '../../package.json' with { type: 'json' }

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}`)
Comment on lines +8 to +19
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "global.setup.ts" | head -20

Repository: TanStack/router

Length of output: 1451

🏁 Script executed:

cat -n ./e2e/react-start/css-modules/tests/setup/global.setup.ts

Repository: 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.ts

Repository: 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 -100

Repository: TanStack/router

Length of output: 86

🏁 Script executed:

# Search for waitForServer in the codebase
rg "waitForServer" --type ts -A 5 -B 2

Repository: 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 -30

Repository: 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.json

Repository: 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 -50

Repository: 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 -30

Repository: 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 preOptimizeDevServer(baseURL: string) {
const browser = await chromium.launch()
const context = await browser.newContext()
const page = await context.newPage()

try {
await page.goto(`${baseURL}/`, { waitUntil: 'domcontentloaded' })
await page.getByTestId('global-styled').waitFor({ state: 'visible' })
await page.waitForLoadState('networkidle')

await page.goto(`${baseURL}/modules`, { waitUntil: 'domcontentloaded' })
await page.getByTestId('module-card').waitFor({ state: 'visible' })
await page.waitForLoadState('networkidle')

await page.goto(`${baseURL}/sass-mixin`, { waitUntil: 'domcontentloaded' })
await page.getByTestId('mixin-styled').waitFor({ state: 'visible' })
await page.waitForLoadState('networkidle')

await page.goto(`${baseURL}/quotes`, { waitUntil: 'domcontentloaded' })
await page.getByTestId('quote-styled').waitFor({ state: 'visible' })
await page.getByTestId('after-quote-styled').waitFor({ state: 'visible' })
await page.waitForLoadState('networkidle')

// Ensure we end in a stable state. Vite's optimize step triggers a reload;
// this waits until no further navigations happen for a short window.
for (let i = 0; i < 40; i++) {
const currentUrl = page.url()
await page.waitForTimeout(250)
if (page.url() === currentUrl) {
await page.waitForTimeout(250)
if (page.url() === currentUrl) return
}
}

throw new Error('Dev server did not reach a stable URL after warmup')
} finally {
await context.close()
await browser.close()
}
}

export default async function setup() {
await e2eStartDummyServer(packageJson.name)

if (process.env.MODE !== 'dev') return

const viteConfig = process.env.VITE_CONFIG // 'nitro' | 'basepath' | 'cloudflare' | undefined
const port = await getTestServerPort(
viteConfig ? `${packageJson.name}-${viteConfig}` : packageJson.name,
)
const basePath = viteConfig === 'basepath' ? '/my-app' : ''
const baseURL = `http://localhost:${port}${basePath}`

await waitForServer(baseURL)
await preOptimizeDevServer(baseURL)
}
67 changes: 18 additions & 49 deletions e2e/solid-start/css-modules/tests/css.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,6 @@ const whitelistErrors = [
test.describe('CSS styles in SSR (dev mode)', () => {
test.use({ whitelistErrors })

// Warmup: trigger Vite's dependency optimization before running tests
// This prevents "optimized dependencies changed. reloading" during actual tests
// We use a real browser context since dep optimization happens on JS load, not HTTP requests
test.beforeAll(async ({ browser, baseURL }) => {
const context = await browser.newContext()
const page = await context.newPage()
try {
// Load both pages to trigger dependency optimization
await page.goto(baseURL!)
await page.waitForTimeout(2000) // Wait for deps to optimize
await page.goto(`${baseURL}/modules`)
await page.waitForTimeout(2000)
// Load again after optimization completes
await page.goto(baseURL!)
await page.waitForTimeout(1000)
} catch {
// Ignore errors during warmup
} finally {
await context.close()
}
})

// Helper to build full URL from baseURL and path
// Playwright's goto with absolute paths (like '/modules') ignores baseURL's path portion
// So we need to manually construct the full URL
Expand Down Expand Up @@ -195,44 +173,35 @@ test.describe('CSS styles in SSR (dev mode)', () => {
// Start from home
await page.goto(buildUrl(baseURL!, '/'))

// Verify initial styles with retry to handle potential Vite dep optimization reload
// Verify initial styles
const globalElement = page.getByTestId('global-styled')
await expect(async () => {
await expect(globalElement).toBeVisible()
const backgroundColor = await globalElement.evaluate(
(el) => getComputedStyle(el).backgroundColor,
)
expect(backgroundColor).toBe('rgb(59, 130, 246)')
}).toPass({ timeout: 10000 })
await expect(globalElement).toBeVisible()
let backgroundColor = await globalElement.evaluate(
(el) => getComputedStyle(el).backgroundColor,
)
expect(backgroundColor).toBe('rgb(59, 130, 246)')

// Navigate to modules page
await page.getByTestId('nav-modules').click()
// Use glob pattern to match with or without basepath
await page.waitForURL('**/modules')

// Verify CSS modules styles with retry to handle potential Vite dep optimization reload
// Verify CSS modules styles
const card = page.getByTestId('module-card')
await expect(async () => {
await expect(card).toBeVisible()
const backgroundColor = await card.evaluate(
(el) => getComputedStyle(el).backgroundColor,
)
expect(backgroundColor).toBe('rgb(240, 253, 244)')
}).toPass({ timeout: 10000 })
await expect(card).toBeVisible()
backgroundColor = await card.evaluate(
(el) => getComputedStyle(el).backgroundColor,
)
expect(backgroundColor).toBe('rgb(240, 253, 244)')

// Navigate back to home
await page.getByTestId('nav-home').click()
// Match home URL with or without trailing slash and optional query string
// Matches: /, /?, /my-app, /my-app/, /my-app?foo=bar
await page.waitForURL(/\/([^/]*)(\/)?($|\?)/)

// Verify global styles still work with retry
await expect(async () => {
await expect(globalElement).toBeVisible()
const backgroundColor = await globalElement.evaluate(
(el) => getComputedStyle(el).backgroundColor,
)
expect(backgroundColor).toBe('rgb(59, 130, 246)')
}).toPass({ timeout: 10000 })
// Verify global styles still work
await expect(globalElement).toBeVisible()
backgroundColor = await globalElement.evaluate(
(el) => getComputedStyle(el).backgroundColor,
)
expect(backgroundColor).toBe('rgb(59, 130, 246)')
})
})
64 changes: 63 additions & 1 deletion e2e/solid-start/css-modules/tests/setup/global.setup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,68 @@
import { e2eStartDummyServer } from '@tanstack/router-e2e-utils'
import { chromium } from '@playwright/test'
import {
e2eStartDummyServer,
getTestServerPort,
} from '@tanstack/router-e2e-utils'
import packageJson from '../../package.json' with { type: 'json' }

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}`)
Comment on lines +8 to +19
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 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.ok is only true for HTTP 200–299; 3xx (redirect) status codes are not “ok” (so ok === false for a 3xx response). [1]
  • In browsers, fetch() uses redirect: "follow" by default, so if the server returns a 3xx, fetch() typically follows the redirect and resolves with the final response (often 200), where response.ok may be true. You can detect that a redirect happened with response.redirected === true (and response.url will be the final URL). [2] [3]
  • You can control redirect handling with the request’s redirect mode: "follow" (default), "error", or "manual". Setting redirect: "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.

Suggested change
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 preOptimizeDevServer(baseURL: string) {
const browser = await chromium.launch()
const context = await browser.newContext()
const page = await context.newPage()

try {
await page.goto(`${baseURL}/`, { waitUntil: 'domcontentloaded' })
await page.getByTestId('global-styled').waitFor({ state: 'visible' })
await page.waitForLoadState('networkidle')

await page.goto(`${baseURL}/modules`, { waitUntil: 'domcontentloaded' })
await page.getByTestId('module-card').waitFor({ state: 'visible' })
await page.waitForLoadState('networkidle')

await page.goto(`${baseURL}/sass-mixin`, { waitUntil: 'domcontentloaded' })
await page.getByTestId('mixin-styled').waitFor({ state: 'visible' })
await page.waitForLoadState('networkidle')

// Ensure we end in a stable state. Vite's optimize step triggers a reload;
// this waits until no further navigations happen for a short window.
for (let i = 0; i < 40; i++) {
const currentUrl = page.url()
await page.waitForTimeout(250)
if (page.url() === currentUrl) {
await page.waitForTimeout(250)
if (page.url() === currentUrl) return
}
}

throw new Error('Dev server did not reach a stable URL after warmup')
} finally {
await context.close()
await browser.close()
}
}

export default async function setup() {
await e2eStartDummyServer(packageJson.name)

if (process.env.MODE !== 'dev') return

const port = await getTestServerPort(packageJson.name)
const baseURL = `http://localhost:${port}`

await waitForServer(baseURL)
await preOptimizeDevServer(baseURL)
}
Loading
Loading