diff --git a/frontend/playwright.visual.config.ts b/frontend/playwright.visual.config.ts index 1ef8e88b4..8f31abf71 100644 --- a/frontend/playwright.visual.config.ts +++ b/frontend/playwright.visual.config.ts @@ -1,10 +1,12 @@ /** * Report-only visual regression + page monitoring config. * - * Read-only by design: every project in this file visits pages with GET only, - * except the two conditional projects below, which do not exist unless their - * env flag is set (so nothing ever shows as "skipped"): + * Read-only by design: every project in this file visits pages with GET only. + * Conditional projects below do not exist unless their env flag is set + * (so nothing ever shows as "skipped"): * + * - crawl-desktop / crawl-mobile: sitemap crawler (crawl.spec.ts). + * Opt-in via VISUAL_CRAWL=1. No workflow sets VISUAL_CRAWL. * - preview-forms: blank-submit validation screenshots on /login + /signup. * Only on preview deploys, via VISUAL_ALLOW_FORM_SUBMIT=1. Never fills * credentials, never creates an account. @@ -21,6 +23,7 @@ import { defineConfig } from '@playwright/test'; const allowFormSubmit = process.env.VISUAL_ALLOW_FORM_SUBMIT === '1'; const allowSignedIn = process.env.VISUAL_AUTH === '1'; +const allowCrawl = process.env.VISUAL_CRAWL === '1'; export default defineConfig({ testDir: './tests/visual', @@ -90,22 +93,26 @@ export default defineConfig({ name: 'smoke', testMatch: ['smoke.spec.ts'], }, - { - name: 'crawl-desktop', - testMatch: ['crawl.spec.ts'], - use: { viewport: { width: 1440, height: 900 }, deviceScaleFactor: 1 }, - }, - { - name: 'crawl-mobile', - testMatch: ['crawl.spec.ts'], - use: { - viewport: { width: 390, height: 844 }, - isMobile: true, - hasTouch: true, - deviceScaleFactor: 1, - }, - }, /* Conditional projects: absent unless the env flag is set. */ + ...(allowCrawl + ? [ + { + name: 'crawl-desktop', + testMatch: ['crawl.spec.ts'], + use: { viewport: { width: 1440, height: 900 }, deviceScaleFactor: 1 }, + }, + { + name: 'crawl-mobile', + testMatch: ['crawl.spec.ts'], + use: { + viewport: { width: 390, height: 844 }, + isMobile: true, + hasTouch: true, + deviceScaleFactor: 1, + }, + }, + ] + : []), ...(allowFormSubmit ? [ { diff --git a/frontend/tests/visual/README.md b/frontend/tests/visual/README.md index 1fb106fd6..297820fcc 100644 --- a/frontend/tests/visual/README.md +++ b/frontend/tests/visual/README.md @@ -38,8 +38,8 @@ VISUAL_BASE_URL=https://regengine.co npx playwright test --config=playwright.vis # One project VISUAL_BASE_URL=https://regengine.co npx playwright test --config=playwright.visual.config.ts --project=smoke -# Sitemap crawler at both widths -VISUAL_BASE_URL=https://regengine.co npx playwright test --config=playwright.visual.config.ts --project=crawl-desktop --project=crawl-mobile +# Sitemap crawler at both widths (opt-in via VISUAL_CRAWL=1) +VISUAL_BASE_URL=https://regengine.co VISUAL_CRAWL=1 npx playwright test --config=playwright.visual.config.ts --project=crawl-desktop --project=crawl-mobile # View the last HTML report npx playwright show-report playwright-visual-report @@ -52,6 +52,7 @@ npx playwright show-report playwright-visual-report | `VISUAL_BASE_URL` | Base URL under test. Default `https://regengine.co` | | `VISUAL_ALLOW_FORM_SUBMIT` | `1` enables the `preview-forms` project (blank submits). Set only by `visual-preview.yml` | | `VISUAL_AUTH` | `1` enables the `signed-in` project. Set by hand for manual runs; no workflow sets it | +| `VISUAL_CRAWL` | `1` enables the `crawl-desktop` / `crawl-mobile` projects. Set by hand for manual runs; no workflow sets it | | `VISUAL_USER_EMAIL` | Expected session email for the signed-in project (asserted, not used to log in) | | `VISUAL_SESSION_TOKEN` | Opaque session cookie value for the signed-in project, provisioned out-of-band | | `VISUAL_UPDATE_BOXES` | `1` records header/nav boxes to `test-results-visual/layout-boxes.candidate.json` (JSONL) instead of asserting | @@ -66,8 +67,8 @@ npx playwright show-report playwright-visual-report | `priority-extras` | priority extras | mixed (set per test) | | `content` | content rules | default | | `smoke` | smoke | default | -| `crawl-desktop` | sitemap crawl | 1440×900 | -| `crawl-mobile` | sitemap crawl | 390×844, touch | +| `crawl-desktop` | sitemap crawl | 1440×900, only if `VISUAL_CRAWL=1` | +| `crawl-mobile` | sitemap crawl | 390×844, touch, only if `VISUAL_CRAWL=1` | | `preview-forms` | blank submits | default, only if `VISUAL_ALLOW_FORM_SUBMIT=1` | | `signed-in` | signed-in reads | default, only if `VISUAL_AUTH=1` | diff --git a/frontend/tests/visual/crawl.spec.ts b/frontend/tests/visual/crawl.spec.ts index a102b6a98..ac3daba37 100644 --- a/frontend/tests/visual/crawl.spec.ts +++ b/frontend/tests/visual/crawl.spec.ts @@ -41,7 +41,61 @@ interface PageFinding { } function baseOriginOf(page: Page): string { - return new URL(test.info().project.use.baseURL ?? page.url()).origin; + const raw = + test.info().project.use.baseURL ?? + process.env.VISUAL_BASE_URL ?? + page.url(); + let origin: string; + try { + origin = new URL(raw).origin; + } catch { + throw new Error(`Cannot determine crawl base origin from "${raw}": invalid URL`); + } + if (origin === 'null' || origin === '') { + const fallback = process.env.VISUAL_BASE_URL ?? 'https://regengine.co'; + try { + const fallbackOrigin = new URL(fallback).origin; + if (fallbackOrigin !== 'null' && fallbackOrigin !== '') { + return fallbackOrigin; + } + } catch { + // Fall through to the clear error below. + } + throw new Error( + `Cannot determine crawl base origin from "${raw}": got opaque origin 'null'. ` + + 'Set VISUAL_BASE_URL to a valid http(s) URL.', + ); + } + return origin; +} + +/** Rewrite a sitemap URL to the base origin so preview runs crawl the preview. */ +function rewriteToBaseOrigin(loc: string, baseOrigin: string): string { + try { + const url = new URL(loc, baseOrigin); + const base = new URL(baseOrigin); + url.protocol = base.protocol; + url.host = base.host; + return url.toString(); + } catch { + return loc; + } +} + +/** Decode XML entities in a value; & is decoded last. */ +function decodeXmlEntities(value: string): string { + return value + .replace(/&#x([0-9A-Fa-f]+);/g, (_m: string, hex: string) => + String.fromCodePoint(parseInt(hex, 16)), + ) + .replace(/&#([0-9]+);/g, (_m: string, dec: string) => + String.fromCodePoint(parseInt(dec, 10)), + ) + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/"/g, '"') + .replace(/'/g, "'") + .replace(/&/g, '&'); } /** Extract values from a sitemap or sitemap-index document. */ @@ -49,7 +103,8 @@ function parseLocs(xml: string): string[] { const locs: string[] = []; const re = /\s*([^<]+?)\s*<\/loc>/gi; let match: RegExpExecArray | null; - while ((match = re.exec(xml)) !== null) locs.push(match[1].trim()); + while ((match = re.exec(xml)) !== null) + locs.push(decodeXmlEntities(match[1].trim())); return locs; } @@ -71,11 +126,13 @@ async function fetchSitemapPaths(page: Page, baseOrigin: string): Promise 0 && locs.every((loc) => isSitemapUrl(loc))) { const childPaths: string[] = []; for (const loc of locs) { - const response = await page.request.get(loc, { + const childUrl = rewriteToBaseOrigin(loc, baseOrigin); + const response = await page.request.get(childUrl, { timeout: LINK_TIMEOUT_MS, maxRedirects: 5, }); - if (!response.ok()) continue; + if (!response.ok()) + throw new Error(`child sitemap GET failed: ${childUrl} status ${response.status()}`); childPaths.push(...parseLocs(await response.text())); } locs = childPaths; @@ -92,6 +149,7 @@ async function fetchSitemapPaths(page: Page, baseOrigin: string): Promise undefined); } catch (error) { + navigationFailed = true; pageErrors.push(`navigation failed: ${String(error).slice(0, 300)}`); } finally { page.off('pageerror', onPageError); @@ -213,34 +273,49 @@ test('crawl sitemap pages for broken links, dead ends, and errors', async ({ } const pageUrl = page.url(); - const hrefs = await page.locator('a[href]').evaluateAll((anchors) => - anchors.map((a) => (a as HTMLAnchorElement).getAttribute('href') ?? ''), - ); - const scopedHrefs = await page - .locator('main a[href], nav a[href], footer a[href]') - .evaluateAll((anchors) => + // Scope links per page: a failed load keeps the previous page's DOM, + // so it must record no links rather than crediting them to this path. + let scopedSameOrigin = 0; + if (!navigationFailed) { + const hrefs = await page.locator('a[href]').evaluateAll((anchors) => anchors.map((a) => (a as HTMLAnchorElement).getAttribute('href') ?? ''), ); - let scopedSameOrigin = 0; - for (const href of scopedHrefs) { - if (normalizeLink(href, pageUrl, baseOrigin)) scopedSameOrigin += 1; - } - for (const href of hrefs) { - const target = normalizeLink(href, pageUrl, baseOrigin); - if (target && !linkSources.has(target)) linkSources.set(target, crawlPath); + const scopedHrefs = await page + .locator('main a[href], nav a[href], footer a[href]') + .evaluateAll((anchors) => + anchors.map((a) => (a as HTMLAnchorElement).getAttribute('href') ?? ''), + ); + const normalizedPageUrl = normalizeLink(pageUrl, pageUrl, baseOrigin); + for (const href of scopedHrefs) { + const scopedTarget = normalizeLink(href, pageUrl, baseOrigin); + if (scopedTarget && scopedTarget !== normalizedPageUrl) + scopedSameOrigin += 1; + } + for (const href of hrefs) { + const target = normalizeLink(href, pageUrl, baseOrigin); + if (target && !linkSources.has(target)) linkSources.set(target, crawlPath); + } } let deadEndReason: string | null = null; - if (status >= 400) { + // A failed navigation leaves stale DOM behind and is already recorded + // in pageErrors, so skip dead-end classification entirely. + if (!navigationFailed && status >= 400) { deadEndReason = `HTTP ${status}`; - } else { + } else if (!navigationFailed) { let finalPath = ''; try { finalPath = new URL(pageUrl).pathname; } catch { finalPath = ''; } - const h1 = ((await page.locator('h1').first().textContent().catch(() => '')) ?? '').trim(); + const h1 = ( + (await page + .locator('h1') + .first() + .textContent({ timeout: 5_000 }) + .catch(() => '')) ?? '' + ).trim(); if (finalPath === '/404' || finalPath === '/not-found' || h1 === '404') { deadEndReason = `ends on 404 page (${finalPath || pageUrl})`; } else if (scopedSameOrigin === 0) {