Skip to content

Commit f92aec0

Browse files
authored
✨ Add --fail-on-diff option and improve SDK resilience (#169)
## Summary - Visual diffs no longer break tests by default - SDKs can now choose whether to fail on diffs - Server returns 200 for diffs with `status: 'diff'` instead of 422 error - Added `--fail-on-diff` CLI flag for `vizzly tdd start` and `vizzly tdd run` ## Changes **Server:** - Visual diffs return `200` with `status: 'diff'` instead of `422` - Consistent response format with `status` field: `'match'`, `'diff'`, `'new'`, or `'baseline-updated'` **CLI:** - `--fail-on-diff` flag on `vizzly tdd start` and `vizzly tdd run` - `failOnDiff` stored in `server.json` for SDK discovery **Ember SDK:** - Read `failOnDiff` from server.json, env var, or per-snapshot option - Gracefully skip snapshots when no server running (like Percy) - `failOnDiff` option on `vizzlySnapshot()` for per-snapshot control ## Usage ```bash # Default: diffs logged as warnings, tests pass vizzly tdd start # Fail tests on visual diffs vizzly tdd start --fail-on-diff # Or via environment variable VIZZLY_FAIL_ON_DIFF=true npm test # Or per-snapshot await vizzlySnapshot('critical-ui', { failOnDiff: true }); ``` ## Test plan - [x] Updated `tdd-handler.test.js` for new 200 response format - [x] Added tests for diff response structure - [x] Added tests for `getServerInfo()` reading `failOnDiff` - [x] All CLI tests pass - [x] All Ember SDK tests pass
1 parent ca3c2a0 commit f92aec0

File tree

9 files changed

+304
-48
lines changed

9 files changed

+304
-48
lines changed

clients/ember/bin/vizzly-browser.js

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import { closeBrowser, launchBrowser } from '../src/launcher/browser.js';
1818
import {
19+
getServerInfo,
1920
setPage,
2021
startSnapshotServer,
2122
stopSnapshotServer,
@@ -66,15 +67,28 @@ async function cleanup() {
6667
*/
6768
async function main() {
6869
try {
69-
// 1. Start snapshot server first
70+
// 1. Start snapshot server first (this also discovers the TDD server and caches its config)
7071
snapshotServer = await startSnapshotServer();
7172
let snapshotUrl = `http://127.0.0.1:${snapshotServer.port}`;
7273

73-
// 2. Launch browser with Playwright
74+
// 2. Determine failOnDiff: env var > server.json > default (false)
75+
// getServerInfo() returns cached info from the TDD server discovery that happened above
76+
let failOnDiff = false;
77+
if (process.env.VIZZLY_FAIL_ON_DIFF === 'true' || process.env.VIZZLY_FAIL_ON_DIFF === '1') {
78+
failOnDiff = true;
79+
} else {
80+
let serverInfo = getServerInfo();
81+
if (serverInfo?.failOnDiff) {
82+
failOnDiff = true;
83+
}
84+
}
85+
86+
// 3. Launch browser with Playwright
7487
// Note: We set the page reference in launchBrowser before navigation
7588
// to avoid a race condition where tests run before page is set
7689
browserInstance = await launchBrowser(browserType, testUrl, {
7790
snapshotUrl,
91+
failOnDiff,
7892
onPageCreated: page => {
7993
// Set page reference immediately when page is created
8094
// This happens BEFORE navigation so tests can capture screenshots

clients/ember/src/launcher/browser.js

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,12 @@ function getChromiumArgs() {
6161
* @param {Object} options - Launch options
6262
* @param {string} options.snapshotUrl - URL of the snapshot HTTP server
6363
* @param {boolean} [options.headless] - Run in headless mode (default: true in CI)
64+
* @param {boolean} [options.failOnDiff] - Whether tests should fail on visual diffs
6465
* @param {Function} [options.onPageCreated] - Callback when page is created (before navigation)
6566
* @returns {Promise<Object>} Browser instance with page reference
6667
*/
6768
export async function launchBrowser(browserType, testUrl, options = {}) {
68-
let { snapshotUrl, headless, onPageCreated } = options;
69+
let { snapshotUrl, headless, failOnDiff, onPageCreated } = options;
6970

7071
// Default headless based on CI environment
7172
if (headless === undefined) {
@@ -93,11 +94,15 @@ export async function launchBrowser(browserType, testUrl, options = {}) {
9394
let context = await browser.newContext();
9495
let page = await context.newPage();
9596

96-
// Inject snapshot URL into page context BEFORE navigation
97-
// This ensures window.__VIZZLY_SNAPSHOT_URL__ is available when tests run
98-
await page.addInitScript(url => {
99-
window.__VIZZLY_SNAPSHOT_URL__ = url;
100-
}, snapshotUrl);
97+
// Inject Vizzly config into page context BEFORE navigation
98+
// This ensures window.__VIZZLY_* is available when tests run
99+
await page.addInitScript(
100+
({ snapshotUrl, failOnDiff }) => {
101+
window.__VIZZLY_SNAPSHOT_URL__ = snapshotUrl;
102+
window.__VIZZLY_FAIL_ON_DIFF__ = failOnDiff;
103+
},
104+
{ snapshotUrl, failOnDiff }
105+
);
101106

102107
// Call onPageCreated callback BEFORE navigation
103108
// This allows setting up the page reference before tests can run

clients/ember/src/launcher/snapshot-server.js

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,14 @@ export function getPage() {
3434
return pageRef;
3535
}
3636

37+
/**
38+
* Cached server info from auto-discovery
39+
*/
40+
let cachedServerInfo = null;
41+
3742
/**
3843
* Auto-discover the Vizzly TDD server by searching for .vizzly/server.json
39-
* @returns {string|null} Server URL or null if not found
44+
* @returns {{url: string, failOnDiff: boolean}|null} Server info or null if not found
4045
*/
4146
function autoDiscoverTddServer() {
4247
let currentDir = process.cwd();
@@ -49,7 +54,11 @@ function autoDiscoverTddServer() {
4954
try {
5055
let serverInfo = JSON.parse(readFileSync(serverJsonPath, 'utf8'));
5156
if (serverInfo.port) {
52-
return `http://localhost:${serverInfo.port}`;
57+
cachedServerInfo = {
58+
url: `http://localhost:${serverInfo.port}`,
59+
failOnDiff: serverInfo.failOnDiff || false,
60+
};
61+
return cachedServerInfo;
5362
}
5463
} catch {
5564
// Invalid JSON, continue searching
@@ -62,6 +71,26 @@ function autoDiscoverTddServer() {
6271
return null;
6372
}
6473

74+
/**
75+
* Get the cached server info (for reading failOnDiff setting)
76+
* @returns {{url: string, failOnDiff: boolean}|null}
77+
*/
78+
export function getServerInfo() {
79+
// If not cached yet, discover it now
80+
if (!cachedServerInfo) {
81+
autoDiscoverTddServer();
82+
}
83+
return cachedServerInfo;
84+
}
85+
86+
/**
87+
* Clear the cached server info (for testing purposes)
88+
* @private
89+
*/
90+
export function clearServerInfoCache() {
91+
cachedServerInfo = null;
92+
}
93+
6594
/**
6695
* Forward screenshot to Vizzly TDD server
6796
* @param {string} name - Screenshot name
@@ -70,9 +99,9 @@ function autoDiscoverTddServer() {
7099
* @returns {Promise<Object>} Response from TDD server
71100
*/
72101
async function forwardToVizzly(name, imageBuffer, properties = {}) {
73-
let tddServerUrl = autoDiscoverTddServer();
102+
let serverInfo = autoDiscoverTddServer();
74103

75-
if (!tddServerUrl) {
104+
if (!serverInfo) {
76105
// Check for cloud mode via environment
77106
if (process.env.VIZZLY_TOKEN) {
78107
// In cloud mode, we'd queue for upload
@@ -85,6 +114,8 @@ async function forwardToVizzly(name, imageBuffer, properties = {}) {
85114
);
86115
}
87116

117+
let tddServerUrl = serverInfo.url;
118+
88119
let payload = {
89120
name,
90121
image: imageBuffer.toString('base64'),

clients/ember/src/test-support/index.js

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,16 @@ function prepareTestingContainer(width = 1280, height = 720, fullPage = false) {
134134
};
135135
}
136136

137+
/**
138+
* Check if tests should fail on visual diffs
139+
* Reads from VIZZLY_FAIL_ON_DIFF environment variable (injected by launcher)
140+
* @returns {boolean}
141+
*/
142+
function shouldFailOnDiff() {
143+
return window.__VIZZLY_FAIL_ON_DIFF__ === true ||
144+
window.__VIZZLY_FAIL_ON_DIFF__ === 'true';
145+
}
146+
137147
/**
138148
* Capture a visual snapshot
139149
*
@@ -154,6 +164,7 @@ function prepareTestingContainer(width = 1280, height = 720, fullPage = false) {
154164
* @param {number} [options.height=720] - Viewport height for the snapshot
155165
* @param {string} [options.scope='app'] - What to capture: 'app' (default), 'container', or 'page'
156166
* @param {Object} [options.properties] - Additional metadata for the snapshot
167+
* @param {boolean} [options.failOnDiff] - Fail the test if visual diff is detected (overrides env var)
157168
* @returns {Promise<Object>} Snapshot result from Vizzly server
158169
*
159170
* @example
@@ -169,8 +180,8 @@ function prepareTestingContainer(width = 1280, height = 720, fullPage = false) {
169180
* await vizzlySnapshot('login-form', { selector: '[data-test-login-form]' });
170181
*
171182
* @example
172-
* // Capture the entire page including QUnit UI (rare use case)
173-
* await vizzlySnapshot('test-runner', { scope: 'page' });
183+
* // Fail test if this specific snapshot has a diff
184+
* await vizzlySnapshot('critical-ui', { failOnDiff: true });
174185
*/
175186
export async function vizzlySnapshot(name, options = {}) {
176187
let {
@@ -180,6 +191,7 @@ export async function vizzlySnapshot(name, options = {}) {
180191
height = 720,
181192
scope = 'app',
182193
properties = {},
194+
failOnDiff = null, // null means use env var, true/false overrides
183195
} = options;
184196

185197
// Get snapshot URL injected by the launcher
@@ -252,13 +264,31 @@ export async function vizzlySnapshot(name, options = {}) {
252264
// This allows tests to pass when Vizzly isn't running (like Percy behavior)
253265
if (errorText.includes('No Vizzly server found')) {
254266
console.warn('[vizzly] Vizzly server not running. Skipping visual snapshot.');
255-
return { skipped: true, reason: 'no-server' };
267+
return { status: 'skipped', reason: 'no-server' };
256268
}
257269

258270
throw new Error(`Vizzly snapshot failed: ${errorText}`);
259271
}
260272

261-
return await response.json();
273+
let result = await response.json();
274+
275+
// Handle visual diff - server returns 200 with status: 'diff'
276+
if (result.status === 'diff') {
277+
// Determine if we should fail based on option or env var
278+
let shouldFail = failOnDiff !== null ? failOnDiff : shouldFailOnDiff();
279+
280+
if (shouldFail) {
281+
throw new Error(
282+
`Visual difference detected for '${name}' (${result.diffPercentage?.toFixed(2)}% diff). ` +
283+
`View diff in Vizzly dashboard.`
284+
);
285+
}
286+
287+
// Log warning but don't fail
288+
console.warn(`[vizzly] Visual difference detected for '${name}'. View diff in Vizzly dashboard.`);
289+
}
290+
291+
return result;
262292
} finally {
263293
// Always restore original styles
264294
cleanup();

clients/ember/tests/unit/snapshot-server.test.js

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import assert from 'node:assert';
2+
import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs';
3+
import { join } from 'node:path';
24
import { afterEach, beforeEach, describe, it } from 'node:test';
35
import {
6+
clearServerInfoCache,
47
getPage,
8+
getServerInfo,
59
setPage,
610
startSnapshotServer,
711
stopSnapshotServer,
@@ -188,4 +192,106 @@ describe('snapshot-server', () => {
188192
assert.strictEqual(response.status, 404);
189193
});
190194
});
195+
196+
describe('getServerInfo()', () => {
197+
let testDir = join(process.cwd(), '.vizzly-test-temp');
198+
199+
beforeEach(() => {
200+
// Clear the cached server info before each test
201+
clearServerInfoCache();
202+
203+
// Clean up any existing test directory
204+
if (existsSync(testDir)) {
205+
rmSync(testDir, { recursive: true });
206+
}
207+
});
208+
209+
afterEach(() => {
210+
// Clear cache after tests
211+
clearServerInfoCache();
212+
213+
// Clean up test directory
214+
if (existsSync(testDir)) {
215+
rmSync(testDir, { recursive: true });
216+
}
217+
});
218+
219+
it('returns null when no server.json exists in isolated directory', () => {
220+
// Create an isolated test directory without server.json
221+
mkdirSync(testDir, { recursive: true });
222+
223+
let originalCwd = process.cwd();
224+
try {
225+
process.chdir(testDir);
226+
clearServerInfoCache();
227+
228+
let info = getServerInfo();
229+
230+
// In an isolated directory without .vizzly/server.json, should return null
231+
// (unless there's a server.json in a parent directory)
232+
if (info !== null) {
233+
assert.ok(typeof info.url === 'string', 'should have url');
234+
assert.ok(typeof info.failOnDiff === 'boolean', 'should have failOnDiff');
235+
}
236+
} finally {
237+
process.chdir(originalCwd);
238+
}
239+
});
240+
241+
it('reads failOnDiff from server.json when present', () => {
242+
// Create a temporary .vizzly directory with server.json
243+
let vizzlyDir = join(testDir, '.vizzly');
244+
mkdirSync(vizzlyDir, { recursive: true });
245+
246+
let serverJson = {
247+
pid: 12345,
248+
port: 47392,
249+
startTime: Date.now(),
250+
failOnDiff: true,
251+
};
252+
writeFileSync(join(vizzlyDir, 'server.json'), JSON.stringify(serverJson));
253+
254+
// Change to test directory to test discovery
255+
let originalCwd = process.cwd();
256+
try {
257+
process.chdir(testDir);
258+
clearServerInfoCache();
259+
260+
let info = getServerInfo();
261+
262+
assert.ok(info !== null, 'should find server.json');
263+
assert.strictEqual(info.url, 'http://localhost:47392', 'should have correct url');
264+
assert.strictEqual(info.failOnDiff, true, 'should read failOnDiff as true');
265+
} finally {
266+
process.chdir(originalCwd);
267+
}
268+
});
269+
270+
it('defaults failOnDiff to false when not specified in server.json', () => {
271+
let vizzlyDir = join(testDir, '.vizzly');
272+
mkdirSync(vizzlyDir, { recursive: true });
273+
274+
let serverJson = {
275+
pid: 12345,
276+
port: 47393,
277+
startTime: Date.now(),
278+
// failOnDiff not specified
279+
};
280+
writeFileSync(join(vizzlyDir, 'server.json'), JSON.stringify(serverJson));
281+
282+
let originalCwd = process.cwd();
283+
try {
284+
process.chdir(testDir);
285+
clearServerInfoCache();
286+
287+
let info = getServerInfo();
288+
289+
assert.ok(info !== null, 'should find server.json');
290+
assert.strictEqual(info.url, 'http://localhost:47393', 'should have correct url');
291+
assert.strictEqual(info.failOnDiff, false, 'should default failOnDiff to false');
292+
} finally {
293+
process.chdir(originalCwd);
294+
}
295+
});
296+
});
191297
});

src/cli.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,7 @@ tddCmd
368368
.option('--environment <env>', 'Environment name', 'test')
369369
.option('--threshold <number>', 'Comparison threshold', parseFloat)
370370
.option('--timeout <ms>', 'Server timeout in milliseconds', '30000')
371+
.option('--fail-on-diff', 'Fail tests when visual differences are detected')
371372
.option('--token <token>', 'API token override')
372373
.option('--daemon-child', 'Internal: run as daemon child process')
373374
.action(async options => {
@@ -416,6 +417,7 @@ tddCmd
416417
'--set-baseline',
417418
'Accept current screenshots as new baseline (overwrites existing)'
418419
)
420+
.option('--fail-on-diff', 'Fail tests when visual differences are detected')
419421
.option('--no-open', 'Skip opening report in browser')
420422
.action(async (command, options) => {
421423
const globalOptions = program.opts();

src/commands/tdd-daemon.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ export async function tddStartCommand(options = {}, globalOptions = {}) {
8686
? ['--threshold', options.threshold.toString()]
8787
: []),
8888
...(options.timeout ? ['--timeout', options.timeout] : []),
89+
...(options.failOnDiff ? ['--fail-on-diff'] : []),
8990
...(options.token ? ['--token', options.token] : []),
9091
...(globalOptions.json ? ['--json'] : []),
9192
...(globalOptions.verbose ? ['--verbose'] : []),
@@ -250,6 +251,7 @@ export async function runDaemonChild(options = {}, globalOptions = {}) {
250251
pid: process.pid,
251252
port: port,
252253
startTime: Date.now(),
254+
failOnDiff: options.failOnDiff || false,
253255
};
254256
writeFileSync(
255257
join(vizzlyDir, 'server.json'),

0 commit comments

Comments
 (0)