diff --git a/clients/ember/bin/vizzly-browser.js b/clients/ember/bin/vizzly-browser.js index 5576eef0..07bce812 100755 --- a/clients/ember/bin/vizzly-browser.js +++ b/clients/ember/bin/vizzly-browser.js @@ -16,6 +16,7 @@ import { closeBrowser, launchBrowser } from '../src/launcher/browser.js'; import { + getServerInfo, setPage, startSnapshotServer, stopSnapshotServer, @@ -66,15 +67,28 @@ async function cleanup() { */ async function main() { try { - // 1. Start snapshot server first + // 1. Start snapshot server first (this also discovers the TDD server and caches its config) snapshotServer = await startSnapshotServer(); let snapshotUrl = `http://127.0.0.1:${snapshotServer.port}`; - // 2. Launch browser with Playwright + // 2. Determine failOnDiff: env var > server.json > default (false) + // getServerInfo() returns cached info from the TDD server discovery that happened above + let failOnDiff = false; + if (process.env.VIZZLY_FAIL_ON_DIFF === 'true' || process.env.VIZZLY_FAIL_ON_DIFF === '1') { + failOnDiff = true; + } else { + let serverInfo = getServerInfo(); + if (serverInfo?.failOnDiff) { + failOnDiff = true; + } + } + + // 3. Launch browser with Playwright // Note: We set the page reference in launchBrowser before navigation // to avoid a race condition where tests run before page is set browserInstance = await launchBrowser(browserType, testUrl, { snapshotUrl, + failOnDiff, onPageCreated: page => { // Set page reference immediately when page is created // This happens BEFORE navigation so tests can capture screenshots diff --git a/clients/ember/src/launcher/browser.js b/clients/ember/src/launcher/browser.js index bd4aded4..3518be79 100644 --- a/clients/ember/src/launcher/browser.js +++ b/clients/ember/src/launcher/browser.js @@ -61,11 +61,12 @@ function getChromiumArgs() { * @param {Object} options - Launch options * @param {string} options.snapshotUrl - URL of the snapshot HTTP server * @param {boolean} [options.headless] - Run in headless mode (default: true in CI) + * @param {boolean} [options.failOnDiff] - Whether tests should fail on visual diffs * @param {Function} [options.onPageCreated] - Callback when page is created (before navigation) * @returns {Promise} Browser instance with page reference */ export async function launchBrowser(browserType, testUrl, options = {}) { - let { snapshotUrl, headless, onPageCreated } = options; + let { snapshotUrl, headless, failOnDiff, onPageCreated } = options; // Default headless based on CI environment if (headless === undefined) { @@ -93,11 +94,15 @@ export async function launchBrowser(browserType, testUrl, options = {}) { let context = await browser.newContext(); let page = await context.newPage(); - // Inject snapshot URL into page context BEFORE navigation - // This ensures window.__VIZZLY_SNAPSHOT_URL__ is available when tests run - await page.addInitScript(url => { - window.__VIZZLY_SNAPSHOT_URL__ = url; - }, snapshotUrl); + // Inject Vizzly config into page context BEFORE navigation + // This ensures window.__VIZZLY_* is available when tests run + await page.addInitScript( + ({ snapshotUrl, failOnDiff }) => { + window.__VIZZLY_SNAPSHOT_URL__ = snapshotUrl; + window.__VIZZLY_FAIL_ON_DIFF__ = failOnDiff; + }, + { snapshotUrl, failOnDiff } + ); // Call onPageCreated callback BEFORE navigation // This allows setting up the page reference before tests can run diff --git a/clients/ember/src/launcher/snapshot-server.js b/clients/ember/src/launcher/snapshot-server.js index 797b3b98..19ae6e22 100644 --- a/clients/ember/src/launcher/snapshot-server.js +++ b/clients/ember/src/launcher/snapshot-server.js @@ -34,9 +34,14 @@ export function getPage() { return pageRef; } +/** + * Cached server info from auto-discovery + */ +let cachedServerInfo = null; + /** * Auto-discover the Vizzly TDD server by searching for .vizzly/server.json - * @returns {string|null} Server URL or null if not found + * @returns {{url: string, failOnDiff: boolean}|null} Server info or null if not found */ function autoDiscoverTddServer() { let currentDir = process.cwd(); @@ -49,7 +54,11 @@ function autoDiscoverTddServer() { try { let serverInfo = JSON.parse(readFileSync(serverJsonPath, 'utf8')); if (serverInfo.port) { - return `http://localhost:${serverInfo.port}`; + cachedServerInfo = { + url: `http://localhost:${serverInfo.port}`, + failOnDiff: serverInfo.failOnDiff || false, + }; + return cachedServerInfo; } } catch { // Invalid JSON, continue searching @@ -62,6 +71,26 @@ function autoDiscoverTddServer() { return null; } +/** + * Get the cached server info (for reading failOnDiff setting) + * @returns {{url: string, failOnDiff: boolean}|null} + */ +export function getServerInfo() { + // If not cached yet, discover it now + if (!cachedServerInfo) { + autoDiscoverTddServer(); + } + return cachedServerInfo; +} + +/** + * Clear the cached server info (for testing purposes) + * @private + */ +export function clearServerInfoCache() { + cachedServerInfo = null; +} + /** * Forward screenshot to Vizzly TDD server * @param {string} name - Screenshot name @@ -70,9 +99,9 @@ function autoDiscoverTddServer() { * @returns {Promise} Response from TDD server */ async function forwardToVizzly(name, imageBuffer, properties = {}) { - let tddServerUrl = autoDiscoverTddServer(); + let serverInfo = autoDiscoverTddServer(); - if (!tddServerUrl) { + if (!serverInfo) { // Check for cloud mode via environment if (process.env.VIZZLY_TOKEN) { // In cloud mode, we'd queue for upload @@ -85,6 +114,8 @@ async function forwardToVizzly(name, imageBuffer, properties = {}) { ); } + let tddServerUrl = serverInfo.url; + let payload = { name, image: imageBuffer.toString('base64'), diff --git a/clients/ember/src/test-support/index.js b/clients/ember/src/test-support/index.js index 556bd8b3..e7df2001 100644 --- a/clients/ember/src/test-support/index.js +++ b/clients/ember/src/test-support/index.js @@ -134,6 +134,16 @@ function prepareTestingContainer(width = 1280, height = 720, fullPage = false) { }; } +/** + * Check if tests should fail on visual diffs + * Reads from VIZZLY_FAIL_ON_DIFF environment variable (injected by launcher) + * @returns {boolean} + */ +function shouldFailOnDiff() { + return window.__VIZZLY_FAIL_ON_DIFF__ === true || + window.__VIZZLY_FAIL_ON_DIFF__ === 'true'; +} + /** * Capture a visual snapshot * @@ -154,6 +164,7 @@ function prepareTestingContainer(width = 1280, height = 720, fullPage = false) { * @param {number} [options.height=720] - Viewport height for the snapshot * @param {string} [options.scope='app'] - What to capture: 'app' (default), 'container', or 'page' * @param {Object} [options.properties] - Additional metadata for the snapshot + * @param {boolean} [options.failOnDiff] - Fail the test if visual diff is detected (overrides env var) * @returns {Promise} Snapshot result from Vizzly server * * @example @@ -169,8 +180,8 @@ function prepareTestingContainer(width = 1280, height = 720, fullPage = false) { * await vizzlySnapshot('login-form', { selector: '[data-test-login-form]' }); * * @example - * // Capture the entire page including QUnit UI (rare use case) - * await vizzlySnapshot('test-runner', { scope: 'page' }); + * // Fail test if this specific snapshot has a diff + * await vizzlySnapshot('critical-ui', { failOnDiff: true }); */ export async function vizzlySnapshot(name, options = {}) { let { @@ -180,6 +191,7 @@ export async function vizzlySnapshot(name, options = {}) { height = 720, scope = 'app', properties = {}, + failOnDiff = null, // null means use env var, true/false overrides } = options; // Get snapshot URL injected by the launcher @@ -252,13 +264,31 @@ export async function vizzlySnapshot(name, options = {}) { // This allows tests to pass when Vizzly isn't running (like Percy behavior) if (errorText.includes('No Vizzly server found')) { console.warn('[vizzly] Vizzly server not running. Skipping visual snapshot.'); - return { skipped: true, reason: 'no-server' }; + return { status: 'skipped', reason: 'no-server' }; } throw new Error(`Vizzly snapshot failed: ${errorText}`); } - return await response.json(); + let result = await response.json(); + + // Handle visual diff - server returns 200 with status: 'diff' + if (result.status === 'diff') { + // Determine if we should fail based on option or env var + let shouldFail = failOnDiff !== null ? failOnDiff : shouldFailOnDiff(); + + if (shouldFail) { + throw new Error( + `Visual difference detected for '${name}' (${result.diffPercentage?.toFixed(2)}% diff). ` + + `View diff in Vizzly dashboard.` + ); + } + + // Log warning but don't fail + console.warn(`[vizzly] Visual difference detected for '${name}'. View diff in Vizzly dashboard.`); + } + + return result; } finally { // Always restore original styles cleanup(); diff --git a/clients/ember/tests/unit/snapshot-server.test.js b/clients/ember/tests/unit/snapshot-server.test.js index 446a2dd7..59bdbc38 100644 --- a/clients/ember/tests/unit/snapshot-server.test.js +++ b/clients/ember/tests/unit/snapshot-server.test.js @@ -1,7 +1,11 @@ import assert from 'node:assert'; +import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; import { afterEach, beforeEach, describe, it } from 'node:test'; import { + clearServerInfoCache, getPage, + getServerInfo, setPage, startSnapshotServer, stopSnapshotServer, @@ -188,4 +192,106 @@ describe('snapshot-server', () => { assert.strictEqual(response.status, 404); }); }); + + describe('getServerInfo()', () => { + let testDir = join(process.cwd(), '.vizzly-test-temp'); + + beforeEach(() => { + // Clear the cached server info before each test + clearServerInfoCache(); + + // Clean up any existing test directory + if (existsSync(testDir)) { + rmSync(testDir, { recursive: true }); + } + }); + + afterEach(() => { + // Clear cache after tests + clearServerInfoCache(); + + // Clean up test directory + if (existsSync(testDir)) { + rmSync(testDir, { recursive: true }); + } + }); + + it('returns null when no server.json exists in isolated directory', () => { + // Create an isolated test directory without server.json + mkdirSync(testDir, { recursive: true }); + + let originalCwd = process.cwd(); + try { + process.chdir(testDir); + clearServerInfoCache(); + + let info = getServerInfo(); + + // In an isolated directory without .vizzly/server.json, should return null + // (unless there's a server.json in a parent directory) + if (info !== null) { + assert.ok(typeof info.url === 'string', 'should have url'); + assert.ok(typeof info.failOnDiff === 'boolean', 'should have failOnDiff'); + } + } finally { + process.chdir(originalCwd); + } + }); + + it('reads failOnDiff from server.json when present', () => { + // Create a temporary .vizzly directory with server.json + let vizzlyDir = join(testDir, '.vizzly'); + mkdirSync(vizzlyDir, { recursive: true }); + + let serverJson = { + pid: 12345, + port: 47392, + startTime: Date.now(), + failOnDiff: true, + }; + writeFileSync(join(vizzlyDir, 'server.json'), JSON.stringify(serverJson)); + + // Change to test directory to test discovery + let originalCwd = process.cwd(); + try { + process.chdir(testDir); + clearServerInfoCache(); + + let info = getServerInfo(); + + assert.ok(info !== null, 'should find server.json'); + assert.strictEqual(info.url, 'http://localhost:47392', 'should have correct url'); + assert.strictEqual(info.failOnDiff, true, 'should read failOnDiff as true'); + } finally { + process.chdir(originalCwd); + } + }); + + it('defaults failOnDiff to false when not specified in server.json', () => { + let vizzlyDir = join(testDir, '.vizzly'); + mkdirSync(vizzlyDir, { recursive: true }); + + let serverJson = { + pid: 12345, + port: 47393, + startTime: Date.now(), + // failOnDiff not specified + }; + writeFileSync(join(vizzlyDir, 'server.json'), JSON.stringify(serverJson)); + + let originalCwd = process.cwd(); + try { + process.chdir(testDir); + clearServerInfoCache(); + + let info = getServerInfo(); + + assert.ok(info !== null, 'should find server.json'); + assert.strictEqual(info.url, 'http://localhost:47393', 'should have correct url'); + assert.strictEqual(info.failOnDiff, false, 'should default failOnDiff to false'); + } finally { + process.chdir(originalCwd); + } + }); + }); }); diff --git a/src/cli.js b/src/cli.js index 991ac65a..9e9aa574 100644 --- a/src/cli.js +++ b/src/cli.js @@ -368,6 +368,7 @@ tddCmd .option('--environment ', 'Environment name', 'test') .option('--threshold ', 'Comparison threshold', parseFloat) .option('--timeout ', 'Server timeout in milliseconds', '30000') + .option('--fail-on-diff', 'Fail tests when visual differences are detected') .option('--token ', 'API token override') .option('--daemon-child', 'Internal: run as daemon child process') .action(async options => { @@ -416,6 +417,7 @@ tddCmd '--set-baseline', 'Accept current screenshots as new baseline (overwrites existing)' ) + .option('--fail-on-diff', 'Fail tests when visual differences are detected') .option('--no-open', 'Skip opening report in browser') .action(async (command, options) => { const globalOptions = program.opts(); diff --git a/src/commands/tdd-daemon.js b/src/commands/tdd-daemon.js index f757fd3c..931570d4 100644 --- a/src/commands/tdd-daemon.js +++ b/src/commands/tdd-daemon.js @@ -86,6 +86,7 @@ export async function tddStartCommand(options = {}, globalOptions = {}) { ? ['--threshold', options.threshold.toString()] : []), ...(options.timeout ? ['--timeout', options.timeout] : []), + ...(options.failOnDiff ? ['--fail-on-diff'] : []), ...(options.token ? ['--token', options.token] : []), ...(globalOptions.json ? ['--json'] : []), ...(globalOptions.verbose ? ['--verbose'] : []), @@ -250,6 +251,7 @@ export async function runDaemonChild(options = {}, globalOptions = {}) { pid: process.pid, port: port, startTime: Date.now(), + failOnDiff: options.failOnDiff || false, }; writeFileSync( join(vizzlyDir, 'server.json'), diff --git a/src/server/handlers/tdd-handler.js b/src/server/handlers/tdd-handler.js index 8c481727..0d38be2f 100644 --- a/src/server/handlers/tdd-handler.js +++ b/src/server/handlers/tdd-handler.js @@ -470,21 +470,20 @@ export const createTddHandler = ( // Update comparison in report data file updateComparison(newComparison); + // Visual diffs return 200 with status: 'diff' - they're not errors + // The SDK/user can decide whether to fail tests based on this if (comparison.status === 'failed') { return { - statusCode: 422, + statusCode: 200, body: { - error: 'Visual difference detected', - details: `Screenshot '${name}' differs from baseline`, - comparison: { - name: comparison.name, - status: comparison.status, - baseline: comparison.baseline, - current: comparison.current, - diff: comparison.diff, - diffPercentage: comparison.diffPercentage, - threshold: comparison.threshold, - }, + status: 'diff', + name: comparison.name, + message: `Visual difference detected for '${name}'`, + baseline: comparison.baseline, + current: comparison.current, + diff: comparison.diff, + diffPercentage: comparison.diffPercentage, + threshold: comparison.threshold, tddMode: true, }, }; @@ -494,14 +493,11 @@ export const createTddHandler = ( return { statusCode: 200, body: { - status: 'success', - message: `Baseline updated for ${name}`, - comparison: { - name: comparison.name, - status: comparison.status, - baseline: comparison.baseline, - current: comparison.current, - }, + status: 'baseline-updated', + name: comparison.name, + message: `Baseline updated for '${name}'`, + baseline: comparison.baseline, + current: comparison.current, tddMode: true, }, }; @@ -517,15 +513,14 @@ export const createTddHandler = ( }; } - // Debug output handled by tdd.js event handler + // Match or new baseline return { statusCode: 200, body: { - success: true, - comparison: { - name: comparison.name, - status: comparison.status, - }, + status: comparison.status === 'new' ? 'new' : 'match', + name: comparison.name, + baseline: comparison.baseline, + current: comparison.current, tddMode: true, }, }; diff --git a/tests/server/handlers/tdd-handler.test.js b/tests/server/handlers/tdd-handler.test.js index 2c153340..3898c39a 100644 --- a/tests/server/handlers/tdd-handler.test.js +++ b/tests/server/handlers/tdd-handler.test.js @@ -525,7 +525,7 @@ describe('server/handlers/tdd-handler', () => { }); describe('handleScreenshot', () => { - it('returns passed comparison result', async () => { + it('returns 200 with status match for passed comparison', async () => { let deps = createMockDeps({ tddServiceOverrides: { compareScreenshot: name => ({ @@ -548,11 +548,39 @@ describe('server/handlers/tdd-handler', () => { ); assert.strictEqual(result.statusCode, 200); - assert.strictEqual(result.body.success, true); - assert.strictEqual(result.body.comparison.status, 'passed'); + assert.strictEqual(result.body.status, 'match'); + assert.strictEqual(result.body.name, 'homepage'); + assert.strictEqual(result.body.tddMode, true); }); - it('returns 422 for failed comparison', async () => { + it('returns 200 with status new for new baseline', async () => { + let deps = createMockDeps({ + tddServiceOverrides: { + compareScreenshot: name => ({ + id: `comp-${name}`, + name, + status: 'new', + baseline: '/baselines/test.png', + current: '/current/test.png', + diff: null, + }), + }, + }); + let handler = createTddHandler({}, '/test', null, null, false, deps); + + let result = await handler.handleScreenshot( + 'build-1', + 'new-screenshot', + 'base64imagedata', + {} + ); + + assert.strictEqual(result.statusCode, 200); + assert.strictEqual(result.body.status, 'new'); + assert.strictEqual(result.body.name, 'new-screenshot'); + }); + + it('returns 200 with status diff for failed comparison', async () => { let deps = createMockDeps({ tddServiceOverrides: { compareScreenshot: name => ({ @@ -576,8 +604,51 @@ describe('server/handlers/tdd-handler', () => { {} ); - assert.strictEqual(result.statusCode, 422); - assert.ok(result.body.error.includes('Visual difference detected')); + // Visual diffs return 200 with status: 'diff' (not 422) + // This allows SDKs to decide whether to fail tests + assert.strictEqual(result.statusCode, 200); + assert.strictEqual(result.body.status, 'diff'); + assert.strictEqual(result.body.name, 'homepage'); + assert.ok(result.body.message.includes('Visual difference detected')); + assert.strictEqual(result.body.diffPercentage, 5.5); + assert.strictEqual(result.body.threshold, 2.0); + assert.ok(result.body.baseline); + assert.ok(result.body.current); + assert.ok(result.body.diff); + }); + + it('returns diff response with all required fields for SDK consumption', async () => { + let deps = createMockDeps({ + tddServiceOverrides: { + compareScreenshot: name => ({ + id: `comp-${name}`, + name, + status: 'failed', + baseline: '/test/.vizzly/baselines/homepage.png', + current: '/test/.vizzly/current/homepage.png', + diff: '/test/.vizzly/diffs/homepage.png', + diffPercentage: 12.34, + threshold: 5.0, + }), + }, + }); + let handler = createTddHandler({}, '/test', null, null, false, deps); + + let result = await handler.handleScreenshot( + 'build-1', + 'homepage', + 'base64imagedata', + {} + ); + + // Verify the response has all fields SDKs need + assert.strictEqual(result.statusCode, 200); + assert.strictEqual(typeof result.body.status, 'string'); + assert.strictEqual(typeof result.body.name, 'string'); + assert.strictEqual(typeof result.body.message, 'string'); + assert.strictEqual(typeof result.body.diffPercentage, 'number'); + assert.strictEqual(typeof result.body.threshold, 'number'); + assert.strictEqual(result.body.tddMode, true); }); it('returns 400 for invalid screenshot name', async () => {