Skip to content
20 changes: 18 additions & 2 deletions src/e2e/browser.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { Browser, BrowserContextOptions, Page, Response } from 'playwright-core'
import { useTestContext } from './context.ts'
import { url } from './server.ts'
import { useTestContext } from './context'
import { url } from './server'
import { join, dirname } from 'node:path'

export async function createBrowser() {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
const ctx = useTestContext()
Expand Down Expand Up @@ -43,9 +44,24 @@ export interface NuxtPage extends Omit<Page, 'goto'> {
}

export async function createPage(path?: string, options?: BrowserContextOptions): Promise<NuxtPage> {
const ctx = useTestContext()
const browser = await getBrowser()
const page = await browser.newPage(options) as unknown as NuxtPage

if (ctx.options.runner === 'vitest') {
const vitest = await import('vitest')
vitest.onTestFailed(async (test) => {
try {
await page.screenshot({
path: join(dirname(test.task.file.filepath), '__screenshots__', `failed-${test.task.fullTestName}.png`),

Copy link
Copy Markdown

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:

# First, let's look at the file structure and the specific line in question
cat -n src/e2e/browser.ts | head -70 | tail -20

Repository: nuxt/test-utils

Length of output: 799


🏁 Script executed:

# Run the verification commands from the review
rg -nP --type=ts -C3 'fullTestName.*\.png|__screenshots__' src

Repository: nuxt/test-utils

Length of output: 446


🏁 Script executed:

# Check for any existing sanitization functions
rg -nP --type=ts -C2 'sanitize|slug|replace\(.+[<>:"/\\|?*]' src/e2e

Repository: nuxt/test-utils

Length of output: 41


🏁 Script executed:

# Check if there are other places where test names are used as filenames
rg -nP --type=ts 'test\.task\.' src/e2e | grep -E '\.(png|jpg|jpeg|txt|json)'

Repository: nuxt/test-utils

Length of output: 195


🏁 Script executed:

# Verify that test names can indeed contain problematic characters
# by checking if there are any test name patterns in config or other files
rg -nP --type=ts 'fullTestName' src

Repository: nuxt/test-utils

Length of output: 195


🏁 Script executed:

# Let's verify what characters are actually allowed in filenames across platforms
# and understand the severity of this issue by checking if there are any existing test names
# that might trigger this problem
rg -nP --type=ts 'describe\(|it\(' src/e2e | head -20

Repository: nuxt/test-utils

Length of output: 261


🏁 Script executed:

# Let's look at actual test files to see what test names might look like
find src -name "*.test.ts" -o -name "*.spec.ts" | head -5

Repository: nuxt/test-utils

Length of output: 41


🏁 Script executed:

# Also check if there are any other usages of screenshot or similar file operations
rg -nP --type=ts 'screenshot|__screenshots__' src

Repository: nuxt/test-utils

Length of output: 249


Sanitize fullTestName before using it as a filename.

Line 56 builds the filename from test.task.fullTestName directly. Test names commonly include characters that are invalid on some filesystems (< > : " / \ | ? *), so screenshot capture can silently fail on those platforms.

πŸ’‘ Proposed fix
+function sanitizeScreenshotName(name: string): string {
+  return name
+    .replace(/[<>:"/\\|?*\u0000-\u001F]/g, '_')
+    .replace(/\s+/g, ' ')
+    .trim()
+    .slice(0, 180)
+}
+
 export async function createPage(path?: string, options?: BrowserContextOptions): Promise<NuxtPage> {
   const ctx = useTestContext()
   const browser = await getBrowser()
   const page = await browser.newPage(options) as unknown as NuxtPage
@@
     vitest.onTestFailed(async (test) => {
       try {
+        const fileName = `failed-${sanitizeScreenshotName(test.task.fullTestName)}.png`
         await page.screenshot({
-          path: join(dirname(test.task.file.filepath), '__screenshots__', `failed-${test.task.fullTestName}.png`),
+          path: join(dirname(test.task.file.filepath), '__screenshots__', fileName),
         })
       }
πŸ€– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/e2e/browser.ts` at line 56, Sanitize test.task.fullTestName before
interpolating into the screenshot filename: update the expression used in the
join(...) call (the line constructing path with `test.task.fullTestName`) to
replace or remove filesystem-invalid characters (e.g., <>:"/\\|?* and control
chars) or use a small helper like `sanitizeFilename(name)`/`slugify` to produce
a safe string, then use that sanitized value in the template
`failed-${sanitizedName}.png`; ensure the helper is imported/defined near the
top and used where `test.task.fullTestName` is currently referenced so
screenshots never fail due to invalid filename characters.

})
}
catch {
// noop
}
})
}

const _goto = page.goto.bind(page)
page.goto = async (url, options): Promise<Response | null> => {
const waitUntil = options?.waitUntil
Expand Down
Loading