Skip to content

fix(ci): isolate visual diff captures #2710

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
riderx wants to merge 1 commit into
base: main
Choose a base branch
from
+165 −40
Open

fix(ci): isolate visual diff captures #2710

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
1 change: 1 addition & 0 deletions .github/workflows/visual-diff.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ jobs:
run: ln -sfn supabase/templates templates

- name: Run visual diff pipeline
timeout-minutes: 15
run: |
bun scripts/visual-diff.ts run \
--base "${{ github.event.pull_request.base.sha }}" \
Expand Down
198 changes: 158 additions & 40 deletions scripts/visual-diff.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { getPlaywrightStripeApiBaseUrl, getStripeEmulatorPort } from './playwrig
import { getSupabaseWorktreeConfig } from './supabase-worktree-config'

type Phase = 'before' | 'after'
type Command = 'capture' | 'diff' | 'run'
type Command = 'capture' | 'capture-route' | 'diff' | 'run'

interface ManagedProcess {
child: ChildProcessWithoutNullStreams
Expand Down Expand Up @@ -45,14 +45,14 @@ interface CliOptions {
}

const repoRoot = process.cwd()
const defaultVisualDiffConfigPath = process.env.VISUAL_DIFF_CONFIG_PATH || resolve(repoRoot, 'playwright/visual-diff.config.ts')

interface LoadedVisualDiffConfig {
visualDiffRoutes: VisualDiffRoute[]
visualDiffViewport: { width: number, height: number }
}

async function loadVisualDiffConfig(): Promise<LoadedVisualDiffConfig> {
const configPath = resolve(repoRoot, 'playwright/visual-diff.config.ts')
async function loadVisualDiffConfig(configPath = defaultVisualDiffConfigPath): Promise<LoadedVisualDiffConfig> {
const module = await import(pathToFileURL(configPath).href)
return {
visualDiffRoutes: module.visualDiffRoutes as VisualDiffRoute[],
Expand All @@ -72,6 +72,7 @@ const managedProcesses: ManagedProcess[] = []
const isWindows = process.platform === 'win32'
const httpReadyTimeoutMs = 5_000
const visualDiffActionTimeoutMs = 60_000
const visualDiffRouteTimeoutMs = 120_000

function sleep(ms: number): Promise<void> {
return new Promise(resolve => setTimeout(resolve, ms))
Expand All @@ -87,8 +88,8 @@ function parseThreshold(raw: string, source: string): number {
function parseArgs(argv: string[]): CliOptions {
const [commandRaw, ...rest] = argv
const command = (commandRaw || 'run') as Command
if (!['capture', 'diff', 'run'].includes(command)) {
throw new Error(`Unknown command "${commandRaw}". Use capture, diff, or run.`)
if (!['capture', 'capture-route', 'diff', 'run'].includes(command)) {
throw new Error(`Unknown command "${commandRaw}". Use capture, capture-route, diff, or run.`)
}

let phase: Phase | undefined
Expand Down Expand Up @@ -134,10 +135,13 @@ function parseArgs(argv: string[]): CliOptions {
throw new Error(`Unknown argument "${arg}"`)
}

if (command === 'capture' && phase !== 'before' && phase !== 'after') {
throw new Error('capture requires --phase before or --phase after')
if ((command === 'capture' || command === 'capture-route') && phase !== 'before' && phase !== 'after') {
throw new Error(`${command} requires --phase before or --phase after`)
}

if (command === 'capture-route' && routes.length !== 1)
throw new Error('capture-route requires exactly one --routes slug')

return {
command,
phase,
Expand Down Expand Up @@ -320,6 +324,62 @@ async function stopProcess(processToStop: ManagedProcess) {
await waitForExit(2000)
}

function waitForManagedProcess(managed: ManagedProcess, timeoutMs: number, label: string) {
const { child } = managed
if (child.exitCode !== null || child.signalCode !== null) {
if (child.exitCode === 0)
return Promise.resolve()
return Promise.reject(new Error(`${label} exited before it completed`))
}

return new Promise<void>((resolveProcess, rejectProcess) => {
let settled = false
let timeout: ReturnType<typeof setTimeout> | undefined
const finish = (callback: () => void) => {
if (settled)
return
settled = true
if (timeout)
clearTimeout(timeout)
callback()
}
const onError = (error: Error) => finish(() => rejectProcess(error))
const onClose = (code: number | null, signal: NodeJS.Signals | null) => finish(() => {
if (code === 0) {
resolveProcess()
return
}
rejectProcess(new Error(`${label} exited with code ${code ?? 'none'}${signal ? ` (${signal})` : ''}`))
})

timeout = setTimeout(() => {
finish(() => rejectProcess(new Error(`Timed out ${label} after ${timeoutMs}ms`)))
}, timeoutMs)
child.once('error', onError)
child.once('close', onClose)
})
}

async function captureRouteInChild(phase: Phase, route: VisualDiffRoute, configPath: string, runnerPath: string) {
const captureProcess = startProcess(
`capture-${phase}-${route.slug}`,
[process.execPath, runnerPath, 'capture-route', '--phase', phase, '--routes', route.slug],
{ VISUAL_DIFF_CONFIG_PATH: configPath },
)
try {
await waitForManagedProcess(captureProcess, visualDiffRouteTimeoutMs, `capturing ${phase} ${route.slug}`)
}
catch (error) {
await stopProcess(captureProcess)
throw error
}
finally {
const index = managedProcesses.indexOf(captureProcess)
if (index >= 0)
managedProcesses.splice(index, 1)
}
}

async function stopAllProcesses() {
for (const managed of [...managedProcesses].reverse())
await stopProcess(managed)
Expand Down Expand Up @@ -481,63 +541,78 @@ async function settlePage(page: Page) {
})
}

async function captureScreenshots(
async function captureRouteScreenshot(
phase: Phase,
routeFilter: string[],
options: { forceFrontend?: boolean, forceBackend?: boolean } = {},
config?: LoadedVisualDiffConfig,
route: VisualDiffRoute,
visualDiffViewport: LoadedVisualDiffConfig['visualDiffViewport'],
) {
const { visualDiffRoutes, visualDiffViewport } = config ?? await loadVisualDiffConfig()
const routes = selectedRoutes(visualDiffRoutes, routeFilter)
const targetDir = phaseDir(phase)
mkdirSync(targetDir, { recursive: true })

console.log(`[visual-diff] preparing ${phase} capture stack`)
const { supabaseUrl, supabaseAnon, apiDomain } = await ensureBackendStack({ force: options.forceBackend })
await prepareScreenshotData(supabaseUrl, supabaseAnon)
await ensureFrontendStack(supabaseUrl, supabaseAnon, apiDomain, { force: options.forceFrontend })
console.log(`[visual-diff] ${phase} frontend ready`)

console.log(`[visual-diff] launching browser for ${phase}`)
console.log(`[visual-diff] launching browser for ${phase} ${route.slug}`)
const browser = await chromium.launch({ headless: true, timeout: visualDiffActionTimeoutMs })
try {
console.log(`[visual-diff] creating browser context for ${phase}`)
console.log(`[visual-diff] creating browser context for ${phase} ${route.slug}`)
const context = await browser.newContext({
baseURL: frontendBaseUrl,
viewport: visualDiffViewport,
deviceScaleFactor: 1,
})
console.log(`[visual-diff] creating page for ${phase}`)
const page = await context.newPage()
page.setDefaultTimeout(visualDiffActionTimeoutMs)
page.setDefaultNavigationTimeout(visualDiffActionTimeoutMs)
let authenticated = false

for (const route of routes) {
console.log(`[visual-diff] capturing ${phase} ${route.slug}`)
if (route.auth && !authenticated) {
await login(page)
authenticated = true
}
if (route.auth) {
console.log(`[visual-diff] logging in for ${phase} ${route.slug}`)
await login(page)
console.log(`[visual-diff] logged in for ${phase} ${route.slug}`)
}

await page.goto(route.path, { waitUntil: 'domcontentloaded', timeout: visualDiffActionTimeoutMs })
console.log(`[visual-diff] navigating ${phase} ${route.slug}`)
await page.goto(route.path, { waitUntil: 'domcontentloaded', timeout: visualDiffActionTimeoutMs })
console.log(`[visual-diff] settling ${phase} ${route.slug}`)
await settlePage(page)
if (route.prepare) {
console.log(`[visual-diff] preparing ${phase} ${route.slug}`)
await route.prepare(page)
await settlePage(page)
if (route.prepare) {
await route.prepare(page)
await settlePage(page)
}
const outputPath = resolve(targetDir, `${route.slug}.png`)
await page.screenshot({ path: outputPath, fullPage: false, timeout: visualDiffActionTimeoutMs })
console.log(`[visual-diff] captured ${phase} ${route.slug} -> ${outputPath}`)
}

const outputPath = resolve(targetDir, `${route.slug}.png`)
console.log(`[visual-diff] screenshotting ${phase} ${route.slug}`)
await page.screenshot({ path: outputPath, fullPage: false, timeout: visualDiffActionTimeoutMs })
console.log(`[visual-diff] captured ${phase} ${route.slug} -> ${outputPath}`)
await context.close()
}
finally {
await browser.close()
}
}

async function captureScreenshots(
phase: Phase,
routeFilter: string[],
options: { forceFrontend?: boolean, forceBackend?: boolean } = {},
config?: LoadedVisualDiffConfig,
configPath = defaultVisualDiffConfigPath,
runnerPath = resolve(repoRoot, 'scripts/visual-diff.ts'),
) {
const { visualDiffRoutes } = config ?? await loadVisualDiffConfig(configPath)
const routes = selectedRoutes(visualDiffRoutes, routeFilter)
mkdirSync(phaseDir(phase), { recursive: true })

console.log(`[visual-diff] preparing ${phase} capture stack`)
const { supabaseUrl, supabaseAnon, apiDomain } = await ensureBackendStack({ force: options.forceBackend })
await prepareScreenshotData(supabaseUrl, supabaseAnon)
await ensureFrontendStack(supabaseUrl, supabaseAnon, apiDomain, { force: options.forceFrontend })
console.log(`[visual-diff] ${phase} frontend ready`)

for (const route of routes) {
console.log(`[visual-diff] capturing ${phase} ${route.slug} in an isolated browser`)
await captureRouteInChild(phase, route, configPath, runnerPath)
console.log(`[visual-diff] captured ${phase} ${route.slug}`)
}
}

function readPng(path: string) {
return PNG.sync.read(readFileSync(path))
}
Expand Down Expand Up @@ -719,6 +794,31 @@ function generateReport(results: DiffResult[], thresholdPercent: number) {
return summary
}

function bundleVisualDiffModule(sourcePath: string, snapshotPath: string) {
const build = spawnSync('bun', [
'build',
sourcePath,
'--bundle',
'--target=bun',
'--packages=external',
`--outfile=${snapshotPath}`,
], {
cwd: repoRoot,
stdio: 'inherit',
env: process.env,
})
if ((build.status ?? 1) !== 0)
throw new Error(`Failed to bundle visual diff module ${sourcePath}`)
}

function snapshotVisualDiffConfig(snapshotPath: string) {
bundleVisualDiffModule(defaultVisualDiffConfigPath, snapshotPath)
}

function snapshotVisualDiffRunner(snapshotPath: string) {
bundleVisualDiffModule(resolve(repoRoot, 'scripts/visual-diff.ts'), snapshotPath)
}

async function checkoutRef(ref: string) {
git(['checkout', '--force', ref])
const install = spawnSync('bun', ['install'], { cwd: repoRoot, stdio: 'inherit', env: process.env })
Expand All @@ -730,13 +830,16 @@ async function runPipeline(options: CliOptions) {
const originalRef = git(['rev-parse', 'HEAD'])
const baseSha = resolveGitRef(options.baseRef, git(['merge-base', 'HEAD', 'origin/main']))
const headSha = resolveGitRef(options.headRef, originalRef)
const visualDiffConfigPath = resolve(outputRoot, `visual-diff-config-${process.pid}.mjs`)
const visualDiffRunnerPath = resolve(outputRoot, `visual-diff-runner-${process.pid}.mjs`)

try {
if (!options.skipGitCheckout && originalRef !== headSha)
await checkoutRef(headSha)

// Keep the head route manifest while Git moves between revisions.
const visualDiffConfig = await loadVisualDiffConfig()
snapshotVisualDiffConfig(visualDiffConfigPath)
const visualDiffConfig = await loadVisualDiffConfig(visualDiffConfigPath)
snapshotVisualDiffRunner(visualDiffRunnerPath)

if (!options.skipGitCheckout) {
await stopFrontendStack()
Expand All @@ -747,6 +850,8 @@ async function runPipeline(options: CliOptions) {
options.routes,
{ forceFrontend: true, forceBackend: true },
visualDiffConfig,
visualDiffConfigPath,
visualDiffRunnerPath,
)
await stopFrontendStack()
await stopBackendStack()
Expand All @@ -761,6 +866,8 @@ async function runPipeline(options: CliOptions) {
forceBackend: !options.skipGitCheckout,
},
visualDiffConfig,
visualDiffConfigPath,
visualDiffRunnerPath,
)
const results = await compareScreenshots(options.thresholdPercent, options.routes, visualDiffConfig)
const summary = generateReport(results, options.thresholdPercent)
Expand All @@ -785,6 +892,8 @@ async function runPipeline(options: CliOptions) {
console.error(`[visual-diff] failed to restore git ref ${originalRef}:`, error)
}
}
rmSync(visualDiffConfigPath, { force: true })
rmSync(visualDiffRunnerPath, { force: true })
}
}

Expand All @@ -798,6 +907,15 @@ async function main() {
return
}

if (options.command === 'capture-route') {
const { visualDiffRoutes, visualDiffViewport } = await loadVisualDiffConfig()
const [route] = selectedRoutes(visualDiffRoutes, options.routes)
if (!route)
throw new Error('capture-route could not resolve its configured route')
await captureRouteScreenshot(options.phase!, route, visualDiffViewport)
return
}

if (options.command === 'diff') {
const results = await compareScreenshots(options.thresholdPercent, options.routes)
generateReport(results, options.thresholdPercent)
Expand Down
3 changes: 3 additions & 0 deletions src/types/supabase.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ export type Database = {
cli_version: string | null
Comment thread
riderx marked this conversation as resolved.
comment: string | null
created_at: string | null
created_by_apikey_rbac_id: string | null
deleted: boolean
deleted_at: string | null
external_url: string | null
Expand All @@ -193,6 +194,7 @@ export type Database = {
cli_version?: string | null
comment?: string | null
created_at?: string | null
created_by_apikey_rbac_id?: string | null
deleted?: boolean
deleted_at?: string | null
external_url?: string | null
Expand All @@ -219,6 +221,7 @@ export type Database = {
cli_version?: string | null
comment?: string | null
created_at?: string | null
created_by_apikey_rbac_id?: string | null
deleted?: boolean
deleted_at?: string | null
external_url?: string | null
Expand Down
3 changes: 3 additions & 0 deletions supabase/functions/_backend/utils/supabase.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ export type Database = {
cli_version: string | null
comment: string | null
created_at: string | null
created_by_apikey_rbac_id: string | null
deleted: boolean
deleted_at: string | null
external_url: string | null
Expand All @@ -193,6 +194,7 @@ export type Database = {
cli_version?: string | null
comment?: string | null
created_at?: string | null
created_by_apikey_rbac_id?: string | null
deleted?: boolean
deleted_at?: string | null
external_url?: string | null
Expand All @@ -219,6 +221,7 @@ export type Database = {
cli_version?: string | null
comment?: string | null
created_at?: string | null
created_by_apikey_rbac_id?: string | null
deleted?: boolean
deleted_at?: string | null
external_url?: string | null
Expand Down
Loading