diff --git a/clients/ember/bin/vizzly-testem-launcher.js b/clients/ember/bin/vizzly-testem-launcher.js index f091875..15c09e3 100755 --- a/clients/ember/bin/vizzly-testem-launcher.js +++ b/clients/ember/bin/vizzly-testem-launcher.js @@ -103,7 +103,10 @@ async function main() { // 2. Determine failOnDiff: env var > server.json > default (false) let failOnDiff = false; - if (process.env.VIZZLY_FAIL_ON_DIFF === 'true' || process.env.VIZZLY_FAIL_ON_DIFF === '1') { + if ( + process.env.VIZZLY_FAIL_ON_DIFF === 'true' || + process.env.VIZZLY_FAIL_ON_DIFF === '1' + ) { failOnDiff = true; } else { let serverInfo = getServerInfo(); diff --git a/clients/ember/src/launcher/screenshot-server.js b/clients/ember/src/launcher/screenshot-server.js index aba66d5..1f6a645 100644 --- a/clients/ember/src/launcher/screenshot-server.js +++ b/clients/ember/src/launcher/screenshot-server.js @@ -181,7 +181,9 @@ function httpPost(url, data) { if (res.statusCode >= 400) { reject( - new Error(`Vizzly server error: ${res.statusCode} - ${responseBody}`) + new Error( + `Vizzly server error: ${res.statusCode} - ${responseBody}` + ) ); return; } diff --git a/clients/ember/src/test-support/index.js b/clients/ember/src/test-support/index.js index 21e9b05..da00c4b 100644 --- a/clients/ember/src/test-support/index.js +++ b/clients/ember/src/test-support/index.js @@ -161,8 +161,10 @@ function prepareTestingContainer(width = 1280, height = 720, fullPage = false) { * @returns {boolean} */ function shouldFailOnDiff() { - return window.__VIZZLY_FAIL_ON_DIFF__ === true || - window.__VIZZLY_FAIL_ON_DIFF__ === 'true'; + return ( + window.__VIZZLY_FAIL_ON_DIFF__ === true || + window.__VIZZLY_FAIL_ON_DIFF__ === 'true' + ); } /** @@ -285,7 +287,9 @@ export async function vizzlyScreenshot(name, options = {}) { // Check if this is a "no server" error - gracefully skip instead of failing // 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 screenshot.'); + console.warn( + '[vizzly] Vizzly server not running. Skipping visual screenshot.' + ); return { status: 'skipped', reason: 'no-server' }; } @@ -302,12 +306,14 @@ export async function vizzlyScreenshot(name, options = {}) { if (shouldFail) { throw new VizzlyDiffError( `Visual difference detected for '${name}' (${result.diffPercentage?.toFixed(2)}% diff). ` + - `View diff in Vizzly dashboard.` + `View diff in Vizzly dashboard.` ); } // Log warning but don't fail - console.warn(`[vizzly] Visual difference detected for '${name}'. View diff in Vizzly dashboard.`); + console.warn( + `[vizzly] Visual difference detected for '${name}'. View diff in Vizzly dashboard.` + ); } return result; @@ -320,7 +326,9 @@ export async function vizzlyScreenshot(name, options = {}) { // Log connection errors only once to avoid spam if (!hasLoggedConnectionError) { hasLoggedConnectionError = true; - console.warn(`[vizzly] Screenshots skipped - server not available. Run 'vizzly tdd start' to enable.`); + console.warn( + `[vizzly] Screenshots skipped - server not available. Run 'vizzly tdd start' to enable.` + ); } return { status: 'skipped', reason: 'error', error: error.message }; } finally { diff --git a/clients/ember/tests/unit/screenshot-server.test.js b/clients/ember/tests/unit/screenshot-server.test.js index 6123034..b8fdc68 100644 --- a/clients/ember/tests/unit/screenshot-server.test.js +++ b/clients/ember/tests/unit/screenshot-server.test.js @@ -231,7 +231,10 @@ describe('screenshot-server', () => { // (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'); + assert.ok( + typeof info.failOnDiff === 'boolean', + 'should have failOnDiff' + ); } } finally { process.chdir(originalCwd); @@ -260,8 +263,16 @@ describe('screenshot-server', () => { 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'); + 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); } @@ -287,8 +298,16 @@ describe('screenshot-server', () => { 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'); + 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/clients/ember/tests/unit/testem-config.test.js b/clients/ember/tests/unit/testem-config.test.js index 93ac4be..bb14bc8 100644 --- a/clients/ember/tests/unit/testem-config.test.js +++ b/clients/ember/tests/unit/testem-config.test.js @@ -155,7 +155,10 @@ describe('testem-config', () => { configure({}, {}); - assert.ok(!existsSync(configPath), 'should not write empty playwright.json'); + assert.ok( + !existsSync(configPath), + 'should not write empty playwright.json' + ); }); it('handles lowercase browser names', () => { diff --git a/clients/static-site/README.md b/clients/static-site/README.md index be9e0f5..b4de1a4 100644 --- a/clients/static-site/README.md +++ b/clients/static-site/README.md @@ -40,7 +40,7 @@ await run('./dist', { viewports: 'mobile:375x667,desktop:1920x1080', concurrency: 3, }, { - logger: console, + output: console, config: vizzlyConfig, services: serviceContainer, }); diff --git a/clients/static-site/package-lock.json b/clients/static-site/package-lock.json index f5e1489..b21c220 100644 --- a/clients/static-site/package-lock.json +++ b/clients/static-site/package-lock.json @@ -1,16 +1,16 @@ { "name": "@vizzly-testing/static-site", - "version": "0.0.11", + "version": "0.2.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@vizzly-testing/static-site", - "version": "0.0.11", + "version": "0.2.0", "license": "MIT", "dependencies": { "cosmiconfig": "^9.0.0", - "fast-xml-parser": "^4.5.0", + "fast-xml-parser": "^5.8.0", "playwright-core": "^1.50.0", "serve-handler": "^6.1.5", "zod": "^4.1.12" @@ -26,7 +26,7 @@ "node": ">=22.0.0" }, "peerDependencies": { - "@vizzly-testing/cli": ">=0.9.0" + "@vizzly-testing/cli": ">=0.24.0" } }, "node_modules/@babel/cli": { @@ -58,10 +58,12 @@ } }, "node_modules/@babel/code-frame": { - "version": "7.27.1", + "version": "7.29.0", + "resolved": "https://registry.npmjs.org/@babel/code-frame/-/code-frame-7.29.0.tgz", + "integrity": "sha512-9NhCeYjq9+3uxgdtp20LSiJXJvN0FeCtNGpJxuMFZ1Kv3cWUNb6DOhJwUvcVCzKGR66cw4njwM6hrJLqgOwbcw==", "license": "MIT", "dependencies": { - "@babel/helper-validator-identifier": "^7.27.1", + "@babel/helper-validator-identifier": "^7.28.5", "js-tokens": "^4.0.0", "picocolors": "^1.1.1" }, @@ -108,12 +110,14 @@ } }, "node_modules/@babel/generator": { - "version": "7.28.3", + "version": "7.29.1", + "resolved": "https://registry.npmjs.org/@babel/generator/-/generator-7.29.1.tgz", + "integrity": "sha512-qsaF+9Qcm2Qv8SRIMMscAvG4O3lJ0F1GuMo5HR/Bp02LopNgnZBC/EkbevHFeGs4ls/oPz9v+Bsmzbkbe+0dUw==", "dev": true, "license": "MIT", "dependencies": { - "@babel/parser": "^7.28.3", - "@babel/types": "^7.28.2", + "@babel/parser": "^7.29.0", + "@babel/types": "^7.29.0", "@jridgewell/gen-mapping": "^0.3.12", "@jridgewell/trace-mapping": "^0.3.28", "jsesc": "^3.0.2" @@ -220,25 +224,29 @@ } }, "node_modules/@babel/helper-module-imports": { - "version": "7.27.1", + "version": "7.28.6", + "resolved": "https://registry.npmjs.org/@babel/helper-module-imports/-/helper-module-imports-7.28.6.tgz", + "integrity": "sha512-l5XkZK7r7wa9LucGw9LwZyyCUscb4x37JWTPz7swwFE/0FMQAGpiWUZn8u9DzkSBWEcK25jmvubfpw2dnAMdbw==", "dev": true, "license": "MIT", "dependencies": { - "@babel/traverse": "^7.27.1", - "@babel/types": "^7.27.1" + "@babel/traverse": "^7.28.6", + "@babel/types": "^7.28.6" }, "engines": { "node": ">=6.9.0" } }, "node_modules/@babel/helper-module-transforms": { - "version": "7.28.3", + "version": "7.28.6", + "resolved": "https://registry.npmjs.org/@babel/helper-module-transforms/-/helper-module-transforms-7.28.6.tgz", + "integrity": "sha512-67oXFAYr2cDLDVGLXTEABjdBJZ6drElUSI7WKp70NrpyISso3plG9SAGEF6y7zbha/wOzUByWWTJvEDVNIUGcA==", "dev": true, "license": "MIT", "dependencies": { - "@babel/helper-module-imports": "^7.27.1", - "@babel/helper-validator-identifier": "^7.27.1", - "@babel/traverse": "^7.28.3" + "@babel/helper-module-imports": "^7.28.6", + "@babel/helper-validator-identifier": "^7.28.5", + "@babel/traverse": "^7.28.6" }, "engines": { "node": ">=6.9.0" @@ -259,7 +267,9 @@ } }, "node_modules/@babel/helper-plugin-utils": { - "version": "7.27.1", + "version": "7.28.6", + "resolved": "https://registry.npmjs.org/@babel/helper-plugin-utils/-/helper-plugin-utils-7.28.6.tgz", + "integrity": "sha512-S9gzZ/bz83GRysI7gAD4wPT/AI3uCnY+9xn+Mx/KPs2JwHJIz1W8PZkg2cqyt3RNOBM8ejcXhV6y8Og7ly/Dug==", "dev": true, "license": "MIT", "engines": { @@ -319,7 +329,9 @@ } }, "node_modules/@babel/helper-validator-identifier": { - "version": "7.27.1", + "version": "7.28.5", + "resolved": "https://registry.npmjs.org/@babel/helper-validator-identifier/-/helper-validator-identifier-7.28.5.tgz", + "integrity": "sha512-qSs4ifwzKJSV39ucNjsvc6WVHs6b7S03sOh2OcHF9UHfVPqWWALUsNUVzhSBiItjRZoLHx7nIarVjqKVusUZ1Q==", "license": "MIT", "engines": { "node": ">=6.9.0" @@ -359,11 +371,13 @@ } }, "node_modules/@babel/parser": { - "version": "7.28.4", + "version": "7.29.3", + "resolved": "https://registry.npmjs.org/@babel/parser/-/parser-7.29.3.tgz", + "integrity": "sha512-b3ctpQwp+PROvU/cttc4OYl4MzfJUWy6FZg+PMXfzmt/+39iHVF0sDfqay8TQM3JA2EUOyKcFZt75jWriQijsA==", "dev": true, "license": "MIT", "dependencies": { - "@babel/types": "^7.28.4" + "@babel/types": "^7.29.0" }, "bin": { "parser": "bin/babel-parser.js" @@ -872,14 +886,16 @@ } }, "node_modules/@babel/plugin-transform-modules-systemjs": { - "version": "7.27.1", + "version": "7.29.4", + "resolved": "https://registry.npmjs.org/@babel/plugin-transform-modules-systemjs/-/plugin-transform-modules-systemjs-7.29.4.tgz", + "integrity": "sha512-N7QmZ0xRZfjHOfZeQLJjwgX2zS9pdGHSVl/cjSGlo4dXMqvurfxXDMKY4RqEKzPozV78VMcd0lxyG13mlbKc4w==", "dev": true, "license": "MIT", "dependencies": { - "@babel/helper-module-transforms": "^7.27.1", - "@babel/helper-plugin-utils": "^7.27.1", - "@babel/helper-validator-identifier": "^7.27.1", - "@babel/traverse": "^7.27.1" + "@babel/helper-module-transforms": "^7.28.6", + "@babel/helper-plugin-utils": "^7.28.6", + "@babel/helper-validator-identifier": "^7.28.5", + "@babel/traverse": "^7.29.0" }, "engines": { "node": ">=6.9.0" @@ -1351,29 +1367,33 @@ } }, "node_modules/@babel/template": { - "version": "7.27.2", + "version": "7.28.6", + "resolved": "https://registry.npmjs.org/@babel/template/-/template-7.28.6.tgz", + "integrity": "sha512-YA6Ma2KsCdGb+WC6UpBVFJGXL58MDA6oyONbjyF/+5sBgxY/dwkhLogbMT2GXXyU84/IhRw/2D1Os1B/giz+BQ==", "dev": true, "license": "MIT", "dependencies": { - "@babel/code-frame": "^7.27.1", - "@babel/parser": "^7.27.2", - "@babel/types": "^7.27.1" + "@babel/code-frame": "^7.28.6", + "@babel/parser": "^7.28.6", + "@babel/types": "^7.28.6" }, "engines": { "node": ">=6.9.0" } }, "node_modules/@babel/traverse": { - "version": "7.28.4", + "version": "7.29.0", + "resolved": "https://registry.npmjs.org/@babel/traverse/-/traverse-7.29.0.tgz", + "integrity": "sha512-4HPiQr0X7+waHfyXPZpWPfWL/J7dcN1mx9gL6WdQVMbPnF3+ZhSMs8tCxN7oHddJE9fhNE7+lxdnlyemKfJRuA==", "dev": true, "license": "MIT", "dependencies": { - "@babel/code-frame": "^7.27.1", - "@babel/generator": "^7.28.3", + "@babel/code-frame": "^7.29.0", + "@babel/generator": "^7.29.0", "@babel/helper-globals": "^7.28.0", - "@babel/parser": "^7.28.4", - "@babel/template": "^7.27.2", - "@babel/types": "^7.28.4", + "@babel/parser": "^7.29.0", + "@babel/template": "^7.28.6", + "@babel/types": "^7.29.0", "debug": "^4.3.1" }, "engines": { @@ -1381,12 +1401,14 @@ } }, "node_modules/@babel/types": { - "version": "7.28.4", + "version": "7.29.0", + "resolved": "https://registry.npmjs.org/@babel/types/-/types-7.29.0.tgz", + "integrity": "sha512-LwdZHpScM4Qz8Xw2iKSzS+cfglZzJGvofQICy7W7v4caru4EaAmyUuO6BGrbyQ2mYV11W0U8j5mBhd14dd3B0A==", "dev": true, "license": "MIT", "dependencies": { "@babel/helper-string-parser": "^7.27.1", - "@babel/helper-validator-identifier": "^7.27.1" + "@babel/helper-validator-identifier": "^7.28.5" }, "engines": { "node": ">=6.9.0" @@ -1555,23 +1577,6 @@ "node": ">=14.21.3" } }, - "node_modules/@isaacs/balanced-match": { - "version": "4.0.1", - "license": "MIT", - "engines": { - "node": "20 || >=22" - } - }, - "node_modules/@isaacs/brace-expansion": { - "version": "5.0.0", - "license": "MIT", - "dependencies": { - "@isaacs/balanced-match": "^4.0.1" - }, - "engines": { - "node": "20 || >=22" - } - }, "node_modules/@isaacs/cliui": { "version": "8.0.2", "dev": true, @@ -1707,13 +1712,25 @@ "license": "MIT", "optional": true }, + "node_modules/@nodable/entities": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/@nodable/entities/-/entities-2.1.0.tgz", + "integrity": "sha512-nyT7T3nbMyBI/lvr6L5TyWbFJAI9FTgVRakNoBqCD+PmID8DzFrrNdLLtHMwMszOtqZa8PAOV24ZqDnQrhQINA==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/nodable" + } + ], + "license": "MIT" + }, "node_modules/@vizzly-testing/cli": { - "version": "0.22.0", - "resolved": "https://registry.npmjs.org/@vizzly-testing/cli/-/cli-0.22.0.tgz", - "integrity": "sha512-OJrJRFFGwgNehvRtrcEF62o5X9Z7YPhBK04OGGK4Py7Y1h+jw/LbSv3vq1+ZAxv/WoweUMp+VeryCWUXAlblrA==", + "version": "0.30.0", + "resolved": "https://registry.npmjs.org/@vizzly-testing/cli/-/cli-0.30.0.tgz", + "integrity": "sha512-vtF2ncb8NatRMBtY+0KKs8PFY7h5B86bymaWzyWWffIqzTcOC0ZCVI6ZMJ8qit6zx8Jh/EKdGHcieKJ5qrYa4A==", "license": "MIT", "dependencies": { - "@vizzly-testing/honeydiff": "^0.7.1", + "@vizzly-testing/honeydiff": "^0.10.0", "ansis": "^4.2.0", "commander": "^14.0.0", "cosmiconfig": "^9.0.0", @@ -1729,6 +1746,27 @@ "node": ">=22.0.0" } }, + "node_modules/@vizzly-testing/cli/node_modules/balanced-match": { + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", + "integrity": "sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==", + "license": "MIT", + "engines": { + "node": "18 || 20 || >=22" + } + }, + "node_modules/@vizzly-testing/cli/node_modules/brace-expansion": { + "version": "5.0.6", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.6.tgz", + "integrity": "sha512-kLpxurY4Z4r9sgMsyG0Z9uzsBlgiU/EFKhj/h91/8yHu0edo7XuixOIH3VcJ8kkxs6/jPzoI6U9Vj3WqbMQ94g==", + "license": "MIT", + "dependencies": { + "balanced-match": "^4.0.2" + }, + "engines": { + "node": "18 || 20 || >=22" + } + }, "node_modules/@vizzly-testing/cli/node_modules/commander": { "version": "14.0.1", "license": "MIT", @@ -1754,24 +1792,24 @@ } }, "node_modules/@vizzly-testing/cli/node_modules/minimatch": { - "version": "10.1.1", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.1.1.tgz", - "integrity": "sha512-enIvLvRAFZYXJzkCYG5RKmPfrFArdLv+R+lbQ53BmIMLIry74bjKzX6iHAm8WYamJkhSSEabrWN5D97XnKObjQ==", + "version": "10.2.5", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.5.tgz", + "integrity": "sha512-MULkVLfKGYDFYejP07QOurDLLQpcjk7Fw+7jXS2R2czRQzR56yHRveU5NDJEOviH+hETZKSkIk5c+T23GjFUMg==", "license": "BlueOak-1.0.0", "dependencies": { - "@isaacs/brace-expansion": "^5.0.0" + "brace-expansion": "^5.0.5" }, "engines": { - "node": "20 || >=22" + "node": "18 || 20 || >=22" }, "funding": { "url": "https://github.com/sponsors/isaacs" } }, "node_modules/@vizzly-testing/honeydiff": { - "version": "0.7.1", - "resolved": "https://registry.npmjs.org/@vizzly-testing/honeydiff/-/honeydiff-0.7.1.tgz", - "integrity": "sha512-GPVXCZr77JjdW//DPTFxn4mf2CEJ7gAvaJb4ZayHFjwFP6N0WyQvHFxcVWGPw1mhxjdv8uMHVEKtJrYQ4R3tTA==", + "version": "0.10.1", + "resolved": "https://registry.npmjs.org/@vizzly-testing/honeydiff/-/honeydiff-0.10.1.tgz", + "integrity": "sha512-XKvW4WkHx9UaDs1cQCR83RC5KR1xmnyune4C7Gifb51+KVhdmZzl/hbRNbzInrK0oQij294xwn+tv8riuNusYg==", "license": "MIT", "engines": { "node": ">=22.0.0" @@ -1890,7 +1928,9 @@ } }, "node_modules/brace-expansion": { - "version": "1.1.12", + "version": "1.1.14", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.14.tgz", + "integrity": "sha512-MWPGfDxnyzKU7rNOW9SP/c50vi3xrmrua/+6hfPbCS2ABNWfx24vPidzvC7krjU/RTo235sV776ymlsMtGKj8g==", "license": "MIT", "dependencies": { "balanced-match": "^1.0.0", @@ -2236,8 +2276,26 @@ "node": ">=0.10.0" } }, + "node_modules/fast-xml-builder": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/fast-xml-builder/-/fast-xml-builder-1.2.0.tgz", + "integrity": "sha512-00aAWieqff+ZJhsXA4g1g7M8k+7AYoMUUHF+/zFb5U6Uv/P0Vl4QZo84/IcufzYalLuEj9928bXN9PbbFzMF0Q==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/NaturalIntelligence" + } + ], + "license": "MIT", + "dependencies": { + "path-expression-matcher": "^1.5.0", + "xml-naming": "^0.1.0" + } + }, "node_modules/fast-xml-parser": { - "version": "4.5.3", + "version": "5.8.0", + "resolved": "https://registry.npmjs.org/fast-xml-parser/-/fast-xml-parser-5.8.0.tgz", + "integrity": "sha512-6bIM7fsJxeo3uXv7OncQYsBAMPJ7V16Slahl/6M98C/i2q+vB1+4a0MtrvYwDFEUrwDSbAmeLDRXsOBwrL7yAg==", "funding": [ { "type": "github", @@ -2246,7 +2304,11 @@ ], "license": "MIT", "dependencies": { - "strnum": "^1.1.1" + "@nodable/entities": "^2.1.0", + "fast-xml-builder": "^1.2.0", + "path-expression-matcher": "^1.5.0", + "strnum": "^2.3.0", + "xml-naming": "^0.1.0" }, "bin": { "fxparser": "src/cli/cli.js" @@ -2557,7 +2619,9 @@ "license": "MIT" }, "node_modules/js-yaml": { - "version": "4.1.0", + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.1.1.tgz", + "integrity": "sha512-qQKT4zQxXl8lLwBtHMWwaTcGfFOZviOJet3Oy/xmGk2gZH677CJM9EvtfdSkgWcATZhj/55JZ0rmy3myCT5lsA==", "license": "MIT", "dependencies": { "argparse": "^2.0.1" @@ -2654,7 +2718,9 @@ } }, "node_modules/minimatch": { - "version": "3.1.2", + "version": "3.1.5", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.5.tgz", + "integrity": "sha512-VgjWUsnnT6n+NUk6eZq77zeFdpW2LWDzP6zFGrCbHXiYNul5Dzqk2HHQ5uFH2DNW5Xbp8+jVzaeNt94ssEEl4w==", "license": "ISC", "dependencies": { "brace-expansion": "^1.1.7" @@ -2728,6 +2794,21 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/path-expression-matcher": { + "version": "1.5.0", + "resolved": "https://registry.npmjs.org/path-expression-matcher/-/path-expression-matcher-1.5.0.tgz", + "integrity": "sha512-cbrerZV+6rvdQrrD+iGMcZFEiiSrbv9Tfdkvnusy6y0x0GKBXREFg/Y65GhIfm0tnLntThhzCnfKwp1WRjeCyQ==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/NaturalIntelligence" + } + ], + "license": "MIT", + "engines": { + "node": ">=14.0.0" + } + }, "node_modules/path-is-absolute": { "version": "1.0.1", "dev": true, @@ -2783,7 +2864,9 @@ "license": "ISC" }, "node_modules/picomatch": { - "version": "2.3.1", + "version": "2.3.2", + "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-2.3.2.tgz", + "integrity": "sha512-V7+vQEJ06Z+c5tSye8S+nHUfI51xoXIXjHQ99cQtKUkQqqO1kO/KCJUfZXuB47h/YBlDhah2H3hdUGXn8ie0oA==", "dev": true, "license": "MIT", "optional": true, @@ -2925,14 +3008,40 @@ "url": "https://github.com/sponsors/isaacs" } }, + "node_modules/rimraf/node_modules/balanced-match": { + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", + "integrity": "sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==", + "dev": true, + "license": "MIT", + "engines": { + "node": "18 || 20 || >=22" + } + }, + "node_modules/rimraf/node_modules/brace-expansion": { + "version": "5.0.6", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.6.tgz", + "integrity": "sha512-kLpxurY4Z4r9sgMsyG0Z9uzsBlgiU/EFKhj/h91/8yHu0edo7XuixOIH3VcJ8kkxs6/jPzoI6U9Vj3WqbMQ94g==", + "dev": true, + "license": "MIT", + "dependencies": { + "balanced-match": "^4.0.2" + }, + "engines": { + "node": "18 || 20 || >=22" + } + }, "node_modules/rimraf/node_modules/glob": { - "version": "11.0.3", + "version": "11.1.0", + "resolved": "https://registry.npmjs.org/glob/-/glob-11.1.0.tgz", + "integrity": "sha512-vuNwKSaKiqm7g0THUBu2x7ckSs3XJLXE+2ssL7/MfTGPLLcrJQ/4Uq1CjPTtO5cCIiRxqvN6Twy1qOwhL0Xjcw==", + "deprecated": "Old versions of glob are not supported, and contain widely publicized security vulnerabilities, which have been fixed in the current version. Please update. Support for old versions may be purchased (at exorbitant rates) by contacting i@izs.me", "dev": true, - "license": "ISC", + "license": "BlueOak-1.0.0", "dependencies": { "foreground-child": "^3.3.1", "jackspeak": "^4.1.1", - "minimatch": "^10.0.3", + "minimatch": "^10.1.1", "minipass": "^7.1.2", "package-json-from-dist": "^1.0.0", "path-scurry": "^2.0.0" @@ -2948,14 +3057,16 @@ } }, "node_modules/rimraf/node_modules/minimatch": { - "version": "10.0.3", + "version": "10.2.5", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.5.tgz", + "integrity": "sha512-MULkVLfKGYDFYejP07QOurDLLQpcjk7Fw+7jXS2R2czRQzR56yHRveU5NDJEOviH+hETZKSkIk5c+T23GjFUMg==", "dev": true, - "license": "ISC", + "license": "BlueOak-1.0.0", "dependencies": { - "@isaacs/brace-expansion": "^5.0.0" + "brace-expansion": "^5.0.5" }, "engines": { - "node": "20 || >=22" + "node": "18 || 20 || >=22" }, "funding": { "url": "https://github.com/sponsors/isaacs" @@ -2970,13 +3081,15 @@ } }, "node_modules/serve-handler": { - "version": "6.1.6", + "version": "6.1.7", + "resolved": "https://registry.npmjs.org/serve-handler/-/serve-handler-6.1.7.tgz", + "integrity": "sha512-CinAq1xWb0vR3twAv9evEU8cNWkXCb9kd5ePAHUKJBkOsUpR1wt/CvGdeca7vqumL1U5cSaeVQ6zZMxiJ3yWsg==", "license": "MIT", "dependencies": { "bytes": "3.0.0", "content-disposition": "0.5.2", "mime-types": "2.1.18", - "minimatch": "3.1.2", + "minimatch": "3.1.5", "path-is-inside": "1.0.2", "path-to-regexp": "3.3.0", "range-parser": "1.2.0" @@ -3088,7 +3201,9 @@ } }, "node_modules/strnum": { - "version": "1.1.2", + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/strnum/-/strnum-2.3.0.tgz", + "integrity": "sha512-ums3KNd42PGyx5xaoVTO1mjU1bH3NpY4vsrVlnv9PNGqQj8wd7rJ6nEypLrJ7z5vxK5RP0yMLo6J/Gsm62DI5Q==", "funding": [ { "type": "github", @@ -3221,6 +3336,21 @@ "dev": true, "license": "ISC" }, + "node_modules/xml-naming": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/xml-naming/-/xml-naming-0.1.0.tgz", + "integrity": "sha512-k8KO9hrMyNk6tUWqUfkTEZbezRRpONVOzUTnc97VnCvyj6Tf9lyUR9EDAIeiVLv56jsMcoXEwjW8Kv5yPY52lw==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/NaturalIntelligence" + } + ], + "license": "MIT", + "engines": { + "node": ">=16.0.0" + } + }, "node_modules/yallist": { "version": "3.1.1", "dev": true, diff --git a/clients/static-site/package.json b/clients/static-site/package.json index f0295af..2d7b872 100644 --- a/clients/static-site/package.json +++ b/clients/static-site/package.json @@ -66,7 +66,7 @@ }, "dependencies": { "cosmiconfig": "^9.0.0", - "fast-xml-parser": "^4.5.0", + "fast-xml-parser": "^5.8.0", "playwright-core": "^1.50.0", "serve-handler": "^6.1.5", "zod": "^4.1.12" diff --git a/clients/static-site/src/browser.js b/clients/static-site/src/browser.js index f5c19ba..9ba17fb 100644 --- a/clients/static-site/src/browser.js +++ b/clients/static-site/src/browser.js @@ -16,10 +16,11 @@ let browsers = { chromium, firefox, webkit }; * @returns {Promise} Browser instance * @throws {Error} If browser type is invalid or browser is not installed */ -export async function launchBrowser(options = {}) { +export async function launchBrowser(options = {}, dependencies = {}) { let { type = 'chromium', headless = true, args = [] } = options; + let browserTypes = dependencies.browsers ?? browsers; - let browserType = browsers[type]; + let browserType = browserTypes[type]; if (!browserType) { throw new Error( `Unknown browser type: ${type}. Supported browsers: chromium, firefox, webkit` diff --git a/clients/static-site/src/crawler.js b/clients/static-site/src/crawler.js index 938b7c2..24342e5 100644 --- a/clients/static-site/src/crawler.js +++ b/clients/static-site/src/crawler.js @@ -3,6 +3,7 @@ * Functions for finding and parsing HTML pages in static site builds */ +import { existsSync } from 'node:fs'; import { readdir } from 'node:fs/promises'; import { join, relative, resolve, sep } from 'node:path'; import { filterByPattern } from './utils/patterns.js'; @@ -133,7 +134,6 @@ async function discoverPagesFromSitemap(buildPath, config) { let sitemapPath = join(buildPath, config.pageDiscovery.sitemapPath); // Check if custom sitemap exists, otherwise try to discover - let { existsSync } = await import('node:fs'); if (!existsSync(sitemapPath)) { sitemapPath = await discoverSitemap(buildPath); } diff --git a/clients/static-site/src/index.js b/clients/static-site/src/index.js index 99eccc9..bf07e11 100644 --- a/clients/static-site/src/index.js +++ b/clients/static-site/src/index.js @@ -4,6 +4,8 @@ * Uses a tab pool for efficient browser tab management */ +import { existsSync, readFileSync } from 'node:fs'; +import { dirname, join, parse } from 'node:path'; import { closeBrowser, launchBrowser } from './browser.js'; import { loadConfig } from './config.js'; import { discoverPages } from './crawler.js'; @@ -17,9 +19,6 @@ import { generateTasks, processAllTasks } from './tasks.js'; * @returns {Promise} True if TDD server is running */ async function isTddModeAvailable(debug = () => {}) { - let { existsSync, readFileSync } = await import('node:fs'); - let { join, parse, dirname } = await import('node:path'); - try { // Look for .vizzly/server.json let currentDir = process.cwd(); @@ -72,11 +71,17 @@ function hasApiToken(config) { * Uses a tab pool for efficient parallel screenshot capture * @param {string} buildPath - Path to static site build * @param {Object} options - CLI options - * @param {Object} context - Plugin context (logger, config, services) + * @param {Object} context - Plugin context (output, config, services) * @returns {Promise} */ export async function run(buildPath, options = {}, context = {}) { - let { logger, config: vizzlyConfig, services } = context; + let { + output: providedOutput, + logger: legacyOutput, + config: vizzlyConfig, + services, + } = context; + let output = providedOutput || legacyOutput; let browser = null; let pool = null; let serverInfo = null; @@ -85,8 +90,10 @@ export async function run(buildPath, options = {}, context = {}) { let buildId = null; let startTime = null; - if (!logger) { - throw new Error('Logger is required but was not provided in context'); + if (!output) { + throw new Error( + 'Output utilities are required but were not provided in context' + ); } try { @@ -96,12 +103,12 @@ export async function run(buildPath, options = {}, context = {}) { // Handle dry-run mode early - just discover and print pages if (options.dryRun) { let pages = await discoverPages(config.buildPath, config); - logger.info( + output.info( `šŸ” Dry run: Found ${pages.length} pages in ${config.buildPath}\n` ); if (pages.length === 0) { - logger.warn(' No pages found matching your configuration.'); + output.warn(' No pages found matching your configuration.'); return; } @@ -110,43 +117,43 @@ export async function run(buildPath, options = {}, context = {}) { let htmlPages = pages.filter(p => p.source === 'html'); if (sitemapPages.length > 0) { - logger.info(` From sitemap (${sitemapPages.length}):`); + output.info(` From sitemap (${sitemapPages.length}):`); for (let page of sitemapPages) { - logger.info(` ${page.path}`); + output.info(` ${page.path}`); } } if (htmlPages.length > 0) { - logger.info(` From HTML scan (${htmlPages.length}):`); + output.info(` From HTML scan (${htmlPages.length}):`); for (let page of htmlPages) { - logger.info(` ${page.path}`); + output.info(` ${page.path}`); } } // Show task count that would be generated let taskCount = pages.length * config.viewports.length; - logger.info(''); - logger.info(`šŸ“ø Would capture ${taskCount} screenshots:`); - logger.info( + output.info(''); + output.info(`šŸ“ø Would capture ${taskCount} screenshots:`); + output.info( ` ${pages.length} pages Ɨ ${config.viewports.length} viewports` ); - logger.info( + output.info( ` Viewports: ${config.viewports.map(v => `${v.name} (${v.width}Ɨ${v.height})`).join(', ')}` ); - logger.info(` Concurrency: ${config.concurrency} tabs`); + output.info(` Concurrency: ${config.concurrency} tabs`); return; } // Determine mode: TDD or Run - let debug = logger.debug?.bind(logger) || (() => {}); + let debug = output.debug?.bind(output) || (() => {}); let isTdd = await isTddModeAvailable(debug); let hasToken = hasApiToken(vizzlyConfig); if (isTdd) { - logger.info('šŸ“ TDD mode: Using local server'); + output.info('šŸ“ TDD mode: Using local server'); } else if (hasToken) { - logger.info('ā˜ļø Run mode: Uploading to cloud'); + output.info('ā˜ļø Run mode: Uploading to cloud'); } let buildUrl = null; @@ -162,7 +169,7 @@ export async function run(buildPath, options = {}, context = {}) { testRunner.once('build-created', buildInfo => { if (buildInfo.url) { buildUrl = buildInfo.url; - logger.info(`šŸ”— ${buildInfo.url}`); + output.info(`šŸ”— ${buildInfo.url}`); } }); @@ -181,7 +188,7 @@ export async function run(buildPath, options = {}, context = {}) { pullRequestNumber = gitInfo.prNumber; } else { // Fallback for older CLI versions - use environment variables - logger.warn( + output.warn( 'āš ļø Upgrade to @vizzly-testing/cli@>=0.25.0 for improved git detection' ); branch = process.env.VIZZLY_BRANCH || 'main'; @@ -223,16 +230,16 @@ export async function run(buildPath, options = {}, context = {}) { process.env.VIZZLY_ENABLED = 'true'; } catch (error) { // Log the error and continue without cloud mode - logger.error(`Failed to initialize cloud mode: ${error.message}`); - logger.warn('āš ļø Falling back to local-only mode'); - logger.info(' Screenshots will not be uploaded to cloud'); + output.error(`Failed to initialize cloud mode: ${error.message}`); + output.warn('āš ļø Falling back to local-only mode'); + output.info(' Screenshots will not be uploaded to cloud'); testRunner = null; } } if (!isTdd && !hasToken) { // Use output module methods for clean formatting - let out = logger.print ? logger : null; + let out = output.print ? output : null; if (out) { out.blank(); out.warn('No TDD server or API token found'); @@ -248,8 +255,8 @@ export async function run(buildPath, options = {}, context = {}) { out.blank(); } else { // Fallback for testing or when output module not available - logger.warn('No TDD server or API token found'); - logger.info('Run "vizzly tdd start" first, or set VIZZLY_TOKEN'); + output.warn('No TDD server or API token found'); + output.info('Run "vizzly tdd start" first, or set VIZZLY_TOKEN'); } return; } @@ -259,10 +266,10 @@ export async function run(buildPath, options = {}, context = {}) { // Discover pages let pages = await discoverPages(config.buildPath, config); - logger.info(`🌐 Found ${pages.length} pages in ${config.buildPath}`); + output.info(`🌐 Found ${pages.length} pages in ${config.buildPath}`); if (pages.length === 0) { - logger.warn('āš ļø No pages found'); + output.warn('āš ļø No pages found'); return; } @@ -272,21 +279,21 @@ export async function run(buildPath, options = {}, context = {}) { // Generate all tasks upfront (pages Ɨ viewports) let tasks = generateTasks(pages, serverInfo.url, config); - logger.info( + output.info( `šŸ“ø Processing ${tasks.length} screenshots (${config.concurrency} concurrent tabs)` ); // Process all tasks through the tab pool - let errors = await processAllTasks(tasks, pool, config, logger); + let errors = await processAllTasks(tasks, pool, config, output); // Report summary if (errors.length > 0) { - logger.warn(`\nāš ļø ${errors.length} screenshot(s) failed:`); + output.warn(`\nāš ļø ${errors.length} screenshot(s) failed:`); errors.forEach(({ page, viewport, error }) => { - logger.error(` ${page}@${viewport}: ${error}`); + output.error(` ${page}@${viewport}: ${error}`); }); } else { - logger.info(`\nāœ… Captured ${tasks.length} screenshots successfully`); + output.info(`\nāœ… Captured ${tasks.length} screenshots successfully`); } // Finalize build in run mode @@ -295,11 +302,11 @@ export async function run(buildPath, options = {}, context = {}) { await testRunner.finalizeBuild(buildId, false, true, executionTime); if (buildUrl) { - logger.info(`šŸ”— View results: ${buildUrl}`); + output.info(`šŸ”— View results: ${buildUrl}`); } } } catch (error) { - logger.error('Failed to process pages:', error.message); + output.error('Failed to process pages:', error.message); // Mark build as failed if in run mode if (testRunner && buildId) { diff --git a/clients/static-site/src/plugin.js b/clients/static-site/src/plugin.js index 5855951..a670559 100644 --- a/clients/static-site/src/plugin.js +++ b/clients/static-site/src/plugin.js @@ -44,10 +44,10 @@ export default { * @param {import('commander').Command} program - Commander program instance * @param {Object} context - Plugin context * @param {Object} context.config - Vizzly configuration - * @param {Object} context.logger - Logger instance + * @param {Object} context.output - Output utilities * @param {Object} context.services - Service container */ - register(program, { config, logger, services }) { + register(program, { config, output, services }) { program .command('static-site ') .description( @@ -95,14 +95,14 @@ export default { let mergedOptions = { ...globalOptions, ...options }; await run(path, mergedOptions, { - logger, + output, config, services, }); } catch (error) { console.error('Failed to run Static Site plugin:', error); - if (logger?.error) { - logger.error('Failed to run Static Site plugin:', error.message); + if (output?.error) { + output.error('Failed to run Static Site plugin:', error); } process.exit(1); } diff --git a/clients/static-site/src/pool.js b/clients/static-site/src/pool.js index c18216b..b0c07bc 100644 --- a/clients/static-site/src/pool.js +++ b/clients/static-site/src/pool.js @@ -36,7 +36,10 @@ export function createTabPool(browser, size, options = {}) { let createContextEntry = async () => { let contextOptions = {}; if (viewport) { - contextOptions.viewport = { width: viewport.width, height: viewport.height }; + contextOptions.viewport = { + width: viewport.width, + height: viewport.height, + }; } let context = await browser.newContext(contextOptions); let page = await context.newPage(); diff --git a/clients/static-site/src/server.js b/clients/static-site/src/server.js index 490b235..9e8fb70 100644 --- a/clients/static-site/src/server.js +++ b/clients/static-site/src/server.js @@ -15,6 +15,7 @@ import handler from 'serve-handler'; */ export async function startStaticServer(rootDir, port = 0) { let absoluteRoot = resolve(rootDir); + let sockets = new Set(); let server = createServer((req, res) => { return handler(req, res, { @@ -24,6 +25,13 @@ export async function startStaticServer(rootDir, port = 0) { }); }); + server.on('connection', socket => { + sockets.add(socket); + socket.on('close', () => { + sockets.delete(socket); + }); + }); + return new Promise((resolve, reject) => { server.listen(port, () => { let address = server.address(); @@ -34,6 +42,7 @@ export async function startStaticServer(rootDir, port = 0) { server, port: actualPort, url, + sockets, }); }); @@ -44,18 +53,32 @@ export async function startStaticServer(rootDir, port = 0) { /** * Stop a static server * @param {Object} serverInfo - Server info from startStaticServer - * @param {number} [timeout=5000] - Max time to wait for server to close (ms) * @returns {Promise} */ -export async function stopStaticServer(serverInfo, timeout = 5000) { +export async function stopStaticServer(serverInfo) { if (!serverInfo || !serverInfo.server) { return; } - return Promise.race([ - new Promise(resolve => { - serverInfo.server.close(() => resolve()); - }), - new Promise(resolve => setTimeout(resolve, timeout)), - ]); + let { server, sockets = new Set() } = serverInfo; + + await new Promise((resolve, reject) => { + server.close(error => { + if (error) { + reject(error); + return; + } + + resolve(); + }); + + if (typeof server.closeAllConnections === 'function') { + server.closeAllConnections(); + return; + } + + for (let socket of sockets) { + socket.destroy(); + } + }); } diff --git a/clients/static-site/src/tasks.js b/clients/static-site/src/tasks.js index 42fbb62..6e50616 100644 --- a/clients/static-site/src/tasks.js +++ b/clients/static-site/src/tasks.js @@ -3,12 +3,12 @@ * Functional approach: tasks are (page, viewport) tuples processed through a tab pool */ +import { navigateToUrl as defaultNavigateToUrl } from './browser.js'; import { getPageConfig as defaultGetPageConfig } from './config.js'; import { generatePageUrl as defaultGeneratePageUrl } from './crawler.js'; import { getBeforeScreenshotHook as defaultGetBeforeScreenshotHook } from './hooks.js'; import { captureAndSendScreenshot as defaultCaptureAndSendScreenshot } from './screenshot.js'; import { setViewport as defaultSetViewport } from './utils/viewport.js'; -import { navigateToUrl as defaultNavigateToUrl } from './browser.js'; /** * Default dependencies for task operations @@ -173,14 +173,14 @@ function createOutputCoordinator() { /** * Queue an error message to be printed * @param {string} message - Error message - * @param {Object} logger - Logger instance + * @param {Object} output - Output utilities */ - logError(message, logger) { + logError(message, output) { if (isInteractiveTTY()) { // Queue error to be printed before next progress update pendingErrors.push(message); } - logger.error(message); + output.error(message); }, /** @@ -204,18 +204,18 @@ function createOutputCoordinator() { * @param {Array} tasks - Array of task objects * @param {Object} pool - Tab pool { acquire, release } * @param {Object} config - Configuration object - * @param {Object} logger - Logger instance + * @param {Object} output - Output utilities * @param {Object} [deps] - Optional dependencies for testing * @returns {Promise} Array of errors encountered */ -export async function processAllTasks(tasks, pool, config, logger, deps = {}) { +export async function processAllTasks(tasks, pool, config, output, deps = {}) { let errors = []; let completed = 0; let total = tasks.length; let startTime = Date.now(); let taskTimes = []; let interactive = isInteractiveTTY(); - let output = createOutputCoordinator(); + let progressOutput = createOutputCoordinator(); // Minimum samples before showing ETA (avoids wild estimates from cold start) let minSamplesForEta = Math.min(5, Math.ceil(total * 0.1)); @@ -300,12 +300,12 @@ export async function processAllTasks(tasks, pool, config, logger, deps = {}) { if (interactive) { // Update single progress line - output.writeProgress( + progressOutput.writeProgress( ` šŸ“ø [${completed}/${total}] ${percent}% ${eta} - ${task.page.path}@${task.viewport.name}` ); } else { // Non-interactive: log each completion - logger.info( + output.info( ` āœ“ [${completed}/${total}] ${task.page.path}@${task.viewport.name} ${eta}` ); } @@ -322,9 +322,9 @@ export async function processAllTasks(tasks, pool, config, logger, deps = {}) { error: result.error.message + retryNote, }); - output.logError( + progressOutput.logError( ` āœ— ${task.page.path}@${task.viewport.name}: ${result.error.message}${retryNote}`, - logger + output ); if (result.tab) { @@ -336,16 +336,16 @@ export async function processAllTasks(tasks, pool, config, logger, deps = {}) { ); // Flush any remaining output - output.flush(); + progressOutput.flush(); // Log total time let totalTime = Date.now() - startTime; - logger.info( + output.info( ` āœ… Completed ${total} screenshots in ${formatDuration(totalTime)}` ); if (errors.length > 0) { - logger.warn(` āš ļø ${errors.length} failed`); + output.warn(` āš ļø ${errors.length} failed`); } return errors; diff --git a/clients/static-site/tests/browser.test.js b/clients/static-site/tests/browser.test.js index 228386e..e2e8e1d 100644 --- a/clients/static-site/tests/browser.test.js +++ b/clients/static-site/tests/browser.test.js @@ -3,16 +3,47 @@ */ import assert from 'node:assert'; -import { describe, it, mock } from 'node:test'; +import { describe, it } from 'node:test'; +import { launchBrowser } from '../src/browser.js'; + +function createBrowserTypes(overrides = {}) { + let calls = []; + let browser = { id: 'browser' }; + + function createBrowserType(name) { + return { + async launch(options) { + calls.push({ name, options }); + return browser; + }, + }; + } + + return { + browser, + calls, + browsers: { + chromium: createBrowserType('chromium'), + firefox: createBrowserType('firefox'), + webkit: createBrowserType('webkit'), + ...overrides, + }, + }; +} + +function createMissingBrowserType(message) { + return { + async launch() { + throw new Error(message); + }, + }; +} describe('browser', () => { describe('launchBrowser', () => { it('throws error for invalid browser type', async () => { - // Import fresh to avoid caching - let { launchBrowser } = await import('../src/browser.js'); - await assert.rejects( - () => launchBrowser({ type: 'invalid-browser' }), + () => launchBrowser({ type: 'invalid-browser' }, createBrowserTypes()), { message: /Unknown browser type: invalid-browser/, } @@ -20,10 +51,8 @@ describe('browser', () => { }); it('includes supported browsers in error message', async () => { - let { launchBrowser } = await import('../src/browser.js'); - await assert.rejects( - () => launchBrowser({ type: 'netscape' }), + () => launchBrowser({ type: 'netscape' }, createBrowserTypes()), { message: /chromium, firefox, webkit/, } @@ -31,117 +60,107 @@ describe('browser', () => { }); it('defaults to chromium when no type specified', async () => { - // This test verifies the default by checking that no error is thrown - // for missing type (actual browser launch would fail without installation) - let { launchBrowser } = await import('../src/browser.js'); - - // We expect this to fail with browser-not-installed, not invalid-type - try { - await launchBrowser({ type: undefined }); - } catch (error) { - // Should NOT be an "Unknown browser type" error - assert.ok( - !error.message.includes('Unknown browser type'), - 'Should default to chromium, not throw unknown browser error' - ); - } + let dependencies = createBrowserTypes(); + + let browser = await launchBrowser( + { type: undefined, headless: false }, + dependencies + ); + + assert.strictEqual(browser, dependencies.browser); + assert.strictEqual(dependencies.calls[0].name, 'chromium'); + assert.strictEqual(dependencies.calls[0].options.headless, false); }); }); describe('browser-specific args', () => { it('chromium args include sandbox and memory flags', async () => { - // We can't easily test the actual args without mocking playwright-core - // but we can verify the function exists and accepts chromium type - let { launchBrowser } = await import('../src/browser.js'); - - try { - await launchBrowser({ type: 'chromium' }); - } catch (error) { - // Expected to fail (no browser installed), but validates type is accepted - assert.ok( - !error.message.includes('Unknown browser type'), - 'chromium should be a valid browser type' - ); - } + let dependencies = createBrowserTypes(); + + await launchBrowser( + { type: 'chromium', args: ['--custom-flag'] }, + dependencies + ); + + let launchArgs = dependencies.calls[0].options.args; + assert.ok(launchArgs.includes('--no-sandbox')); + assert.ok(launchArgs.includes('--disable-dev-shm-usage')); + assert.ok(launchArgs.includes('--custom-flag')); }); - it('firefox is a valid browser type', async () => { - let { launchBrowser } = await import('../src/browser.js'); - - try { - await launchBrowser({ type: 'firefox' }); - } catch (error) { - assert.ok( - !error.message.includes('Unknown browser type'), - 'firefox should be a valid browser type' - ); - } + it('passes non-chromium browser args through unchanged', async () => { + let dependencies = createBrowserTypes(); + + await launchBrowser( + { type: 'firefox', args: ['--custom-flag'] }, + dependencies + ); + + assert.deepStrictEqual(dependencies.calls[0].options.args, [ + '--custom-flag', + ]); }); it('webkit is a valid browser type', async () => { - let { launchBrowser } = await import('../src/browser.js'); - - try { - await launchBrowser({ type: 'webkit' }); - } catch (error) { - assert.ok( - !error.message.includes('Unknown browser type'), - 'webkit should be a valid browser type' - ); - } + let dependencies = createBrowserTypes(); + + await launchBrowser({ type: 'webkit' }, dependencies); + + assert.strictEqual(dependencies.calls[0].name, 'webkit'); }); }); describe('error messages', () => { it('browser-not-installed error includes install command', async () => { - let { launchBrowser } = await import('../src/browser.js'); - - try { - await launchBrowser({ type: 'chromium' }); - // If we get here, browser is installed - skip assertion - } catch (error) { - if (error.message.includes('is not installed')) { - assert.ok( - error.message.includes('npx playwright install chromium'), - 'Should include chromium install command' - ); + let dependencies = createBrowserTypes({ + chromium: createMissingBrowserType("Executable doesn't exist at /tmp"), + }); + + await assert.rejects( + () => launchBrowser({ type: 'chromium' }, dependencies), + { + message: /npx playwright install chromium/, } - // Otherwise it's a different error (browser is installed but failed for other reason) - } + ); }); it('browser-not-installed error includes CI guidance', async () => { - let { launchBrowser } = await import('../src/browser.js'); - - try { - await launchBrowser({ type: 'firefox' }); - } catch (error) { - if (error.message.includes('is not installed')) { - assert.ok( - error.message.includes('--with-deps'), - 'Should include --with-deps for CI environments' - ); - assert.ok( - error.message.includes('playwright.dev/docs/ci'), - 'Should link to Playwright CI docs' - ); + let dependencies = createBrowserTypes({ + firefox: createMissingBrowserType('download new browsers'), + }); + + await assert.rejects( + () => launchBrowser({ type: 'firefox' }, dependencies), + { + message: /--with-deps[\s\S]*playwright\.dev\/docs\/ci/, } - } + ); }); it('firefox install error suggests correct browser', async () => { - let { launchBrowser } = await import('../src/browser.js'); - - try { - await launchBrowser({ type: 'firefox' }); - } catch (error) { - if (error.message.includes('is not installed')) { - assert.ok( - error.message.includes('npx playwright install firefox'), - 'Should suggest installing firefox, not chromium' - ); + let dependencies = createBrowserTypes({ + firefox: createMissingBrowserType('npx playwright install'), + }); + + await assert.rejects( + () => launchBrowser({ type: 'firefox' }, dependencies), + { + message: /npx playwright install firefox/, } - } + ); + }); + + it('rethrows non-install launch failures', async () => { + let dependencies = createBrowserTypes({ + firefox: createMissingBrowserType('permission denied'), + }); + + await assert.rejects( + () => launchBrowser({ type: 'firefox' }, dependencies), + { + message: /permission denied/, + } + ); }); }); }); diff --git a/clients/static-site/tests/dry-run.test.js b/clients/static-site/tests/dry-run.test.js index d8ddedd..1c3aee7 100644 --- a/clients/static-site/tests/dry-run.test.js +++ b/clients/static-site/tests/dry-run.test.js @@ -5,9 +5,9 @@ import assert from 'node:assert'; import { mkdir, rm, writeFile } from 'node:fs/promises'; -import { join } from 'node:path'; import { tmpdir } from 'node:os'; -import { describe, it, before, after, mock } from 'node:test'; +import { join } from 'node:path'; +import { after, before, describe, it } from 'node:test'; import { run } from '../src/index.js'; describe('dry-run mode', () => { diff --git a/clients/static-site/tests/sdk-integration.test.js b/clients/static-site/tests/sdk-integration.test.js index b78c1ef..eab4fa0 100644 --- a/clients/static-site/tests/sdk-integration.test.js +++ b/clients/static-site/tests/sdk-integration.test.js @@ -13,6 +13,7 @@ import assert from 'node:assert'; import { spawn } from 'node:child_process'; +import { once } from 'node:events'; import { existsSync, mkdirSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; @@ -25,6 +26,15 @@ import { captureScreenshot } from '../src/screenshot.js'; import { startStaticServer, stopStaticServer } from '../src/server.js'; import { generateTasks, processAllTasks } from '../src/tasks.js'; +async function stopProcess(child) { + if (!child || child.exitCode !== null || child.signalCode !== null) { + return; + } + + child.kill('SIGTERM'); + await once(child, 'exit'); +} + // Paths let testDir = join(tmpdir(), `vizzly-static-site-e2e-${Date.now()}`); let testSitePath = resolve(import.meta.dirname, '../../../test-site'); @@ -104,11 +114,7 @@ describe('Static-Site E2E with shared test-site', { skip: !runE2E }, () => { if (serverInfo) await stopStaticServer(serverInfo); if (tddServer && !externalServer) { - tddServer.kill('SIGTERM'); - await new Promise(resolve => { - tddServer.on('exit', resolve); - setTimeout(resolve, 2000); - }); + await stopProcess(tddServer); } if (!externalServer) { @@ -140,7 +146,10 @@ describe('Static-Site E2E with shared test-site', { skip: !runE2E }, () => { let pages = await discoverPages(testSitePath, config); - assert.ok(pages.length >= 4, `Should find at least 4 pages, found ${pages.length}`); + assert.ok( + pages.length >= 4, + `Should find at least 4 pages, found ${pages.length}` + ); let pagePaths = pages.map(p => p.path); assert.ok( @@ -175,7 +184,11 @@ describe('Static-Site E2E with shared test-site', { skip: !runE2E }, () => { }; let pages = await discoverPages(testSitePath, config); - assert.strictEqual(pages.length, 2, `Should find exactly 2 pages, found ${pages.length}: ${pages.map(p => p.path).join(', ')}`); + assert.strictEqual( + pages.length, + 2, + `Should find exactly 2 pages, found ${pages.length}: ${pages.map(p => p.path).join(', ')}` + ); }); it('excludes pages with exclude patterns', async () => { @@ -202,7 +215,10 @@ describe('Static-Site E2E with shared test-site', { skip: !runE2E }, () => { !pagePaths.some(p => p.includes('contact')), 'Should exclude contact page' ); - assert.ok(pages.length >= 2, `Should have at least 2 pages (/ and /features), found ${pages.length}`); + assert.ok( + pages.length >= 2, + `Should have at least 2 pages (/ and /features), found ${pages.length}` + ); }); }); @@ -306,7 +322,11 @@ describe('Static-Site E2E with shared test-site', { skip: !runE2E }, () => { let tasks = generateTasks(pages, serverInfo.url, config); - assert.strictEqual(tasks.length, 4, 'Should generate 4 tasks (2 pages Ɨ 2 viewports)'); + assert.strictEqual( + tasks.length, + 4, + 'Should generate 4 tasks (2 pages Ɨ 2 viewports)' + ); // Verify task structure let firstTask = tasks[0]; @@ -351,7 +371,12 @@ describe('Static-Site E2E with shared test-site', { skip: !runE2E }, () => { describe('Multi-Page Screenshots', () => { it('captures all test-site pages', async () => { - let pages = ['index.html', 'features.html', 'pricing.html', 'contact.html']; + let pages = [ + 'index.html', + 'features.html', + 'pricing.html', + 'contact.html', + ]; let results = []; for (let pageName of pages) { @@ -453,9 +478,19 @@ describe('Static-Site SDK (unit tests)', () => { let tasks = generateTasks(pages, 'http://localhost:3000', config); assert.strictEqual(tasks.length, 1); - assert.ok(tasks[0].page.path.includes('about'), 'Task page path should include page name'); - assert.strictEqual(tasks[0].viewport.name, 'desktop', 'Task should have viewport'); - assert.ok(tasks[0].url.includes('about'), 'Task URL should include page path'); + assert.ok( + tasks[0].page.path.includes('about'), + 'Task page path should include page name' + ); + assert.strictEqual( + tasks[0].viewport.name, + 'desktop', + 'Task should have viewport' + ); + assert.ok( + tasks[0].url.includes('about'), + 'Task URL should include page path' + ); }); it('handles include patterns correctly', async () => { @@ -472,7 +507,11 @@ describe('Static-Site SDK (unit tests)', () => { }; let pages = await discoverPages(testSitePath, config); - assert.strictEqual(pages.length, 1, `Should find only index page, found ${pages.length}: ${pages.map(p => p.path).join(', ')}`); + assert.strictEqual( + pages.length, + 1, + `Should find only index page, found ${pages.length}: ${pages.map(p => p.path).join(', ')}` + ); assert.strictEqual(pages[0].path, '/'); }); }); diff --git a/clients/static-site/tests/server.test.js b/clients/static-site/tests/server.test.js new file mode 100644 index 0000000..b2a14b5 --- /dev/null +++ b/clients/static-site/tests/server.test.js @@ -0,0 +1,36 @@ +import assert from 'node:assert/strict'; +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, it } from 'node:test'; + +import { startStaticServer, stopStaticServer } from '../src/server.js'; + +describe('static-site server', () => { + let workspace; + let serverInfo; + + beforeEach(async () => { + workspace = await mkdtemp(join(tmpdir(), 'vizzly-static-server-')); + await mkdir(join(workspace, 'site')); + await writeFile(join(workspace, 'site', 'index.html'), '

Hello

'); + }); + + afterEach(async () => { + await stopStaticServer(serverInfo); + await rm(workspace, { recursive: true, force: true }); + }); + + it('serves files and stops accepting requests after shutdown', async () => { + serverInfo = await startStaticServer(join(workspace, 'site')); + + let response = await fetch(`${serverInfo.url}/index.html`); + assert.equal(response.status, 200); + assert.match(await response.text(), /Hello/); + + await stopStaticServer(serverInfo); + serverInfo = null; + + await assert.rejects(fetch(response.url)); + }); +}); diff --git a/clients/static-site/tests/tasks.test.js b/clients/static-site/tests/tasks.test.js index 466ff44..86702c9 100644 --- a/clients/static-site/tests/tasks.test.js +++ b/clients/static-site/tests/tasks.test.js @@ -4,7 +4,7 @@ import assert from 'node:assert'; import { describe, it, mock } from 'node:test'; -import { generateTasks, processTask, processAllTasks } from '../src/tasks.js'; +import { generateTasks, processAllTasks, processTask } from '../src/tasks.js'; describe('generateTasks', () => { it('generates tasks for each page Ɨ viewport combination', () => { @@ -18,7 +18,7 @@ describe('generateTasks', () => { }; let deps = { - getPageConfig: (cfg, page) => ({ + getPageConfig: (cfg, _page) => ({ viewports: cfg.viewports, screenshot: {}, }), @@ -55,7 +55,7 @@ describe('generateTasks', () => { }; let deps = { - getPageConfig: (cfg, page) => ({ + getPageConfig: (cfg, _page) => ({ viewports: cfg.viewports, screenshot: {}, }), diff --git a/clients/static-site/tests/utils/sitemap.test.js b/clients/static-site/tests/utils/sitemap.test.js index 4dfed67..95febf1 100644 --- a/clients/static-site/tests/utils/sitemap.test.js +++ b/clients/static-site/tests/utils/sitemap.test.js @@ -4,8 +4,8 @@ import assert from 'node:assert'; import { mkdir, rm, writeFile } from 'node:fs/promises'; -import { join } from 'node:path'; import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import { after, before, describe, it } from 'node:test'; import { parseSitemapFile, diff --git a/clients/storybook/README.md b/clients/storybook/README.md index 18e8d0e..f4c4f78 100644 --- a/clients/storybook/README.md +++ b/clients/storybook/README.md @@ -54,7 +54,7 @@ await run('./storybook-static', { viewports: 'mobile:375x667,desktop:1920x1080', concurrency: 3, }, { - logger: console, + output: console, }); ``` diff --git a/clients/storybook/src/crawler.js b/clients/storybook/src/crawler.js index 746c065..d5dde4e 100644 --- a/clients/storybook/src/crawler.js +++ b/clients/storybook/src/crawler.js @@ -129,7 +129,9 @@ export function filterStories(stories, config) { let storyConfig = extractStoryConfig(story); if (storyConfig?.skip) { if (verbose) { - console.error(` [filter] Skipping ${story.id} (parameters.vizzly.skip)`); + console.error( + ` [filter] Skipping ${story.id} (parameters.vizzly.skip)` + ); } return false; } diff --git a/clients/storybook/src/index.js b/clients/storybook/src/index.js index 27f8edd..4a1fe1c 100644 --- a/clients/storybook/src/index.js +++ b/clients/storybook/src/index.js @@ -4,6 +4,8 @@ * Uses a tab pool for efficient browser tab management */ +import { existsSync, readFileSync } from 'node:fs'; +import { dirname, join, parse } from 'node:path'; import { closeBrowser, launchBrowser } from './browser.js'; import { loadConfig } from './config.js'; import { discoverStories } from './crawler.js'; @@ -16,9 +18,6 @@ import { generateTasks, processAllTasks } from './tasks.js'; * @returns {Promise} True if TDD server is running */ async function isTddModeAvailable() { - let { existsSync, readFileSync } = await import('node:fs'); - let { join, parse, dirname } = await import('node:path'); - try { // Look for .vizzly/server.json let currentDir = process.cwd(); @@ -64,11 +63,17 @@ function hasApiToken(config) { * Uses a tab pool for efficient parallel screenshot capture * @param {string} storybookPath - Path to static Storybook build * @param {Object} options - CLI options - * @param {Object} context - Plugin context (logger, config, services) + * @param {Object} context - Plugin context (output, config, services) * @returns {Promise} */ export async function run(storybookPath, options = {}, context = {}) { - let { logger, config: vizzlyConfig, services } = context; + let { + output: providedOutput, + logger: legacyOutput, + config: vizzlyConfig, + services, + } = context; + let output = providedOutput || legacyOutput; let browser = null; let pool = null; let serverInfo = null; @@ -77,8 +82,10 @@ export async function run(storybookPath, options = {}, context = {}) { let buildId = null; let startTime = null; - if (!logger) { - throw new Error('Logger is required but was not provided in context'); + if (!output) { + throw new Error( + 'Output utilities are required but were not provided in context' + ); } try { @@ -90,9 +97,9 @@ export async function run(storybookPath, options = {}, context = {}) { let hasToken = hasApiToken(vizzlyConfig); if (isTdd) { - logger.info('šŸ“ TDD mode: Using local server'); + output.info('šŸ“ TDD mode: Using local server'); } else if (hasToken) { - logger.info('ā˜ļø Run mode: Uploading to cloud'); + output.info('ā˜ļø Run mode: Uploading to cloud'); } let buildUrl = null; @@ -108,7 +115,7 @@ export async function run(storybookPath, options = {}, context = {}) { testRunner.once('build-created', buildInfo => { if (buildInfo.url) { buildUrl = buildInfo.url; - logger.info(`šŸ”— ${buildInfo.url}`); + output.info(`šŸ”— ${buildInfo.url}`); } }); @@ -125,7 +132,7 @@ export async function run(storybookPath, options = {}, context = {}) { pullRequestNumber = gitInfo.prNumber; } else { // Fallback for older CLI versions - use environment variables - logger.warn( + output.warn( 'āš ļø Upgrade to @vizzly-testing/cli@>=0.25.0 for improved git detection' ); branch = process.env.VIZZLY_BRANCH || 'main'; @@ -167,16 +174,16 @@ export async function run(storybookPath, options = {}, context = {}) { process.env.VIZZLY_ENABLED = 'true'; } catch (error) { // Log the error and continue without cloud mode - logger.error(`Failed to initialize cloud mode: ${error.message}`); - logger.warn('āš ļø Falling back to local-only mode'); - logger.info(' Screenshots will not be uploaded to cloud'); + output.error(`Failed to initialize cloud mode: ${error.message}`); + output.warn('āš ļø Falling back to local-only mode'); + output.info(' Screenshots will not be uploaded to cloud'); testRunner = null; } } if (!isTdd && !hasToken) { - logger.warn('āš ļø No TDD server or API token found'); - logger.info(' Run `vizzly tdd start` or set VIZZLY_TOKEN'); + output.warn('āš ļø No TDD server or API token found'); + output.info(' Run `vizzly tdd start` or set VIZZLY_TOKEN'); } // Start HTTP server to serve Storybook static files @@ -184,12 +191,12 @@ export async function run(storybookPath, options = {}, context = {}) { // Discover stories let stories = await discoverStories(config.storybookPath, config); - logger.info( + output.info( `šŸ“š Found ${stories.length} stories in ${config.storybookPath}` ); if (stories.length === 0) { - logger.warn('āš ļø No stories found'); + output.warn('āš ļø No stories found'); return; } @@ -199,18 +206,18 @@ export async function run(storybookPath, options = {}, context = {}) { // Generate all tasks upfront (stories Ɨ viewports) let tasks = generateTasks(stories, serverInfo.url, config); - logger.info( + output.info( `šŸ“ø Processing ${tasks.length} screenshots (${config.concurrency} concurrent tabs)` ); // Process all tasks through the tab pool - let errors = await processAllTasks(tasks, pool, config, logger); + let errors = await processAllTasks(tasks, pool, config, output); // Report summary if (errors.length > 0) { - logger.warn(`\nāš ļø ${errors.length} screenshot(s) failed:`); + output.warn(`\nāš ļø ${errors.length} screenshot(s) failed:`); errors.forEach(({ story, viewport, error }) => { - logger.error(` ${story}@${viewport}: ${error}`); + output.error(` ${story}@${viewport}: ${error}`); }); } @@ -220,11 +227,11 @@ export async function run(storybookPath, options = {}, context = {}) { await testRunner.finalizeBuild(buildId, false, true, executionTime); if (buildUrl) { - logger.info(`šŸ”— View results: ${buildUrl}`); + output.info(`šŸ”— View results: ${buildUrl}`); } } } catch (error) { - logger.error('Failed to process stories:', error.message); + output.error('Failed to process stories:', error.message); // Mark build as failed if in run mode if (testRunner && buildId) { diff --git a/clients/storybook/src/navigation.js b/clients/storybook/src/navigation.js index 36230f2..4987968 100644 --- a/clients/storybook/src/navigation.js +++ b/clients/storybook/src/navigation.js @@ -47,7 +47,9 @@ export async function navigateToStory(page, storyId, baseUrl, options = {}) { await fullPageNavigation(page, storyId, baseUrl, timeout); if (verbose) { - console.error(` [nav] ${storyId}: full-page took ${Date.now() - start}ms`); + console.error( + ` [nav] ${storyId}: full-page took ${Date.now() - start}ms` + ); } if (entry) { @@ -71,7 +73,9 @@ export async function navigateToStory(page, storyId, baseUrl, options = {}) { await clientSideNavigation(page, storyId, timeout); if (verbose) { - console.error(` [nav] ${storyId}: client-side took ${Date.now() - start}ms`); + console.error( + ` [nav] ${storyId}: client-side took ${Date.now() - start}ms` + ); } entry.currentStoryId = storyId; } catch (error) { @@ -83,7 +87,9 @@ export async function navigateToStory(page, storyId, baseUrl, options = {}) { await fullPageNavigation(page, storyId, baseUrl, timeout); if (verbose) { - console.error(` [nav] ${storyId}: fallback full-page took ${Date.now() - start}ms`); + console.error( + ` [nav] ${storyId}: fallback full-page took ${Date.now() - start}ms` + ); } entry.currentStoryId = storyId; } @@ -152,10 +158,10 @@ async function clientSideNavigation(page, storyId, timeout) { // Navigate to the story preview.channel.emit('setCurrentStory', { storyId: id }); - // Timeout fallback - use configured timeout + // Trigger the full-page fallback when Storybook does not emit a render. timeoutId = setTimeout(() => { preview.channel.off('storyRendered', handleRendered); - resolve(); // Resolve anyway - story might have rendered + reject(new Error(`Storybook did not render ${id} before timeout`)); }, timeoutMs); }); }, diff --git a/clients/storybook/src/plugin.js b/clients/storybook/src/plugin.js index c3cabf5..312757b 100644 --- a/clients/storybook/src/plugin.js +++ b/clients/storybook/src/plugin.js @@ -38,10 +38,10 @@ export default { * @param {import('commander').Command} program - Commander program instance * @param {Object} context - Plugin context * @param {Object} context.config - Vizzly configuration - * @param {Object} context.logger - Logger instance + * @param {Object} context.output - Output utilities * @param {Object} context.services - Service container */ - register(program, { config, logger, services }) { + register(program, { config, output, services }) { program .command('storybook ') .description('Capture screenshots from static Storybook build') @@ -78,14 +78,14 @@ export default { let mergedOptions = { ...globalOptions, ...options }; await run(path, mergedOptions, { - logger, + output, config, services, }); } catch (error) { console.error('Failed to run Storybook plugin:', error); - if (logger?.error) { - logger.error('Failed to run Storybook plugin:', error.message); + if (output?.error) { + output.error('Failed to run Storybook plugin:', error); } process.exit(1); } diff --git a/clients/storybook/src/pool.js b/clients/storybook/src/pool.js index 1732d2e..ea0d254 100644 --- a/clients/storybook/src/pool.js +++ b/clients/storybook/src/pool.js @@ -36,7 +36,10 @@ export function createTabPool(browser, size, options = {}) { let createContextEntry = async () => { let contextOptions = {}; if (viewport) { - contextOptions.viewport = { width: viewport.width, height: viewport.height }; + contextOptions.viewport = { + width: viewport.width, + height: viewport.height, + }; } let context = await browser.newContext(contextOptions); let page = await context.newPage(); diff --git a/clients/storybook/src/server.js b/clients/storybook/src/server.js index 7f0843a..bbe799f 100644 --- a/clients/storybook/src/server.js +++ b/clients/storybook/src/server.js @@ -15,6 +15,7 @@ import handler from 'serve-handler'; */ export async function startStaticServer(rootDir, port = 0) { let absoluteRoot = resolve(rootDir); + let sockets = new Set(); let server = createServer((req, res) => { return handler(req, res, { @@ -24,6 +25,13 @@ export async function startStaticServer(rootDir, port = 0) { }); }); + server.on('connection', socket => { + sockets.add(socket); + socket.on('close', () => { + sockets.delete(socket); + }); + }); + return new Promise((resolve, reject) => { server.listen(port, () => { let address = server.address(); @@ -34,6 +42,7 @@ export async function startStaticServer(rootDir, port = 0) { server, port: actualPort, url, + sockets, }); }); @@ -51,7 +60,25 @@ export async function stopStaticServer(serverInfo) { return; } - return new Promise(resolve => { - serverInfo.server.close(() => resolve()); + let { server, sockets = new Set() } = serverInfo; + + await new Promise((resolve, reject) => { + server.close(error => { + if (error) { + reject(error); + return; + } + + resolve(); + }); + + if (typeof server.closeAllConnections === 'function') { + server.closeAllConnections(); + return; + } + + for (let socket of sockets) { + socket.destroy(); + } }); } diff --git a/clients/storybook/src/tasks.js b/clients/storybook/src/tasks.js index b13d3c9..4cec525 100644 --- a/clients/storybook/src/tasks.js +++ b/clients/storybook/src/tasks.js @@ -106,7 +106,11 @@ export async function processTask(tab, task, deps = {}) { // Log timing breakdown in verbose mode if (verbose) { - let total = timings.viewport + timings.navigate + (timings.hook || 0) + timings.screenshot; + let total = + timings.viewport + + timings.navigate + + (timings.hook || 0) + + timings.screenshot; let hookStr = timings.hook ? ` hook=${timings.hook}ms` : ''; console.error( ` [timing] ${storyId}@${viewport.name}: viewport=${timings.viewport}ms nav=${timings.navigate}ms${hookStr} screenshot=${timings.screenshot}ms (total=${total}ms)` @@ -205,14 +209,14 @@ function createOutputCoordinator() { /** * Queue an error message to be printed * @param {string} message - Error message - * @param {Object} logger - Logger instance + * @param {Object} output - Output utilities */ - logError(message, logger) { + logError(message, output) { if (isInteractiveTTY()) { // Queue error to be printed before next progress update pendingErrors.push(message); } - logger.error(message); + output.error(message); }, /** @@ -236,18 +240,18 @@ function createOutputCoordinator() { * @param {Array} tasks - Array of task objects * @param {Object} pool - Tab pool { acquire, release } * @param {Object} config - Configuration object - * @param {Object} logger - Logger instance + * @param {Object} output - Output utilities * @param {Object} [deps] - Optional dependencies for testing * @returns {Promise} Array of errors encountered */ -export async function processAllTasks(tasks, pool, config, logger, deps = {}) { +export async function processAllTasks(tasks, pool, config, output, deps = {}) { let errors = []; let completed = 0; let total = tasks.length; let startTime = Date.now(); let taskTimes = []; let interactive = isInteractiveTTY(); - let output = createOutputCoordinator(); + let progressOutput = createOutputCoordinator(); // Minimum samples before showing ETA (avoids wild estimates from cold start) let minSamplesForEta = Math.min(5, Math.ceil(total * 0.1)); @@ -337,12 +341,12 @@ export async function processAllTasks(tasks, pool, config, logger, deps = {}) { if (interactive) { // Update single progress line - output.writeProgress( + progressOutput.writeProgress( ` šŸ“ø [${completed}/${total}] ${percent}% ${eta} - ${formatStory(task.story)}@${task.viewport.name}` ); } else { // Non-interactive: log each completion - logger.info( + output.info( ` āœ“ [${completed}/${total}] ${formatStory(task.story)}@${task.viewport.name} ${eta}` ); } @@ -359,9 +363,9 @@ export async function processAllTasks(tasks, pool, config, logger, deps = {}) { error: result.error.message + retryNote, }); - output.logError( + progressOutput.logError( ` āœ— ${formatStory(task.story)}@${task.viewport.name}: ${result.error.message}${retryNote}`, - logger + output ); if (result.tab) { @@ -373,16 +377,16 @@ export async function processAllTasks(tasks, pool, config, logger, deps = {}) { ); // Flush any remaining output - output.flush(); + progressOutput.flush(); // Log total time let totalTime = Date.now() - startTime; - logger.info( + output.info( ` āœ… Completed ${total} screenshots in ${formatDuration(totalTime)}` ); if (errors.length > 0) { - logger.warn(` āš ļø ${errors.length} failed`); + output.warn(` āš ļø ${errors.length} failed`); } return errors; diff --git a/clients/storybook/tests/concurrency.test.js b/clients/storybook/tests/concurrency.test.js index 1eb9a8d..21e4e32 100644 --- a/clients/storybook/tests/concurrency.test.js +++ b/clients/storybook/tests/concurrency.test.js @@ -5,27 +5,7 @@ import assert from 'node:assert/strict'; import { describe, it } from 'node:test'; -// Simple concurrency control - process items with limited parallelism -async function mapWithConcurrency(items, fn, concurrency) { - let results = []; - let executing = []; - - for (let item of items) { - let promise = fn(item).then(result => { - executing.splice(executing.indexOf(promise), 1); - return result; - }); - - results.push(promise); - executing.push(promise); - - if (executing.length >= concurrency) { - await Promise.race(executing); - } - } - - await Promise.all(results); -} +import { mapWithConcurrency } from '../src/tasks.js'; describe('mapWithConcurrency', () => { it('should process all items', async () => { @@ -54,7 +34,7 @@ describe('mapWithConcurrency', () => { async _item => { activeCount++; maxConcurrent = Math.max(maxConcurrent, activeCount); - await new Promise(resolve => setTimeout(resolve, 10)); + await new Promise(resolve => setImmediate(resolve)); activeCount--; }, 2 diff --git a/clients/storybook/tests/crawler.test.js b/clients/storybook/tests/crawler.test.js index b373fde..498893c 100644 --- a/clients/storybook/tests/crawler.test.js +++ b/clients/storybook/tests/crawler.test.js @@ -191,7 +191,10 @@ describe('filterStories', () => { ]; let filtered = filterStories(storiesWithBoth, config); - assert.equal(filtered.find(s => s.id === 'card--both-skipped'), undefined); + assert.equal( + filtered.find(s => s.id === 'card--both-skipped'), + undefined + ); }); it('should apply both include and skip filters', () => { diff --git a/clients/storybook/tests/navigation.test.js b/clients/storybook/tests/navigation.test.js index 7a92f01..28175a6 100644 --- a/clients/storybook/tests/navigation.test.js +++ b/clients/storybook/tests/navigation.test.js @@ -109,6 +109,51 @@ describe('navigateToStory', () => { assert.strictEqual(tab._poolEntry.currentStoryId, 'button--secondary'); }); + it('falls back to full navigation when Storybook never renders the story', async () => { + let originalWindow = globalThis.window; + let gotoCalls = []; + let listeners = new Map(); + + globalThis.window = { + __STORYBOOK_PREVIEW__: { + channel: { + on: (event, handler) => { + listeners.set(event, handler); + }, + off: event => { + listeners.delete(event); + }, + emit: () => {}, + }, + }, + }; + + let tab = { + _poolEntry: { + storybookInitialized: true, + currentStoryId: 'button--primary', + }, + goto: mock.fn(async url => { + gotoCalls.push(url); + }), + evaluate: mock.fn(async (fn, args) => fn(args)), + }; + + try { + await navigateToStory(tab, 'button--secondary', 'http://localhost:6006', { + timeout: 0, + }); + } finally { + globalThis.window = originalWindow; + } + + assert.strictEqual(tab.evaluate.mock.callCount(), 1); + assert.strictEqual(gotoCalls.length, 1); + assert.ok(gotoCalls[0].includes('button--secondary')); + assert.strictEqual(listeners.has('storyRendered'), false); + assert.strictEqual(tab._poolEntry.currentStoryId, 'button--secondary'); + }); + it('falls back to domcontentloaded on timeout', async () => { let gotoCalls = []; let callCount = 0; diff --git a/clients/storybook/tests/sdk-integration.test.js b/clients/storybook/tests/sdk-integration.test.js index adf4ca7..57cff66 100644 --- a/clients/storybook/tests/sdk-integration.test.js +++ b/clients/storybook/tests/sdk-integration.test.js @@ -13,13 +13,18 @@ import assert from 'node:assert'; import { execSync, spawn } from 'node:child_process'; +import { once } from 'node:events'; import { existsSync, mkdirSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { after, before, describe, it } from 'node:test'; import { closeBrowser, launchBrowser, navigateToUrl } from '../src/browser.js'; -import { discoverStories, generateStoryUrl, readIndexJson } from '../src/crawler.js'; +import { + discoverStories, + generateStoryUrl, + readIndexJson, +} from '../src/crawler.js'; import { getBeforeScreenshotHook, getStoryConfig } from '../src/hooks.js'; import { captureAndSendScreenshot } from '../src/screenshot.js'; import { startStaticServer, stopStaticServer } from '../src/server.js'; @@ -41,6 +46,15 @@ async function prepareStoryPage(browser, url, viewport) { return page; } +async function stopProcess(child) { + if (!child || child.exitCode !== null || child.signalCode !== null) { + return; + } + + child.kill('SIGTERM'); + await once(child, 'exit'); +} + // Paths let testDir = join(tmpdir(), `vizzly-storybook-e2e-${Date.now()}`); let exampleStorybookPath = resolve(import.meta.dirname, '../example-storybook'); @@ -57,14 +71,6 @@ describe('Storybook E2E with example-storybook', { skip: !runE2E }, () => { let serverInfo = null; let browser = null; - // Mock logger for tests (unused but kept for future use) - let _logger = { - info: () => {}, - warn: () => {}, - error: () => {}, - debug: () => {}, - }; - before(async () => { // Check if example-storybook is built, if not build it if (!existsSync(storybookBuildPath)) { @@ -134,11 +140,7 @@ describe('Storybook E2E with example-storybook', { skip: !runE2E }, () => { if (serverInfo) await stopStaticServer(serverInfo); if (tddServer && !externalServer) { - tddServer.kill('SIGTERM'); - await new Promise(resolve => { - tddServer.on('exit', resolve); - setTimeout(resolve, 2000); - }); + await stopProcess(tddServer); } if (!externalServer) { diff --git a/clients/storybook/tests/server.test.js b/clients/storybook/tests/server.test.js new file mode 100644 index 0000000..cb84065 --- /dev/null +++ b/clients/storybook/tests/server.test.js @@ -0,0 +1,39 @@ +import assert from 'node:assert/strict'; +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, it } from 'node:test'; + +import { startStaticServer, stopStaticServer } from '../src/server.js'; + +describe('storybook static server', () => { + let workspace; + let serverInfo; + + beforeEach(async () => { + workspace = await mkdtemp(join(tmpdir(), 'vizzly-storybook-server-')); + await mkdir(join(workspace, 'dist')); + await writeFile( + join(workspace, 'dist', 'index.html'), + '

Storybook

' + ); + }); + + afterEach(async () => { + await stopStaticServer(serverInfo); + await rm(workspace, { recursive: true, force: true }); + }); + + it('serves files and stops accepting requests after shutdown', async () => { + serverInfo = await startStaticServer(join(workspace, 'dist')); + + let response = await fetch(`${serverInfo.url}/index.html`); + assert.equal(response.status, 200); + assert.match(await response.text(), /Storybook/); + + await stopStaticServer(serverInfo); + serverInfo = null; + + await assert.rejects(fetch(response.url)); + }); +}); diff --git a/clients/storybook/tests/tasks.test.js b/clients/storybook/tests/tasks.test.js index 531dac5..cfd4b50 100644 --- a/clients/storybook/tests/tasks.test.js +++ b/clients/storybook/tests/tasks.test.js @@ -4,7 +4,7 @@ import assert from 'node:assert'; import { describe, it, mock } from 'node:test'; -import { generateTasks, processTask, processAllTasks } from '../src/tasks.js'; +import { generateTasks, processAllTasks, processTask } from '../src/tasks.js'; describe('generateTasks', () => { it('generates tasks for each story Ɨ viewport combination', () => { @@ -84,7 +84,9 @@ describe('generateTasks', () => { let tasks = generateTasks(stories, baseUrl, config, deps); // Tasks should be grouped by viewport - let viewportOrder = tasks.map(t => `${t.viewport.width}x${t.viewport.height}`); + let viewportOrder = tasks.map( + t => `${t.viewport.width}x${t.viewport.height}` + ); // Same viewports should be adjacent let desktopIndices = viewportOrder .map((v, i) => (v === '1920x1080' ? i : -1)) @@ -95,11 +97,15 @@ describe('generateTasks', () => { // All desktop tasks should be contiguous (indices are consecutive) assert.ok( - desktopIndices.every((idx, i) => i === 0 || idx === desktopIndices[i - 1] + 1) + desktopIndices.every( + (idx, i) => i === 0 || idx === desktopIndices[i - 1] + 1 + ) ); // All mobile tasks should be contiguous assert.ok( - mobileIndices.every((idx, i) => i === 0 || idx === mobileIndices[i - 1] + 1) + mobileIndices.every( + (idx, i) => i === 0 || idx === mobileIndices[i - 1] + 1 + ) ); }); diff --git a/clients/vitest/tests/e2e/example.test.js b/clients/vitest/tests/e2e/example.test.js index e9a3556..0185feb 100644 --- a/clients/vitest/tests/e2e/example.test.js +++ b/clients/vitest/tests/e2e/example.test.js @@ -25,7 +25,7 @@ beforeAll(async () => { document.head.appendChild(link); // Wait for CSS to load - await new Promise((resolve) => { + await new Promise(resolve => { link.onload = resolve; link.onerror = resolve; }); diff --git a/examples/custom-plugin/README.md b/examples/custom-plugin/README.md index d8d2fe6..4f1fdaf 100644 --- a/examples/custom-plugin/README.md +++ b/examples/custom-plugin/README.md @@ -8,7 +8,7 @@ This example plugin adds several commands to demonstrate different plugin capabi - `vizzly hello` - Simple greeting command - `vizzly greet ` - Command with arguments and options -- `vizzly check-api` - Command that accesses Vizzly services +- `vizzly check-services` - Command that accesses Vizzly services - `vizzly list-screenshots` - Command with file system operations ## Installation @@ -52,7 +52,7 @@ vizzly --help # Try the commands vizzly hello vizzly greet "World" --loud -vizzly check-api +vizzly check-services vizzly list-screenshots ``` @@ -87,7 +87,7 @@ let gitInfo = await git.detect({ buildPrefix: 'MyPlugin' }); // Build lifecycle let buildId = await testRunner.createBuild(options); -await testRunner.finalizeBuild(buildId, wait, success, executionTime); +await testRunner.finalizeBuild(buildId, isTddMode, success, executionTime); // Server control await serverManager.start(buildId, tddMode, setBaseline); @@ -97,14 +97,12 @@ await serverManager.stop(); ## Creating Your Own Plugin 1. Create a new directory for your plugin -2. Create `package.json` with `vizzly.plugin` field: +2. Create `package.json` with a `vizzlyPlugin` field: ```json { "name": "@vizzly-testing/my-plugin", - "vizzly": { - "plugin": "./plugin.js" - } + "vizzlyPlugin": "./plugin.js" } ``` @@ -114,12 +112,12 @@ await serverManager.stop(); export default { name: 'my-plugin', version: '1.0.0', - register(program, { config, logger, services }) { + register(program, { config, output, services }) { program .command('my-command') .description('My custom command') .action(() => { - logger.info('Running my command!'); + output.info('Running my command!'); }); } }; diff --git a/package.json b/package.json index a94ea56..2cacf5b 100644 --- a/package.json +++ b/package.json @@ -73,10 +73,10 @@ "test:swift:e2e": "npm run build && node clients/swift/scripts/run-e2e.js", "test:tui": "node --test --test-reporter=spec tests/tui/*.test.js", "test:tui:docker": "./tests/tui/run-tui-tests.sh", - "lint": "biome check src tests", - "lint:fix": "biome check --write --unsafe src tests", - "format": "biome format --write src tests", - "format:check": "biome format src tests", + "lint": "biome check src tests clients/storybook/src clients/storybook/tests clients/static-site/src clients/static-site/tests clients/vitest/src clients/vitest/tests clients/ember/src clients/ember/tests clients/ember/bin", + "lint:fix": "biome check --write --unsafe src tests clients/storybook/src clients/storybook/tests clients/static-site/src clients/static-site/tests clients/vitest/src clients/vitest/tests clients/ember/src clients/ember/tests clients/ember/bin", + "format": "biome format --write src tests clients/storybook/src clients/storybook/tests clients/static-site/src clients/static-site/tests clients/vitest/src clients/vitest/tests clients/ember/src clients/ember/tests clients/ember/bin", + "format:check": "biome format src tests clients/storybook/src clients/storybook/tests clients/static-site/src clients/static-site/tests clients/vitest/src clients/vitest/tests clients/ember/src clients/ember/tests clients/ember/bin", "fix": "npm run format && npm run lint:fix" }, "engines": { diff --git a/src/cli.js b/src/cli.js index 2aeb807..d487a6c 100644 --- a/src/cli.js +++ b/src/cli.js @@ -55,6 +55,7 @@ import { tddStartCommand, tddStatusCommand, tddStopCommand, + validateTddStartOptions, } from './commands/tdd-daemon.js'; import { uploadCommand, validateUploadOptions } from './commands/upload.js'; import { validateWhoamiOptions, whoamiCommand } from './commands/whoami.js'; @@ -65,6 +66,7 @@ import { generateStaticReport, getReportFileUrl, } from './services/static-report-generator.js'; +import { withTimeout } from './utils/async-utils.js'; import { openBrowser } from './utils/browser.js'; import { colors } from './utils/colors.js'; import { loadConfig } from './utils/config-loader.js'; @@ -376,24 +378,21 @@ let plugins = []; try { plugins = await loadPlugins(configPath, config); - for (const plugin of plugins) { + for (let plugin of plugins) { try { // Add timeout protection for plugin registration (5 seconds) - const registerPromise = plugin.register(program, { + let registerPromise = plugin.register(program, { config, services: pluginServices, output, - // Backwards compatibility alias for plugins using old API - logger: output, }); - const timeoutPromise = new Promise((_, reject) => - setTimeout( - () => reject(new Error('Plugin registration timeout (5s)')), - 5000 - ) + + await withTimeout( + registerPromise, + 5000, + 'Plugin registration timeout (5s)' ); - await Promise.race([registerPromise, timeoutPromise]); output.debug(`Registered plugin: ${plugin.name}`); } catch (error) { output.warn(`Failed to register plugin ${plugin.name}: ${error.message}`); @@ -418,15 +417,13 @@ program .argument('', 'Path to screenshots directory or file') .option('-b, --build-name ', 'Build name for grouping') .option('-m, --metadata ', 'Additional metadata as JSON') - .option('--batch-size ', 'Upload batch size', v => parseInt(v, 10)) - .option('--upload-timeout ', 'Upload timeout in milliseconds', v => - parseInt(v, 10) - ) + .option('--batch-size ', 'Upload batch size', Number) + .option('--upload-timeout ', 'Upload timeout in milliseconds', Number) .option('--branch ', 'Git branch') .option('--commit ', 'Git commit SHA') .option('--message ', 'Commit message') .option('--environment ', 'Environment name', 'test') - .option('--threshold ', 'Comparison threshold', parseFloat) + .option('--threshold ', 'Comparison threshold', Number) .option('--token ', 'API token override') .option('--wait', 'Wait for build completion') .option('--upload-all', 'Upload all screenshots without SHA deduplication') @@ -461,7 +458,12 @@ tddCmd .option('--baseline-build ', 'Use specific build as baseline') .option('--baseline-comparison ', 'Use specific comparison as baseline') .option('--environment ', 'Environment name', 'test') - .option('--threshold ', 'Comparison threshold', parseFloat) + .option('--threshold ', 'Comparison threshold', Number) + .option( + '--min-cluster-size ', + 'Minimum changed-pixel cluster size', + Number + ) .option('--timeout ', 'Server timeout in milliseconds', '30000') .option('--fail-on-diff', 'Fail tests when visual differences are detected') .option('--token ', 'API token override') @@ -475,6 +477,15 @@ tddCmd return; } + let validationErrors = validateTddStartOptions(options); + if (validationErrors.length > 0) { + output.error('Validation errors:'); + for (let error of validationErrors) { + output.printErr(` - ${error}`); + } + process.exit(1); + } + await tddStartCommand(options, globalOptions); }); @@ -512,7 +523,12 @@ tddCmd .option('--port ', 'Port for TDD server', '47392') .option('--branch ', 'Git branch override') .option('--environment ', 'Environment name', 'test') - .option('--threshold ', 'Comparison threshold', parseFloat) + .option('--threshold ', 'Comparison threshold', Number) + .option( + '--min-cluster-size ', + 'Minimum changed-pixel cluster size', + Number + ) .option('--token ', 'API token override') .option('--timeout ', 'Server timeout in milliseconds', '30000') .option('--baseline-build ', 'Use specific build as baseline') @@ -602,6 +618,14 @@ program .option('--commit ', 'Git commit SHA') .option('--message ', 'Commit message') .option('--environment ', 'Environment name', 'test') + .option('--threshold ', 'Comparison threshold', Number) + .option( + '--min-cluster-size ', + 'Minimum changed-pixel cluster size', + Number + ) + .option('--batch-size ', 'Upload batch size', Number) + .option('--upload-timeout ', 'Upload timeout in milliseconds', Number) .option('--token ', 'API token override') .option('--wait', 'Wait for build completion') .option('--timeout ', 'Server timeout in milliseconds', '30000') @@ -664,13 +688,8 @@ program .option('--environment ', 'Filter by environment') .option('-p, --project ', 'Filter by project slug') .option('--org ', 'Filter by organization slug') - .option( - '--limit ', - 'Maximum results to return (1-250)', - val => parseInt(val, 10), - 20 - ) - .option('--offset ', 'Skip first N results', val => parseInt(val, 10), 0) + .option('--limit ', 'Maximum results to return (1-250)', Number, 20) + .option('--offset ', 'Skip first N results', Number, 0) .option('--comparisons', 'Include comparisons when fetching a specific build') .addHelpText( 'after', @@ -710,13 +729,8 @@ program .option('--name ', 'Search comparisons by name (supports wildcards)') .option('--status ', 'Filter by status (identical, new, changed)') .option('--branch ', 'Filter by branch (for name search)') - .option( - '--limit ', - 'Maximum results to return (1-250)', - val => parseInt(val, 10), - 50 - ) - .option('--offset ', 'Skip first N results', val => parseInt(val, 10), 0) + .option('--limit ', 'Maximum results to return (1-250)', Number, 50) + .option('--offset ', 'Skip first N results', Number, 0) .option('-p, --project ', 'Filter by project slug') .option('--org ', 'Filter by organization slug') .addHelpText( @@ -789,17 +803,17 @@ contextCmd .option( '--similar-limit ', 'Maximum similar fingerprint matches to return (1-50)', - val => parseInt(val, 10) + Number ) .option( '--recent-limit ', 'Maximum recent same-name comparisons to return (1-50)', - val => parseInt(val, 10) + Number ) .option( '--window-size ', 'Historical hotspot analysis window size (1-50)', - val => parseInt(val, 10) + Number ) .addHelpText( 'after', @@ -835,12 +849,12 @@ contextCmd .option( '--recent-limit ', 'Maximum recent comparisons to return (1-50)', - val => parseInt(val, 10) + Number ) .option( '--window-size ', 'Historical hotspot analysis window size (1-50)', - val => parseInt(val, 10) + Number ) .addHelpText( 'after', @@ -873,9 +887,7 @@ contextCmd .option('--source ', 'Context source: auto, cloud, or local', 'auto') .option('-p, --project ', 'Project scope for user auth lookups') .option('--org ', 'Organization slug when project slug is ambiguous') - .option('--limit ', 'Maximum matches to return (1-50)', val => - parseInt(val, 10) - ) + .option('--limit ', 'Maximum matches to return (1-50)', Number) .addHelpText( 'after', ` @@ -905,10 +917,8 @@ contextCmd .option('--source ', 'Context source: auto, cloud, or local', 'auto') .option('-p, --project ', 'Project scope for user auth lookups') .option('--org ', 'Organization slug when project slug is ambiguous') - .option('--limit ', 'Maximum comparisons to return (1-100)', val => - parseInt(val, 10) - ) - .option('--offset ', 'Skip first N comparisons', val => parseInt(val, 10)) + .option('--limit ', 'Maximum comparisons to return (1-100)', Number) + .option('--offset ', 'Skip first N comparisons', Number) .addHelpText( 'after', ` @@ -1179,13 +1189,8 @@ program .command('projects') .description('List projects you have access to') .option('--org ', 'Filter by organization slug') - .option( - '--limit ', - 'Maximum results to return (1-250)', - val => parseInt(val, 10), - 50 - ) - .option('--offset ', 'Skip first N results', val => parseInt(val, 10), 0) + .option('--limit ', 'Maximum results to return (1-250)', Number, 50) + .option('--offset ', 'Skip first N results', Number, 0) .addHelpText( 'after', ` @@ -1240,7 +1245,6 @@ program .description('Upload static files as a preview for a build') .argument('[path]', 'Path to static files (dist/, build/, out/)') .option('-b, --build ', 'Build ID to attach preview to') - .option('-p, --parallel-id ', 'Look up build by parallel ID') .option('--base ', 'Override auto-detected base path') .option('--open', 'Open preview URL in browser after upload') .option('--dry-run', 'Show what would be uploaded without uploading') diff --git a/src/commands/doctor.js b/src/commands/doctor.js index 1755f32..ec0b511 100644 --- a/src/commands/doctor.js +++ b/src/commands/doctor.js @@ -1,24 +1,17 @@ import { URL } from 'node:url'; -import { createApiClient, getBuilds } from '../api/index.js'; -import { ConfigError } from '../errors/vizzly-error.js'; -import { loadConfig } from '../utils/config-loader.js'; -import { getContext } from '../utils/context.js'; -import { getApiToken } from '../utils/environment-config.js'; -import * as output from '../utils/output.js'; +import { + createApiClient as defaultCreateApiClient, + getBuilds as defaultGetBuilds, +} from '../api/index.js'; +import { loadConfig as defaultLoadConfig } from '../utils/config-loader.js'; +import { getContext as defaultGetContext } from '../utils/context.js'; +import { getApiToken as defaultGetApiToken } from '../utils/environment-config.js'; +import * as defaultOutput from '../utils/output.js'; -/** - * Doctor command implementation - Run diagnostics to check environment - * @param {Object} options - Command options - * @param {Object} globalOptions - Global CLI options - */ -export async function doctorCommand(options = {}, globalOptions = {}) { - output.configure({ - json: globalOptions.json, - verbose: globalOptions.verbose, - color: !globalOptions.noColor, - }); +export let MIN_NODE_MAJOR = 22; - let diagnostics = { +export function createDoctorDiagnostics() { + return { environment: { nodeVersion: null, nodeVersionValid: null, @@ -36,73 +29,144 @@ export async function doctorCommand(options = {}, globalOptions = {}) { error: null, }, }; +} + +export function getNodeVersionCheck( + nodeVersion = process.version, + minMajor = MIN_NODE_MAJOR +) { + let nodeMajor = parseNodeMajorVersion(nodeVersion); + let ok = nodeMajor !== null && nodeMajor >= minMajor; + let value = ok + ? `${nodeVersion} (supported)` + : nodeMajor === null + ? `${nodeVersion} (unrecognized Node.js version)` + : `${nodeVersion} (requires >= ${minMajor})`; + + return { + diagnostic: { + nodeVersion, + nodeVersionValid: ok, + }, + check: { + name: 'Node.js', + value, + ok, + }, + }; +} + +function parseNodeMajorVersion(nodeVersion) { + let match = /^v?(\d+)\.\d+\.\d+$/.exec(nodeVersion); + return match ? Number(match[1]) : null; +} + +export function getApiUrlCheck(apiUrl) { + try { + let url = new URL(apiUrl); + if (!['http:', 'https:'].includes(url.protocol)) { + throw new Error('URL must use http or https'); + } + + return { + apiUrl, + apiUrlValid: true, + check: { name: 'API URL', value: apiUrl, ok: true }, + }; + } catch { + return { + apiUrl, + apiUrlValid: false, + check: { + name: 'API URL', + value: 'invalid (check VIZZLY_API_URL)', + ok: false, + }, + }; + } +} + +export function getThresholdCheck(thresholdValue) { + let threshold = Number(thresholdValue); + // CIEDE2000 threshold: 0 = exact, 1 = JND, 2 = recommended, 3+ = permissive + let thresholdValid = Number.isFinite(threshold) && threshold >= 0; + + return { + threshold, + thresholdValid, + check: thresholdValid + ? { + name: 'Threshold', + value: `${threshold} (CIEDE2000)`, + ok: true, + } + : { name: 'Threshold', value: 'invalid', ok: false }, + }; +} + +/** + * Doctor command implementation - Run diagnostics to check environment + * @param {Object} options - Command options + * @param {Object} globalOptions - Global CLI options + * @param {Object} deps - Dependencies for testing + */ +export async function doctorCommand( + options = {}, + globalOptions = {}, + deps = {} +) { + let { + createApiClient = defaultCreateApiClient, + getApiToken = defaultGetApiToken, + getBuilds = defaultGetBuilds, + getContext = defaultGetContext, + loadConfig = defaultLoadConfig, + nodeVersion = process.version, + output = defaultOutput, + exit = code => process.exit(code), + } = deps; + + output.configure({ + json: globalOptions.json, + verbose: globalOptions.verbose, + color: !globalOptions.noColor, + }); + let diagnostics = createDoctorDiagnostics(); let hasErrors = false; let checks = []; try { // Determine if we'll attempt remote checks (API connectivity) - let willCheckConnectivity = Boolean(options.api || getApiToken()); + let hasApiToken = Boolean(getApiToken()); + let willCheckConnectivity = Boolean(options.api || hasApiToken); // Show header output.header('doctor', willCheckConnectivity ? 'full' : 'local'); - // Node.js version check (require >= 20) - let nodeVersion = process.version; - let nodeMajor = parseInt(nodeVersion.slice(1).split('.')[0], 10); - diagnostics.environment.nodeVersion = nodeVersion; - diagnostics.environment.nodeVersionValid = nodeMajor >= 20; - if (nodeMajor >= 20) { - checks.push({ - name: 'Node.js', - value: `${nodeVersion} (supported)`, - ok: true, - }); - } else { - checks.push({ - name: 'Node.js', - value: `${nodeVersion} (requires >= 20)`, - ok: false, - }); + let nodeCheck = getNodeVersionCheck(nodeVersion); + diagnostics.environment = nodeCheck.diagnostic; + checks.push(nodeCheck.check); + if (!nodeCheck.check.ok) { hasErrors = true; } // Load configuration (apply global CLI overrides like --config only) let config = await loadConfig(globalOptions.config); - // Validate apiUrl - diagnostics.configuration.apiUrl = config.apiUrl; - try { - let url = new URL(config.apiUrl); - if (!['http:', 'https:'].includes(url.protocol)) { - throw new ConfigError('URL must use http or https'); - } - diagnostics.configuration.apiUrlValid = true; - checks.push({ name: 'API URL', value: config.apiUrl, ok: true }); - } catch (_e) { - diagnostics.configuration.apiUrlValid = false; - checks.push({ - name: 'API URL', - value: 'invalid (check VIZZLY_API_URL)', - ok: false, - }); + let apiUrlCheck = getApiUrlCheck(config.apiUrl); + diagnostics.configuration.apiUrl = apiUrlCheck.apiUrl; + diagnostics.configuration.apiUrlValid = apiUrlCheck.apiUrlValid; + checks.push(apiUrlCheck.check); + if (!apiUrlCheck.check.ok) { hasErrors = true; } - // Validate threshold (0..1 inclusive) - let threshold = Number(config?.comparison?.threshold); - diagnostics.configuration.threshold = threshold; - // CIEDE2000 threshold: 0 = exact, 1 = JND, 2 = recommended, 3+ = permissive - let thresholdValid = Number.isFinite(threshold) && threshold >= 0; - diagnostics.configuration.thresholdValid = thresholdValid; - if (thresholdValid) { - checks.push({ - name: 'Threshold', - value: `${threshold} (CIEDE2000)`, - ok: true, - }); - } else { - checks.push({ name: 'Threshold', value: 'invalid', ok: false }); + let thresholdCheck = getThresholdCheck(config?.comparison?.threshold); + diagnostics.configuration.threshold = thresholdCheck.threshold; + diagnostics.configuration.thresholdValid = thresholdCheck.thresholdValid; + checks.push(thresholdCheck.check); + if (!thresholdCheck.check.ok) { hasErrors = true; } @@ -112,8 +176,7 @@ export async function doctorCommand(options = {}, globalOptions = {}) { checks.push({ name: 'Port', value: String(port), ok: true }); // Optional: API connectivity check when --api is provided or VIZZLY_TOKEN is present - let autoApi = Boolean(getApiToken()); - if (options.api || autoApi) { + if (willCheckConnectivity) { diagnostics.connectivity.checked = true; if (!config.apiKey) { diagnostics.connectivity.ok = false; @@ -214,7 +277,7 @@ export async function doctorCommand(options = {}, globalOptions = {}) { output.error('Failed to run preflight', error); } finally { output.cleanup(); - if (hasErrors) process.exit(1); + if (hasErrors) exit(1); } } diff --git a/src/commands/preview.js b/src/commands/preview.js index ae79c12..2bb02d4 100644 --- a/src/commands/preview.js +++ b/src/commands/preview.js @@ -5,10 +5,10 @@ * The build is automatically detected from session file or environment. */ -import { exec, execSync } from 'node:child_process'; +import { execFile, execFileSync } from 'node:child_process'; import { randomBytes } from 'node:crypto'; import { existsSync, statSync } from 'node:fs'; -import { readFile, realpath, stat, unlink } from 'node:fs/promises'; +import { readdir, readFile, realpath, stat, unlink } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { promisify } from 'node:util'; @@ -21,27 +21,17 @@ import { openBrowser as defaultOpenBrowser } from '../utils/browser.js'; import { loadConfig as defaultLoadConfig } from '../utils/config-loader.js'; import { detectBranch as defaultDetectBranch } from '../utils/git.js'; import * as defaultOutput from '../utils/output.js'; +import { createWildcardMatcher } from '../utils/patterns.js'; import { formatSessionAge as defaultFormatSessionAge, readSession as defaultReadSession, } from '../utils/session.js'; -let execAsync = promisify(exec); +let execFileAsync = promisify(execFile); // Maximum files to show in dry-run output (use --verbose for all) let DRY_RUN_FILE_LIMIT = 50; -/** - * Validate path for shell safety - prevents command injection - * @param {string} path - Path to validate - * @returns {boolean} true if path is safe for shell use - */ -function isPathSafe(path) { - // Reject paths with shell metacharacters that could enable command injection - let dangerousChars = /[`$;&|<>(){}[\]\\!*?'"]/; - return !dangerousChars.test(path); -} - /** * Check if a command exists on the system * @param {string} command - Command to check @@ -50,13 +40,17 @@ function isPathSafe(path) { function commandExists(command) { try { let checkCmd = process.platform === 'win32' ? 'where' : 'which'; - execSync(`${checkCmd} ${command}`, { stdio: 'ignore' }); + execFileSync(checkCmd, [command], { stdio: 'ignore' }); return true; } catch { return false; } } +function quotePowerShellString(value) { + return `'${String(value).replace(/'/g, "''")}'`; +} + /** * Get the appropriate zip command for the current platform * @returns {{ command: string, available: boolean }} @@ -134,11 +128,8 @@ let DEFAULT_EXCLUDED_FILES = [ function matchesPattern(filename, patterns) { for (let pattern of patterns) { if (pattern.includes('*')) { - // Simple glob matching - convert to regex - let regex = new RegExp( - `^${pattern.replace(/\./g, '\\.').replace(/\*/g, '.*')}$` - ); - if (regex.test(filename)) return true; + let matches = createWildcardMatcher(pattern, { anchored: true }); + if (matches(filename)) return true; } else { if (filename === pattern) return true; } @@ -165,68 +156,60 @@ async function createZipWithSystem(sourceDir, outputPath, exclusions = {}) { ); } - // Validate paths to prevent command injection - // Note: outputPath is internally generated (tmpdir + random), so always safe - // sourceDir comes from user input, so we validate it - if (!isPathSafe(sourceDir)) { - throw new Error( - 'Path contains unsupported characters. Please use a path without special shell characters.' - ); - } - - // Validate exclusion patterns to prevent command injection - // Only allow safe characters in patterns: alphanumeric, dots, asterisks, underscores, hyphens, slashes - let safePatternRegex = /^[a-zA-Z0-9.*_\-/]+$/; - for (let pattern of [...dirs, ...files]) { - if (!safePatternRegex.test(pattern)) { - throw new Error( - `Exclusion pattern contains unsafe characters: ${pattern}. Only alphanumeric, ., *, _, -, / are allowed.` - ); - } - } - if (command === 'zip') { // Standard zip command - create ZIP from directory contents - // Using cwd option is safe as it's not part of the command string // -r: recursive, -q: quiet, -x: exclude patterns - let excludeArgs = [ - ...dirs.map(dir => `-x "${dir}/*"`), - ...files.map(pattern => `-x "${pattern}"`), - ].join(' '); - await execAsync(`zip -r -q "${outputPath}" . ${excludeArgs}`, { + let excludeArgs = []; + for (let dir of dirs) { + excludeArgs.push('-x', `${dir}/*`); + } + for (let pattern of files) { + excludeArgs.push('-x', pattern); + } + + await execFileAsync('zip', ['-r', '-q', outputPath, '.', ...excludeArgs], { cwd: sourceDir, maxBuffer: 1024 * 1024 * 100, // 100MB buffer }); } else if (command === 'powershell') { // Windows PowerShell - Compress-Archive doesn't support exclusions, // so we create a temp directory with only the files we want - let safeSrcDir = sourceDir.replace(/'/g, "''"); - let safeOutPath = outputPath.replace(/'/g, "''"); + let safeSrcDir = quotePowerShellString(sourceDir); + let safeOutPath = quotePowerShellString(outputPath); // Build exclusion filter for PowerShell // We use Get-ChildItem with -Exclude and pipe to Compress-Archive - let excludePatterns = [...dirs, ...files].map(p => `'${p}'`).join(','); + let excludePatterns = [...dirs, ...files] + .map(quotePowerShellString) + .join(','); if (excludePatterns) { // Use robocopy to copy files excluding patterns, then zip // This is more reliable than PowerShell's native filtering - await execAsync( - `powershell -Command "` + - `$src = '${safeSrcDir}'; ` + - `$dst = '${safeOutPath}'; ` + - `$exclude = @(${excludePatterns}); ` + - `$items = Get-ChildItem -Path $src -Recurse -File | Where-Object { ` + - `$rel = $_.FullName.Substring($src.Length + 1); ` + - `$dominated = $false; ` + - `foreach ($ex in $exclude) { if ($rel -like $ex -or $rel -like \\"$ex/*\\" -or $_.Name -like $ex) { $dominated = $true; break } }; ` + - `-not $dominated ` + - `}; ` + - `if ($items) { $items | Compress-Archive -DestinationPath $dst -Force }"`, + await execFileAsync( + 'powershell', + [ + '-Command', + `$src = ${safeSrcDir}; ` + + `$dst = ${safeOutPath}; ` + + `$exclude = @(${excludePatterns}); ` + + `$items = Get-ChildItem -Path $src -Recurse -File | Where-Object { ` + + `$rel = $_.FullName.Substring($src.Length + 1); ` + + `$dominated = $false; ` + + `foreach ($ex in $exclude) { if ($rel -like $ex -or $rel -like "$ex/*" -or $_.Name -like $ex) { $dominated = $true; break } }; ` + + `-not $dominated ` + + `}; ` + + `if ($items) { $items | Compress-Archive -DestinationPath $dst -Force }`, + ], { maxBuffer: 1024 * 1024 * 100 } ); } else { - await execAsync( - `powershell -Command "Compress-Archive -LiteralPath '${safeSrcDir}\\*' -DestinationPath '${safeOutPath}' -Force"`, + await execFileAsync( + 'powershell', + [ + '-Command', + `Compress-Archive -LiteralPath ${quotePowerShellString(`${sourceDir}\\*`)} -DestinationPath ${safeOutPath} -Force`, + ], { maxBuffer: 1024 * 1024 * 100 } ); } @@ -244,7 +227,6 @@ async function createZipWithSystem(sourceDir, outputPath, exclusions = {}) { * @returns {Promise<{ count: number, totalSize: number, files?: Array<{path: string, size: number}> }>} */ async function countFiles(dir, options = {}) { - let { readdir } = await import('node:fs/promises'); let { collectPaths = false, excludedDirs = DEFAULT_EXCLUDED_DIRS, @@ -385,15 +367,6 @@ export async function previewCommand( let buildId = options.build; let buildSource = 'flag'; - if (!buildId && options.parallelId) { - // TODO: Look up build by parallel ID - output.error( - 'Parallel ID lookup not yet implemented. Use --build to specify build ID directly.' - ); - exit(1); - return { success: false, reason: 'parallel-id-not-implemented' }; - } - if (!buildId) { // Try to read from session let currentBranch = await detectBranch(); @@ -654,11 +627,10 @@ export async function previewCommand( await unlink(zipPath).catch(() => {}); } - let compressionRatio = ((1 - zipBuffer.length / totalSize) * 100).toFixed( - 0 - ); + let compressionRatio = 1 - zipBuffer.length / totalSize; + let compressionPercent = Math.round(compressionRatio * 100); output.updateSpinner( - `Compressed to ${formatBytes(zipBuffer.length)} (${compressionRatio}% smaller)` + `Compressed to ${formatBytes(zipBuffer.length)} (${compressionPercent}% smaller)` ); // Upload (reuse client created earlier) @@ -675,7 +647,7 @@ export async function previewCommand( files: result.uploaded || fileCount, bytes: totalSize, compressedBytes: zipBuffer.length, - compressionRatio: parseFloat(compressionRatio) / 100, + compressionRatio, newBytes: result.newBytes, reusedBlobs: result.reusedBlobs || 0, deduplicationRatio: result.deduplicationRatio, diff --git a/src/commands/run.js b/src/commands/run.js index 17ef93a..3185613 100644 --- a/src/commands/run.js +++ b/src/commands/run.js @@ -14,11 +14,12 @@ import { import { VizzlyError } from '../errors/vizzly-error.js'; import { createServerManager as defaultCreateServerManager } from '../server-manager/index.js'; import { createBuildObject as defaultCreateBuildObject } from '../services/build-manager.js'; -import { createUploader as defaultCreateUploader } from '../services/uploader.js'; import { finalizeBuild as defaultFinalizeBuild, runTests as defaultRunTests, } from '../test-runner/index.js'; +import { createUploader as defaultCreateUploader } from '../uploader/index.js'; +import { getAppBaseUrl } from '../utils/api-url.js'; import { loadConfig as defaultLoadConfig } from '../utils/config-loader.js'; import { detectBranch as defaultDetectBranch, @@ -30,6 +31,39 @@ import { import * as defaultOutput from '../utils/output.js'; import { writeSession as defaultWriteSession } from '../utils/session.js'; +export async function resolveBuildDisplayUrl({ + result, + config, + createApiClient = defaultCreateApiClient, + getTokenContext = defaultGetTokenContext, +}) { + if (result.url) { + return result.url; + } + + if (!config.apiKey) { + return undefined; + } + + let baseUrl = getAppBaseUrl(config.apiUrl); + + try { + let client = createApiClient({ + baseUrl: config.apiUrl, + token: config.apiKey, + command: 'run', + }); + let tokenContext = await getTokenContext(client); + if (tokenContext.organization?.slug && tokenContext.project?.slug) { + return `${baseUrl}/${tokenContext.organization.slug}/${tokenContext.project.slug}/builds/${result.buildId}`; + } + } catch { + return `${baseUrl}/builds/${result.buildId}`; + } + + return undefined; +} + /** * Run command implementation * @param {string} testCommand - Test command to execute @@ -311,27 +345,12 @@ export async function runCommand( // JSON output mode - output structured data and exit if (globalOptions.json) { let executionTimeMs = Date.now() - startTime; - - // Get URL from result, or construct one as fallback - let displayUrl = result.url; - if (!displayUrl && config.apiKey) { - try { - let client = createApiClient({ - baseUrl: config.apiUrl, - token: config.apiKey, - command: 'run', - }); - let tokenContext = await getTokenContext(client); - let baseUrl = config.apiUrl.replace(/\/api.*$/, ''); - if (tokenContext.organization?.slug && tokenContext.project?.slug) { - displayUrl = `${baseUrl}/${tokenContext.organization.slug}/${tokenContext.project.slug}/builds/${result.buildId}`; - } - } catch { - // Fallback to simple URL if context fetch fails - let baseUrl = config.apiUrl.replace(/\/api.*$/, ''); - displayUrl = `${baseUrl}/builds/${result.buildId}`; - } - } + let displayUrl = await resolveBuildDisplayUrl({ + result, + config, + createApiClient, + getTokenContext, + }); let jsonResult = { buildId: result.buildId, @@ -362,26 +381,12 @@ export async function runCommand( ` ${colors.brand.textTertiary('Screenshots')} ${colors.white(result.screenshotsCaptured)}` ); - // Get URL from result, or construct one as fallback - let displayUrl = result.url; - if (!displayUrl && config.apiKey) { - try { - let client = createApiClient({ - baseUrl: config.apiUrl, - token: config.apiKey, - command: 'run', - }); - let tokenContext = await getTokenContext(client); - let baseUrl = config.apiUrl.replace(/\/api.*$/, ''); - if (tokenContext.organization?.slug && tokenContext.project?.slug) { - displayUrl = `${baseUrl}/${tokenContext.organization.slug}/${tokenContext.project.slug}/builds/${result.buildId}`; - } - } catch { - // Fallback to simple URL if context fetch fails - let baseUrl = config.apiUrl.replace(/\/api.*$/, ''); - displayUrl = `${baseUrl}/builds/${result.buildId}`; - } - } + let displayUrl = await resolveBuildDisplayUrl({ + result, + config, + createApiClient, + getTokenContext, + }); if (displayUrl) { output.print( @@ -404,7 +409,7 @@ export async function runCommand( ) { // Extract exit code from error message if available let exitCodeMatch = error.message.match(/exited with code (\d+)/); - let exitCode = exitCodeMatch ? parseInt(exitCodeMatch[1], 10) : 1; + let exitCode = exitCodeMatch ? Number(exitCodeMatch[1]) : 1; // JSON output for test command failure if (globalOptions.json) { @@ -462,29 +467,12 @@ export async function runCommand( // JSON output for --wait mode if (globalOptions.json) { let executionTimeMs = Date.now() - startTime; - - // Get URL from result, or construct one as fallback - let displayUrl = result.url; - if (!displayUrl && config.apiKey) { - try { - let client = createApiClient({ - baseUrl: config.apiUrl, - token: config.apiKey, - command: 'run', - }); - let tokenContext = await getTokenContext(client); - let baseUrl = config.apiUrl.replace(/\/api.*$/, ''); - if ( - tokenContext.organization?.slug && - tokenContext.project?.slug - ) { - displayUrl = `${baseUrl}/${tokenContext.organization.slug}/${tokenContext.project.slug}/builds/${result.buildId}`; - } - } catch { - let baseUrl = config.apiUrl.replace(/\/api.*$/, ''); - displayUrl = `${baseUrl}/builds/${result.buildId}`; - } - } + let displayUrl = await resolveBuildDisplayUrl({ + result, + config, + createApiClient, + getTokenContext, + }); let exitCode = buildResult.failedComparisons > 0 ? 1 : 0; let jsonResult = { @@ -573,32 +561,48 @@ export function validateRunOptions(testCommand, options) { } if (options.port) { - let port = parseInt(options.port, 10); - if (Number.isNaN(port) || port < 1 || port > 65535) { + let port = Number(options.port); + if (!Number.isInteger(port) || port < 1 || port > 65535) { errors.push('Port must be a valid number between 1 and 65535'); } } if (options.timeout) { - let timeout = parseInt(options.timeout, 10); - if (Number.isNaN(timeout) || timeout < 1000) { + let timeout = Number(options.timeout); + if (!Number.isInteger(timeout) || timeout < 1000) { errors.push('Timeout must be at least 1000 milliseconds'); } } if (options.batchSize !== undefined) { - let n = parseInt(options.batchSize, 10); - if (!Number.isFinite(n) || n <= 0) { + let n = Number(options.batchSize); + if (!Number.isInteger(n) || n <= 0) { errors.push('Batch size must be a positive integer'); } } if (options.uploadTimeout !== undefined) { - let n = parseInt(options.uploadTimeout, 10); - if (!Number.isFinite(n) || n <= 0) { + let n = Number(options.uploadTimeout); + if (!Number.isInteger(n) || n <= 0) { errors.push('Upload timeout must be a positive integer (milliseconds)'); } } + if (options.threshold !== undefined) { + let threshold = Number(options.threshold); + if (!Number.isFinite(threshold) || threshold < 0) { + errors.push( + 'Threshold must be a non-negative number (CIEDE2000 Delta E)' + ); + } + } + + if (options.minClusterSize !== undefined) { + let minClusterSize = Number(options.minClusterSize); + if (!Number.isInteger(minClusterSize) || minClusterSize < 1) { + errors.push('Min cluster size must be a positive integer'); + } + } + return errors; } diff --git a/src/commands/status.js b/src/commands/status.js index 5d3028c..abff6c3 100644 --- a/src/commands/status.js +++ b/src/commands/status.js @@ -8,10 +8,249 @@ import { getBuild as defaultGetBuild, getPreviewInfo as defaultGetPreviewInfo, } from '../api/index.js'; +import { getAppBaseUrl } from '../utils/api-url.js'; import { loadConfig as defaultLoadConfig } from '../utils/config-loader.js'; import { getApiUrl as defaultGetApiUrl } from '../utils/environment-config.js'; import * as defaultOutput from '../utils/output.js'; +function createStatusDeps(deps = {}) { + return { + loadConfig: deps.loadConfig || defaultLoadConfig, + createApiClient: deps.createApiClient || defaultCreateApiClient, + getBuild: deps.getBuild || defaultGetBuild, + getPreviewInfo: deps.getPreviewInfo || defaultGetPreviewInfo, + getApiUrl: deps.getApiUrl || defaultGetApiUrl, + output: deps.output || defaultOutput, + exit: deps.exit || (code => process.exit(code)), + }; +} + +function configureOutput(output, globalOptions) { + output.configure({ + json: globalOptions.json, + verbose: globalOptions.verbose, + color: !globalOptions.noColor, + }); +} + +function createStatusClient({ createApiClient, config }) { + return createApiClient({ + baseUrl: config.apiUrl, + token: config.apiKey, + command: 'status', + }); +} + +export function normalizeBuildStatus(buildStatus) { + return buildStatus.build || buildStatus; +} + +export function createStatusData(build, previewInfo = null) { + return { + buildId: build.id, + status: build.status, + name: build.name, + createdAt: build.created_at, + updatedAt: build.updated_at, + completedAt: build.completed_at, + environment: build.environment, + branch: build.branch, + commit: build.commit_sha, + commitMessage: build.commit_message, + screenshotsTotal: build.screenshot_count || 0, + comparisonsTotal: build.total_comparisons || 0, + newComparisons: build.new_comparisons || 0, + changedComparisons: build.changed_comparisons || 0, + identicalComparisons: build.identical_comparisons || 0, + approvalStatus: build.approval_status, + executionTime: build.execution_time_ms, + isBaseline: build.is_baseline, + userAgent: build.user_agent, + preview: previewInfo + ? { + url: previewInfo.preview_url, + status: previewInfo.status, + fileCount: previewInfo.file_count, + expiresAt: previewInfo.expires_at, + } + : null, + }; +} + +export function createBuildInfo(build) { + let buildInfo = { + Name: build.name || build.id, + Status: build.status.toUpperCase(), + Environment: build.environment, + }; + + if (build.branch) { + buildInfo.Branch = build.branch; + } + + if (build.commit_sha) { + buildInfo.Commit = `${build.commit_sha.substring(0, 8)} - ${build.commit_message || 'No message'}`; + } + + return buildInfo; +} + +export function createComparisonStats(build, colors) { + let stats = []; + let newCount = build.new_comparisons || 0; + let changedCount = build.changed_comparisons || 0; + let identicalCount = build.identical_comparisons || 0; + + if (newCount > 0) { + stats.push(`${colors.brand.info(newCount)} new`); + } + if (changedCount > 0) { + stats.push(`${colors.brand.warning(changedCount)} changed`); + } + if (identicalCount > 0) { + stats.push(`${colors.brand.success(identicalCount)} identical`); + } + + return stats.join(colors.brand.textMuted(' Ā· ')); +} + +export function createBuildUrl(baseUrl, build) { + if (!baseUrl || !build.project_id) { + return null; + } + + return `${getAppBaseUrl(baseUrl)}/projects/${build.project_id}/builds/${build.id}`; +} + +export function shouldFailStatus(build) { + return build.status === 'failed' || build.failed_jobs > 0; +} + +export function getProcessingProgress(build) { + if (build.status !== 'processing' && build.status !== 'pending') { + return null; + } + + let completedJobs = build.completed_jobs || 0; + let failedJobs = build.failed_jobs || 0; + let processingScreenshots = build.processing_screenshots || 0; + let totalJobs = completedJobs + failedJobs + processingScreenshots; + + if (totalJobs <= 0) { + return null; + } + + return ((completedJobs + failedJobs) / totalJobs) * 100; +} + +function writeStatusTiming({ build, output }) { + if (build.created_at) { + output.hint(`Created ${new Date(build.created_at).toLocaleString()}`); + } + + if (build.completed_at) { + output.hint(`Completed ${new Date(build.completed_at).toLocaleString()}`); + } else if (build.status !== 'completed' && build.status !== 'failed') { + output.hint( + `Started ${new Date(build.started_at || build.created_at).toLocaleString()}` + ); + } + + if (build.execution_time_ms) { + output.hint(`Took ${Math.round(build.execution_time_ms / 1000)}s`); + } +} + +function createVerboseInfo(build) { + let verboseInfo = {}; + + if ( + build.approved_screenshots > 0 || + build.rejected_screenshots > 0 || + build.pending_screenshots > 0 + ) { + verboseInfo.Approvals = `${build.approved_screenshots || 0} approved, ${build.rejected_screenshots || 0} rejected, ${build.pending_screenshots || 0} pending`; + } + + if (build.avg_diff_percentage != null) { + verboseInfo['Avg Diff'] = + `${(build.avg_diff_percentage * 100).toFixed(2)}%`; + } + + if (build.github_pull_request_number) { + verboseInfo['GitHub PR'] = `#${build.github_pull_request_number}`; + } + + if (build.is_baseline) { + verboseInfo.Baseline = 'Yes'; + } + + verboseInfo['User Agent'] = build.user_agent || 'Unknown'; + verboseInfo['Build ID'] = build.id; + verboseInfo['Project ID'] = build.project_id; + + return verboseInfo; +} + +function writeHumanStatus({ + build, + buildUrl, + globalOptions, + output, + previewInfo, +}) { + output.header('status', build.status); + output.keyValue(createBuildInfo(build)); + output.blank(); + + let colors = output.getColors(); + let comparisonStats = createComparisonStats(build, colors); + + output.labelValue('Screenshots', String(build.screenshot_count || 0)); + + if (comparisonStats) { + output.labelValue('Comparisons', comparisonStats); + } + + if (build.approval_status) { + output.labelValue('Approval', build.approval_status); + } + + output.blank(); + writeStatusTiming({ build, output }); + + if (buildUrl) { + output.blank(); + output.labelValue('View', output.link('Build', buildUrl)); + } + + if (previewInfo?.preview_url) { + output.labelValue( + 'Preview', + output.link('Preview', previewInfo.preview_url) + ); + if (previewInfo.expires_at) { + let expiresDate = new Date(previewInfo.expires_at); + output.hint(`Preview expires ${expiresDate.toLocaleDateString()}`); + } + } + + if (globalOptions.verbose) { + output.blank(); + output.divider(); + output.blank(); + output.keyValue(createVerboseInfo(build)); + } + + let progress = getProcessingProgress(build); + if (progress !== null) { + output.blank(); + output.print( + ` ${output.progressBar(progress, 100)} ${Math.round(progress)}%` + ); + } +} + /** * Status command implementation * @param {string} buildId - Build ID to check status for @@ -26,20 +265,16 @@ export async function statusCommand( deps = {} ) { let { - loadConfig = defaultLoadConfig, - createApiClient = defaultCreateApiClient, - getBuild = defaultGetBuild, - getPreviewInfo = defaultGetPreviewInfo, - getApiUrl = defaultGetApiUrl, - output = defaultOutput, - exit = code => process.exit(code), - } = deps; + loadConfig, + createApiClient, + getBuild, + getPreviewInfo, + getApiUrl, + output, + exit, + } = createStatusDeps(deps); - output.configure({ - json: globalOptions.json, - verbose: globalOptions.verbose, - color: !globalOptions.noColor, - }); + configureOutput(output, globalOptions); try { // Load configuration with CLI overrides @@ -51,16 +286,14 @@ export async function statusCommand( output.error( 'API token required. Use --token or set VIZZLY_TOKEN environment variable' ); + output.cleanup(); exit(1); + return { success: false, result: { reason: 'missing_token' } }; } // Get build details via functional API output.startSpinner('Fetching build status...'); - let client = createApiClient({ - baseUrl: config.apiUrl, - token: config.apiKey, - command: 'status', - }); + let client = createStatusClient({ createApiClient, config }); let buildStatus = await getBuild(client, buildId); // Also fetch preview info (if exists) @@ -68,193 +301,30 @@ export async function statusCommand( output.stopSpinner(); // Extract build data from API response - let build = buildStatus.build || buildStatus; + let build = normalizeBuildStatus(buildStatus); // Output in JSON mode if (globalOptions.json) { - let statusData = { - buildId: build.id, - status: build.status, - name: build.name, - createdAt: build.created_at, - updatedAt: build.updated_at, - completedAt: build.completed_at, - environment: build.environment, - branch: build.branch, - commit: build.commit_sha, - commitMessage: build.commit_message, - screenshotsTotal: build.screenshot_count || 0, - comparisonsTotal: build.total_comparisons || 0, - newComparisons: build.new_comparisons || 0, - changedComparisons: build.changed_comparisons || 0, - identicalComparisons: build.identical_comparisons || 0, - approvalStatus: build.approval_status, - executionTime: build.execution_time_ms, - isBaseline: build.is_baseline, - userAgent: build.user_agent, - preview: previewInfo - ? { - url: previewInfo.preview_url, - status: previewInfo.status, - fileCount: previewInfo.file_count, - expiresAt: previewInfo.expires_at, - } - : null, - }; - output.data(statusData); + output.data(createStatusData(build, previewInfo)); output.cleanup(); return; } // Human-readable output - output.header('status', build.status); - - // Build info section - let buildInfo = { - Name: build.name || build.id, - Status: build.status.toUpperCase(), - Environment: build.environment, - }; - - if (build.branch) { - buildInfo.Branch = build.branch; - } - - if (build.commit_sha) { - buildInfo.Commit = `${build.commit_sha.substring(0, 8)} - ${build.commit_message || 'No message'}`; - } - - output.keyValue(buildInfo); - output.blank(); - - // Comparison stats with visual indicators - let colors = output.getColors(); - let stats = []; - let newCount = build.new_comparisons || 0; - let changedCount = build.changed_comparisons || 0; - let identicalCount = build.identical_comparisons || 0; - let screenshotCount = build.screenshot_count || 0; - - output.labelValue('Screenshots', String(screenshotCount)); - - if (newCount > 0) { - stats.push(`${colors.brand.info(newCount)} new`); - } - if (changedCount > 0) { - stats.push(`${colors.brand.warning(changedCount)} changed`); - } - if (identicalCount > 0) { - stats.push(`${colors.brand.success(identicalCount)} identical`); - } - - if (stats.length > 0) { - output.labelValue( - 'Comparisons', - stats.join(colors.brand.textMuted(' Ā· ')) - ); - } - - if (build.approval_status) { - output.labelValue('Approval', build.approval_status); - } - - output.blank(); - - // Timing info - if (build.created_at) { - output.hint(`Created ${new Date(build.created_at).toLocaleString()}`); - } - - if (build.completed_at) { - output.hint(`Completed ${new Date(build.completed_at).toLocaleString()}`); - } else if (build.status !== 'completed' && build.status !== 'failed') { - output.hint( - `Started ${new Date(build.started_at || build.created_at).toLocaleString()}` - ); - } - - if (build.execution_time_ms) { - output.hint(`Took ${Math.round(build.execution_time_ms / 1000)}s`); - } - // Show build URL if we can construct it let baseUrl = config.baseUrl || getApiUrl(); - if (baseUrl && build.project_id) { - let buildUrl = - baseUrl.replace('/api', '') + - `/projects/${build.project_id}/builds/${build.id}`; - output.blank(); - output.labelValue('View', output.link('Build', buildUrl)); - } - - // Show preview URL if available - if (previewInfo?.preview_url) { - output.labelValue( - 'Preview', - output.link('Preview', previewInfo.preview_url) - ); - if (previewInfo.expires_at) { - let expiresDate = new Date(previewInfo.expires_at); - output.hint(`Preview expires ${expiresDate.toLocaleDateString()}`); - } - } - - // Show additional info in verbose mode - if (globalOptions.verbose) { - output.blank(); - output.divider(); - output.blank(); - - let verboseInfo = {}; - - if ( - build.approved_screenshots > 0 || - build.rejected_screenshots > 0 || - build.pending_screenshots > 0 - ) { - verboseInfo.Approvals = `${build.approved_screenshots || 0} approved, ${build.rejected_screenshots || 0} rejected, ${build.pending_screenshots || 0} pending`; - } - - if (build.avg_diff_percentage !== null) { - verboseInfo['Avg Diff'] = - `${(build.avg_diff_percentage * 100).toFixed(2)}%`; - } - - if (build.github_pull_request_number) { - verboseInfo['GitHub PR'] = `#${build.github_pull_request_number}`; - } - - if (build.is_baseline) { - verboseInfo.Baseline = 'Yes'; - } - - verboseInfo['User Agent'] = build.user_agent || 'Unknown'; - verboseInfo['Build ID'] = build.id; - verboseInfo['Project ID'] = build.project_id; - - output.keyValue(verboseInfo); - } - - // Show progress if build is still processing - if (build.status === 'processing' || build.status === 'pending') { - let totalJobs = - build.completed_jobs + build.failed_jobs + build.processing_screenshots; - if (totalJobs > 0) { - let progress = (build.completed_jobs + build.failed_jobs) / totalJobs; - output.blank(); - output.print( - ` ${output.progressBar(progress * 100, 100)} ${Math.round(progress * 100)}%` - ); - } - } + let buildUrl = createBuildUrl(baseUrl, build); + writeHumanStatus({ build, buildUrl, globalOptions, output, previewInfo }); output.cleanup(); // Exit with appropriate code based on build status - if (build.status === 'failed' || build.failed_jobs > 0) { + if (shouldFailStatus(build)) { exit(1); } } catch (error) { + output.stopSpinner(); + // Don't fail CI for Vizzly infrastructure issues (5xx errors) let status = error.context?.status; if (status >= 500) { @@ -264,6 +334,7 @@ export async function statusCommand( } output.error('Failed to get build status', error); + output.cleanup(); exit(1); return { success: false, error }; } diff --git a/src/commands/tdd-daemon.js b/src/commands/tdd-daemon.js index d0d7904..edda84a 100644 --- a/src/commands/tdd-daemon.js +++ b/src/commands/tdd-daemon.js @@ -9,9 +9,376 @@ import { import { homedir } from 'node:os'; import { basename, join } from 'node:path'; import { getServerRegistry } from '../tdd/server-registry.js'; +import { withTimeout } from '../utils/async-utils.js'; import * as output from '../utils/output.js'; import { tddCommand } from './tdd.js'; +let defaultTimers = { setTimeout, clearTimeout }; + +export function getLocalDaemonFiles(directory = process.cwd()) { + let vizzlyDir = join(directory, '.vizzly'); + return { + vizzlyDir, + pidFile: join(vizzlyDir, 'server.pid'), + serverFile: join(vizzlyDir, 'server.json'), + logFile: join(vizzlyDir, 'server.log'), + }; +} + +export function removeFileIfExists(filePath, deps = {}) { + let fileExists = deps.existsSync || existsSync; + let unlinkFile = deps.unlinkSync || unlinkSync; + + if (fileExists(filePath)) { + unlinkFile(filePath); + return true; + } + + return false; +} + +export function cleanupLocalDaemonFiles(directory = process.cwd(), deps = {}) { + let { pidFile, serverFile } = getLocalDaemonFiles(directory); + return { + pidFileRemoved: removeFileIfExists(pidFile, deps), + serverFileRemoved: removeFileIfExists(serverFile, deps), + }; +} + +export function buildLegacyServerInfo({ pid, port, now = Date.now }) { + return { + pid, + port: port.toString(), + startTime: now(), + }; +} + +export function writeLegacyGlobalServerFile( + { pid, port }, + { + home = homedir, + exists = existsSync, + mkdir = mkdirSync, + writeFile = writeFileSync, + now = Date.now, + } = {} +) { + let globalVizzlyDir = join(home(), '.vizzly'); + if (!exists(globalVizzlyDir)) { + mkdir(globalVizzlyDir, { recursive: true }); + } + + let globalServerFile = join(globalVizzlyDir, 'server.json'); + let serverInfo = buildLegacyServerInfo({ pid, port, now }); + writeFile(globalServerFile, JSON.stringify(serverInfo, null, 2)); + return { path: globalServerFile, serverInfo }; +} + +export function cleanupLegacyGlobalServerFile({ + home = homedir, + exists = existsSync, + unlink = unlinkSync, +} = {}) { + let globalServerFile = join(home(), '.vizzly', 'server.json'); + return removeFileIfExists(globalServerFile, { + existsSync: exists, + unlinkSync: unlink, + }); +} + +export function unregisterDaemonServer({ + port, + directory = process.cwd(), + registry = getServerRegistry(), +}) { + registry.unregister({ port, directory }); +} + +export function cleanupDaemonState({ + port, + directory = process.cwd(), + registry = getServerRegistry(), + localFileDeps = {}, + legacyFileDeps = {}, +} = {}) { + let localFiles = cleanupLocalDaemonFiles(directory, localFileDeps); + let legacyGlobalServerFileRemoved = + cleanupLegacyGlobalServerFile(legacyFileDeps); + + try { + if (port !== undefined) { + unregisterDaemonServer({ port, directory, registry }); + } else { + registry.unregister({ directory }); + } + } catch { + // Non-fatal; stale file cleanup is still useful on its own. + } + + return { + ...localFiles, + legacyGlobalServerFileRemoved, + }; +} + +export function buildDashboardUrl(port = 47392) { + return `http://localhost:${port}`; +} + +export function buildOpenDashboardCommand(url, platform = process.platform) { + if (platform === 'darwin') { + return { command: 'open', args: [url] }; + } + + if (platform === 'win32') { + return { command: 'cmd', args: ['/c', 'start', '', url] }; + } + + return { command: 'xdg-open', args: [url] }; +} + +function wait(ms, timers = defaultTimers) { + return new Promise(resolve => { + timers.setTimeout(resolve, ms); + }); +} + +function isProcessRunning(pid) { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +function parsePositiveInteger(value) { + let text = String(value).trim(); + if (!/^\d+$/.test(text)) { + return null; + } + + let number = Number(text); + return Number.isSafeInteger(number) && number > 0 ? number : null; +} + +export function readDaemonPidFile(pidFile, deps = {}) { + let fileExists = deps.existsSync || existsSync; + let readFile = deps.readFileSync || readFileSync; + + if (!fileExists(pidFile)) { + return null; + } + + try { + return parsePositiveInteger(readFile(pidFile, 'utf8')); + } catch { + return null; + } +} + +export async function findDaemonPidByPort(port, { spawnProcess = spawn } = {}) { + try { + let lsofProcess = spawnProcess('lsof', ['-ti', `:${port}`], { + stdio: 'pipe', + }); + + let lsofOutput = ''; + lsofProcess.stdout.on('data', data => { + lsofOutput += data.toString(); + }); + + return await new Promise(resolve => { + lsofProcess.on('close', code => { + if (code === 0 && lsofOutput.trim()) { + let foundPid = parsePositiveInteger(lsofOutput.trim().split('\n')[0]); + resolve(foundPid); + return; + } + + resolve(null); + }); + + lsofProcess.on('error', () => { + resolve(null); + }); + }); + } catch { + return null; + } +} + +export async function resolveDaemonPid({ + port, + pidFile = getLocalDaemonFiles().pidFile, + readPid = readDaemonPidFile, + findByPort = findDaemonPidByPort, + fileDeps = {}, +} = {}) { + let pid = readPid(pidFile, fileDeps); + if (pid) { + return pid; + } + + return await findByPort(port); +} + +export function buildDaemonChildArgs({ + entrypoint = process.argv[1], + port, + options = {}, + globalOptions = {}, +}) { + return [ + entrypoint, + 'tdd', + 'start', + '--daemon-child', + '--port', + port.toString(), + ...(options.open ? ['--open'] : []), + ...(options.baselineBuild + ? ['--baseline-build', options.baselineBuild] + : []), + ...(options.baselineComparison + ? ['--baseline-comparison', options.baselineComparison] + : []), + ...(options.environment ? ['--environment', options.environment] : []), + ...(options.threshold !== undefined + ? ['--threshold', options.threshold.toString()] + : []), + ...(options.minClusterSize !== undefined + ? ['--min-cluster-size', options.minClusterSize.toString()] + : []), + ...(options.timeout ? ['--timeout', options.timeout] : []), + ...(options.failOnDiff ? ['--fail-on-diff'] : []), + ...(options.token ? ['--token', options.token] : []), + ...(globalOptions.json ? ['--json'] : []), + ...(globalOptions.verbose ? ['--verbose'] : []), + ...(globalOptions.noColor ? ['--no-color'] : []), + ]; +} + +export function validateTddStartOptions(options = {}) { + let errors = []; + + if (options.port) { + let port = Number(options.port); + if (!Number.isInteger(port) || port < 1 || port > 65535) { + errors.push('Port must be a valid number between 1 and 65535'); + } + } + + if (options.timeout) { + let timeout = Number(options.timeout); + if (!Number.isInteger(timeout) || timeout < 1000) { + errors.push('Timeout must be at least 1000 milliseconds'); + } + } + + if (options.threshold !== undefined) { + let threshold = Number(options.threshold); + if (!Number.isFinite(threshold) || threshold < 0) { + errors.push( + 'Threshold must be a non-negative number (CIEDE2000 Delta E)' + ); + } + } + + if (options.minClusterSize !== undefined) { + let minClusterSize = Number(options.minClusterSize); + if (!Number.isInteger(minClusterSize) || minClusterSize < 1) { + errors.push('Min cluster size must be a positive integer'); + } + } + + return errors; +} + +export async function waitForDaemonChildInit( + child, + { timeoutMs = 30000, timers = defaultTimers } = {} +) { + let cleanup = () => {}; + let initPromise = new Promise(resolve => { + let handleDisconnect = () => { + cleanup(); + resolve({ ok: true }); + }; + + let handleExit = () => { + cleanup(); + resolve({ ok: false, reason: 'exit' }); + }; + + cleanup = () => { + child.off('disconnect', handleDisconnect); + child.off('exit', handleExit); + }; + + child.on('disconnect', handleDisconnect); + child.on('exit', handleExit); + }); + + try { + return await withTimeout( + initPromise, + timeoutMs, + 'TDD server initialization timed out', + timers + ); + } catch (error) { + cleanup(); + return { ok: false, reason: 'timeout', error }; + } +} + +export async function waitForServerRunning( + port, + { + maxAttempts = 10, + delayMs = 200, + isRunning = isServerRunning, + timers = defaultTimers, + } = {} +) { + for (let attempt = 0; attempt < maxAttempts; attempt++) { + if (await isRunning(port)) { + return true; + } + + if (attempt < maxAttempts - 1) { + await wait(delayMs * (attempt + 1), timers); + } + } + + return false; +} + +export async function waitForProcessExit( + pid, + { + timeoutMs = 2000, + intervalMs = 100, + processRunning = isProcessRunning, + timers = defaultTimers, + } = {} +) { + let elapsedMs = 0; + + while (elapsedMs < timeoutMs) { + if (!processRunning(pid)) { + return true; + } + + let nextDelay = Math.min(intervalMs, timeoutMs - elapsedMs); + await wait(nextDelay, timers); + elapsedMs += nextDelay; + } + + return !processRunning(pid); +} + /** * Start TDD server in daemon mode * @param {Object} options - Command options @@ -38,7 +405,7 @@ export async function tddStartCommand(options = {}, globalOptions = {}) { status: 'already_running', port: existingServer.port, pid: existingServer.pid, - dashboardUrl: `http://localhost:${existingServer.port}`, + dashboardUrl: buildDashboardUrl(existingServer.port), }); return; } @@ -62,13 +429,7 @@ export async function tddStartCommand(options = {}, globalOptions = {}) { return; } else { // Stale entry - clean it up (registry and local files) - registry.unregister({ directory: process.cwd() }); - - let vizzlyDir = join(process.cwd(), '.vizzly'); - let pidFile = join(vizzlyDir, 'server.pid'); - let serverFile = join(vizzlyDir, 'server.json'); - if (existsSync(pidFile)) unlinkSync(pidFile); - if (existsSync(serverFile)) unlinkSync(serverFile); + cleanupDaemonState({ directory: process.cwd(), registry }); } } @@ -102,7 +463,7 @@ export async function tddStartCommand(options = {}, globalOptions = {}) { try { // Ensure .vizzly directory exists - let vizzlyDir = join(process.cwd(), '.vizzly'); + let { vizzlyDir } = getLocalDaemonFiles(); if (!existsSync(vizzlyDir)) { mkdirSync(vizzlyDir, { recursive: true }); } @@ -118,33 +479,9 @@ export async function tddStartCommand(options = {}, globalOptions = {}) { } // Spawn child process with stdio inherited during init for direct error visibility - const child = spawn( + let child = spawn( process.execPath, - [ - process.argv[1], // CLI entry point - 'tdd', - 'start', - '--daemon-child', // Special flag for child process - '--port', - port.toString(), - ...(options.open ? ['--open'] : []), - ...(options.baselineBuild - ? ['--baseline-build', options.baselineBuild] - : []), - ...(options.baselineComparison - ? ['--baseline-comparison', options.baselineComparison] - : []), - ...(options.environment ? ['--environment', options.environment] : []), - ...(options.threshold !== undefined - ? ['--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'] : []), - ...(globalOptions.noColor ? ['--no-color'] : []), - ], + buildDaemonChildArgs({ port, options, globalOptions }), { detached: true, stdio: ['ignore', 'inherit', 'inherit', 'ipc'], @@ -152,39 +489,9 @@ export async function tddStartCommand(options = {}, globalOptions = {}) { } ); - // Wait for child to signal successful init or exit with error - let initComplete = false; - let initFailed = false; - - await new Promise(resolve => { - // Child disconnects IPC when initialization succeeds - child.on('disconnect', () => { - initComplete = true; - resolve(); - }); - - // Child exits before disconnecting = initialization failed - child.on('exit', () => { - if (!initComplete) { - initFailed = true; - resolve(); - } - }); + let initResult = await waitForDaemonChildInit(child); - // Timeout after 30 seconds to prevent indefinite wait - const timeoutId = setTimeout(() => { - if (!initComplete && !initFailed) { - initFailed = true; - resolve(); - } - }, 30000); - - // Clear timeout if we resolve early - child.on('disconnect', () => clearTimeout(timeoutId)); - child.on('exit', () => clearTimeout(timeoutId)); - }); - - if (initFailed) { + if (!initResult.ok) { if (options.baselineBuild && !globalOptions.verbose) { output.stopSpinner(); } @@ -196,14 +503,7 @@ export async function tddStartCommand(options = {}, globalOptions = {}) { child.unref(); // Verify server started with retries - const maxRetries = 10; - const retryDelay = 200; // Start with 200ms - let running = false; - - for (let i = 0; i < maxRetries && !running; i++) { - await new Promise(resolve => setTimeout(resolve, retryDelay * (i + 1))); - running = await isServerRunning(port); - } + let running = await waitForServerRunning(port); if (options.baselineBuild && !globalOptions.verbose) { output.stopSpinner(); @@ -224,14 +524,14 @@ export async function tddStartCommand(options = {}, globalOptions = {}) { registry.cleanupStale(); // Register this server with log file path for menubar to read - let serverLogFile = join(process.cwd(), '.vizzly', 'server.log'); + let { logFile } = getLocalDaemonFiles(); registry.register({ pid: child.pid, port: port, directory: process.cwd(), name: basename(process.cwd()), startedAt: new Date().toISOString(), - logFile: serverLogFile, + logFile, }); } catch { // Non-fatal @@ -239,23 +539,13 @@ export async function tddStartCommand(options = {}, globalOptions = {}) { // Also write legacy server.json for SDK discovery (backwards compatibility) try { - const globalVizzlyDir = join(homedir(), '.vizzly'); - if (!existsSync(globalVizzlyDir)) { - mkdirSync(globalVizzlyDir, { recursive: true }); - } - const globalServerFile = join(globalVizzlyDir, 'server.json'); - const serverInfo = { - pid: child.pid, - port: port.toString(), - startTime: Date.now(), - }; - writeFileSync(globalServerFile, JSON.stringify(serverInfo, null, 2)); + writeLegacyGlobalServerFile({ pid: child.pid, port }); } catch { // Non-fatal, SDK can still use health check } // JSON output for successful start - let dashboardUrl = `http://localhost:${port}`; + let dashboardUrl = buildDashboardUrl(port); if (globalOptions.json) { output.data({ status: 'started', @@ -317,11 +607,8 @@ export async function tddStartCommand(options = {}, globalOptions = {}) { * @private */ export async function runDaemonChild(options = {}, globalOptions = {}) { - const vizzlyDir = join(process.cwd(), '.vizzly'); - const port = options.port || 47392; - - // Set up log file for menubar app to read - const logFile = join(vizzlyDir, 'server.log'); + let { pidFile, serverFile, logFile } = getLocalDaemonFiles(); + let port = options.port || 47392; // Configure output to write JSON logs to file (before tddCommand configures it) output.configure({ @@ -333,7 +620,7 @@ export async function runDaemonChild(options = {}, globalOptions = {}) { try { // Use existing tddCommand but with daemon mode - const { cleanup } = await tddCommand( + let { cleanup } = await tddCommand( null, // No test command - server only { ...options, @@ -348,44 +635,21 @@ export async function runDaemonChild(options = {}, globalOptions = {}) { } // Store our PID for the stop command - const pidFile = join(vizzlyDir, 'server.pid'); writeFileSync(pidFile, process.pid.toString()); - const serverInfo = { + let serverInfo = { pid: process.pid, port: port, startTime: Date.now(), failOnDiff: options.failOnDiff || false, - logFile: logFile, + logFile, }; - writeFileSync( - join(vizzlyDir, 'server.json'), - JSON.stringify(serverInfo, null, 2) - ); + writeFileSync(serverFile, JSON.stringify(serverInfo, null, 2)); // Set up graceful shutdown - const handleShutdown = async () => { + let handleShutdown = async () => { try { - // Clean up PID files - if (existsSync(pidFile)) unlinkSync(pidFile); - const serverFile = join(vizzlyDir, 'server.json'); - if (existsSync(serverFile)) unlinkSync(serverFile); - - // Unregister from global registry (for menubar app) - try { - let registry = getServerRegistry(); - registry.unregister({ port: port, directory: process.cwd() }); - } catch { - // Non-fatal - } - - // Clean up legacy global server file - try { - const globalServerFile = join(homedir(), '.vizzly', 'server.json'); - if (existsSync(globalServerFile)) unlinkSync(globalServerFile); - } catch { - // Non-fatal - } + cleanupDaemonState({ port }); // Use the cleanup function from tddCommand await cleanup(); @@ -421,51 +685,8 @@ export async function tddStopCommand(options = {}, globalOptions = {}) { color: !globalOptions.noColor, }); - const vizzlyDir = join(process.cwd(), '.vizzly'); - const pidFile = join(vizzlyDir, 'server.pid'); - const serverFile = join(vizzlyDir, 'server.json'); - - // First try to find process by PID file - let pid = null; - if (existsSync(pidFile)) { - try { - pid = parseInt(readFileSync(pidFile, 'utf8').trim(), 10); - } catch { - // Invalid PID file - } - } - - // If no PID file or invalid, try to find by port using lsof - const port = options.port || 47392; - if (!pid) { - try { - const lsofProcess = spawn('lsof', ['-ti', `:${port}`], { stdio: 'pipe' }); - - let lsofOutput = ''; - lsofProcess.stdout.on('data', data => { - lsofOutput += data.toString(); - }); - - await new Promise(resolve => { - lsofProcess.on('close', code => { - if (code === 0 && lsofOutput.trim()) { - const foundPid = parseInt(lsofOutput.trim().split('\n')[0], 10); - if (foundPid && !Number.isNaN(foundPid)) { - pid = foundPid; - } - } - resolve(); - }); - - lsofProcess.on('error', () => { - // lsof not available, that's ok - resolve(); - }); - }); - } catch { - // lsof failed, that's ok too - } - } + let port = options.port || 47392; + let pid = await resolveDaemonPid({ port }); if (!pid) { // JSON output for not running @@ -479,45 +700,30 @@ export async function tddStopCommand(options = {}, globalOptions = {}) { } // Clean up any stale files - if (existsSync(pidFile)) unlinkSync(pidFile); - if (existsSync(serverFile)) unlinkSync(serverFile); + cleanupDaemonState({ port }); return; } try { - let _colors = output.getColors(); - // Try to kill the process gracefully process.kill(pid, 'SIGTERM'); output.startSpinner('Stopping TDD server...'); - // Give it a moment to shut down gracefully - await new Promise(resolve => setTimeout(resolve, 2000)); + let exited = await waitForProcessExit(pid); // Check if it's still running - try { - process.kill(pid, 0); // Just check if process exists - // If we get here, process is still running, force kill it + if (!exited) { process.kill(pid, 'SIGKILL'); output.stopSpinner(); output.debug('tdd', 'Force killed process'); - } catch { + } else { // Process is gone, which is what we want output.stopSpinner(); } // Clean up files - if (existsSync(pidFile)) unlinkSync(pidFile); - if (existsSync(serverFile)) unlinkSync(serverFile); - - // Unregister from global registry (for menubar app) - try { - let registry = getServerRegistry(); - registry.unregister({ port: port, directory: process.cwd() }); - } catch { - // Non-fatal - } + cleanupDaemonState({ port }); // JSON output for successful stop if (globalOptions.json) { @@ -534,16 +740,7 @@ export async function tddStopCommand(options = {}, globalOptions = {}) { if (error.code === 'ESRCH') { // Process not found - clean up stale files output.warn('TDD server was not running (cleaning up stale files)'); - if (existsSync(pidFile)) unlinkSync(pidFile); - if (existsSync(serverFile)) unlinkSync(serverFile); - - // Still unregister from registry - try { - let registry = getServerRegistry(); - registry.unregister({ port: port, directory: process.cwd() }); - } catch { - // Non-fatal - } + cleanupDaemonState({ port }); } else { output.error('Error stopping TDD server', error); } @@ -562,9 +759,7 @@ export async function tddStatusCommand(_options, globalOptions = {}) { color: !globalOptions.noColor, }); - const vizzlyDir = join(process.cwd(), '.vizzly'); - const pidFile = join(vizzlyDir, 'server.pid'); - const serverFile = join(vizzlyDir, 'server.json'); + let { pidFile, serverFile } = getLocalDaemonFiles(); if (!existsSync(pidFile)) { // JSON output for not running @@ -580,7 +775,12 @@ export async function tddStatusCommand(_options, globalOptions = {}) { } try { - const pid = parseInt(readFileSync(pidFile, 'utf8').trim(), 10); + let pid = parsePositiveInteger(readFileSync(pidFile, 'utf8')); + if (!pid) { + output.warn('TDD server pid file is invalid (cleaning up stale files)'); + cleanupDaemonState(); + return; + } // Check if process is actually running process.kill(pid, 0); // Signal 0 just checks if process exists @@ -591,7 +791,7 @@ export async function tddStatusCommand(_options, globalOptions = {}) { } // Try to check health endpoint - const health = await checkServerHealth(serverInfo.port); + let health = await checkServerHealth(serverInfo.port); if (health.running) { // Calculate uptime @@ -599,16 +799,16 @@ export async function tddStatusCommand(_options, globalOptions = {}) { let uptimeStr = ''; if (serverInfo.startTime) { uptimeMs = Date.now() - serverInfo.startTime; - const uptime = Math.floor(uptimeMs / 1000); - const hours = Math.floor(uptime / 3600); - const minutes = Math.floor((uptime % 3600) / 60); - const seconds = uptime % 60; + let uptime = Math.floor(uptimeMs / 1000); + let hours = Math.floor(uptime / 3600); + let minutes = Math.floor((uptime % 3600) / 60); + let seconds = uptime % 60; if (hours > 0) uptimeStr += `${hours}h `; if (minutes > 0 || hours > 0) uptimeStr += `${minutes}m `; uptimeStr += `${seconds}s`; } - let dashboardUrl = `http://localhost:${serverInfo.port}`; + let dashboardUrl = buildDashboardUrl(serverInfo.port); // JSON output for running status if (globalOptions.json) { @@ -653,10 +853,7 @@ export async function tddStatusCommand(_options, globalOptions = {}) { } catch (error) { if (error.code === 'ESRCH') { output.warn('TDD server process not found (cleaning up stale files)'); - unlinkSync(pidFile); - if (existsSync(serverFile)) { - unlinkSync(serverFile); - } + cleanupDaemonState(); } else { output.error('Error checking TDD server status', error); } @@ -669,7 +866,7 @@ export async function tddStatusCommand(_options, globalOptions = {}) { */ async function isServerRunning(port = 47392) { try { - const health = await checkServerHealth(port); + let health = await checkServerHealth(port); return health.running; } catch { return false; @@ -682,8 +879,8 @@ async function isServerRunning(port = 47392) { */ async function checkServerHealth(port = 47392) { try { - const response = await fetch(`http://localhost:${port}/health`); - const data = await response.json(); + let response = await fetch(`http://localhost:${port}/health`); + let data = await response.json(); return { running: response.ok, port: data.port, @@ -699,19 +896,10 @@ async function checkServerHealth(port = 47392) { * @private */ function openDashboard(port = 47392) { - const url = `http://localhost:${port}`; - - // Cross-platform open command - let openCmd; - if (process.platform === 'darwin') { - openCmd = 'open'; - } else if (process.platform === 'win32') { - openCmd = 'start'; - } else { - openCmd = 'xdg-open'; - } + let url = buildDashboardUrl(port); + let { command, args } = buildOpenDashboardCommand(url); - spawn(openCmd, [url], { + spawn(command, args, { detached: true, stdio: 'ignore', }).unref(); diff --git a/src/commands/tdd.js b/src/commands/tdd.js index 5fa3751..525c117 100644 --- a/src/commands/tdd.js +++ b/src/commands/tdd.js @@ -113,7 +113,7 @@ export async function tddCommand( // Show config in verbose mode output.debug( 'config', - `port=${config.server.port} threshold=${config.comparison.threshold}` + `port=${config.server.port} threshold=${config.comparison.threshold} minClusterSize=${config.comparison.minClusterSize}` ); } @@ -153,6 +153,7 @@ export async function tddCommand( commit, environment: config.build.environment, threshold: config.comparison.threshold, + minClusterSize: config.comparison.minClusterSize, allowNoToken: config.allowNoToken || false, baselineBuildId: config.baselineBuildId, baselineComparisonId: config.baselineComparisonId, @@ -310,27 +311,34 @@ export function validateTddOptions(testCommand, options) { } if (options.port) { - let port = parseInt(options.port, 10); - if (Number.isNaN(port) || port < 1 || port > 65535) { + let port = Number(options.port); + if (!Number.isInteger(port) || port < 1 || port > 65535) { errors.push('Port must be a valid number between 1 and 65535'); } } if (options.timeout) { - let timeout = parseInt(options.timeout, 10); - if (Number.isNaN(timeout) || timeout < 1000) { + let timeout = Number(options.timeout); + if (!Number.isInteger(timeout) || timeout < 1000) { errors.push('Timeout must be at least 1000 milliseconds'); } } if (options.threshold !== undefined) { - let threshold = parseFloat(options.threshold); - if (Number.isNaN(threshold) || threshold < 0) { + let threshold = Number(options.threshold); + if (!Number.isFinite(threshold) || threshold < 0) { errors.push( 'Threshold must be a non-negative number (CIEDE2000 Delta E)' ); } } + if (options.minClusterSize !== undefined) { + let minClusterSize = Number(options.minClusterSize); + if (!Number.isInteger(minClusterSize) || minClusterSize < 1) { + errors.push('Min cluster size must be a positive integer'); + } + } + return errors; } diff --git a/src/commands/upload.js b/src/commands/upload.js index 024a29e..56d3242 100644 --- a/src/commands/upload.js +++ b/src/commands/upload.js @@ -3,7 +3,7 @@ import { finalizeBuild as defaultFinalizeBuild, getTokenContext as defaultGetTokenContext, } from '../api/index.js'; -import { createUploader as defaultCreateUploader } from '../services/uploader.js'; +import { createUploader as defaultCreateUploader } from '../uploader/index.js'; import { getAppBaseUrl } from '../utils/api-url.js'; import { loadConfig as defaultLoadConfig } from '../utils/config-loader.js'; import { diff --git a/src/index.js b/src/index.js index b7a4204..49cfb57 100644 --- a/src/index.js +++ b/src/index.js @@ -11,13 +11,24 @@ import 'dotenv/config'; // Client exports for convenience export { configure, setEnabled, vizzlyScreenshot } from './client/index.js'; // Errors -export { UploadError } from './errors/vizzly-error.js'; +export { + AuthError, + BuildError, + ConfigError, + NetworkError, + ScreenshotError, + TimeoutError, + UploadError, + ValidationError, + VizzlyError, +} from './errors/vizzly-error.js'; +export { createPluginServices } from './plugin-api.js'; // Primary SDK export -export { createVizzly } from './sdk/index.js'; +export { createVizzly, VizzlySDK } from './sdk/index.js'; export { createServices } from './services/index.js'; -// Core services (for advanced usage) -export { createUploader } from './services/uploader.js'; export { createTDDService } from './tdd/tdd-service.js'; +// Core services (for advanced usage) +export { createUploader } from './uploader/index.js'; // Configuration helper export { defineConfig } from './utils/config-helpers.js'; // Utilities diff --git a/src/plugin-loader.js b/src/plugin-loader.js index 71e1c98..f42fa3f 100644 --- a/src/plugin-loader.js +++ b/src/plugin-loader.js @@ -1,5 +1,5 @@ import { readFileSync } from 'node:fs'; -import { dirname, resolve } from 'node:path'; +import { dirname, isAbsolute, relative, resolve, sep } from 'node:path'; import { pathToFileURL } from 'node:url'; import { glob } from 'glob'; import { z } from 'zod'; @@ -12,15 +12,15 @@ import * as output from './utils/output.js'; * @returns {Promise} Array of loaded plugins */ export async function loadPlugins(configPath, config) { - const plugins = []; - const loadedNames = new Set(); + let plugins = []; + let loadedNames = new Set(); // 1. Auto-discover plugins from @vizzly-testing/* packages - const discoveredPlugins = await discoverInstalledPlugins(); + let discoveredPlugins = await discoverInstalledPlugins(); - for (const pluginInfo of discoveredPlugins) { + for (let pluginInfo of discoveredPlugins) { try { - const plugin = await loadPlugin(pluginInfo.path); + let plugin = await loadPlugin(pluginInfo.path); if (plugin && !loadedNames.has(plugin.name)) { plugins.push(plugin); loadedNames.add(plugin.name); @@ -37,16 +37,16 @@ export async function loadPlugins(configPath, config) { // 2. Load explicit plugins from config if (config?.plugins && Array.isArray(config.plugins)) { - for (const pluginSpec of config.plugins) { + for (let pluginSpec of config.plugins) { try { - const pluginPath = resolvePluginPath(pluginSpec, configPath); - const plugin = await loadPlugin(pluginPath); + let pluginPath = resolvePluginPath(pluginSpec, configPath); + let plugin = await loadPlugin(pluginPath); if (plugin && !loadedNames.has(plugin.name)) { plugins.push(plugin); loadedNames.add(plugin.name); } else if (plugin && loadedNames.has(plugin.name)) { - const existingPlugin = plugins.find(p => p.name === plugin.name); + let existingPlugin = plugins.find(p => p.name === plugin.name); output.warn( `Plugin ${plugin.name} already loaded (v${existingPlugin.version || 'unknown'}), ` + `skipping v${plugin.version || 'unknown'} from config` @@ -67,12 +67,12 @@ export async function loadPlugins(configPath, config) { * Discover installed plugins from node_modules/@vizzly-testing/* * @returns {Promise} Array of plugin info objects */ -async function discoverInstalledPlugins() { - const plugins = []; +export async function discoverInstalledPlugins() { + let plugins = []; try { // Find all @vizzly-testing packages - const packageJsonPaths = await glob( + let packageJsonPaths = await glob( 'node_modules/@vizzly-testing/*/package.json', { cwd: process.cwd(), @@ -80,39 +80,20 @@ async function discoverInstalledPlugins() { } ); - for (const pkgPath of packageJsonPaths) { + for (let pkgPath of packageJsonPaths) { try { - const packageJson = JSON.parse(readFileSync(pkgPath, 'utf-8')); + let packageJson = JSON.parse(readFileSync(pkgPath, 'utf-8')); // Check if package has a plugin field // Support both new `vizzlyPlugin` and legacy `vizzly.plugin` for backwards compatibility - const pluginField = - packageJson.vizzlyPlugin || packageJson.vizzly?.plugin; + let pluginField = getPluginField(packageJson); if (pluginField) { - const pluginRelativePath = pluginField; - - // Security: Ensure plugin path is relative and doesn't traverse up - if ( - pluginRelativePath.startsWith('/') || - pluginRelativePath.includes('..') - ) { - output.warn( - `Invalid plugin path in ${packageJson.name}: path must be relative and cannot traverse directories` - ); - continue; - } - - // Resolve plugin path relative to package directory - const packageDir = dirname(pkgPath); - const pluginPath = resolve(packageDir, pluginRelativePath); - - // Additional security: Ensure resolved path is still within package directory - if (!pluginPath.startsWith(packageDir)) { - output.warn( - `Plugin path escapes package directory: ${packageJson.name}` - ); - continue; - } + let packageDir = dirname(pkgPath); + let pluginPath = resolvePackagePluginPath( + packageJson.name, + packageDir, + pluginField + ); plugins.push({ packageName: packageJson.name, @@ -132,28 +113,65 @@ async function discoverInstalledPlugins() { return plugins; } +function getPluginField(packageJson) { + return packageJson.vizzlyPlugin || packageJson.vizzly?.plugin; +} + +export function resolvePackagePluginPath( + packageName, + packageDir, + pluginRelativePath +) { + if (typeof pluginRelativePath !== 'string' || pluginRelativePath === '') { + throw new Error( + `Invalid plugin path in ${packageName}: path must be a non-empty string` + ); + } + + if (isAbsolute(pluginRelativePath)) { + throw new Error( + `Invalid plugin path in ${packageName}: path must be relative` + ); + } + + let pluginPath = resolve(packageDir, pluginRelativePath); + let relativePath = relative(packageDir, pluginPath); + + if ( + relativePath === '..' || + relativePath.startsWith(`..${sep}`) || + isAbsolute(relativePath) + ) { + throw new Error( + `Invalid plugin path in ${packageName}: path cannot escape package directory` + ); + } + + return pluginPath; +} + /** * Load a plugin from a file path * @param {string} pluginPath - Path to plugin file * @returns {Promise} Loaded plugin or null */ -async function loadPlugin(pluginPath) { +export async function loadPlugin(pluginPath) { try { // Convert to file URL for ESM import - const pluginUrl = pathToFileURL(pluginPath).href; + let pluginUrl = pathToFileURL(pluginPath).href; // Dynamic import - const pluginModule = await import(pluginUrl); + let pluginModule = await import(pluginUrl); // Get the default export - const plugin = pluginModule.default || pluginModule; + let plugin = pluginModule.default || pluginModule; // Validate plugin structure validatePluginStructure(plugin); return plugin; } catch (error) { - const newError = new Error( + let newError = new Error( `Failed to load plugin from ${pluginPath}: ${error.message}` ); newError.cause = error; @@ -190,9 +208,7 @@ function validatePluginStructure(plugin) { // configSchema is optional and primarily for documentation } catch (error) { if (error instanceof z.ZodError) { - const messages = error.issues.map( - e => `${e.path.join('.')}: ${e.message}` - ); + let messages = error.issues.map(e => `${e.path.join('.')}: ${e.message}`); throw new Error(`Invalid plugin structure: ${messages.join(', ')}`); } throw error; @@ -205,25 +221,32 @@ function validatePluginStructure(plugin) { * @param {string|null} configPath - Path to config file * @returns {string} Resolved plugin path */ -function resolvePluginPath(pluginSpec, configPath) { +export function resolvePluginPath(pluginSpec, configPath) { + if (typeof pluginSpec !== 'string' || pluginSpec === '') { + throw new Error('Plugin spec must be a non-empty string'); + } + // If it's a package name (starts with @ or is alphanumeric), try to resolve from node_modules if (pluginSpec.startsWith('@') || /^[a-zA-Z0-9-]+$/.test(pluginSpec)) { // Try to resolve as a package try { - const packageJsonPath = resolve( + let packageJsonPath = resolve( process.cwd(), 'node_modules', pluginSpec, 'package.json' ); - const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8')); + let packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8')); // Support both new `vizzlyPlugin` and legacy `vizzly.plugin` - const pluginField = - packageJson.vizzlyPlugin || packageJson.vizzly?.plugin; + let pluginField = getPluginField(packageJson); if (pluginField) { - const packageDir = dirname(packageJsonPath); - return resolve(packageDir, pluginField); + let packageDir = dirname(packageJsonPath); + return resolvePackagePluginPath( + packageJson.name || pluginSpec, + packageDir, + pluginField + ); } throw new Error( @@ -239,10 +262,10 @@ function resolvePluginPath(pluginSpec, configPath) { // Otherwise treat as a file path if (configPath) { // Resolve relative to config file - const configDir = dirname(configPath); + let configDir = dirname(configPath); return resolve(configDir, pluginSpec); - } else { - // Resolve relative to cwd - return resolve(process.cwd(), pluginSpec); } + + // Resolve relative to cwd + return resolve(process.cwd(), pluginSpec); } diff --git a/src/reporter/src/components/dashboard/dashboard-filters.jsx b/src/reporter/src/components/dashboard/dashboard-filters.jsx index 10ac380..11d4a60 100644 --- a/src/reporter/src/components/dashboard/dashboard-filters.jsx +++ b/src/reporter/src/components/dashboard/dashboard-filters.jsx @@ -25,14 +25,8 @@ function getDeviceIcon(width) { return ComputerDesktopIcon; } -/** - * Get device label for viewport - */ -function _getDeviceLabel(viewportStr) { - let width = parseInt(viewportStr.split('Ɨ')[0], 10); - if (width <= 480) return 'Mobile'; - if (width <= 1024) return 'Tablet'; - return 'Desktop'; +function getViewportWidth(viewport) { + return Number(viewport.split('Ɨ')[0]); } /** @@ -299,7 +293,7 @@ export default function DashboardFilters({ // Render viewport option with device icon let renderViewportOption = useCallback(viewport => { - let DeviceIcon = getDeviceIcon(parseInt(viewport.split('Ɨ')[0], 10)); + let DeviceIcon = getDeviceIcon(getViewportWidth(viewport)); return ( <> @@ -313,7 +307,7 @@ export default function DashboardFilters({ if (value === 'all') { return All viewports; } - let DeviceIcon = getDeviceIcon(parseInt(value.split('Ɨ')[0], 10)); + let DeviceIcon = getDeviceIcon(getViewportWidth(value)); return ( @@ -553,7 +547,7 @@ export default function DashboardFilters({ {selectedViewport !== 'all' && ( setSelectedViewport('all')} /> )} diff --git a/src/sdk/index.js b/src/sdk/index.js index 850b5b5..92b99b4 100644 --- a/src/sdk/index.js +++ b/src/sdk/index.js @@ -18,8 +18,8 @@ import { startServer as startScreenshotServer, stopServer as stopScreenshotServer, } from '../screenshot-server/index.js'; -import { createUploader } from '../services/uploader.js'; import { createTDDService } from '../tdd/tdd-service.js'; +import { createUploader } from '../uploader/index.js'; import { loadConfig } from '../utils/config-loader.js'; import { resolveImageBuffer } from '../utils/file-helpers.js'; import * as output from '../utils/output.js'; @@ -448,9 +448,9 @@ export class VizzlySDK extends EventEmitter { } } -// Export service creators for advanced usage -export { createUploader } from '../services/uploader.js'; export { createTDDService } from '../tdd/tdd-service.js'; +// Export service creators for advanced usage +export { createUploader } from '../uploader/index.js'; // Re-export key utilities and errors export { loadConfig } from '../utils/config-loader.js'; export * as output from '../utils/output.js'; diff --git a/src/server/routers/events.js b/src/server/routers/events.js index 97fda28..e15ce07 100644 --- a/src/server/routers/events.js +++ b/src/server/routers/events.js @@ -11,130 +11,207 @@ let FILE_POLL_INTERVAL_MS = 50; let REPORT_READ_RETRY_MS = 25; let MAX_REPORT_READ_RETRIES = 3; +let defaultTimers = { + setTimeout, + clearTimeout, + setInterval, + clearInterval, +}; + /** - * Create events router for SSE - * @param {Object} context - Router context - * @param {string} context.workingDir - Working directory for report data - * @returns {Function} Route handler + * Read and parse JSON from disk, returning null on missing or invalid files. */ -export function createEventsRouter(context) { - const { workingDir = process.cwd() } = context || {}; - const reportDataPath = join(workingDir, '.vizzly', 'report-data.json'); - const baselineMetadataPath = join( - workingDir, - '.vizzly', - 'baselines', - 'metadata.json' - ); +export function readJsonFile(path) { + if (!existsSync(path)) { + return null; + } - /** - * Read and parse baseline metadata, returning null on error - */ - const readBaselineMetadata = () => { - if (!existsSync(baselineMetadataPath)) { - return null; - } - try { - return JSON.parse(readFileSync(baselineMetadataPath, 'utf8')); - } catch { - return null; - } - }; + try { + return JSON.parse(readFileSync(path, 'utf8')); + } catch { + return null; + } +} - /** - * Read and parse report data with baseline metadata included - */ - const readReportData = () => { - if (!existsSync(reportDataPath)) { - return null; - } - try { - const data = JSON.parse(readFileSync(reportDataPath, 'utf8')); - // Include baseline metadata for stats view - data.baseline = readBaselineMetadata(); - return data; - } catch { - return null; - } - }; +/** + * Read report data and attach baseline metadata for the stats view. + */ +export function readReportDataFile({ reportDataPath, baselineMetadataPath }) { + let data = readJsonFile(reportDataPath); + if (!data) { + return null; + } - /** - * Send SSE event to response - */ - const sendEvent = (res, eventType, data) => { - if (res.writableEnded) return; - res.write(`event: ${eventType}\n`); - res.write(`data: ${JSON.stringify(data)}\n\n`); + return { + ...data, + baseline: readJsonFile(baselineMetadataPath), }; +} - /** - * Build a lookup map from comparisons array keyed by id - */ - const buildComparisonMap = comparisons => { - let map = new Map(); - for (let c of comparisons) { - map.set(c.id, c); +/** + * Build a lookup map from comparisons array keyed by id + */ +function buildComparisonMap(comparisons) { + let map = new Map(); + for (let c of comparisons) { + map.set(c.id, c); + } + return map; +} + +function comparisonChanged(oldComp, newComp) { + return JSON.stringify(oldComp) !== JSON.stringify(newComp); +} + +/** + * Extract summary fields (everything except comparisons) for diffing + */ +function extractSummary(data) { + let { comparisons: _c, ...summary } = data; + return summary; +} + +/** + * Check if summary-level fields changed between old and new data + */ +function summaryChanged(oldData, newData) { + let oldSummary = extractSummary(oldData); + let newSummary = extractSummary(newData); + return JSON.stringify(oldSummary) !== JSON.stringify(newSummary); +} + +/** + * Build incremental SSE events by diffing old vs new report data. + */ +export function buildReportDataEvents(oldData, newData) { + if (!oldData) { + return [{ type: 'reportData', data: newData }]; + } + + let events = []; + let oldComparisons = oldData.comparisons || []; + let newComparisons = newData.comparisons || []; + + let oldMap = buildComparisonMap(oldComparisons); + let newMap = buildComparisonMap(newComparisons); + + // New or changed comparisons send the full comparison object, not a partial delta. + for (let [id, newComp] of newMap) { + let oldComp = oldMap.get(id); + if (!oldComp || comparisonChanged(oldComp, newComp)) { + events.push({ type: 'comparisonUpdate', data: newComp }); } - return map; - }; + } - const comparisonChanged = (oldComp, newComp) => { - return JSON.stringify(oldComp) !== JSON.stringify(newComp); - }; + for (let [id] of oldMap) { + if (!newMap.has(id)) { + events.push({ type: 'comparisonRemoved', data: { id } }); + } + } - /** - * Extract summary fields (everything except comparisons) for diffing - */ - const extractSummary = data => { - let { comparisons: _c, ...summary } = data; - return summary; - }; + if (summaryChanged(oldData, newData)) { + events.push({ type: 'summaryUpdate', data: extractSummary(newData) }); + } - /** - * Check if summary-level fields changed between old and new data - */ - const summaryChanged = (oldData, newData) => { - let oldSummary = extractSummary(oldData); - let newSummary = extractSummary(newData); - return JSON.stringify(oldSummary) !== JSON.stringify(newSummary); - }; + return events; +} - /** - * Send incremental updates by diffing old vs new report data. - * Returns true if any events were sent. - */ - const sendIncrementalUpdates = (res, oldData, newData) => { - let sent = false; - let oldComparisons = oldData.comparisons || []; - let newComparisons = newData.comparisons || []; - - let oldMap = buildComparisonMap(oldComparisons); - let newMap = buildComparisonMap(newComparisons); - - // New or changed comparisons — sends the full comparison object, not a partial delta - for (let [id, newComp] of newMap) { - let oldComp = oldMap.get(id); - if (!oldComp || comparisonChanged(oldComp, newComp)) { - sendEvent(res, 'comparisonUpdate', newComp); - sent = true; - } +/** + * Watch report-data.json with fs.watch plus a lightweight mtime fallback. + */ +export function watchReportDataFile({ + workingDir, + reportDataPath, + onReportDataChanged, + timers = defaultTimers, +}) { + let watcher = null; + let filePollInterval = null; + let vizzlyDir = join(workingDir, '.vizzly'); + + if (existsSync(vizzlyDir)) { + try { + watcher = watch( + vizzlyDir, + { recursive: false }, + (_eventType, filename) => { + // Some platforms occasionally omit the filename for directory watch + // events. In that case, fall back to re-reading report data. + if (!filename || filename === 'report-data.json') { + onReportDataChanged(); + } + } + ); + watcher.on('error', () => { + if (watcher) { + watcher.close(); + watcher = null; + } + }); + } catch { + // File watching not available, mtime polling remains as fallback. } + } - // Removed comparisons - for (let [id] of oldMap) { - if (!newMap.has(id)) { - sendEvent(res, 'comparisonRemoved', { id }); - sent = true; + let lastPolledMtime = existsSync(reportDataPath) + ? statSync(reportDataPath).mtimeMs + : null; + + filePollInterval = timers.setInterval(() => { + let nextMtime = null; + if (existsSync(reportDataPath)) { + try { + nextMtime = statSync(reportDataPath).mtimeMs; + } catch { + nextMtime = null; } } + if (nextMtime !== lastPolledMtime) { + lastPolledMtime = nextMtime; + onReportDataChanged(); + } + }, FILE_POLL_INTERVAL_MS); - // Summary-level changes (total, passed, failed, etc.) - if (summaryChanged(oldData, newData)) { - sendEvent(res, 'summaryUpdate', extractSummary(newData)); - sent = true; + return () => { + if (watcher) { + watcher.close(); + watcher = null; } + if (filePollInterval) { + timers.clearInterval(filePollInterval); + filePollInterval = null; + } + }; +} - return sent; +/** + * Create events router for SSE + * @param {Object} context - Router context + * @param {string} context.workingDir - Working directory for report data + * @returns {Function} Route handler + */ +export function createEventsRouter(context) { + let { + workingDir = process.cwd(), + readReportData = readReportDataFile, + watchReportData = watchReportDataFile, + timers = defaultTimers, + } = context || {}; + let reportDataPath = join(workingDir, '.vizzly', 'report-data.json'); + let baselineMetadataPath = join( + workingDir, + '.vizzly', + 'baselines', + 'metadata.json' + ); + + /** + * Send SSE event to response + */ + let sendEvent = (res, eventType, data) => { + if (res.writableEnded) return; + res.write(`event: ${eventType}\n`); + res.write(`data: ${JSON.stringify(data)}\n\n`); }; return async function handleEventsRoute(req, res, pathname) { @@ -151,30 +228,28 @@ export function createEventsRouter(context) { }); // Send initial full data immediately - let lastSentData = readReportData(); + let lastSentData = readReportData({ reportDataPath, baselineMetadataPath }); if (lastSentData) { sendEvent(res, 'reportData', lastSentData); } // Debounce file change events (fs.watch can fire multiple times) let debounceTimer = null; - let watcher = null; - let filePollInterval = null; - const scheduleUpdate = () => { + let scheduleUpdate = () => { if (debounceTimer) { - clearTimeout(debounceTimer); + timers.clearTimeout(debounceTimer); } - debounceTimer = setTimeout(sendUpdate, FILE_WATCH_DEBOUNCE_MS); + debounceTimer = timers.setTimeout(sendUpdate, FILE_WATCH_DEBOUNCE_MS); }; - const sendUpdate = (retryCount = 0) => { - const newData = readReportData(); + let sendUpdate = (retryCount = 0) => { + let newData = readReportData({ reportDataPath, baselineMetadataPath }); if (!newData) { if ( existsSync(reportDataPath) && retryCount < MAX_REPORT_READ_RETRIES ) { - debounceTimer = setTimeout( + debounceTimer = timers.setTimeout( () => sendUpdate(retryCount + 1), REPORT_READ_RETRY_MS ); @@ -182,84 +257,39 @@ export function createEventsRouter(context) { return; } - if (!lastSentData) { - // No previous data — send full payload - sendEvent(res, 'reportData', newData); - } else { - // Diff and send incremental updates - let sent = sendIncrementalUpdates(res, lastSentData, newData); - // If nothing changed, skip (no event needed) - if (!sent) return; + let events = buildReportDataEvents(lastSentData, newData); + if (events.length === 0) { + return; } - lastSentData = newData; - }; - // Watch for file changes - const vizzlyDir = join(workingDir, '.vizzly'); - if (existsSync(vizzlyDir)) { - try { - watcher = watch( - vizzlyDir, - { recursive: false }, - (_eventType, filename) => { - // Some platforms occasionally omit the filename for directory watch - // events. In that case, fall back to re-reading report data. - if (!filename || filename === 'report-data.json') { - scheduleUpdate(); - } - } - ); - watcher.on('error', () => { - if (watcher) { - watcher.close(); - watcher = null; - } - }); - } catch { - // File watching not available, client will fall back to polling + for (let event of events) { + sendEvent(res, event.type, event.data); } - } + lastSentData = newData; + }; - let lastPolledMtime = existsSync(reportDataPath) - ? statSync(reportDataPath).mtimeMs - : null; - filePollInterval = setInterval(() => { - let nextMtime = null; - if (existsSync(reportDataPath)) { - try { - nextMtime = statSync(reportDataPath).mtimeMs; - } catch { - nextMtime = null; - } - } - if (nextMtime !== lastPolledMtime) { - lastPolledMtime = nextMtime; - scheduleUpdate(); - } - }, FILE_POLL_INTERVAL_MS); + let cleanupReportWatcher = watchReportData({ + workingDir, + reportDataPath, + onReportDataChanged: scheduleUpdate, + timers, + }); // Heartbeat to keep connection alive (every 30 seconds) - const heartbeatInterval = setInterval(() => { + let heartbeatInterval = timers.setInterval(() => { if (!res.writableEnded) { sendEvent(res, 'heartbeat', { timestamp: Date.now() }); } }, 30000); // Cleanup on connection close - const cleanup = () => { + let cleanup = () => { if (debounceTimer) { - clearTimeout(debounceTimer); + timers.clearTimeout(debounceTimer); debounceTimer = null; } - clearInterval(heartbeatInterval); - if (watcher) { - watcher.close(); - watcher = null; - } - if (filePollInterval) { - clearInterval(filePollInterval); - filePollInterval = null; - } + timers.clearInterval(heartbeatInterval); + cleanupReportWatcher(); }; req.on('close', cleanup); diff --git a/src/services/build-manager.js b/src/services/build-manager.js index 1aba253..eb5240d 100644 --- a/src/services/build-manager.js +++ b/src/services/build-manager.js @@ -1,131 +1,43 @@ /** - * Build Manager - Pure functions for build lifecycle management + * Build Manager - local build object creation for the test runner. */ -import crypto from 'node:crypto'; +import { randomUUID } from 'node:crypto'; /** - * Generate unique build ID for local build management only. - * Note: The API generates its own UUIDs for actual builds - this local ID - * is only used for CLI internal tracking and is not sent to the API. - * @returns {string} Build ID - */ -export function generateBuildId() { - return `build-${crypto.randomUUID()}`; -} - -/** - * Create build object + * Create a local build object for test-runner orchestration. + * + * The API creates persisted build IDs. This object is only used inside the + * CLI process to give the runner a consistent shape before API creation. + * * @param {Object} buildOptions - Build configuration + * @param {Object} [deps] + * @param {Function} [deps.randomId] + * @param {Function} [deps.now] + * @param {Function} [deps.timestamp] * @returns {Object} Build object */ -export function createBuildObject(buildOptions) { - const { +export function createBuildObject(buildOptions, deps = {}) { + let { name, branch, commit, environment = 'test', metadata = {}, } = buildOptions; + let randomId = deps.randomId || randomUUID; + let now = deps.now || (() => new Date().toISOString()); + let timestamp = deps.timestamp || Date.now; return { - id: generateBuildId(), - name: name || `build-${Date.now()}`, + id: `build-${randomId()}`, + name: name || `build-${timestamp()}`, branch, commit, environment, metadata, status: 'pending', - createdAt: new Date().toISOString(), + createdAt: now(), screenshots: [], }; } - -/** - * Update build with new status and data - * @param {Object} build - Current build - * @param {string} status - New status - * @param {Object} updates - Additional updates - * @returns {Object} Updated build - */ -export function updateBuild(build, status, updates = {}) { - return { - ...build, - status, - updatedAt: new Date().toISOString(), - ...updates, - }; -} - -/** - * Add screenshot to build - * @param {Object} build - Current build - * @param {Object} screenshot - Screenshot data - * @returns {Object} Updated build - */ -export function addScreenshotToBuild(build, screenshot) { - return { - ...build, - screenshots: [ - ...build.screenshots, - { - ...screenshot, - addedAt: new Date().toISOString(), - }, - ], - }; -} - -/** - * Finalize build with result - * @param {Object} build - Current build - * @param {Object} result - Build result - * @returns {Object} Finalized build - */ -export function finalizeBuildObject(build, result = {}) { - const finalStatus = result.success ? 'completed' : 'failed'; - - return { - ...build, - status: finalStatus, - completedAt: new Date().toISOString(), - result, - }; -} - -/** - * Create queued build item - * @param {Object} buildOptions - Build options - * @returns {Object} Queued build item - */ -export function createQueuedBuild(buildOptions) { - return { - ...buildOptions, - queuedAt: new Date().toISOString(), - }; -} - -/** - * Validate build options - * @param {Object} buildOptions - Build options to validate - * @returns {Object} Validation result - */ -export function validateBuildOptions(buildOptions) { - let errors = []; - - if (!buildOptions.name && !buildOptions.branch) { - errors.push('Either name or branch is required'); - } - - if ( - buildOptions.environment && - !['test', 'staging', 'production'].includes(buildOptions.environment) - ) { - errors.push('Environment must be one of: test, staging, production'); - } - - return { - valid: errors.length === 0, - errors, - }; -} diff --git a/src/services/index.js b/src/services/index.js index c5abbaf..16cb232 100644 --- a/src/services/index.js +++ b/src/services/index.js @@ -9,7 +9,7 @@ * This factory is only used by cli.js to provide services to plugins. */ -import { ServerManager } from './server-manager.js'; +import { createServerManager } from '../server-manager/index.js'; import { TestRunner } from './test-runner.js'; /** @@ -26,9 +26,7 @@ import { TestRunner } from './test-runner.js'; * @returns {Object} Services object for plugins */ export function createServices(config) { - let serverManager = new ServerManager(config, { - services: {}, - }); + let serverManager = createServerManager(config, {}); let testRunner = new TestRunner(config, serverManager); diff --git a/src/services/server-manager.js b/src/services/server-manager.js deleted file mode 100644 index da4449e..0000000 --- a/src/services/server-manager.js +++ /dev/null @@ -1,86 +0,0 @@ -/** - * Server Manager Service - * Manages the HTTP server with functional handlers - * - * This class is a thin wrapper around the functional operations in - * src/server-manager/. It maintains backwards compatibility while - * delegating to pure functions for testability. - */ - -import { existsSync, mkdirSync, unlinkSync, writeFileSync } from 'node:fs'; -import { createApiClient } from '../api/index.js'; -import { createApiHandler } from '../server/handlers/api-handler.js'; -import { createTddHandler } from '../server/handlers/tdd-handler.js'; -import { createHttpServer } from '../server/http-server.js'; -import { - buildServerInterface, - getTddResults, - startServer, - stopServer, -} from '../server-manager/index.js'; - -export class ServerManager { - constructor(config, options = {}) { - this.config = config; - this.httpServer = null; - this.handler = null; - this.services = options.services || {}; - this.tddMode = false; - - // Dependency injection for testing - defaults to real implementations - this.deps = options.deps || { - createHttpServer, - createTddHandler, - createApiHandler, - createApiClient, - fs: { mkdirSync, writeFileSync, existsSync, unlinkSync }, - }; - } - - async start(buildId = null, tddMode = false, setBaseline = false) { - this.buildId = buildId; - this.tddMode = tddMode; - this.setBaseline = setBaseline; - - let result = await startServer({ - config: this.config, - buildId, - tddMode, - setBaseline, - projectRoot: process.cwd(), - services: this.services, - deps: this.deps, - }); - - this.httpServer = result.httpServer; - this.handler = result.handler; - } - - async stop() { - await stopServer({ - httpServer: this.httpServer, - handler: this.handler, - projectRoot: process.cwd(), - deps: this.deps, - }); - } - - // Expose server interface for compatibility - get server() { - return buildServerInterface({ - handler: this.handler, - httpServer: this.httpServer, - }); - } - - /** - * Get TDD results (comparisons, screenshot count, etc.) - * Only available in TDD mode after tests have run - */ - async getTddResults() { - return getTddResults({ - tddMode: this.tddMode, - handler: this.handler, - }); - } -} diff --git a/src/services/static-report-generator.js b/src/services/static-report-generator.js index fc22c64..013cbf0 100644 --- a/src/services/static-report-generator.js +++ b/src/services/static-report-generator.js @@ -15,7 +15,7 @@ import { writeFileSync, } from 'node:fs'; import { dirname, join } from 'node:path'; -import { fileURLToPath } from 'node:url'; +import { fileURLToPath, pathToFileURL } from 'node:url'; let __dirname = dirname(fileURLToPath(import.meta.url)); @@ -141,6 +141,14 @@ ${css} `; } +export async function loadStaticReportRenderer( + ssrModulePath = getSSRModulePath() +) { + let moduleUrl = pathToFileURL(ssrModulePath).href; + let { renderStaticReport } = await import(moduleUrl); + return renderStaticReport; +} + /** * Generate a static report from the current TDD results * @@ -179,17 +187,13 @@ export async function generateStaticReport(workingDir, options = {}) { // Transform image URLs to relative paths let transformedData = transformImageUrls(reportData); - // Load and use the SSR module - let ssrModulePath = getSSRModulePath(); - let { renderStaticReport } = await import(ssrModulePath); + let renderStaticReport = + options.renderStaticReport || (await loadStaticReportRenderer()); let renderedContent = renderStaticReport(transformedData); - // Get CSS - let reporterDistPath = getReporterDistPath(); - let css = readFileSync( - join(reporterDistPath, 'reporter-bundle.css'), - 'utf8' - ); + let css = + options.css ?? + readFileSync(join(getReporterDistPath(), 'reporter-bundle.css'), 'utf8'); // Create output directory mkdirSync(outputDir, { recursive: true }); @@ -224,5 +228,5 @@ export async function generateStaticReport(workingDir, options = {}) { * Get the file:// URL for a report path */ export function getReportFileUrl(reportPath) { - return `file://${reportPath}`; + return pathToFileURL(reportPath).href; } diff --git a/src/services/test-runner.js b/src/services/test-runner.js index 36384e4..9f116f8 100644 --- a/src/services/test-runner.js +++ b/src/services/test-runner.js @@ -2,9 +2,8 @@ * Test Runner Service * Orchestrates the test execution flow * - * This class is a thin wrapper around the functional operations in - * src/test-runner/. It maintains backwards compatibility while - * delegating to pure functions for testability. + * This EventEmitter adapter keeps the stable plugin-facing runner contract + * while delegating execution to the functional operations in src/test-runner/. */ import { spawn } from 'node:child_process'; @@ -138,47 +137,6 @@ export class TestRunner extends EventEmitter { }); } - async executeTestCommand(testCommand, env) { - return new Promise((resolve, reject) => { - let proc = this.deps.spawn(testCommand, { - env, - stdio: 'inherit', - shell: true, - }); - - this.testProcess = proc; - - proc.on('error', error => { - reject( - this.deps.createError( - `Failed to run test command: ${error.message}`, - 'TEST_COMMAND_FAILED' - ) - ); - }); - - proc.on('exit', (code, signal) => { - if (signal === 'SIGINT') { - reject( - this.deps.createError( - 'Test command was interrupted', - 'TEST_COMMAND_INTERRUPTED' - ) - ); - } else if (code !== 0) { - reject( - this.deps.createError( - `Test command exited with code ${code}`, - 'TEST_COMMAND_FAILED' - ) - ); - } else { - resolve(); - } - }); - }); - } - async cancel() { await cancelTests({ testProcess: this.testProcess, diff --git a/src/services/uploader.js b/src/services/uploader.js deleted file mode 100644 index 0dafaaf..0000000 --- a/src/services/uploader.js +++ /dev/null @@ -1,101 +0,0 @@ -/** - * Vizzly Screenshot Uploader - * Handles screenshot uploads to the Vizzly platform - * - * This module is a thin wrapper around the functional operations in - * src/uploader/. It maintains backwards compatibility while - * delegating to pure functions for testability. - */ - -import { readFile, stat } from 'node:fs/promises'; -import { glob } from 'glob'; -import { checkShas, createApiClient, createBuild } from '../api/index.js'; -import { - TimeoutError, - UploadError, - ValidationError, -} from '../errors/vizzly-error.js'; -import { resolveBatchSize, resolveTimeout } from '../uploader/index.js'; -import { - upload as uploadOperation, - waitForBuild as waitForBuildOperation, -} from '../uploader/operations.js'; -import { getDefaultBranch } from '../utils/git.js'; -import * as output from '../utils/output.js'; - -/** - * Create a new uploader instance - */ -export function createUploader( - { apiKey, apiUrl, userAgent, command, upload: uploadConfig = {} } = {}, - options = {} -) { - let signal = options.signal || new AbortController().signal; - let client = createApiClient({ - baseUrl: apiUrl, - token: apiKey, - command: command || 'upload', - sdkUserAgent: userAgent, - allowNoToken: true, - }); - - // Resolve tunable parameters - let batchSize = resolveBatchSize(options, uploadConfig); - let timeout = resolveTimeout(options, uploadConfig); - - // Dependency injection for testing - let deps = options.deps || { - client, - createBuild, - getDefaultBranch, - glob, - readFile, - stat, - checkShas, - createError: (message, code, context) => { - let error = new UploadError(message, context); - error.code = code; - return error; - }, - createValidationError: (message, context) => - new ValidationError(message, context), - createUploadError: (message, context) => new UploadError(message, context), - createTimeoutError: (message, context) => - new TimeoutError(message, context), - output, - }; - - /** - * Upload screenshots to Vizzly - */ - async function upload(uploadOptions) { - return uploadOperation({ - uploadOptions, - config: { apiKey, apiUrl }, - signal, - batchSize, - deps: { - ...deps, - client: deps.client || client, - }, - }); - } - - /** - * Wait for a build to complete - */ - async function waitForBuild(buildId, waitTimeout = timeout) { - return waitForBuildOperation({ - buildId, - timeout: waitTimeout, - signal, - client: deps.client || client, - deps: { - createError: deps.createError, - createTimeoutError: deps.createTimeoutError, - }, - }); - } - - return { upload, waitForBuild }; -} diff --git a/src/tdd/index.js b/src/tdd/index.js index 3d18a27..a13a45c 100644 --- a/src/tdd/index.js +++ b/src/tdd/index.js @@ -58,10 +58,7 @@ export { compareImages, isDimensionMismatchError, } from './services/comparison-service.js'; -export { - downloadHotspots, - extractScreenshotNames, -} from './services/hotspot-service.js'; +export { downloadHotspots } from './services/hotspot-service.js'; export { buildResults, calculateSummary, diff --git a/src/tdd/metadata/region-metadata.js b/src/tdd/metadata/region-metadata.js index b639e08..085243a 100644 --- a/src/tdd/metadata/region-metadata.js +++ b/src/tdd/metadata/region-metadata.js @@ -56,38 +56,3 @@ export function saveRegionMetadata(workingDir, regionData, summary = {}) { writeFileSync(regionsPath, JSON.stringify(content, null, 2)); } - -/** - * Get regions for a specific screenshot with caching support - * - * This is a pure function that takes a cache object as parameter - * for stateless operation. The cache is mutated if data needs to be loaded. - * - * @param {Object} cache - Cache object { data: Object|null, loaded: boolean } - * @param {string} workingDir - Working directory - * @param {string} screenshotName - Name of the screenshot - * @returns {Object|null} Region data or null if not available - */ -export function getRegionsForScreenshot(cache, workingDir, screenshotName) { - // Check cache first - if (cache.data?.[screenshotName]) { - return cache.data[screenshotName]; - } - - // Load from disk if not yet loaded - if (!cache.loaded) { - cache.data = loadRegionMetadata(workingDir); - cache.loaded = true; - } - - return cache.data?.[screenshotName] || null; -} - -/** - * Create an empty region cache object - * - * @returns {{ data: null, loaded: boolean }} - */ -export function createRegionCache() { - return { data: null, loaded: false }; -} diff --git a/src/tdd/server-registry.js b/src/tdd/server-registry.js index 4ab495e..65a1673 100644 --- a/src/tdd/server-registry.js +++ b/src/tdd/server-registry.js @@ -1,4 +1,4 @@ -import { execSync } from 'node:child_process'; +import { execFileSync } from 'node:child_process'; import { randomBytes } from 'node:crypto'; import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; import { createServer } from 'node:net'; @@ -10,9 +10,11 @@ import { join } from 'node:path'; * Enables the menubar app to discover and manage multiple concurrent servers. */ export class ServerRegistry { - constructor() { - this.vizzlyHome = process.env.VIZZLY_HOME || join(homedir(), '.vizzly'); + constructor({ vizzlyHome, logger = console } = {}) { + this.vizzlyHome = + vizzlyHome || process.env.VIZZLY_HOME || join(homedir(), '.vizzly'); this.registryPath = join(this.vizzlyHome, 'servers.json'); + this.logger = logger; } /** @@ -38,7 +40,9 @@ export class ServerRegistry { } } catch (_err) { // Corrupted file, start fresh - console.warn('Warning: Could not read server registry, starting fresh'); + this.logger.warn( + 'Warning: Could not read server registry, starting fresh' + ); } return { version: 1, servers: [] }; } @@ -60,11 +64,11 @@ export class ServerRegistry { throw new Error('Missing required fields: pid, port, directory'); } - let port = Number(serverInfo.port); - let pid = Number(serverInfo.pid); + let port = parsePositiveInteger(serverInfo.port); + let pid = parsePositiveInteger(serverInfo.pid); - if (Number.isNaN(port) || Number.isNaN(pid)) { - throw new Error('Invalid port or pid - must be numbers'); + if (port === null || pid === null) { + throw new Error('Invalid port or pid - must be positive integers'); } let registry = this.read(); @@ -178,16 +182,7 @@ export class ServerRegistry { * The menubar app listens for this in addition to file watching. */ notifyMenubar() { - if (process.platform !== 'darwin') return; - - try { - execSync('notifyutil -p dev.vizzly.serverChanged', { - stdio: 'ignore', - timeout: 500, - }); - } catch { - // Non-fatal - menubar will still see changes via file watching - } + notifyServerRegistryChanged(); } /** @@ -229,6 +224,36 @@ export class ServerRegistry { } } +function parsePositiveInteger(value) { + if (typeof value === 'number') { + return Number.isInteger(value) && value > 0 ? value : null; + } + + if (typeof value === 'string' && /^[1-9]\d*$/.test(value)) { + return Number(value); + } + + return null; +} + +export function notifyServerRegistryChanged({ + platform = process.platform, + execFile = execFileSync, +} = {}) { + if (platform !== 'darwin') return false; + + try { + execFile('notifyutil', ['-p', 'dev.vizzly.serverChanged'], { + stdio: 'ignore', + timeout: 500, + }); + return true; + } catch { + // Non-fatal - menubar will still see changes via file watching + return false; + } +} + /** * Check if a port is free (not in use by any process) * @param {number} port - Port to check diff --git a/src/tdd/services/hotspot-service.js b/src/tdd/services/hotspot-service.js index 32e69a9..a1a0fe6 100644 --- a/src/tdd/services/hotspot-service.js +++ b/src/tdd/services/hotspot-service.js @@ -45,17 +45,3 @@ export async function downloadHotspots(options) { return { success: false, error: error.message }; } } - -/** - * Extract screenshot names from a list of screenshots - * - * @param {Array} screenshots - Screenshots with name property - * @returns {string[]} - */ -export function extractScreenshotNames(screenshots) { - if (!screenshots || !Array.isArray(screenshots)) { - return []; - } - - return screenshots.map(s => s.name).filter(Boolean); -} diff --git a/src/tdd/services/region-service.js b/src/tdd/services/region-service.js deleted file mode 100644 index 05a6fb8..0000000 --- a/src/tdd/services/region-service.js +++ /dev/null @@ -1,59 +0,0 @@ -/** - * Region Service - * - * Functions for downloading and managing user-defined hotspot regions from the cloud. - * Regions are 2D bounding boxes that users have confirmed as dynamic content areas. - */ - -import { saveRegionMetadata } from '../metadata/region-metadata.js'; - -/** - * Download user-defined regions from cloud API - * - * @param {Object} options - * @param {Object} options.api - ApiService instance - * @param {string} options.workingDir - Working directory - * @param {string[]} options.screenshotNames - Names of screenshots to get regions for - * @param {boolean} options.includeCandidates - Include candidate regions (default: false) - * @returns {Promise<{ success: boolean, count: number, regionCount: number, error?: string }>} - */ -export async function downloadRegions(options) { - let { api, workingDir, screenshotNames, includeCandidates = false } = options; - - if (!screenshotNames || screenshotNames.length === 0) { - return { success: true, count: 0, regionCount: 0 }; - } - - try { - let response = await api.getRegions(screenshotNames, { includeCandidates }); - - if (!response?.regions) { - return { success: false, error: 'API returned no region data' }; - } - - // Save regions to disk - saveRegionMetadata(workingDir, response.regions, response.summary); - - // Calculate stats - let count = Object.keys(response.regions).length; - let regionCount = response.summary?.total_regions || 0; - - return { success: true, count, regionCount }; - } catch (error) { - return { success: false, error: error.message }; - } -} - -/** - * Extract screenshot names from a list of screenshots - * - * @param {Array} screenshots - Screenshots with name property - * @returns {string[]} - */ -export function extractScreenshotNames(screenshots) { - if (!screenshots || !Array.isArray(screenshots)) { - return []; - } - - return screenshots.map(s => s.name).filter(Boolean); -} diff --git a/src/tdd/tdd-service.js b/src/tdd/tdd-service.js index 0b6a0d5..beceee0 100644 --- a/src/tdd/tdd-service.js +++ b/src/tdd/tdd-service.js @@ -256,7 +256,7 @@ export class TddService { // State this.baselineData = null; this.comparisons = []; - this.threshold = config.comparison?.threshold || 2.0; + this.threshold = config.comparison?.threshold ?? 2.0; this.minClusterSize = config.comparison?.minClusterSize ?? 2; this.signatureProperties = config.signatureProperties ?? []; @@ -1199,7 +1199,7 @@ export class TddService { } this.baselineData = metadata; - this.threshold = metadata.threshold || this.threshold; + this.threshold = metadata.threshold ?? this.threshold; this.signatureProperties = metadata.signatureProperties || this.signatureProperties; diff --git a/src/uploader/core.js b/src/uploader/core.js index d984d87..e9c485d 100644 --- a/src/uploader/core.js +++ b/src/uploader/core.js @@ -14,6 +14,7 @@ import { basename } from 'node:path'; export const DEFAULT_BATCH_SIZE = 50; export const DEFAULT_SHA_CHECK_BATCH_SIZE = 100; export const DEFAULT_TIMEOUT = 30000; // 30 seconds +export const DEFAULT_BUILD_POLL_INTERVAL = 1000; // 1 second // ============================================================================ // Validation @@ -340,8 +341,8 @@ export function resolveTimeout(options, uploadConfig) { * @param {number} timeout - Timeout in ms * @returns {boolean} True if timed out */ -export function isTimedOut(startTime, timeout) { - return Date.now() - startTime >= timeout; +export function isTimedOut(startTime, timeout, now = Date.now()) { + return now - startTime >= timeout; } /** @@ -349,8 +350,8 @@ export function isTimedOut(startTime, timeout) { * @param {number} startTime - Start timestamp * @returns {number} Elapsed time in ms */ -export function getElapsedTime(startTime) { - return Date.now() - startTime; +export function getElapsedTime(startTime, now = Date.now()) { + return now - startTime; } /** diff --git a/src/uploader/index.js b/src/uploader/index.js index aaa3ca5..deade4b 100644 --- a/src/uploader/index.js +++ b/src/uploader/index.js @@ -1,9 +1,91 @@ /** * Uploader Module * - * Exports pure functions (core) and I/O operations for screenshot uploading. + * Exports pure functions (core), I/O operations, and the public uploader + * factory for screenshot uploading. */ +import { readFile, stat } from 'node:fs/promises'; +import { glob } from 'glob'; +import { checkShas, createApiClient, createBuild } from '../api/index.js'; +import { + TimeoutError, + UploadError, + ValidationError, +} from '../errors/vizzly-error.js'; +import { getDefaultBranch } from '../utils/git.js'; +import * as output from '../utils/output.js'; +import { resolveBatchSize, resolveTimeout } from './core.js'; +import { + upload as uploadOperation, + waitForBuild as waitForBuildOperation, +} from './operations.js'; + +export function createUploader( + { apiKey, apiUrl, userAgent, command, upload: uploadConfig = {} } = {}, + options = {} +) { + let signal = options.signal || new AbortController().signal; + let client = createApiClient({ + baseUrl: apiUrl, + token: apiKey, + command: command || 'upload', + sdkUserAgent: userAgent, + allowNoToken: true, + }); + + let batchSize = resolveBatchSize(options, uploadConfig); + let timeout = resolveTimeout(options, uploadConfig); + let deps = options.deps || { + client, + createBuild, + getDefaultBranch, + glob, + readFile, + stat, + checkShas, + createError: (message, code, context) => { + let error = new UploadError(message, context); + error.code = code; + return error; + }, + createValidationError: (message, context) => + new ValidationError(message, context), + createUploadError: (message, context) => new UploadError(message, context), + createTimeoutError: (message, context) => + new TimeoutError(message, context), + output, + }; + + async function upload(uploadOptions) { + return uploadOperation({ + uploadOptions, + config: { apiKey, apiUrl }, + signal, + batchSize, + deps: { + ...deps, + client: deps.client || client, + }, + }); + } + + async function waitForBuild(buildId, waitTimeout = timeout) { + return waitForBuildOperation({ + buildId, + timeout: waitTimeout, + signal, + client: deps.client || client, + deps: { + createError: deps.createError, + createTimeoutError: deps.createTimeoutError, + }, + }); + } + + return { upload, waitForBuild }; +} + // Core - pure functions export { buildBuildInfo, @@ -18,6 +100,7 @@ export { buildWaitResult, computeSha256, DEFAULT_BATCH_SIZE, + DEFAULT_BUILD_POLL_INTERVAL, DEFAULT_SHA_CHECK_BATCH_SIZE, DEFAULT_TIMEOUT, extractBrowserFromFilename, diff --git a/src/uploader/operations.js b/src/uploader/operations.js index ef2f386..d571d1a 100644 --- a/src/uploader/operations.js +++ b/src/uploader/operations.js @@ -15,6 +15,7 @@ import { buildUploadingProgress, buildUploadResult, buildWaitResult, + DEFAULT_BUILD_POLL_INTERVAL, DEFAULT_SHA_CHECK_BATCH_SIZE, extractStatusCodeFromError, fileToScreenshotFormat, @@ -27,6 +28,38 @@ import { validateScreenshotsDir, } from './core.js'; +function delay(ms, signal) { + if (ms <= 0) { + return Promise.resolve(); + } + + return new Promise((resolve, reject) => { + let timeoutId; + + function cleanup() { + clearTimeout(timeoutId); + signal?.removeEventListener?.('abort', handleAbort); + } + + function handleAbort() { + cleanup(); + reject(new Error('Operation cancelled')); + } + + timeoutId = setTimeout(() => { + cleanup(); + resolve(); + }, ms); + + if (signal?.aborted) { + handleAbort(); + return; + } + + signal?.addEventListener?.('abort', handleAbort, { once: true }); + }); +} + // ============================================================================ // File Discovery // ============================================================================ @@ -232,10 +265,16 @@ export async function uploadFiles({ * @returns {Promise} Build result */ export async function waitForBuild({ buildId, timeout, signal, client, deps }) { - let { createError, createTimeoutError } = deps; - let startTime = Date.now(); + let { + createError, + createTimeoutError, + now = () => Date.now(), + pollInterval = DEFAULT_BUILD_POLL_INTERVAL, + wait = delay, + } = deps; + let startTime = now(); - while (!isTimedOut(startTime, timeout)) { + while (!isTimedOut(startTime, timeout, now())) { if (signal.aborted) { throw createError('Operation cancelled', 'UPLOAD_CANCELLED', { buildId }); } @@ -263,12 +302,19 @@ export async function waitForBuild({ buildId, timeout, signal, client, deps }) { 'BUILD_FAILED' ); } + + let remaining = timeout - getElapsedTime(startTime, now()); + if (remaining <= 0) { + break; + } + + await wait(Math.min(pollInterval, remaining), signal); } throw createTimeoutError(`Build timed out after ${timeout}ms`, { buildId, timeout, - elapsed: getElapsedTime(startTime), + elapsed: getElapsedTime(startTime, now()), }); } diff --git a/tests/commands/doctor.test.js b/tests/commands/doctor.test.js new file mode 100644 index 0000000..0eaaf6c --- /dev/null +++ b/tests/commands/doctor.test.js @@ -0,0 +1,294 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import { + createDoctorDiagnostics, + doctorCommand, + getApiUrlCheck, + getNodeVersionCheck, + getThresholdCheck, + validateDoctorOptions, +} from '../../src/commands/doctor.js'; + +function createMockOutput() { + let calls = []; + return { + calls, + configure: opts => calls.push({ method: 'configure', args: [opts] }), + data: value => calls.push({ method: 'data', args: [value] }), + error: (message, error) => + calls.push({ method: 'error', args: [message, error] }), + header: (command, mode) => + calls.push({ method: 'header', args: [command, mode] }), + printErr: message => calls.push({ method: 'printErr', args: [message] }), + startSpinner: message => + calls.push({ method: 'startSpinner', args: [message] }), + stopSpinner: () => calls.push({ method: 'stopSpinner', args: [] }), + warn: message => calls.push({ method: 'warn', args: [message] }), + cleanup: () => calls.push({ method: 'cleanup', args: [] }), + getColors: () => ({ + brand: { + success: value => value, + danger: value => value, + textTertiary: value => value, + }, + cyan: value => value, + dim: value => value, + gray: value => value, + green: value => value, + underline: value => value, + white: value => value, + yellow: value => value, + }), + }; +} + +let validConfig = { + apiUrl: 'https://api.example.test', + apiKey: 'token-123', + comparison: { threshold: 2 }, + server: { port: 47888 }, +}; + +describe('commands/doctor', () => { + describe('validateDoctorOptions', () => { + it('returns no errors', () => { + assert.deepStrictEqual(validateDoctorOptions({}), []); + }); + }); + + describe('diagnostic helpers', () => { + it('creates the default diagnostics shape', () => { + assert.deepStrictEqual(createDoctorDiagnostics(), { + environment: { + nodeVersion: null, + nodeVersionValid: null, + }, + configuration: { + apiUrl: null, + apiUrlValid: null, + threshold: null, + thresholdValid: null, + port: null, + }, + connectivity: { + checked: false, + ok: null, + error: null, + }, + }); + }); + + it('requires Node 22 or newer', () => { + let unsupported = getNodeVersionCheck('v21.9.0'); + let supported = getNodeVersionCheck('v22.0.0'); + let supportedWithoutPrefix = getNodeVersionCheck('22.1.0'); + + assert.strictEqual(unsupported.check.ok, false); + assert.match(unsupported.check.value, />= 22/); + assert.strictEqual(supported.check.ok, true); + assert.strictEqual(supportedWithoutPrefix.check.ok, true); + }); + + it('reports malformed Node versions as unsupported', () => { + let result = getNodeVersionCheck('v22abc'); + + assert.strictEqual(result.check.ok, false); + assert.strictEqual(result.diagnostic.nodeVersionValid, false); + assert.match(result.check.value, /unrecognized/); + }); + + it('accepts only HTTP API URLs', () => { + assert.strictEqual(getApiUrlCheck('https://api.test').apiUrlValid, true); + assert.strictEqual(getApiUrlCheck('http://api.test').apiUrlValid, true); + assert.strictEqual( + getApiUrlCheck('file:///tmp/vizzly').apiUrlValid, + false + ); + assert.strictEqual(getApiUrlCheck('not a url').apiUrlValid, false); + }); + + it('accepts non-negative CIEDE2000 thresholds', () => { + assert.deepStrictEqual(getThresholdCheck(2), { + threshold: 2, + thresholdValid: true, + check: { + name: 'Threshold', + value: '2 (CIEDE2000)', + ok: true, + }, + }); + assert.strictEqual(getThresholdCheck(-1).thresholdValid, false); + assert.strictEqual(getThresholdCheck('nope').thresholdValid, false); + }); + }); + + describe('doctorCommand', () => { + it('reports local diagnostics as JSON without API connectivity', async () => { + let output = createMockOutput(); + + await doctorCommand( + {}, + { json: true }, + { + getApiToken: () => null, + loadConfig: async () => validConfig, + nodeVersion: 'v22.2.0', + output, + } + ); + + let dataCall = output.calls.find(call => call.method === 'data'); + assert.strictEqual(dataCall.args[0].passed, true); + assert.strictEqual( + dataCall.args[0].diagnostics.environment.nodeVersionValid, + true + ); + assert.strictEqual( + dataCall.args[0].diagnostics.connectivity.checked, + false + ); + assert.ok(output.calls.some(call => call.method === 'cleanup')); + }); + + it('checks API connectivity when requested', async () => { + let output = createMockOutput(); + let capturedClientOptions = null; + let capturedBuildOptions = null; + + await doctorCommand( + { api: true }, + { json: true }, + { + createApiClient: options => { + capturedClientOptions = options; + return { kind: 'client' }; + }, + getApiToken: () => null, + getBuilds: async (_client, options) => { + capturedBuildOptions = options; + return { builds: [] }; + }, + loadConfig: async () => validConfig, + nodeVersion: 'v22.2.0', + output, + } + ); + + assert.deepStrictEqual(capturedClientOptions, { + baseUrl: 'https://api.example.test', + token: 'token-123', + command: 'doctor', + }); + assert.deepStrictEqual(capturedBuildOptions, { limit: 1 }); + + let dataCall = output.calls.find(call => call.method === 'data'); + assert.strictEqual( + dataCall.args[0].diagnostics.connectivity.checked, + true + ); + assert.strictEqual(dataCall.args[0].diagnostics.connectivity.ok, true); + }); + + it('fails when API connectivity is requested without a token', async () => { + let output = createMockOutput(); + let exitCode = null; + + await doctorCommand( + { api: true }, + { json: true }, + { + getApiToken: () => null, + loadConfig: async () => ({ ...validConfig, apiKey: null }), + nodeVersion: 'v22.2.0', + output, + exit: code => { + exitCode = code; + }, + } + ); + + assert.strictEqual(exitCode, 1); + let dataCall = output.calls.find(call => call.method === 'data'); + assert.strictEqual(dataCall.args[0].passed, false); + assert.strictEqual( + dataCall.args[0].diagnostics.connectivity.error, + 'Missing API token (VIZZLY_TOKEN)' + ); + }); + + it('prints human-readable diagnostics and context', async () => { + let output = createMockOutput(); + + await doctorCommand( + {}, + {}, + { + getApiToken: () => null, + getContext: () => [ + { type: 'success', label: 'Logged in', value: 'rob@example.com' }, + ], + loadConfig: async () => validConfig, + nodeVersion: 'v22.2.0', + output, + } + ); + + assert.ok( + output.calls.some( + call => call.method === 'header' && call.args[1] === 'local' + ) + ); + assert.ok( + output.calls.some( + call => call.method === 'printErr' && call.args[0].includes('Node.js') + ) + ); + assert.ok( + output.calls.some( + call => + call.method === 'printErr' && + call.args[0].includes('rob@example.com') + ) + ); + }); + + it('exits with status 1 when local diagnostics fail', async () => { + let output = createMockOutput(); + let exitCode = null; + + await doctorCommand( + {}, + { json: true }, + { + getApiToken: () => null, + loadConfig: async () => ({ + ...validConfig, + apiUrl: 'file:///tmp/vizzly', + comparison: { threshold: -1 }, + }), + nodeVersion: 'v21.9.0', + output, + exit: code => { + exitCode = code; + }, + } + ); + + assert.strictEqual(exitCode, 1); + let dataCall = output.calls.find(call => call.method === 'data'); + assert.strictEqual(dataCall.args[0].passed, false); + assert.strictEqual( + dataCall.args[0].diagnostics.environment.nodeVersionValid, + false + ); + assert.strictEqual( + dataCall.args[0].diagnostics.configuration.apiUrlValid, + false + ); + assert.strictEqual( + dataCall.args[0].diagnostics.configuration.thresholdValid, + false + ); + }); + }); +}); diff --git a/tests/commands/preview.test.js b/tests/commands/preview.test.js index f745029..63e3c97 100644 --- a/tests/commands/preview.test.js +++ b/tests/commands/preview.test.js @@ -532,6 +532,37 @@ describe('previewCommand', () => { ); }); + it('treats regex syntax literally in --exclude patterns', async () => { + writeFileSync( + join(distDir, 'widget[primary]-main.js'), + 'console.log("skip")' + ); + writeFileSync(join(distDir, 'widgetp-main.js'), 'console.log("keep")'); + let output = createMockOutput(); + + let result = await previewCommand( + distDir, + { + dryRun: true, + build: 'build-123', + exclude: ['widget[primary]*.js'], + }, + {}, + { + loadConfig: async () => ({ + apiKey: null, + apiUrl: 'https://api.test', + }), + output, + exit: () => {}, + } + ); + + let filePaths = result.files.map(f => f.path); + assert.ok(!filePaths.includes('widget[primary]-main.js')); + assert.ok(filePaths.includes('widgetp-main.js')); + }); + it('excludes directories matching --exclude patterns with trailing slash', async () => { let output = createMockOutput(); @@ -842,4 +873,44 @@ describe('previewCommand', () => { assert.strictEqual(uploadCalled, true, 'Should call upload'); assert.strictEqual(result.success, true); }); + + it('uploads previews from paths with spaces', async () => { + let spacedDir = join(testDir, 'dist with spaces'); + mkdirSync(spacedDir, { recursive: true }); + writeFileSync(join(spacedDir, 'index.html'), ''); + + let output = createMockOutput(); + let uploadedBytes = 0; + + let result = await previewCommand( + spacedDir, + { build: 'build-123' }, + {}, + { + loadConfig: async () => ({ + apiKey: 'test-token', + apiUrl: 'https://api.test', + }), + createApiClient: () => ({}), + getBuild: async () => ({ + id: 'build-123', + project: { id: 'proj-1', name: 'Test Project', isPublic: true }, + }), + uploadPreviewZip: async (_client, _buildId, zipBuffer) => { + uploadedBytes = zipBuffer.length; + return { + previewUrl: 'https://preview.test', + uploaded: 1, + totalBytes: zipBuffer.length, + newBytes: zipBuffer.length, + }; + }, + output, + exit: () => {}, + } + ); + + assert.strictEqual(result.success, true); + assert.ok(uploadedBytes > 0, 'Should upload a generated ZIP'); + }); }); diff --git a/tests/commands/run.test.js b/tests/commands/run.test.js index e7f3999..69a2151 100644 --- a/tests/commands/run.test.js +++ b/tests/commands/run.test.js @@ -1,6 +1,10 @@ import assert from 'node:assert'; import { describe, it } from 'node:test'; -import { runCommand, validateRunOptions } from '../../src/commands/run.js'; +import { + resolveBuildDisplayUrl, + runCommand, + validateRunOptions, +} from '../../src/commands/run.js'; /** * Create mock output object that tracks calls @@ -56,6 +60,77 @@ function createMockConfig(overrides = {}) { } describe('commands/run', () => { + describe('resolveBuildDisplayUrl', () => { + it('uses the API result URL when one is already present', async () => { + let url = await resolveBuildDisplayUrl({ + result: { + buildId: 'build-123', + url: 'https://app.test/acme/web/builds/build-123', + }, + config: createMockConfig(), + createApiClient: () => { + throw new Error('should not fetch token context'); + }, + }); + + assert.strictEqual(url, 'https://app.test/acme/web/builds/build-123'); + }); + + it('builds an organization/project URL from token context', async () => { + let clientArgs; + let url = await resolveBuildDisplayUrl({ + result: { buildId: 'build-123' }, + config: createMockConfig({ + apiUrl: 'https://api.test/api/v1', + apiKey: 'token-123', + }), + createApiClient: args => { + clientArgs = args; + return { client: true }; + }, + getTokenContext: async client => { + assert.deepStrictEqual(client, { client: true }); + return { + organization: { slug: 'acme' }, + project: { slug: 'web' }, + }; + }, + }); + + assert.deepStrictEqual(clientArgs, { + baseUrl: 'https://api.test/api/v1', + token: 'token-123', + command: 'run', + }); + assert.strictEqual(url, 'https://api.test/acme/web/builds/build-123'); + }); + + it('falls back to the build URL when token context lookup fails', async () => { + let url = await resolveBuildDisplayUrl({ + result: { buildId: 'build-123' }, + config: createMockConfig({ + apiUrl: 'https://api.test/api/v1', + apiKey: 'token-123', + }), + createApiClient: () => ({ client: true }), + getTokenContext: async () => { + throw new Error('context unavailable'); + }, + }); + + assert.strictEqual(url, 'https://api.test/builds/build-123'); + }); + + it('returns undefined when no URL can be resolved without a token', async () => { + let url = await resolveBuildDisplayUrl({ + result: { buildId: 'build-123' }, + config: createMockConfig({ apiKey: null }), + }); + + assert.strictEqual(url, undefined); + }); + }); + describe('runCommand', () => { it('returns error when no API key and allowNoToken not set', async () => { let output = createMockOutput(); @@ -1222,6 +1297,13 @@ describe('commands/run', () => { ); }); + it('should fail with decimal port number', () => { + let errors = validateRunOptions('npm test', { port: '3000.5' }); + assert.ok( + errors.includes('Port must be a valid number between 1 and 65535') + ); + }); + it('should fail with port out of range (too low)', () => { let errors = validateRunOptions('npm test', { port: '0' }); assert.ok( @@ -1250,6 +1332,13 @@ describe('commands/run', () => { ); }); + it('should fail with decimal timeout', () => { + let errors = validateRunOptions('npm test', { timeout: '5000.5' }); + assert.ok( + errors.includes('Timeout must be at least 1000 milliseconds') + ); + }); + it('should fail with timeout too low', () => { let errors = validateRunOptions('npm test', { timeout: '500' }); assert.ok( @@ -1269,6 +1358,11 @@ describe('commands/run', () => { assert.ok(errors.includes('Batch size must be a positive integer')); }); + it('should fail with decimal batch size', () => { + let errors = validateRunOptions('npm test', { batchSize: '2.5' }); + assert.ok(errors.includes('Batch size must be a positive integer')); + }); + it('should fail with zero batch size', () => { let errors = validateRunOptions('npm test', { batchSize: '0' }); assert.ok(errors.includes('Batch size must be a positive integer')); @@ -1297,6 +1391,17 @@ describe('commands/run', () => { ); }); + it('should fail with decimal upload timeout', () => { + let errors = validateRunOptions('npm test', { + uploadTimeout: '2500.5', + }); + assert.ok( + errors.includes( + 'Upload timeout must be a positive integer (milliseconds)' + ) + ); + }); + it('should fail with zero upload timeout', () => { let errors = validateRunOptions('npm test', { uploadTimeout: '0' }); assert.ok( @@ -1307,6 +1412,61 @@ describe('commands/run', () => { }); }); + describe('comparison validation', () => { + it('should pass with exact-match threshold and min cluster size', () => { + let errors = validateRunOptions('npm test', { + threshold: '0', + minClusterSize: '1', + }); + + assert.strictEqual(errors.length, 0); + }); + + it('should fail with invalid threshold', () => { + let errors = validateRunOptions('npm test', { + threshold: 'invalid', + }); + + assert.ok( + errors.includes( + 'Threshold must be a non-negative number (CIEDE2000 Delta E)' + ) + ); + }); + + it('should fail when threshold has trailing text', () => { + let errors = validateRunOptions('npm test', { + threshold: '2abc', + }); + + assert.ok( + errors.includes( + 'Threshold must be a non-negative number (CIEDE2000 Delta E)' + ) + ); + }); + + it('should fail with non-integer min cluster size', () => { + let errors = validateRunOptions('npm test', { + minClusterSize: '2.5', + }); + + assert.ok( + errors.includes('Min cluster size must be a positive integer') + ); + }); + + it('should fail with zero min cluster size', () => { + let errors = validateRunOptions('npm test', { + minClusterSize: '0', + }); + + assert.ok( + errors.includes('Min cluster size must be a positive integer') + ); + }); + }); + describe('multiple validation errors', () => { it('should return all validation errors', () => { let errors = validateRunOptions('', { diff --git a/tests/commands/status.test.js b/tests/commands/status.test.js index 6cf26e9..a7e1160 100644 --- a/tests/commands/status.test.js +++ b/tests/commands/status.test.js @@ -1,6 +1,13 @@ -import assert from 'node:assert'; +import assert from 'node:assert/strict'; import { describe, it } from 'node:test'; import { + createBuildInfo, + createBuildUrl, + createComparisonStats, + createStatusData, + getProcessingProgress, + normalizeBuildStatus, + shouldFailStatus, statusCommand, validateStatusOptions, } from '../../src/commands/status.js'; @@ -45,6 +52,66 @@ function createMockOutput() { }; } +function createBuild(overrides = {}) { + return { + id: 'build-123', + status: 'completed', + name: 'Homepage', + created_at: '2026-05-18T12:00:00.000Z', + updated_at: '2026-05-18T12:01:00.000Z', + completed_at: '2026-05-18T12:02:00.000Z', + environment: 'ci', + branch: 'main', + commit_sha: 'abcdef1234567890', + commit_message: 'Update homepage', + screenshot_count: 3, + total_comparisons: 3, + new_comparisons: 1, + changed_comparisons: 1, + identical_comparisons: 1, + approval_status: 'pending', + execution_time_ms: 4250, + is_baseline: false, + user_agent: 'vizzly-test', + project_id: 'project-123', + failed_jobs: 0, + ...overrides, + }; +} + +function createStatusHarness(build = createBuild(), previewInfo = null) { + let output = createMockOutput(); + let clientConfig = null; + let exitCode = null; + + return { + output, + get clientConfig() { + return clientConfig; + }, + get exitCode() { + return exitCode; + }, + deps: { + loadConfig: async () => ({ + apiKey: 'test-token', + apiUrl: 'https://api.test/api', + }), + createApiClient: config => { + clientConfig = config; + return { kind: 'client' }; + }, + getBuild: async () => ({ build }), + getPreviewInfo: async () => previewInfo, + getApiUrl: () => 'https://app.test/api', + output, + exit: code => { + exitCode = code; + }, + }, + }; +} + describe('commands/status', () => { describe('validateStatusOptions', () => { it('returns no errors for valid build ID', () => { @@ -63,7 +130,209 @@ describe('commands/status', () => { }); }); + describe('status helpers', () => { + it('normalizes wrapped and unwrapped build responses', () => { + let build = createBuild(); + + assert.strictEqual(normalizeBuildStatus({ build }), build); + assert.strictEqual(normalizeBuildStatus(build), build); + }); + + it('creates JSON status data with preview details', () => { + let data = createStatusData(createBuild(), { + preview_url: 'https://preview.test', + status: 'ready', + file_count: 12, + expires_at: '2026-05-19T12:00:00.000Z', + }); + + assert.deepStrictEqual(data, { + buildId: 'build-123', + status: 'completed', + name: 'Homepage', + createdAt: '2026-05-18T12:00:00.000Z', + updatedAt: '2026-05-18T12:01:00.000Z', + completedAt: '2026-05-18T12:02:00.000Z', + environment: 'ci', + branch: 'main', + commit: 'abcdef1234567890', + commitMessage: 'Update homepage', + screenshotsTotal: 3, + comparisonsTotal: 3, + newComparisons: 1, + changedComparisons: 1, + identicalComparisons: 1, + approvalStatus: 'pending', + executionTime: 4250, + isBaseline: false, + userAgent: 'vizzly-test', + preview: { + url: 'https://preview.test', + status: 'ready', + fileCount: 12, + expiresAt: '2026-05-19T12:00:00.000Z', + }, + }); + }); + + it('creates human build info and comparison stats', () => { + let build = createBuild(); + let colors = createMockOutput().getColors(); + + assert.deepStrictEqual(createBuildInfo(build), { + Name: 'Homepage', + Status: 'COMPLETED', + Environment: 'ci', + Branch: 'main', + Commit: 'abcdef12 - Update homepage', + }); + assert.strictEqual( + createComparisonStats(build, colors), + '1 new Ā· 1 changed Ā· 1 identical' + ); + }); + + it('creates build URLs, progress, and failure status', () => { + assert.strictEqual( + createBuildUrl('https://app.test/api', createBuild()), + 'https://app.test/projects/project-123/builds/build-123' + ); + assert.strictEqual( + createBuildUrl('https://api.test/api/v1', createBuild()), + 'https://api.test/projects/project-123/builds/build-123' + ); + assert.strictEqual(createBuildUrl(null, createBuild()), null); + assert.strictEqual( + getProcessingProgress( + createBuild({ + status: 'processing', + completed_jobs: 2, + failed_jobs: 1, + processing_screenshots: 1, + }) + ), + 75 + ); + assert.strictEqual(getProcessingProgress(createBuild()), null); + assert.strictEqual( + shouldFailStatus(createBuild({ status: 'failed' })), + true + ); + assert.strictEqual( + shouldFailStatus(createBuild({ failed_jobs: 1 })), + true + ); + assert.strictEqual(shouldFailStatus(createBuild()), false); + }); + }); + describe('statusCommand', () => { + it('fetches build and preview data for JSON output', async () => { + let harness = createStatusHarness(createBuild(), { + preview_url: 'https://preview.test', + status: 'ready', + file_count: 4, + expires_at: '2026-05-19T12:00:00.000Z', + }); + + await statusCommand('build-123', {}, { json: true }, harness.deps); + + assert.deepStrictEqual(harness.clientConfig, { + baseUrl: 'https://api.test/api', + token: 'test-token', + command: 'status', + }); + + let dataCall = harness.output.calls.find(call => call.method === 'data'); + assert.strictEqual(dataCall.args[0].buildId, 'build-123'); + assert.deepStrictEqual(dataCall.args[0].preview, { + url: 'https://preview.test', + status: 'ready', + fileCount: 4, + expiresAt: '2026-05-19T12:00:00.000Z', + }); + assert.ok(harness.output.calls.some(call => call.method === 'cleanup')); + }); + + it('prints human-readable status and exits non-zero for failed builds', async () => { + let harness = createStatusHarness( + createBuild({ status: 'failed', failed_jobs: 1 }), + { preview_url: 'https://preview.test' } + ); + + await statusCommand('build-123', {}, {}, harness.deps); + + assert.strictEqual(harness.exitCode, 1); + assert.ok( + harness.output.calls.some( + call => call.method === 'header' && call.args[1] === 'failed' + ) + ); + assert.ok( + harness.output.calls.some( + call => call.method === 'labelValue' && call.args[0] === 'Preview' + ) + ); + }); + + it('prints processing progress when jobs are active', async () => { + let harness = createStatusHarness( + createBuild({ + status: 'processing', + completed_jobs: 2, + failed_jobs: 0, + processing_screenshots: 2, + }) + ); + + await statusCommand('build-123', {}, {}, harness.deps); + + assert.ok( + harness.output.calls.some( + call => call.method === 'print' && call.args[0].includes('50%') + ) + ); + }); + + it('does not render NaN average diff in verbose output', async () => { + let harness = createStatusHarness( + createBuild({ avg_diff_percentage: undefined }) + ); + + await statusCommand('build-123', {}, { verbose: true }, harness.deps); + + let keyValueCalls = harness.output.calls.filter( + call => call.method === 'keyValue' + ); + let verboseInfo = keyValueCalls.at(-1).args[0]; + assert.equal(Object.hasOwn(verboseInfo, 'Avg Diff'), false); + }); + + it('cleans up and exits when no API token is configured', async () => { + let output = createMockOutput(); + let exitCode = null; + + let result = await statusCommand( + 'build-123', + {}, + {}, + { + loadConfig: async () => ({ apiUrl: 'https://api.test' }), + output, + exit: code => { + exitCode = code; + }, + } + ); + + assert.strictEqual(exitCode, 1); + assert.deepStrictEqual(result, { + success: false, + result: { reason: 'missing_token' }, + }); + assert.ok(output.calls.some(call => call.method === 'cleanup')); + }); + it('does not fail CI when API returns 5xx error', async () => { let output = createMockOutput(); let exitCode = null; @@ -136,6 +405,7 @@ describe('commands/status', () => { assert.strictEqual(exitCode, 1); assert.ok(output.calls.some(c => c.method === 'error')); + assert.ok(output.calls.some(c => c.method === 'cleanup')); }); }); }); diff --git a/tests/commands/tdd-daemon.test.js b/tests/commands/tdd-daemon.test.js new file mode 100644 index 0000000..84b2496 --- /dev/null +++ b/tests/commands/tdd-daemon.test.js @@ -0,0 +1,650 @@ +import assert from 'node:assert/strict'; +import { EventEmitter } from 'node:events'; +import { describe, it } from 'node:test'; +import { + buildDaemonChildArgs, + buildDashboardUrl, + buildLegacyServerInfo, + buildOpenDashboardCommand, + cleanupDaemonState, + cleanupLegacyGlobalServerFile, + cleanupLocalDaemonFiles, + findDaemonPidByPort, + getLocalDaemonFiles, + readDaemonPidFile, + removeFileIfExists, + resolveDaemonPid, + validateTddStartOptions, + waitForDaemonChildInit, + waitForProcessExit, + waitForServerRunning, + writeLegacyGlobalServerFile, +} from '../../src/commands/tdd-daemon.js'; + +function createManualTimers() { + let timers = new Map(); + let nextId = 1; + + return { + setTimeout(fn, ms) { + let id = nextId++; + timers.set(id, { fn, ms }); + return id; + }, + clearTimeout(id) { + timers.delete(id); + }, + trigger(id) { + let timer = timers.get(id); + timers.delete(id); + timer?.fn(); + }, + get(id) { + return timers.get(id); + }, + }; +} + +async function flushMicrotasks() { + await Promise.resolve(); +} + +async function triggerTimer(timers, id) { + for (let i = 0; i < 5 && !timers.get(id); i++) { + await flushMicrotasks(); + } + timers.trigger(id); + await flushMicrotasks(); +} + +function createLsofProcess({ output = '', closeCode = 0, emitError = false }) { + let child = new EventEmitter(); + child.stdout = new EventEmitter(); + + queueMicrotask(() => { + if (emitError) { + child.emit('error', new Error('lsof unavailable')); + return; + } + + if (output) { + child.stdout.emit('data', output); + } + child.emit('close', closeCode); + }); + + return child; +} + +describe('commands/tdd-daemon helpers', () => { + describe('daemon file helpers', () => { + it('resolves local daemon files for a workspace', () => { + assert.deepStrictEqual(getLocalDaemonFiles('/repo/app'), { + vizzlyDir: '/repo/app/.vizzly', + pidFile: '/repo/app/.vizzly/server.pid', + serverFile: '/repo/app/.vizzly/server.json', + logFile: '/repo/app/.vizzly/server.log', + }); + }); + + it('removes files only when they exist', () => { + let removed = []; + let existing = new Set(['/repo/app/.vizzly/server.pid']); + + assert.strictEqual( + removeFileIfExists('/repo/app/.vizzly/server.pid', { + existsSync: path => existing.has(path), + unlinkSync: path => { + removed.push(path); + existing.delete(path); + }, + }), + true + ); + assert.strictEqual( + removeFileIfExists('/repo/app/.vizzly/server.json', { + existsSync: path => existing.has(path), + unlinkSync: path => removed.push(path), + }), + false + ); + assert.deepStrictEqual(removed, ['/repo/app/.vizzly/server.pid']); + }); + + it('cleans local pid and server files together', () => { + let removed = []; + let existing = new Set([ + '/repo/app/.vizzly/server.pid', + '/repo/app/.vizzly/server.json', + ]); + + let result = cleanupLocalDaemonFiles('/repo/app', { + existsSync: path => existing.has(path), + unlinkSync: path => removed.push(path), + }); + + assert.deepStrictEqual(result, { + pidFileRemoved: true, + serverFileRemoved: true, + }); + assert.deepStrictEqual(removed, [ + '/repo/app/.vizzly/server.pid', + '/repo/app/.vizzly/server.json', + ]); + }); + + it('writes the legacy global server file for SDK discovery', () => { + let createdDirectories = []; + let writes = []; + + let result = writeLegacyGlobalServerFile( + { pid: 1234, port: 47400 }, + { + home: () => '/home/test', + exists: () => false, + mkdir: (path, options) => { + createdDirectories.push({ path, options }); + }, + writeFile: (path, contents) => { + writes.push({ path, contents: JSON.parse(contents) }); + }, + now: () => 987654321, + } + ); + + assert.deepStrictEqual(createdDirectories, [ + { path: '/home/test/.vizzly', options: { recursive: true } }, + ]); + assert.deepStrictEqual(writes, [ + { + path: '/home/test/.vizzly/server.json', + contents: { + pid: 1234, + port: '47400', + startTime: 987654321, + }, + }, + ]); + assert.deepStrictEqual(result, { + path: '/home/test/.vizzly/server.json', + serverInfo: buildLegacyServerInfo({ + pid: 1234, + port: 47400, + now: () => 987654321, + }), + }); + }); + + it('cleans the legacy global server file when present', () => { + let removed = []; + let didRemove = cleanupLegacyGlobalServerFile({ + home: () => '/home/test', + exists: path => path === '/home/test/.vizzly/server.json', + unlink: path => removed.push(path), + }); + + assert.strictEqual(didRemove, true); + assert.deepStrictEqual(removed, ['/home/test/.vizzly/server.json']); + }); + + it('cleans local files, legacy global state, and registry entries together', () => { + let removed = []; + let registryCalls = []; + let localFiles = new Set([ + '/repo/app/.vizzly/server.pid', + '/repo/app/.vizzly/server.json', + ]); + let legacyFiles = new Set(['/home/test/.vizzly/server.json']); + + let result = cleanupDaemonState({ + port: 47400, + directory: '/repo/app', + registry: { + unregister: args => registryCalls.push(args), + }, + localFileDeps: { + existsSync: path => localFiles.has(path), + unlinkSync: path => removed.push(path), + }, + legacyFileDeps: { + home: () => '/home/test', + exists: path => legacyFiles.has(path), + unlink: path => removed.push(path), + }, + }); + + assert.deepStrictEqual(result, { + pidFileRemoved: true, + serverFileRemoved: true, + legacyGlobalServerFileRemoved: true, + }); + assert.deepStrictEqual(removed, [ + '/repo/app/.vizzly/server.pid', + '/repo/app/.vizzly/server.json', + '/home/test/.vizzly/server.json', + ]); + assert.deepStrictEqual(registryCalls, [ + { port: 47400, directory: '/repo/app' }, + ]); + }); + + it('cleans daemon files even when registry cleanup fails', () => { + let removed = []; + + let result = cleanupDaemonState({ + directory: '/repo/app', + registry: { + unregister: () => { + throw new Error('registry unavailable'); + }, + }, + localFileDeps: { + existsSync: path => path === '/repo/app/.vizzly/server.pid', + unlinkSync: path => removed.push(path), + }, + legacyFileDeps: { + home: () => '/home/test', + exists: () => false, + unlink: path => removed.push(path), + }, + }); + + assert.deepStrictEqual(result, { + pidFileRemoved: true, + serverFileRemoved: false, + legacyGlobalServerFileRemoved: false, + }); + assert.deepStrictEqual(removed, ['/repo/app/.vizzly/server.pid']); + }); + }); + + describe('daemon pid discovery', () => { + it('reads a daemon pid from a valid pid file', () => { + let pid = readDaemonPidFile('/repo/app/.vizzly/server.pid', { + existsSync: () => true, + readFileSync: () => '1234\n', + }); + + assert.strictEqual(pid, 1234); + }); + + it('treats missing, unreadable, and invalid pid files as no process', () => { + assert.strictEqual( + readDaemonPidFile('/repo/app/.vizzly/server.pid', { + existsSync: () => false, + readFileSync: () => '1234', + }), + null + ); + assert.strictEqual( + readDaemonPidFile('/repo/app/.vizzly/server.pid', { + existsSync: () => true, + readFileSync: () => { + throw new Error('permission denied'); + }, + }), + null + ); + assert.strictEqual( + readDaemonPidFile('/repo/app/.vizzly/server.pid', { + existsSync: () => true, + readFileSync: () => 'not-a-pid', + }), + null + ); + assert.strictEqual( + readDaemonPidFile('/repo/app/.vizzly/server.pid', { + existsSync: () => true, + readFileSync: () => '1234abc', + }), + null + ); + assert.strictEqual( + readDaemonPidFile('/repo/app/.vizzly/server.pid', { + existsSync: () => true, + readFileSync: () => '0', + }), + null + ); + }); + + it('finds the first process listening on the daemon port', async () => { + let calls = []; + let pid = await findDaemonPidByPort(47400, { + spawnProcess: (command, args, options) => { + calls.push({ command, args, options }); + return createLsofProcess({ output: '4321\n9876\n' }); + }, + }); + + assert.strictEqual(pid, 4321); + assert.deepStrictEqual(calls, [ + { + command: 'lsof', + args: ['-ti', ':47400'], + options: { stdio: 'pipe' }, + }, + ]); + }); + + it('returns no pid when port lookup fails or returns invalid output', async () => { + assert.strictEqual( + await findDaemonPidByPort(47400, { + spawnProcess: () => createLsofProcess({ output: '', closeCode: 1 }), + }), + null + ); + assert.strictEqual( + await findDaemonPidByPort(47400, { + spawnProcess: () => createLsofProcess({ output: 'nope\n' }), + }), + null + ); + assert.strictEqual( + await findDaemonPidByPort(47400, { + spawnProcess: () => createLsofProcess({ output: '4321abc\n' }), + }), + null + ); + assert.strictEqual( + await findDaemonPidByPort(47400, { + spawnProcess: () => createLsofProcess({ emitError: true }), + }), + null + ); + assert.strictEqual( + await findDaemonPidByPort(47400, { + spawnProcess: () => { + throw new Error('spawn failed'); + }, + }), + null + ); + }); + + it('prefers the pid file before falling back to port discovery', async () => { + let findByPortCalls = 0; + let pid = await resolveDaemonPid({ + port: 47400, + pidFile: '/repo/app/.vizzly/server.pid', + readPid: pidFile => { + assert.strictEqual(pidFile, '/repo/app/.vizzly/server.pid'); + return 1234; + }, + findByPort: () => { + findByPortCalls++; + return 4321; + }, + }); + + assert.strictEqual(pid, 1234); + assert.strictEqual(findByPortCalls, 0); + }); + + it('falls back to port discovery when the pid file is stale', async () => { + let pid = await resolveDaemonPid({ + port: 47400, + readPid: () => null, + findByPort: port => { + assert.strictEqual(port, 47400); + return 4321; + }, + }); + + assert.strictEqual(pid, 4321); + }); + }); + + describe('buildDaemonChildArgs', () => { + it('builds daemon child args from explicit options', () => { + let args = buildDaemonChildArgs({ + entrypoint: '/repo/bin/vizzly.js', + port: 47400, + options: { + open: true, + baselineBuild: 'build-123', + baselineComparison: 'comparison-456', + environment: 'staging', + threshold: 0.05, + minClusterSize: 4, + timeout: '45000', + failOnDiff: true, + token: 'token-abc', + }, + globalOptions: { + json: true, + verbose: true, + noColor: true, + }, + }); + + assert.deepStrictEqual(args, [ + '/repo/bin/vizzly.js', + 'tdd', + 'start', + '--daemon-child', + '--port', + '47400', + '--open', + '--baseline-build', + 'build-123', + '--baseline-comparison', + 'comparison-456', + '--environment', + 'staging', + '--threshold', + '0.05', + '--min-cluster-size', + '4', + '--timeout', + '45000', + '--fail-on-diff', + '--token', + 'token-abc', + '--json', + '--verbose', + '--no-color', + ]); + }); + }); + + describe('validateTddStartOptions', () => { + it('accepts valid comparison options', () => { + assert.deepStrictEqual( + validateTddStartOptions({ threshold: '0', minClusterSize: '1' }), + [] + ); + }); + + it('rejects invalid comparison options', () => { + assert.deepStrictEqual( + validateTddStartOptions({ + threshold: '-1', + minClusterSize: '2.5', + }), + [ + 'Threshold must be a non-negative number (CIEDE2000 Delta E)', + 'Min cluster size must be a positive integer', + ] + ); + }); + }); + + describe('dashboard open helpers', () => { + it('builds the local dashboard URL for a port', () => { + assert.strictEqual(buildDashboardUrl(47400), 'http://localhost:47400'); + }); + + it('uses open on macOS', () => { + assert.deepStrictEqual( + buildOpenDashboardCommand('http://localhost:47400', 'darwin'), + { + command: 'open', + args: ['http://localhost:47400'], + } + ); + }); + + it('uses xdg-open on Linux', () => { + assert.deepStrictEqual( + buildOpenDashboardCommand('http://localhost:47400', 'linux'), + { + command: 'xdg-open', + args: ['http://localhost:47400'], + } + ); + }); + + it('uses cmd start on Windows because start is a shell built-in', () => { + assert.deepStrictEqual( + buildOpenDashboardCommand('http://localhost:47400', 'win32'), + { + command: 'cmd', + args: ['/c', 'start', '', 'http://localhost:47400'], + } + ); + }); + }); + + describe('waitForDaemonChildInit', () => { + it('resolves when the daemon child disconnects after initialization', async () => { + let timers = createManualTimers(); + let child = new EventEmitter(); + + let promise = waitForDaemonChildInit(child, { timers }); + child.emit('disconnect'); + + let result = await promise; + + assert.deepStrictEqual(result, { ok: true }); + assert.strictEqual(timers.get(1), undefined); + assert.strictEqual(child.listenerCount('disconnect'), 0); + assert.strictEqual(child.listenerCount('exit'), 0); + }); + + it('returns an exit result when the daemon child exits first', async () => { + let timers = createManualTimers(); + let child = new EventEmitter(); + + let promise = waitForDaemonChildInit(child, { timers }); + child.emit('exit'); + + let result = await promise; + + assert.deepStrictEqual(result, { ok: false, reason: 'exit' }); + assert.strictEqual(timers.get(1), undefined); + assert.strictEqual(child.listenerCount('disconnect'), 0); + assert.strictEqual(child.listenerCount('exit'), 0); + }); + + it('returns a timeout result and removes listeners', async () => { + let timers = createManualTimers(); + let child = new EventEmitter(); + + let promise = waitForDaemonChildInit(child, { timers }); + await triggerTimer(timers, 1); + + let result = await promise; + + assert.strictEqual(result.ok, false); + assert.strictEqual(result.reason, 'timeout'); + assert.match(result.error.message, /initialization timed out/); + assert.strictEqual(child.listenerCount('disconnect'), 0); + assert.strictEqual(child.listenerCount('exit'), 0); + }); + }); + + describe('waitForServerRunning', () => { + it('checks immediately and returns when the health check succeeds', async () => { + let calls = []; + + let running = await waitForServerRunning(47400, { + isRunning: async port => { + calls.push(port); + return true; + }, + }); + + assert.strictEqual(running, true); + assert.deepStrictEqual(calls, [47400]); + }); + + it('waits between concrete health checks until the server responds', async () => { + let timers = createManualTimers(); + let attempts = 0; + let promise = waitForServerRunning(47400, { + maxAttempts: 3, + delayMs: 200, + timers, + isRunning: async () => { + attempts++; + return attempts === 3; + }, + }); + + await triggerTimer(timers, 1); + await triggerTimer(timers, 2); + + assert.strictEqual(await promise, true); + assert.strictEqual(attempts, 3); + }); + + it('returns false when all health checks fail', async () => { + let timers = createManualTimers(); + let promise = waitForServerRunning(47400, { + maxAttempts: 2, + delayMs: 200, + timers, + isRunning: async () => false, + }); + + await triggerTimer(timers, 1); + + assert.strictEqual(await promise, false); + }); + }); + + describe('waitForProcessExit', () => { + it('returns true immediately when the process is already gone', async () => { + let exited = await waitForProcessExit(123, { + processRunning: () => false, + }); + + assert.strictEqual(exited, true); + }); + + it('returns true once the process exits during the grace period', async () => { + let timers = createManualTimers(); + let checks = 0; + let promise = waitForProcessExit(123, { + timeoutMs: 300, + intervalMs: 100, + timers, + processRunning: () => { + checks++; + return checks < 2; + }, + }); + + await triggerTimer(timers, 1); + + assert.strictEqual(await promise, true); + assert.strictEqual(checks, 2); + }); + + it('returns false when the process is still running after the grace period', async () => { + let timers = createManualTimers(); + let promise = waitForProcessExit(123, { + timeoutMs: 200, + intervalMs: 100, + timers, + processRunning: () => true, + }); + + await triggerTimer(timers, 1); + await triggerTimer(timers, 2); + + assert.strictEqual(await promise, false); + }); + }); +}); diff --git a/tests/commands/tdd.test.js b/tests/commands/tdd.test.js index 8a7e665..1f59233 100644 --- a/tests/commands/tdd.test.js +++ b/tests/commands/tdd.test.js @@ -34,7 +34,7 @@ function createMockConfig(overrides = {}) { apiUrl: 'https://api.test', server: { port: 47392, timeout: 30000 }, build: { environment: 'test' }, - comparison: { threshold: 0.1 }, + comparison: { threshold: 0.1, minClusterSize: 2 }, ...overrides, }; } @@ -398,6 +398,37 @@ describe('commands/tdd', () => { assert.strictEqual(capturedRunOptions.commit, 'def456'); }); + it('passes comparison config through to the TDD run', async () => { + let output = createMockOutput(); + let capturedRunOptions = null; + + await tddCommand( + 'npm test', + {}, + {}, + { + loadConfig: async () => + createMockConfig({ + comparison: { threshold: 0, minClusterSize: 6 }, + }), + createServerManager: () => ({ + start: async () => {}, + stop: async () => {}, + }), + runTests: async ({ runOptions }) => { + capturedRunOptions = runOptions; + return { screenshotsCaptured: 0, comparisons: [] }; + }, + detectBranch: async () => 'main', + detectCommit: async () => 'abc123', + output, + } + ); + + assert.strictEqual(capturedRunOptions.threshold, 0); + assert.strictEqual(capturedRunOptions.minClusterSize, 6); + }); + it('invokes onBuildCreated callback', async () => { let output = createMockOutput(); @@ -698,6 +729,13 @@ describe('commands/tdd', () => { ); }); + it('should fail with decimal port number', () => { + let errors = validateTddOptions('npm test', { port: '3000.5' }); + assert.ok( + errors.includes('Port must be a valid number between 1 and 65535') + ); + }); + it('should fail with port out of range (too low)', () => { let errors = validateTddOptions('npm test', { port: '0' }); assert.ok( @@ -726,6 +764,13 @@ describe('commands/tdd', () => { ); }); + it('should fail with decimal timeout', () => { + let errors = validateTddOptions('npm test', { timeout: '5000.5' }); + assert.ok( + errors.includes('Timeout must be at least 1000 milliseconds') + ); + }); + it('should fail with timeout too low', () => { let errors = validateTddOptions('npm test', { timeout: '500' }); assert.ok( @@ -759,6 +804,15 @@ describe('commands/tdd', () => { ); }); + it('should fail when threshold has trailing text', () => { + let errors = validateTddOptions('npm test', { threshold: '2abc' }); + assert.ok( + errors.includes( + 'Threshold must be a non-negative number (CIEDE2000 Delta E)' + ) + ); + }); + it('should fail with threshold below 0', () => { let errors = validateTddOptions('npm test', { threshold: '-0.1' }); assert.ok( @@ -774,6 +828,27 @@ describe('commands/tdd', () => { }); }); + describe('min cluster size validation', () => { + it('should pass with a positive integer', () => { + let errors = validateTddOptions('npm test', { minClusterSize: '2' }); + assert.strictEqual(errors.length, 0); + }); + + it('should fail with zero', () => { + let errors = validateTddOptions('npm test', { minClusterSize: '0' }); + assert.ok( + errors.includes('Min cluster size must be a positive integer') + ); + }); + + it('should fail with a decimal', () => { + let errors = validateTddOptions('npm test', { minClusterSize: '2.5' }); + assert.ok( + errors.includes('Min cluster size must be a positive integer') + ); + }); + }); + describe('multiple validation errors', () => { it('should return all validation errors', () => { let errors = validateTddOptions('', { diff --git a/tests/plugin-loader.test.js b/tests/plugin-loader.test.js new file mode 100644 index 0000000..177fd30 --- /dev/null +++ b/tests/plugin-loader.test.js @@ -0,0 +1,160 @@ +import assert from 'node:assert/strict'; +import { mkdir, mkdtemp, realpath, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, it } from 'node:test'; +import { + loadPlugin, + resolvePackagePluginPath, + resolvePluginPath, +} from '../src/plugin-loader.js'; + +async function createTempWorkspace() { + return mkdtemp(join(tmpdir(), 'vizzly-plugin-loader-')); +} + +async function writePackage(workspace, packageName, packageJson) { + let packageDir = join(workspace, 'node_modules', packageName); + await mkdir(packageDir, { recursive: true }); + await writeFile( + join(packageDir, 'package.json'), + JSON.stringify(packageJson, null, 2) + ); + return packageDir; +} + +describe('plugin-loader', () => { + let originalCwd; + let workspace; + + beforeEach(async () => { + originalCwd = process.cwd(); + workspace = await realpath(await createTempWorkspace()); + process.chdir(workspace); + }); + + afterEach(async () => { + process.chdir(originalCwd); + await rm(workspace, { recursive: true, force: true }); + }); + + describe('resolvePackagePluginPath', () => { + it('resolves package plugin paths inside the package directory', () => { + let packageDir = join(workspace, 'node_modules', 'storybook-plugin'); + + let pluginPath = resolvePackagePluginPath( + 'storybook-plugin', + packageDir, + 'dist/plugin.js' + ); + + assert.equal(pluginPath, join(packageDir, 'dist', 'plugin.js')); + }); + + it('rejects absolute package plugin paths', () => { + let packageDir = join(workspace, 'node_modules', 'bad-plugin'); + + assert.throws( + () => + resolvePackagePluginPath('bad-plugin', packageDir, '/tmp/plugin.js'), + /path must be relative/ + ); + }); + + it('rejects package plugin paths that escape the package directory', () => { + let packageDir = join(workspace, 'node_modules', 'bad-plugin'); + + assert.throws( + () => + resolvePackagePluginPath( + 'bad-plugin', + packageDir, + '../other/plugin.js' + ), + /cannot escape package directory/ + ); + }); + }); + + describe('resolvePluginPath', () => { + it('resolves explicit package plugins through vizzlyPlugin', async () => { + let packageDir = await writePackage(workspace, 'storybook-plugin', { + name: 'storybook-plugin', + vizzlyPlugin: 'dist/plugin.js', + }); + + let pluginPath = resolvePluginPath('storybook-plugin', null); + + assert.equal(pluginPath, join(packageDir, 'dist', 'plugin.js')); + }); + + it('keeps legacy package plugin field support behind the same path safety checks', async () => { + let packageDir = await writePackage(workspace, 'legacy-plugin', { + name: 'legacy-plugin', + vizzly: { plugin: 'plugin.js' }, + }); + + let pluginPath = resolvePluginPath('legacy-plugin', null); + + assert.equal(pluginPath, join(packageDir, 'plugin.js')); + }); + + it('rejects unsafe plugin fields from explicit package specs', async () => { + await writePackage(workspace, 'bad-plugin', { + name: 'bad-plugin', + vizzlyPlugin: '../outside.js', + }); + + assert.throws( + () => resolvePluginPath('bad-plugin', null), + /cannot escape package directory/ + ); + }); + + it('resolves file paths relative to the config file', () => { + let configPath = join(workspace, 'configs', 'vizzly.config.js'); + + let pluginPath = resolvePluginPath('./plugins/custom.js', configPath); + + assert.equal( + pluginPath, + join(workspace, 'configs', 'plugins', 'custom.js') + ); + }); + }); + + describe('loadPlugin', () => { + it('loads a valid ESM plugin default export', async () => { + let pluginPath = join(workspace, 'plugin.js'); + await writeFile( + pluginPath, + `export default { + name: 'custom-plugin', + version: '1.0.0', + register() {} +};` + ); + + let plugin = await loadPlugin(pluginPath); + + assert.equal(plugin.name, 'custom-plugin'); + assert.equal(plugin.version, '1.0.0'); + assert.equal(typeof plugin.register, 'function'); + }); + + it('rejects plugins without a register function', async () => { + let pluginPath = join(workspace, 'invalid-plugin.js'); + await writeFile( + pluginPath, + `export default { + name: 'invalid-plugin' +};` + ); + + await assert.rejects( + () => loadPlugin(pluginPath), + /Invalid plugin structure/ + ); + }); + }); +}); diff --git a/tests/server/routers/assets.test.js b/tests/server/routers/assets.test.js index 63da92d..faf6e2a 100644 --- a/tests/server/routers/assets.test.js +++ b/tests/server/routers/assets.test.js @@ -3,49 +3,10 @@ import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, it } from 'node:test'; import { createAssetsRouter } from '../../../src/server/routers/assets.js'; - -/** - * Creates a mock HTTP request - */ -function createMockRequest(method = 'GET') { - return { method }; -} - -/** - * Creates a mock HTTP response with tracking - */ -function createMockResponse() { - let headers = {}; - let statusCode = null; - let body = null; - - return { - get statusCode() { - return statusCode; - }, - set statusCode(code) { - statusCode = code; - }, - setHeader(name, value) { - headers[name] = value; - }, - getHeader(name) { - return headers[name]; - }, - end(content) { - body = content; - }, - get headers() { - return headers; - }, - get body() { - return body; - }, - getParsedBody() { - return body && typeof body === 'string' ? JSON.parse(body) : body; - }, - }; -} +import { + createMockRequest, + createMockResponse, +} from '../../helpers/http-mocks.js'; describe('server/routers/assets', () => { let testDir = join(process.cwd(), '.test-assets-router'); diff --git a/tests/server/routers/auth.test.js b/tests/server/routers/auth.test.js index 013da59..4f06eba 100644 --- a/tests/server/routers/auth.test.js +++ b/tests/server/routers/auth.test.js @@ -1,60 +1,10 @@ import assert from 'node:assert'; -import { EventEmitter } from 'node:events'; import { describe, it } from 'node:test'; import { createAuthRouter } from '../../../src/server/routers/auth.js'; - -/** - * Creates a mock HTTP request with body support - */ -function createMockRequest(method = 'GET', body = null) { - let emitter = new EventEmitter(); - emitter.method = method; - - if (body !== null) { - process.nextTick(() => { - emitter.emit('data', JSON.stringify(body)); - emitter.emit('end'); - }); - } - - return emitter; -} - -/** - * Creates a mock HTTP response with tracking - */ -function createMockResponse() { - let headers = {}; - let statusCode = null; - let body = null; - - return { - get statusCode() { - return statusCode; - }, - set statusCode(code) { - statusCode = code; - }, - setHeader(name, value) { - headers[name] = value; - }, - getHeader(name) { - return headers[name]; - }, - end(content) { - body = content; - }, - get headers() { - return headers; - }, - get body() { - return body; - }, - getParsedBody() { - return body ? JSON.parse(body) : null; - }, - }; -} +import { + createMockRequest, + createMockResponse, +} from '../../helpers/http-mocks.js'; /** * Creates a mock auth service diff --git a/tests/server/routers/cloud-proxy.test.js b/tests/server/routers/cloud-proxy.test.js index 497b7b0..e6a03b5 100644 --- a/tests/server/routers/cloud-proxy.test.js +++ b/tests/server/routers/cloud-proxy.test.js @@ -1,60 +1,10 @@ import assert from 'node:assert'; -import { EventEmitter } from 'node:events'; import { describe, it } from 'node:test'; import { createCloudProxyRouter } from '../../../src/server/routers/cloud-proxy.js'; - -/** - * Creates a mock HTTP request with body support - */ -function createMockRequest(method = 'GET', body = null) { - let emitter = new EventEmitter(); - emitter.method = method; - - if (body !== null) { - process.nextTick(() => { - emitter.emit('data', JSON.stringify(body)); - emitter.emit('end'); - }); - } - - return emitter; -} - -/** - * Creates a mock HTTP response with tracking - */ -function createMockResponse() { - let headers = {}; - let statusCode = null; - let body = null; - - return { - get statusCode() { - return statusCode; - }, - set statusCode(code) { - statusCode = code; - }, - setHeader(name, value) { - headers[name] = value; - }, - getHeader(name) { - return headers[name]; - }, - end(content) { - body = content; - }, - get headers() { - return headers; - }, - get body() { - return body; - }, - getParsedBody() { - return body ? JSON.parse(body) : null; - }, - }; -} +import { + createMockRequest, + createMockResponse, +} from '../../helpers/http-mocks.js'; /** * Creates a mock URL object diff --git a/tests/server/routers/config.test.js b/tests/server/routers/config.test.js index fad17dd..d7a75c2 100644 --- a/tests/server/routers/config.test.js +++ b/tests/server/routers/config.test.js @@ -1,60 +1,10 @@ import assert from 'node:assert'; -import { EventEmitter } from 'node:events'; import { describe, it } from 'node:test'; import { createConfigRouter } from '../../../src/server/routers/config.js'; - -/** - * Creates a mock HTTP request with body support - */ -function createMockRequest(method = 'GET', body = null) { - let emitter = new EventEmitter(); - emitter.method = method; - - if (body !== null) { - process.nextTick(() => { - emitter.emit('data', JSON.stringify(body)); - emitter.emit('end'); - }); - } - - return emitter; -} - -/** - * Creates a mock HTTP response with tracking - */ -function createMockResponse() { - let headers = {}; - let statusCode = null; - let body = null; - - return { - get statusCode() { - return statusCode; - }, - set statusCode(code) { - statusCode = code; - }, - setHeader(name, value) { - headers[name] = value; - }, - getHeader(name) { - return headers[name]; - }, - end(content) { - body = content; - }, - get headers() { - return headers; - }, - get body() { - return body; - }, - getParsedBody() { - return body ? JSON.parse(body) : null; - }, - }; -} +import { + createMockRequest, + createMockResponse, +} from '../../helpers/http-mocks.js'; /** * Creates a mock config service diff --git a/tests/server/routers/dashboard.test.js b/tests/server/routers/dashboard.test.js index 591a009..f059f50 100644 --- a/tests/server/routers/dashboard.test.js +++ b/tests/server/routers/dashboard.test.js @@ -3,49 +3,10 @@ import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, it } from 'node:test'; import { createDashboardRouter } from '../../../src/server/routers/dashboard.js'; - -/** - * Creates a mock HTTP request - */ -function createMockRequest(method = 'GET') { - return { method }; -} - -/** - * Creates a mock HTTP response with tracking - */ -function createMockResponse() { - let headers = {}; - let statusCode = null; - let body = null; - - return { - get statusCode() { - return statusCode; - }, - set statusCode(code) { - statusCode = code; - }, - setHeader(name, value) { - headers[name] = value; - }, - getHeader(name) { - return headers[name]; - }, - end(content) { - body = content; - }, - get headers() { - return headers; - }, - get body() { - return body; - }, - getParsedBody() { - return body && typeof body === 'string' ? JSON.parse(body) : body; - }, - }; -} +import { + createMockRequest, + createMockResponse, +} from '../../helpers/http-mocks.js'; describe('server/routers/dashboard', () => { let testDir = join(process.cwd(), '.test-dashboard-router'); diff --git a/tests/server/routers/events.test.js b/tests/server/routers/events.test.js index 669d5d3..f8c279b 100644 --- a/tests/server/routers/events.test.js +++ b/tests/server/routers/events.test.js @@ -62,6 +62,66 @@ function createMockResponse() { }; } +function createManualTimers() { + let timeouts = []; + let intervals = new Set(); + + return { + setTimeout(fn) { + let timeout = { fn }; + timeouts.push(timeout); + return timeout; + }, + clearTimeout(timeout) { + timeouts = timeouts.filter(candidate => candidate !== timeout); + }, + setInterval() { + let interval = {}; + intervals.add(interval); + return interval; + }, + clearInterval(interval) { + intervals.delete(interval); + }, + flushTimeouts() { + let pending = timeouts; + timeouts = []; + for (let timeout of pending) { + timeout.fn(); + } + }, + }; +} + +function createEventsHarness(workingDir) { + let timers = createManualTimers(); + let triggerReportDataChanged = () => { + throw new Error('report data watcher was not registered'); + }; + let cleanupCalled = false; + let handler = createEventsRouter({ + workingDir, + timers, + watchReportData({ onReportDataChanged }) { + triggerReportDataChanged = onReportDataChanged; + return () => { + cleanupCalled = true; + }; + }, + }); + + return { + handler, + timers, + triggerReportDataChanged() { + triggerReportDataChanged(); + }, + get cleanupCalled() { + return cleanupCalled; + }, + }; +} + describe('server/routers/events', () => { let testDir = join(process.cwd(), '.test-events-router'); let originalCwd = process.cwd(); @@ -230,20 +290,19 @@ describe('server/routers/events', () => { }); it('sends update when report-data.json changes', async () => { - let handler = createEventsRouter({ workingDir: testDir }); + let harness = createEventsHarness(testDir); let req = createMockRequest('GET'); let res = createMockResponse(); - await handler(req, res, '/api/events'); + await harness.handler(req, res, '/api/events'); - // Write initial data - this triggers the file watcher writeFileSync( join(testDir, '.vizzly', 'report-data.json'), JSON.stringify({ comparisons: [{ id: 'updated' }] }) ); - // Wait for debounce (100ms) + buffer - await new Promise(resolve => setTimeout(resolve, 200)); + harness.triggerReportDataChanged(); + harness.timers.flushTimeouts(); let output = res.getOutput(); assert.ok(output.includes('event: reportData')); @@ -254,33 +313,35 @@ describe('server/routers/events', () => { }); it('debounces rapid file changes', async () => { - let handler = createEventsRouter({ workingDir: testDir }); + let harness = createEventsHarness(testDir); let req = createMockRequest('GET'); let res = createMockResponse(); - await handler(req, res, '/api/events'); + await harness.handler(req, res, '/api/events'); // Rapid file changes writeFileSync( join(testDir, '.vizzly', 'report-data.json'), JSON.stringify({ version: 1 }) ); + harness.triggerReportDataChanged(); writeFileSync( join(testDir, '.vizzly', 'report-data.json'), JSON.stringify({ version: 2 }) ); + harness.triggerReportDataChanged(); writeFileSync( join(testDir, '.vizzly', 'report-data.json'), JSON.stringify({ version: 3 }) ); + harness.triggerReportDataChanged(); - // Wait for debounce - await new Promise(resolve => setTimeout(resolve, 200)); + harness.timers.flushTimeouts(); let output = res.getOutput(); // Should only have one event with the final version let eventCount = (output.match(/event: reportData/g) || []).length; - assert.ok(eventCount >= 1, 'Should have at least one event'); + assert.strictEqual(eventCount, 1, 'Should send one debounced event'); assert.ok(output.includes('"version":3'), 'Should contain final version'); // Clean up @@ -288,11 +349,11 @@ describe('server/routers/events', () => { }); it('does not send to closed connections', async () => { - let handler = createEventsRouter({ workingDir: testDir }); + let harness = createEventsHarness(testDir); let req = createMockRequest('GET'); let res = createMockResponse(); - await handler(req, res, '/api/events'); + await harness.handler(req, res, '/api/events'); // Mark connection as ended res.end(); @@ -303,11 +364,10 @@ describe('server/routers/events', () => { JSON.stringify({ test: true }) ); - await new Promise(resolve => setTimeout(resolve, 200)); + harness.triggerReportDataChanged(); + harness.timers.flushTimeouts(); - // Should not have written after end() - // The initial chunks should be empty since no initial data - assert.ok(true, 'Should not crash when writing to closed connection'); + assert.strictEqual(res.getOutput(), ''); // Clean up req.emit('close'); @@ -341,11 +401,11 @@ describe('server/routers/events', () => { JSON.stringify(initialData) ); - let handler = createEventsRouter({ workingDir: testDir }); + let harness = createEventsHarness(testDir); let req = createMockRequest('GET'); let res = createMockResponse(); - await handler(req, res, '/api/events'); + await harness.handler(req, res, '/api/events'); // Add a new comparison let updatedData = { @@ -360,7 +420,8 @@ describe('server/routers/events', () => { JSON.stringify(updatedData) ); - await new Promise(resolve => setTimeout(resolve, 200)); + harness.triggerReportDataChanged(); + harness.timers.flushTimeouts(); let output = res.getOutput(); assert.ok(output.includes('event: comparisonUpdate')); @@ -382,11 +443,11 @@ describe('server/routers/events', () => { JSON.stringify(initialData) ); - let handler = createEventsRouter({ workingDir: testDir }); + let harness = createEventsHarness(testDir); let req = createMockRequest('GET'); let res = createMockResponse(); - await handler(req, res, '/api/events'); + await harness.handler(req, res, '/api/events'); // Change the comparison's status let updatedData = { @@ -400,7 +461,8 @@ describe('server/routers/events', () => { JSON.stringify(updatedData) ); - await new Promise(resolve => setTimeout(resolve, 200)); + harness.triggerReportDataChanged(); + harness.timers.flushTimeouts(); let output = res.getOutput(); assert.ok(output.includes('event: comparisonUpdate')); @@ -429,11 +491,11 @@ describe('server/routers/events', () => { JSON.stringify(initialData) ); - let handler = createEventsRouter({ workingDir: testDir }); + let harness = createEventsHarness(testDir); let req = createMockRequest('GET'); let res = createMockResponse(); - await handler(req, res, '/api/events'); + await harness.handler(req, res, '/api/events'); // Remove comparison b let updatedData = { @@ -445,7 +507,8 @@ describe('server/routers/events', () => { JSON.stringify(updatedData) ); - await new Promise(resolve => setTimeout(resolve, 200)); + harness.triggerReportDataChanged(); + harness.timers.flushTimeouts(); let output = res.getOutput(); assert.ok(output.includes('event: comparisonRemoved')); @@ -466,11 +529,11 @@ describe('server/routers/events', () => { JSON.stringify(initialData) ); - let handler = createEventsRouter({ workingDir: testDir }); + let harness = createEventsHarness(testDir); let req = createMockRequest('GET'); let res = createMockResponse(); - await handler(req, res, '/api/events'); + await harness.handler(req, res, '/api/events'); // Change summary fields only, same comparisons let updatedData = { @@ -484,7 +547,8 @@ describe('server/routers/events', () => { JSON.stringify(updatedData) ); - await new Promise(resolve => setTimeout(resolve, 200)); + harness.triggerReportDataChanged(); + harness.timers.flushTimeouts(); let output = res.getOutput(); assert.ok(output.includes('event: summaryUpdate')); @@ -508,11 +572,11 @@ describe('server/routers/events', () => { JSON.stringify(initialData) ); - let handler = createEventsRouter({ workingDir: testDir }); + let harness = createEventsHarness(testDir); let req = createMockRequest('GET'); let res = createMockResponse(); - await handler(req, res, '/api/events'); + await harness.handler(req, res, '/api/events'); let chunksAfterInitial = res.chunks.length; @@ -522,7 +586,8 @@ describe('server/routers/events', () => { JSON.stringify(initialData) ); - await new Promise(resolve => setTimeout(resolve, 200)); + harness.triggerReportDataChanged(); + harness.timers.flushTimeouts(); // No new chunks should have been written assert.strictEqual( @@ -535,11 +600,11 @@ describe('server/routers/events', () => { }); it('ignores changes to other files in .vizzly directory', async () => { - let handler = createEventsRouter({ workingDir: testDir }); + let harness = createEventsHarness(testDir); let req = createMockRequest('GET'); let res = createMockResponse(); - await handler(req, res, '/api/events'); + await harness.handler(req, res, '/api/events'); // Write to a different file writeFileSync( @@ -547,7 +612,7 @@ describe('server/routers/events', () => { JSON.stringify({ ignored: true }) ); - await new Promise(resolve => setTimeout(resolve, 200)); + harness.timers.flushTimeouts(); // Should not have sent any events let output = res.getOutput(); diff --git a/tests/server/routers/health.test.js b/tests/server/routers/health.test.js index 0b14ebf..7634e91 100644 --- a/tests/server/routers/health.test.js +++ b/tests/server/routers/health.test.js @@ -3,49 +3,10 @@ import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, it } from 'node:test'; import { createHealthRouter } from '../../../src/server/routers/health.js'; - -/** - * Creates a mock HTTP request - */ -function createMockRequest(method = 'GET') { - return { method }; -} - -/** - * Creates a mock HTTP response with tracking - */ -function createMockResponse() { - let headers = {}; - let statusCode = null; - let body = null; - - return { - get statusCode() { - return statusCode; - }, - set statusCode(code) { - statusCode = code; - }, - setHeader(name, value) { - headers[name] = value; - }, - getHeader(name) { - return headers[name]; - }, - end(content) { - body = content; - }, - get headers() { - return headers; - }, - get body() { - return body; - }, - getParsedBody() { - return body ? JSON.parse(body) : null; - }, - }; -} +import { + createMockRequest, + createMockResponse, +} from '../../helpers/http-mocks.js'; describe('server/routers/health', () => { let testDir = join(process.cwd(), '.test-health-router'); diff --git a/tests/services/build-manager.test.js b/tests/services/build-manager.test.js index 4b7ec35..8b57ab1 100644 --- a/tests/services/build-manager.test.js +++ b/tests/services/build-manager.test.js @@ -1,219 +1,51 @@ import assert from 'node:assert'; import { describe, it } from 'node:test'; -import { - addScreenshotToBuild, - createBuildObject, - createQueuedBuild, - finalizeBuildObject, - generateBuildId, - updateBuild, - validateBuildOptions, -} from '../../src/services/build-manager.js'; +import { createBuildObject } from '../../src/services/build-manager.js'; describe('services/build-manager', () => { - describe('generateBuildId', () => { - it('generates a unique build ID with prefix', () => { - let id = generateBuildId(); - - assert.ok(id.startsWith('build-')); - assert.ok(id.length > 10); - }); - - it('generates different IDs on each call', () => { - let id1 = generateBuildId(); - let id2 = generateBuildId(); - - assert.notStrictEqual(id1, id2); - }); - }); - describe('createBuildObject', () => { - it('creates build with required fields', () => { - let build = createBuildObject({ + it('creates the local build shape used by the runner', () => { + let build = createBuildObject( + { + name: 'Test Build', + branch: 'main', + commit: 'abc123', + metadata: { ci: true }, + }, + { + randomId: () => 'local-id', + now: () => '2026-05-18T12:00:00.000Z', + } + ); + + assert.deepStrictEqual(build, { + id: 'build-local-id', name: 'Test Build', branch: 'main', commit: 'abc123', - }); - - assert.ok(build.id.startsWith('build-')); - assert.strictEqual(build.name, 'Test Build'); - assert.strictEqual(build.branch, 'main'); - assert.strictEqual(build.commit, 'abc123'); - assert.strictEqual(build.environment, 'test'); - assert.deepStrictEqual(build.metadata, {}); - assert.strictEqual(build.status, 'pending'); - assert.ok(build.createdAt); - assert.deepStrictEqual(build.screenshots, []); - }); - - it('uses default name when not provided', () => { - let build = createBuildObject({ branch: 'main' }); - - assert.ok(build.name.startsWith('build-')); - }); - - it('uses custom environment and metadata', () => { - let build = createBuildObject({ - name: 'Build', - environment: 'production', + environment: 'test', metadata: { ci: true }, - }); - - assert.strictEqual(build.environment, 'production'); - assert.deepStrictEqual(build.metadata, { ci: true }); - }); - }); - - describe('updateBuild', () => { - it('updates build status', () => { - let build = { - id: 'build-1', status: 'pending', - name: 'Test', - }; - - let updated = updateBuild(build, 'running'); - - assert.strictEqual(updated.status, 'running'); - assert.ok(updated.updatedAt); - assert.strictEqual(updated.name, 'Test'); - }); - - it('merges additional updates', () => { - let build = { id: 'build-1', status: 'pending' }; - - let updated = updateBuild(build, 'completed', { result: 'success' }); - - assert.strictEqual(updated.status, 'completed'); - assert.strictEqual(updated.result, 'success'); - }); - - it('does not mutate original build', () => { - let build = { id: 'build-1', status: 'pending' }; - - updateBuild(build, 'running'); - - assert.strictEqual(build.status, 'pending'); - }); - }); - - describe('addScreenshotToBuild', () => { - it('adds screenshot to build', () => { - let build = { - id: 'build-1', + createdAt: '2026-05-18T12:00:00.000Z', screenshots: [], - }; - let screenshot = { name: 'homepage', path: '/img/home.png' }; - - let updated = addScreenshotToBuild(build, screenshot); - - assert.strictEqual(updated.screenshots.length, 1); - assert.strictEqual(updated.screenshots[0].name, 'homepage'); - assert.ok(updated.screenshots[0].addedAt); - }); - - it('appends to existing screenshots', () => { - let build = { - id: 'build-1', - screenshots: [{ name: 'first' }], - }; - - let updated = addScreenshotToBuild(build, { name: 'second' }); - - assert.strictEqual(updated.screenshots.length, 2); - }); - - it('does not mutate original build', () => { - let build = { id: 'build-1', screenshots: [] }; - - addScreenshotToBuild(build, { name: 'test' }); - - assert.strictEqual(build.screenshots.length, 0); - }); - }); - - describe('finalizeBuildObject', () => { - it('sets status to completed on success', () => { - let build = { id: 'build-1', status: 'running' }; - - let finalized = finalizeBuildObject(build, { success: true }); - - assert.strictEqual(finalized.status, 'completed'); - assert.ok(finalized.completedAt); - assert.deepStrictEqual(finalized.result, { success: true }); - }); - - it('sets status to failed on failure', () => { - let build = { id: 'build-1', status: 'running' }; - - let finalized = finalizeBuildObject(build, { success: false }); - - assert.strictEqual(finalized.status, 'failed'); - }); - - it('handles empty result', () => { - let build = { id: 'build-1', status: 'running' }; - - let finalized = finalizeBuildObject(build); - - assert.strictEqual(finalized.status, 'failed'); // no success = failed - assert.deepStrictEqual(finalized.result, {}); - }); - }); - - describe('createQueuedBuild', () => { - it('creates queued build with timestamp', () => { - let options = { name: 'Build', branch: 'main' }; - - let queued = createQueuedBuild(options); - - assert.strictEqual(queued.name, 'Build'); - assert.strictEqual(queued.branch, 'main'); - assert.ok(queued.queuedAt); - }); - }); - - describe('validateBuildOptions', () => { - it('returns valid when name is provided', () => { - let result = validateBuildOptions({ name: 'Build' }); - - assert.strictEqual(result.valid, true); - assert.deepStrictEqual(result.errors, []); - }); - - it('returns valid when branch is provided', () => { - let result = validateBuildOptions({ branch: 'main' }); - - assert.strictEqual(result.valid, true); - }); - - it('returns invalid when neither name nor branch provided', () => { - let result = validateBuildOptions({}); - - assert.strictEqual(result.valid, false); - assert.ok(result.errors.some(e => e.includes('name or branch'))); - }); - - it('validates environment values', () => { - let validResult = validateBuildOptions({ - name: 'Build', - environment: 'production', - }); - assert.strictEqual(validResult.valid, true); - - let invalidResult = validateBuildOptions({ - name: 'Build', - environment: 'invalid', }); - assert.strictEqual(invalidResult.valid, false); - assert.ok(invalidResult.errors.some(e => e.includes('Environment'))); }); - it('accepts all valid environments', () => { - for (let env of ['test', 'staging', 'production']) { - let result = validateBuildOptions({ name: 'Build', environment: env }); - assert.strictEqual(result.valid, true, `Should accept ${env}`); - } + it('keeps existing default behavior for callers with minimal options', () => { + let build = createBuildObject( + { branch: 'main' }, + { + randomId: () => 'minimal-id', + now: () => '2026-05-18T12:00:00.000Z', + timestamp: () => 1779120000000, + } + ); + + assert.strictEqual(build.id, 'build-minimal-id'); + assert.strictEqual(build.name, 'build-1779120000000'); + assert.strictEqual(build.branch, 'main'); + assert.strictEqual(build.environment, 'test'); + assert.deepStrictEqual(build.metadata, {}); }); }); }); diff --git a/tests/services/static-report-generator.test.js b/tests/services/static-report-generator.test.js index be99409..1cf1ae8 100644 --- a/tests/services/static-report-generator.test.js +++ b/tests/services/static-report-generator.test.js @@ -10,6 +10,11 @@ import { tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; import { afterEach, beforeEach, describe, it } from 'node:test'; import { fileURLToPath } from 'node:url'; +import { + generateStaticReport, + getReportFileUrl, + loadStaticReportRenderer, +} from '../../src/services/static-report-generator.js'; let __dirname = dirname(fileURLToPath(import.meta.url)); @@ -96,6 +101,23 @@ function setupMockVizzlyDir(workingDir, options = {}) { return vizzlyDir; } +async function generateInjectedStaticReport(workingDir, options = {}) { + let capturedReportData; + let result = await generateStaticReport(workingDir, { + css: options.css ?? '.report { color: red; }', + outputDir: options.outputDir, + renderStaticReport: reportData => { + capturedReportData = reportData; + return ( + options.renderedContent || + `
${JSON.stringify(reportData)}
` + ); + }, + }); + + return { result, capturedReportData }; +} + describe('services/static-report-generator', () => { let tempDir; @@ -109,10 +131,6 @@ describe('services/static-report-generator', () => { describe('generateStaticReport', () => { it('should return error when report data is missing', async () => { - let { generateStaticReport } = await import( - '../../src/services/static-report-generator.js' - ); - let result = await generateStaticReport(tempDir); assert.strictEqual(result.success, false); @@ -120,20 +138,50 @@ describe('services/static-report-generator', () => { assert.ok(result.error.includes('No report data found')); }); - it('should generate HTML report with correct structure', async t => { - // Skip if SSR build artifacts don't exist (CI runs tests before build) - if (!ssrBuildExists()) { - t.skip('SSR build not available - run npm run build first'); - return; - } - + it('generates an HTML report with injected renderer and CSS', async () => { + let capturedReportData; setupMockVizzlyDir(tempDir); - let { generateStaticReport } = await import( - '../../src/services/static-report-generator.js' + let result = await generateStaticReport(tempDir, { + css: '.report { color: red; }', + renderStaticReport: reportData => { + capturedReportData = reportData; + return '
Rendered report
'; + }, + }); + + assert.strictEqual(result.success, true); + assert.ok(result.reportPath.endsWith('index.html')); + assert.ok(existsSync(result.reportPath)); + assert.strictEqual( + capturedReportData.comparisons[0].baseline, + './images/baselines/test.png' + ); + assert.strictEqual( + capturedReportData.comparisons[0].current, + './images/current/test.png' + ); + assert.strictEqual( + capturedReportData.comparisons[0].diff, + './images/diffs/test.png' ); - let result = await generateStaticReport(tempDir); + let html = readFileSync(result.reportPath, 'utf8'); + assert.ok(html.includes('