diff --git a/package-lock.json b/package-lock.json index f0d4eaf3..376890df 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@vizzly-testing/cli", - "version": "0.1.0", + "version": "0.3.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@vizzly-testing/cli", - "version": "0.1.0", + "version": "0.3.1", "license": "MIT", "dependencies": { "commander": "^11.1.0", diff --git a/src/cli.js b/src/cli.js index 4b357e1e..5dbe1428 100644 --- a/src/cli.js +++ b/src/cli.js @@ -87,58 +87,30 @@ program process.exit(1); } - await tddCommand(command, options, globalOptions); + const result = await tddCommand(command, options, globalOptions); + if (result && !result.success && result.exitCode > 0) { + process.exit(result.exitCode); + } }); program .command('run') .description('Run tests with Vizzly integration') .argument('', 'Test command to run') - .option('--tdd', 'Enable TDD mode with auto-reload') .option('--port ', 'Port for screenshot server', '47392') .option('-b, --build-name ', 'Custom build name') .option('--branch ', 'Git branch override') .option('--commit ', 'Git commit SHA') .option('--message ', 'Commit message') .option('--environment ', 'Environment name', 'test') - .option('--threshold ', 'Comparison threshold', parseFloat) .option('--token ', 'API token override') .option('--wait', 'Wait for build completion') .option('--timeout ', 'Server timeout in milliseconds', '30000') - .option('--eager', 'Create build immediately (default: lazy)') .option('--allow-no-token', 'Allow running without API token') - .option('--baseline-build ', 'Use specific build as baseline') - .option('--baseline-comparison ', 'Use specific comparison as baseline') + .option('--upload-all', 'Upload all screenshots without SHA deduplication') .action(async (command, options) => { const globalOptions = program.opts(); - // Forward --tdd flag to TDD command (shortcut) - if (options.tdd) { - // Forward to tdd command with appropriate options - const tddOptions = { - port: options.port, - branch: options.branch, - environment: options.environment, - threshold: options.threshold, - token: options.token, - timeout: options.timeout, - baselineBuild: options.baselineBuild, - baselineComparison: options.baselineComparison, - allowNoToken: options.allowNoToken, - }; - - // Validate options using TDD validator - const validationErrors = validateTddOptions(command, tddOptions); - if (validationErrors.length > 0) { - console.error('Validation errors:'); - validationErrors.forEach(error => console.error(` - ${error}`)); - process.exit(1); - } - - await tddCommand(command, tddOptions, globalOptions); - return; - } - // Validate options const validationErrors = validateRunOptions(command, options); if (validationErrors.length > 0) { @@ -147,7 +119,18 @@ program process.exit(1); } - await runCommand(command, options, globalOptions); + try { + const result = await runCommand(command, options, globalOptions); + if (result && !result.success && result.exitCode > 0) { + process.exit(result.exitCode); + } + } catch (error) { + console.error('Command failed:', error.message); + if (globalOptions.verbose) { + console.error('Stack trace:', error.stack); + } + process.exit(1); + } }); program diff --git a/src/client/index.js b/src/client/index.js index 57c7d3a9..828550b8 100644 --- a/src/client/index.js +++ b/src/client/index.js @@ -75,7 +75,7 @@ function createSimpleClient(serverUrl) { buildId: getBuildId(), name, image: imageBuffer.toString('base64'), - properties: options.properties || {}, + properties: options, threshold: options.threshold || 0, variant: options.variant, fullPage: options.fullPage || false, diff --git a/src/commands/run.js b/src/commands/run.js index b4593785..9860823e 100644 --- a/src/commands/run.js +++ b/src/commands/run.js @@ -81,7 +81,6 @@ export async function runCommand( testCommand, port: config.server.port, timeout: config.server.timeout, - tddMode: options.tdd || false, branch, commit: commit?.substring(0, 7), message, @@ -93,8 +92,12 @@ export async function runCommand( // Create service container and get test runner service ui.startSpinner('Initializing test runner...'); - const configWithVerbose = { ...config, verbose: globalOptions.verbose }; - const command = options.tdd ? 'tdd' : 'run'; + const configWithVerbose = { + ...config, + verbose: globalOptions.verbose, + uploadAll: options.uploadAll || false, + }; + const command = 'run'; const container = await createServiceContainer(configWithVerbose, command); testRunner = await container.get('testRunner'); // Assign to outer scope variable ui.stopSpinner(); @@ -130,22 +133,36 @@ export async function runCommand( testRunner.on('build-created', buildInfo => { buildUrl = buildInfo.url; + // Debug: Log build creation details + if (globalOptions.verbose) { + ui.info(`Build created: ${buildInfo.buildId} - ${buildInfo.name}`); + } // Use UI for consistent formatting if (buildUrl) { ui.info(`Vizzly: ${buildUrl}`); } }); + testRunner.on('build-failed', buildError => { + ui.error('Failed to create build', buildError); + }); + testRunner.on('error', error => { + ui.stopSpinner(); // Stop spinner to ensure error is visible ui.error('Test runner error occurred', error, 0); // Don't exit immediately, let runner handle it }); + testRunner.on('build-finalize-failed', errorInfo => { + ui.warning( + `Failed to finalize build ${errorInfo.buildId}: ${errorInfo.error}` + ); + }); + // Prepare run options const runOptions = { testCommand, port: config.server.port, timeout: config.server.timeout, - tdd: options.tdd || false, buildName, branch, commit, @@ -154,9 +171,8 @@ export async function runCommand( threshold: config.comparison.threshold, eager: config.eager || false, allowNoToken: config.allowNoToken || false, - baselineBuildId: config.baselineBuildId, - baselineComparisonId: config.baselineComparisonId, wait: config.wait || options.wait || false, + uploadAll: options.uploadAll || false, }; // Start test run @@ -194,15 +210,29 @@ export async function runCommand( ui.error( `${buildResult.failedComparisons} visual comparisons failed`, {}, - 1 + 0 ); + // Return error status without calling process.exit in tests + return { success: false, exitCode: 1 }; } } } ui.cleanup(); } catch (error) { - ui.error('Test run failed', error); + ui.stopSpinner(); // Ensure spinner is stopped before showing error + + // Provide more context about where the error occurred + let errorContext = 'Test run failed'; + if (error.message && error.message.includes('build')) { + errorContext = 'Build creation failed'; + } else if (error.message && error.message.includes('screenshot')) { + errorContext = 'Screenshot processing failed'; + } else if (error.message && error.message.includes('server')) { + errorContext = 'Server startup failed'; + } + + ui.error(errorContext, error); } finally { // Remove event listeners to prevent memory leaks process.removeListener('SIGINT', sigintHandler); diff --git a/src/commands/tdd.js b/src/commands/tdd.js index b86b420c..3bc078c2 100644 --- a/src/commands/tdd.js +++ b/src/commands/tdd.js @@ -185,7 +185,9 @@ export async function tddCommand( (result.comparisons && result.comparisons.some(c => c.status === 'failed')) ) { - ui.error('Visual differences detected in TDD mode', {}, 1); + ui.error('Visual differences detected in TDD mode', {}, 0); + // Return error status without calling process.exit in tests + return { success: false, exitCode: 1 }; } ui.cleanup(); diff --git a/src/commands/upload.js b/src/commands/upload.js index 79a4844a..f3f0f04a 100644 --- a/src/commands/upload.js +++ b/src/commands/upload.js @@ -63,12 +63,16 @@ export async function uploadCommand( // Note: ConsoleUI handles cleanup via global process listeners + let buildId = null; + let config = null; + const uploadStartTime = Date.now(); + try { ui.info('Starting upload process...'); // Load configuration with CLI overrides const allOptions = { ...globalOptions, ...options }; - const config = await loadConfig(globalOptions.config, allOptions); + config = await loadConfig(globalOptions.config, allOptions); // Validate API token if (!config.apiKey) { @@ -115,19 +119,47 @@ export async function uploadCommand( current, total, phase, + buildId: progressBuildId, } = progressData; - ui.progress( - progressMessage || - `${phase || 'Processing'}: ${current || 0}/${total || 0}`, - current, - total - ); + + // Track buildId when it becomes available + if (progressBuildId) { + buildId = progressBuildId; + } + + let displayMessage = progressMessage; + if (!displayMessage && phase) { + if (current !== undefined && total !== undefined) { + displayMessage = `${phase}: ${current}/${total}`; + } else { + displayMessage = phase; + } + } + + ui.progress(displayMessage || 'Processing...', current, total); }, }; // Start upload ui.progress('Starting upload...'); const result = await uploader.upload(uploadOptions); + buildId = result.buildId; // Ensure we have the buildId + + // Mark build as completed + if (result.buildId) { + ui.progress('Finalizing build...'); + try { + const apiService = new ApiService({ + baseUrl: config.apiUrl, + token: config.apiKey, + command: 'upload', + }); + const executionTime = Date.now() - uploadStartTime; + await apiService.finalizeBuild(result.buildId, true, executionTime); + } catch (error) { + ui.warning(`Failed to finalize build: ${error.message}`); + } + } ui.success('Upload completed successfully'); @@ -171,6 +203,20 @@ export async function uploadCommand( ui.cleanup(); } catch (error) { + // Mark build as failed if we have a buildId and config + if (buildId && config) { + try { + const apiService = new ApiService({ + baseUrl: config.apiUrl, + token: config.apiKey, + command: 'upload', + }); + const executionTime = Date.now() - uploadStartTime; + await apiService.finalizeBuild(buildId, false, executionTime); + } catch { + // Silent fail on cleanup + } + } ui.error('Upload failed', error); } } diff --git a/src/services/api-service.js b/src/services/api-service.js index 450670ee..dfe56b86 100644 --- a/src/services/api-service.js +++ b/src/services/api-service.js @@ -20,6 +20,7 @@ export class ApiService { constructor(options = {}) { this.baseUrl = options.baseUrl || getApiUrl(); this.token = options.token || getApiToken(); + this.uploadAll = options.uploadAll || false; // Build User-Agent string const command = options.command || 'run'; // Default to 'run' for API service @@ -119,7 +120,7 @@ export class ApiService { return this.request('/api/sdk/builds', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(metadata), + body: JSON.stringify({ build: metadata }), }); } @@ -156,7 +157,22 @@ export class ApiService { * @returns {Promise} Upload result */ async uploadScreenshot(buildId, name, buffer, metadata = {}) { - // Calculate SHA256 of the image + // Skip SHA deduplication entirely if uploadAll flag is set + if (this.uploadAll) { + // Upload directly without SHA calculation or checking + return this.request(`/api/sdk/builds/${buildId}/screenshots`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + name, + image_data: buffer.toString('base64'), + properties: metadata ?? {}, + // No SHA included when bypassing deduplication + }), + }); + } + + // Normal flow with SHA deduplication const sha256 = crypto.createHash('sha256').update(buffer).digest('hex'); // Check if this SHA already exists diff --git a/src/services/server-manager.js b/src/services/server-manager.js index 71cef2e8..58858373 100644 --- a/src/services/server-manager.js +++ b/src/services/server-manager.js @@ -1,47 +1,54 @@ /** * Server Manager Service - * Manages the Vizzly HTTP server + * Manages the HTTP server with functional handlers */ import { BaseService } from './base-service.js'; -import { VizzlyServer } from '../server/index.js'; +import { createHttpServer } from '../server/http-server.js'; +import { createTddHandler } from '../server/handlers/tdd-handler.js'; +import { createApiHandler } from '../server/handlers/api-handler.js'; import { EventEmitter } from 'events'; export class ServerManager extends BaseService { constructor(config, logger) { super(config, { logger }); - this.server = null; + this.httpServer = null; + this.handler = null; + this.emitter = null; } - async start(buildId = null, buildInfo = null, mode = 'lazy') { - if (this.started) { - this.logger.warn(`${this.constructor.name} already started`); - return; - } - - // Create event emitter for server events - const emitter = new EventEmitter(); - - this.server = new VizzlyServer({ - port: this.config?.server?.port || 47392, - config: this.config, - buildId, - buildInfo, - vizzlyApi: - buildInfo || mode === 'eager' ? await this.createApiService() : null, - tddMode: mode === 'tdd', // TDD mode only when explicitly set - baselineBuild: this.config?.baselineBuildId, - baselineComparison: this.config?.baselineComparisonId, - workingDir: process.cwd(), - emitter, // Pass the emitter to the server - }); - - await super.start(); + async start(buildId = null, tddMode = false) { + this.buildId = buildId; + this.tddMode = tddMode; + return super.start(); } async onStart() { - if (this.server) { - await this.server.start(); + this.emitter = new EventEmitter(); + const port = this.config?.server?.port || 47392; + + if (this.tddMode) { + this.handler = createTddHandler( + this.config, + process.cwd(), + this.config?.baselineBuildId, + this.config?.baselineComparisonId + ); + + await this.handler.initialize(); + + if (this.buildId) { + this.handler.registerBuild(this.buildId); + } + } else { + const apiService = await this.createApiService(); + this.handler = createApiHandler(apiService); + } + + this.httpServer = createHttpServer(port, this.handler, this.emitter); + + if (this.httpServer) { + await this.httpServer.start(); } } @@ -56,8 +63,26 @@ export class ServerManager extends BaseService { } async onStop() { - if (this.server) { - await this.server.stop(); + if (this.httpServer) { + await this.httpServer.stop(); + } + if (this.handler?.cleanup) { + try { + this.handler.cleanup(); + } catch (error) { + this.logger.debug('Handler cleanup error:', error.message); + // Don't throw - cleanup errors shouldn't fail the stop process + } } } + + // Expose server interface for compatibility + get server() { + return { + emitter: this.emitter, + getScreenshotCount: buildId => + this.handler?.getScreenshotCount?.(buildId) || 0, + finishBuild: buildId => this.handler?.finishBuild?.(buildId), + }; + } } diff --git a/src/services/test-runner.js b/src/services/test-runner.js index 7cb0bf73..f3c51212 100644 --- a/src/services/test-runner.js +++ b/src/services/test-runner.js @@ -42,73 +42,32 @@ export class TestRunner extends BaseService { } try { - let buildInfo = null; let buildUrl = null; let screenshotCount = 0; - if (tdd) { - // TDD mode: create local build for fast feedback - this.logger.debug('TDD mode: creating local build...'); - const build = await this.buildManager.createBuild(options); - buildId = build.id; - this.logger.debug(`TDD build created with ID: ${build.id}`); - } else if (options.eager) { - // Eager mode: create build immediately via API - this.logger.debug('Eager mode: creating build via API...'); + // Create build based on mode + buildId = await this.createBuild(options, tdd); + if (!tdd && buildId) { + // Get build URL for API mode const apiService = await this.createApiService(); if (apiService) { - const buildResult = await apiService.createBuild({ - build: { - name: options.buildName || `Test Run ${new Date().toISOString()}`, - branch: options.branch || 'main', - environment: options.environment || 'test', - commit_sha: options.commit, - commit_message: options.message, - }, - }); - buildId = buildResult.id; - buildUrl = buildResult.url; - this.logger.debug(`Eager build created with ID: ${buildId}`); - if (buildUrl) { - this.logger.info(`Build URL: ${buildUrl}`); + try { + const build = await apiService.getBuild(buildId); + buildUrl = build.url; + if (buildUrl) { + this.logger.info(`Build URL: ${buildUrl}`); + } + } catch (error) { + this.logger.debug('Could not retrieve build URL:', error.message); } - - // Emit build created event for eager mode - this.emit('build-created', { - buildId: buildResult.id, - url: buildResult.url, - name: buildResult.name || options.buildName, - }); - } else { - this.logger.warn( - 'No API key available for eager build creation, falling back to lazy mode' - ); } - } else { - // Lazy mode: prepare build info for API creation on first screenshot - buildInfo = { - buildName: - options.buildName || `Test Run ${new Date().toISOString()}`, - branch: options.branch || 'main', - environment: options.environment || 'test', - commitSha: options.commit, - commitMessage: options.message, - }; } - // Start server with appropriate configuration - const mode = tdd ? 'tdd' : options.eager ? 'eager' : 'lazy'; - await this.serverManager.start(buildId, buildInfo, mode); + // Start server with appropriate handler + await this.serverManager.start(buildId, tdd); // Forward server events - if (this.serverManager.server && this.serverManager.server.emitter) { - this.serverManager.server.emitter.on('build-created', buildInfo => { - // Update local buildId and buildUrl from server - buildId = buildInfo.buildId; - buildUrl = buildInfo.url; - this.emit('build-created', buildInfo); - }); - + if (this.serverManager.server?.emitter) { this.serverManager.server.emitter.on( 'screenshot-captured', screenshotInfo => { @@ -118,14 +77,10 @@ export class TestRunner extends BaseService { ); } - if (tdd) { - this.logger.debug('TDD service ready for comparisons'); - } - const env = { ...process.env, VIZZLY_SERVER_URL: `http://localhost:${this.config.server.port}`, - VIZZLY_BUILD_ID: buildId || 'lazy', // Use 'lazy' for API-driven builds + VIZZLY_BUILD_ID: buildId, VIZZLY_ENABLED: 'true', VIZZLY_SET_BASELINE: options.setBaseline || options['set-baseline'] ? 'true' : 'false', @@ -133,7 +88,7 @@ export class TestRunner extends BaseService { await this.executeTestCommand(testCommand, env); - // Finalize builds based on mode + // Finalize build const executionTime = Date.now() - startTime; await this.finalizeBuild(buildId, tdd, true, executionTime); @@ -147,19 +102,52 @@ export class TestRunner extends BaseService { } catch (error) { this.logger.error('Test run failed:', error); - // Finalize builds on failure too + // Finalize build on failure const executionTime = Date.now() - startTime; await this.finalizeBuild(buildId, tdd, false, executionTime); throw error; } finally { await this.serverManager.stop(); - if ( - tdd && - this.tddService && - typeof this.tddService.stop === 'function' - ) { - await this.tddService.stop(); + } + } + + async createBuild(options, tdd) { + if (tdd) { + // TDD mode: create local build + this.logger.debug('TDD mode: creating local build...'); + const build = await this.buildManager.createBuild(options); + this.logger.debug(`TDD build created with ID: ${build.id}`); + return build.id; + } else { + // API mode: create build via API + this.logger.debug('Creating build via API...'); + const apiService = await this.createApiService(); + if (apiService) { + const buildResult = await apiService.createBuild({ + build: { + name: options.buildName || `Test Run ${new Date().toISOString()}`, + branch: options.branch || 'main', + environment: options.environment || 'test', + commit_sha: options.commit, + commit_message: options.message, + }, + }); + this.logger.debug(`Build created with ID: ${buildResult.id}`); + + // Emit build created event + this.emit('build-created', { + buildId: buildResult.id, + url: buildResult.url, + name: buildResult.name || options.buildName, + }); + + return buildResult.id; + } else { + throw new VizzlyError( + 'No API key available for build creation', + 'API_KEY_MISSING' + ); } } } @@ -168,10 +156,11 @@ export class TestRunner extends BaseService { if (!this.config.apiKey) return null; const { ApiService } = await import('./api-service.js'); - return new ApiService( - { ...this.config, command: 'run' }, - { logger: this.logger } - ); + return new ApiService({ + ...this.config, + command: 'run', + uploadAll: this.config.uploadAll, + }); } async finalizeBuild(buildId, isTddMode, success, executionTime) { @@ -182,15 +171,15 @@ export class TestRunner extends BaseService { try { if (isTddMode) { - // TDD mode: use buildManager for local builds - if (this.buildManager.getCurrentBuild()) { - await this.buildManager.finalizeBuild(buildId, { success }); + // TDD mode: use server handler to finalize + if (this.serverManager.server?.finishBuild) { + await this.serverManager.server.finishBuild(buildId); this.logger.debug( `TDD build ${buildId} finalized with success: ${success}` ); } } else { - // API mode (eager/lazy): use API service to update build status + // API mode: use API service to update build status const apiService = await this.createApiService(); if (apiService) { await apiService.finalizeBuild(buildId, success, executionTime); @@ -199,6 +188,12 @@ export class TestRunner extends BaseService { } catch (error) { // Don't fail the entire run if build finalization fails this.logger.warn(`Failed to finalize build ${buildId}:`, error.message); + // Emit event for UI handling + this.emit('build-finalize-failed', { + buildId, + error: error.message, + stack: error.stack, + }); } } diff --git a/src/services/uploader.js b/src/services/uploader.js index 63564ba2..58921ba5 100644 --- a/src/services/uploader.js +++ b/src/services/uploader.js @@ -17,7 +17,7 @@ import { ValidationError, } from '../errors/vizzly-error.js'; -const DEFAULT_BATCH_SIZE = 10; +const DEFAULT_BATCH_SIZE = 50; const DEFAULT_SHA_CHECK_BATCH_SIZE = 100; const DEFAULT_TIMEOUT = 30000; // 30 seconds @@ -86,11 +86,20 @@ export function createUploader( }); } - onProgress({ phase: 'scanning', total: files.length }); + onProgress({ + phase: 'scanning', + message: `Found ${files.length} screenshots`, + total: files.length, + }); // Process files to get metadata const fileMetadata = await processFiles(files, signal, current => - onProgress({ phase: 'processing', current, total: files.length }) + onProgress({ + phase: 'processing', + message: `Processing files`, + current, + total: files.length, + }) ); // Create build first to get buildId for SHA checking @@ -116,6 +125,7 @@ export function createUploader( onProgress({ phase: 'deduplication', + message: `Checking for duplicates (${toUpload.length} to upload, ${existing.length} existing)`, toUpload: toUpload.length, existing: existing.length, total: files.length, @@ -134,6 +144,7 @@ export function createUploader( onProgress: current => onProgress({ phase: 'uploading', + message: `Uploading screenshots`, current, total: toUpload.length, }), @@ -141,6 +152,7 @@ export function createUploader( onProgress({ phase: 'completed', + message: `Upload completed`, buildId: result.buildId, url: result.url, }); diff --git a/src/utils/console-ui.js b/src/utils/console-ui.js index 92dd4897..fc464823 100644 --- a/src/utils/console-ui.js +++ b/src/utils/console-ui.js @@ -167,13 +167,14 @@ export class ConsoleUI { if (this.json || !process.stdout.isTTY) return; this.stopSpinner(); + this.currentMessage = message; const frames = ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏']; let i = 0; this.spinner = setInterval(() => { const frame = frames[i++ % frames.length]; - const line = `${this.colors.blue(frame)} ${message}`; + const line = `${this.colors.blue(frame)} ${this.currentMessage || message}`; // Clear previous line and write new one process.stdout.write('\r' + ' '.repeat(this.lastLine.length) + '\r'); @@ -224,17 +225,5 @@ export class ConsoleUI { } } -// Ensure spinner is cleaned up on process exit -process.on('exit', () => { - // Clear any remaining spinner - if (process.stdout.isTTY) { - process.stdout.write('\r' + ' '.repeat(80) + '\r'); - } -}); - -process.on('SIGINT', () => { - if (process.stdout.isTTY) { - process.stdout.write('\r' + ' '.repeat(80) + '\r'); - } - process.exit(1); -}); +// Note: Global process event listeners are handled in individual commands +// to avoid interference between tests and proper cleanup