diff --git a/SDK-API.md b/docs/internal/SDK-API.md similarity index 63% rename from SDK-API.md rename to docs/internal/SDK-API.md index 5d7a9ed1..fdb3d369 100644 --- a/SDK-API.md +++ b/docs/internal/SDK-API.md @@ -4,10 +4,6 @@ This document describes the public API endpoints available for Vizzly SDKs and C ## Base URL -``` -https://vizzly.dev/api/sdk/ -``` - All SDK endpoints are prefixed with `/api/sdk/` ## Authentication @@ -78,7 +74,7 @@ Retrieves build information with optional related data. **Query Parameters:** -- `include` (optional): `"screenshots"` or `"comparisons"` to include related data +- `include` (optional): `"screenshots"`, `"comparisons"`, or `"review-summary"` to include related data **Response (200):** @@ -111,7 +107,26 @@ Retrieves build information with optional related data. "created_at": "2024-01-15T10:30:00Z" } ], - "comparisons": [] // Only included if include=comparisons + "comparisons": [], // Only included if include=comparisons + "review_summary": { + // Only included if include=review-summary + "build_id": "build-uuid", + "review_status": "in_progress", + "assignments": [ + { + "id": "assignment-uuid", + "user_id": "user-uuid", + "user_name": "Jane Smith", + "status": "pending", + "assigned_at": "2024-01-15T10:00:00Z" + } + ], + "statistics": { + "total_assignments": 1, + "pending": 1, + "completed": 0 + } + } } } ``` @@ -409,7 +424,7 @@ Uploads multiple screenshot files and/or references existing screenshots by SHA2 ### Get Comparison Details -Retrieves detailed information about a specific comparison, including diff images and screenshot properties. +Retrieves detailed information about a specific comparison, including diff images and commit context for debugging. **Endpoint:** `GET /api/sdk/comparisons/:comparisonId` @@ -421,19 +436,16 @@ Retrieves detailed information about a specific comparison, including diff image "id": "comparison-uuid", "build_id": "build-uuid", "build_name": "CLI Build", + "build_branch": "feature/new-ui", + "build_commit_sha": "abc123def456", + "build_commit_message": "✨ Add new dashboard", + "build_common_ancestor_sha": "def456ghi789", "name": "homepage-desktop-chrome", "status": "completed", "diff_percentage": 2.5, - "threshold": 2.0, + "threshold": 0.1, "has_diff": true, - "current_name": "homepage-desktop-chrome", - "current_browser": "chrome", - "current_viewport_width": 1920, - "current_viewport_height": 1080, - "baseline_name": "homepage-desktop-chrome", - "baseline_browser": "chrome", - "baseline_viewport_width": 1920, - "baseline_viewport_height": 1080, + "approval_status": "pending", "current_screenshot_url": "https://storage.example.com/current.png", "baseline_screenshot_url": "https://storage.example.com/baseline.png", "diff_url": "https://storage.example.com/diff.png", @@ -442,76 +454,222 @@ Retrieves detailed information about a specific comparison, including diff image } ``` -**Note:** The `current_browser`, `current_viewport_width`, `current_viewport_height`, `baseline_browser`, `baseline_viewport_width`, and `baseline_viewport_height` fields are essential for matching screenshots with the same name but different dimensions or browsers. These fields are used to generate unique screenshot signatures (name|viewport_width|browser) for proper baseline matching. +## Build Collaboration -### Search Comparisons +### Create Build Comment -Search for comparisons by screenshot name across builds. Useful for debugging visual regressions by tracking a specific screenshot across multiple builds. +Creates a comment on a build for team collaboration and feedback. -**Endpoint:** `GET /api/sdk/comparisons/search` +**Endpoint:** `POST /api/sdk/builds/:buildId/comments` -**Query Parameters:** +**Request Body:** + +```json +{ + "content": "All visual diffs look good, approving this build for deployment.", + "type": "general" +} +``` + +**Comment Types:** + +- `"general"`: General comment or discussion +- `"approval"`: Comment indicating approval +- `"rejection"`: Comment indicating rejection with concerns + +**Response (201):** + +```json +{ + "comment": { + "id": "comment-uuid", + "build_id": "build-uuid", + "user_id": "user-uuid", + "author_name": "John Doe", + "content": "All visual diffs look good, approving this build for deployment.", + "type": "general", + "created_at": "2024-01-15T10:30:00Z", + "updated_at": "2024-01-15T10:30:00Z" + } +} +``` -- `name` (required): Screenshot name to search for (supports partial matching with LIKE) -- `branch` (optional): Filter results by branch name -- `limit` (optional, default: 50): Maximum number of results to return -- `offset` (optional, default: 0): Number of results to skip for pagination +### List Build Comments + +Retrieves all comments for a build, including nested replies. + +**Endpoint:** `GET /api/sdk/builds/:buildId/comments` **Response (200):** ```json { - "comparisons": [ + "comments": [ { - "id": "comparison-uuid", - "name": "homepage-desktop-chrome", - "status": "completed", - "diff_percentage": 2.5, - "threshold": 2.0, - "has_diff": true, + "id": "comment-uuid", "build_id": "build-uuid", - "build_name": "CLI Build", - "build_branch": "feature/new-ui", - "build_status": "completed", - "build_created_at": "2024-01-15T10:30:00Z", - "current_screenshot": { - "id": "current-screenshot-uuid", - "original_url": "https://storage.example.com/current.png" - }, - "baseline_screenshot": { - "id": "baseline-screenshot-uuid", - "original_url": "https://storage.example.com/baseline.png" - }, - "diff_image": { - "id": "diff-image-uuid", - "url": "https://storage.example.com/diff.png" - }, - "created_at": "2024-01-15T10:30:00Z" + "user_id": "user-uuid", + "author_name": "John Doe", + "content": "All visual diffs look good, approving this build for deployment.", + "type": "general", + "created_at": "2024-01-15T10:30:00Z", + "updated_at": "2024-01-15T10:30:00Z", + "replies": [] } - ], - "pagination": { - "total": 25, - "limit": 50, - "offset": 0, - "hasMore": false + ] +} +``` + +**Note:** For MCP/Claude Code integration, the `profile_photo_url` and `email` fields are filtered out to reduce token usage. + +## Comparison Approvals + +### Update Comparison Approval Status + +Updates the approval status of a comparison. + +**Endpoint:** `PUT /api/sdk/comparisons/:comparisonId/approval` + +**Request Body:** + +```json +{ + "approval_status": "approved", + "comment": "Visual changes look intentional, layout improvements are good." +} +``` + +**Valid Approval Status Values:** + +- `"pending"`: No decision made yet (default) +- `"approved"`: Comparison approved, changes are intentional +- `"rejected"`: Comparison rejected, changes need investigation +- `"auto_approved"`: Automatically approved (identical screenshots) + +**Response (200):** + +```json +{ + "message": "Comparison approval status updated successfully", + "comparison": { + "id": "comparison-uuid", + "approval_status": "approved", + "approval_comment": "Visual changes look intentional, layout improvements are good.", + "approved_by": "user-uuid", + "approved_at": "2024-01-15T10:30:00Z" } } ``` -**Usage Example:** +### Approve Comparison -```bash -# Search for all comparisons of "homepage" screenshot -curl -X GET "/api/sdk/comparisons/search?name=homepage" \ - -H "Authorization: Bearer your-api-token" \ - -H "User-Agent: Vizzly-CLI/1.0.0" +Convenient endpoint to approve a comparison with optional comment. -# Search with branch filter and pagination -curl -X GET "/api/sdk/comparisons/search?name=dashboard&branch=main&limit=20&offset=0" \ - -H "Authorization: Bearer your-api-token" \ - -H "User-Agent: Vizzly-CLI/1.0.0" +**Endpoint:** `POST /api/sdk/comparisons/:comparisonId/approve` + +**Request Body:** + +```json +{ + "comment": "Homepage redesign looks excellent, ready for production." +} +``` + +**Response (200):** + +```json +{ + "message": "Comparison approved successfully", + "comparison": { + "id": "comparison-uuid", + "approval_status": "approved", + "approval_comment": "Homepage redesign looks excellent, ready for production.", + "approved_by": "user-uuid", + "approved_at": "2024-01-15T10:30:00Z" + } +} +``` + +### Reject Comparison + +Convenient endpoint to reject a comparison with required reason. + +**Endpoint:** `POST /api/sdk/comparisons/:comparisonId/reject` + +**Request Body:** + +```json +{ + "reason": "Button alignment is off by 2px, needs to be fixed before deployment." +} +``` + +**Response (200):** + +```json +{ + "message": "Comparison rejected successfully", + "comparison": { + "id": "comparison-uuid", + "approval_status": "rejected", + "approval_comment": "Button alignment is off by 2px, needs to be fixed before deployment.", + "approved_by": "user-uuid", + "approved_at": "2024-01-15T10:30:00Z" + } +} +``` + +## Review Management + +### Get Build Review Summary + +Retrieves review status, assignments, and statistics for a build. + +**Endpoint:** `GET /api/sdk/builds/:buildId/review-summary` + +**Response (200):** + +```json +{ + "build_id": "build-uuid", + "review_status": "in_progress", + "assignments": [ + { + "id": "assignment-uuid", + "user_id": "user-uuid", + "user_name": "Jane Smith", + "status": "pending", + "assigned_at": "2024-01-15T10:00:00Z", + "due_date": null + }, + { + "id": "assignment-uuid-2", + "user_id": "user-uuid-2", + "user_name": "John Doe", + "status": "completed", + "assigned_at": "2024-01-15T10:00:00Z", + "completed_at": "2024-01-15T10:30:00Z", + "due_date": null + } + ], + "statistics": { + "total_assignments": 2, + "pending": 1, + "completed": 1, + "approved": 0, + "rejected": 0 + } +} ``` +**Review Status Values:** + +- `"pending"`: No reviewers assigned yet +- `"in_progress"`: Reviewers assigned, waiting for completion +- `"completed"`: All reviewers have completed their review +- `"approved"`: Build approved by all reviewers +- `"rejected"`: Build rejected by one or more reviewers + ## Token Management ### Get Token Context @@ -709,6 +867,144 @@ jobs: - Always call the finalize endpoint after all shards complete - The finalize job should depend on all shard jobs completing successfully +## Hotspot Analysis (TDD Mode) + +Hotspot endpoints provide historical data about regions that frequently change across builds (timestamps, dynamic IDs, ads, etc.). This data helps the CLI's TDD mode intelligently filter out known dynamic content. + +### Get Screenshot Hotspots + +Retrieves hotspot analysis for a specific screenshot name. + +**Endpoint:** `GET /api/sdk/screenshots/:screenshotName/hotspots` + +**Query Parameters:** + +- `windowSize` (optional, default: 20, max: 50): Number of historical builds to analyze + +**Response (200):** + +```json +{ + "screenshot_name": "homepage-desktop-chrome", + "hotspot_analysis": { + "regions": [ + { + "y1": 100, + "y2": 150, + "height": 50, + "frequency": 0.85, + "lineCount": 50 + } + ], + "total_builds_analyzed": 15, + "confidence": "high", + "confidence_score": 85, + "confidence_factors": { + "build_count": 15, + "data_source": "approved", + "frequency_consistency": 0.9 + }, + "data_source": "approved", + "frequency_threshold": 0.7 + } +} +``` + +**Confidence Values:** + +- `"high"`: Strong historical data, reliable hotspot detection +- `"medium"`: Moderate data, reasonably reliable +- `"low"`: Limited data, use with caution +- `"insufficient_data"`: Not enough historical builds to determine hotspots + +**Data Source Values:** + +- `"approved"`: Hotspots derived from approved builds only (highest quality) +- `"mixed"`: Includes both approved and unapproved builds +- `"none"`: No historical data available + +### Batch Get Screenshot Hotspots + +Retrieves hotspot analysis for multiple screenshots in a single request. More efficient for TDD baseline downloads. + +**Endpoint:** `POST /api/sdk/screenshots/hotspots` + +**Request Body:** + +```json +{ + "screenshot_names": [ + "homepage-desktop-chrome", + "dashboard-mobile-safari", + "settings-tablet-firefox" + ], + "windowSize": 20 +} +``` + +**Constraints:** + +- Maximum 100 screenshot names per request +- Screenshot names must be valid (no path traversal, max 255 characters) + +**Response (200):** + +```json +{ + "hotspots": { + "homepage-desktop-chrome": { + "regions": [ + { + "y1": 100, + "y2": 150, + "height": 50, + "frequency": 0.85, + "lineCount": 50 + } + ], + "total_builds_analyzed": 15, + "confidence": "high", + "confidence_score": 85, + "data_source": "approved" + }, + "dashboard-mobile-safari": { + "regions": [], + "total_builds_analyzed": 8, + "confidence": "medium", + "confidence_score": 60, + "data_source": "mixed" + } + }, + "summary": { + "requested": 3, + "returned": 2 + } +} +``` + +**Notes:** + +- Only screenshots with `total_builds_analyzed > 0` are included in the response +- Screenshots without historical data are omitted from the `hotspots` object +- The `summary` shows how many screenshots were requested vs returned + +### Usage Example (CLI TDD Mode) + +```bash +# Fetch hotspots for baseline download +curl -X POST /api/sdk/screenshots/hotspots \ + -H "Authorization: Bearer your-api-token" \ + -H "User-Agent: Vizzly-CLI/1.0.0" \ + -H "Content-Type: application/json" \ + -d '{"screenshot_names": ["homepage", "dashboard", "settings"]}' +``` + +The CLI uses this data to: + +1. Identify regions that historically change frequently +2. Apply intelligent filtering during local comparisons +3. Reduce false positives from dynamic content like timestamps or ads + ## Implementation Notes - All image data should be base64 encoded when sent via JSON diff --git a/src/server/routers/baseline.js b/src/server/routers/baseline.js index 4c6f188d..25861976 100644 --- a/src/server/routers/baseline.js +++ b/src/server/routers/baseline.js @@ -16,9 +16,14 @@ import { * @param {Object} context - Router context * @param {Object} context.screenshotHandler - Screenshot handler * @param {Object} context.tddService - TDD service for baseline downloads + * @param {Object} context.authService - Auth service for OAuth requests * @returns {Function} Route handler */ -export function createBaselineRouter({ screenshotHandler, tddService }) { +export function createBaselineRouter({ + screenshotHandler, + tddService, + authService, +}) { return async function handleBaselineRoute(req, res, pathname) { // Accept a single screenshot as baseline if (req.method === 'POST' && pathname === '/api/baseline/accept') { @@ -165,13 +170,66 @@ export function createBaselineRouter({ screenshotHandler, tddService }) { try { let body = await parseJsonBody(req); - let { buildId } = body; + let { buildId, organizationSlug, projectSlug } = body; if (!buildId) { sendError(res, 400, 'buildId is required'); return true; } + // Try OAuth authentication first (allows access to any project user has membership to) + // This is the preferred method when user is logged in via the dashboard + if (authService && organizationSlug && projectSlug) { + try { + output.info(`Downloading baselines from build ${buildId}...`); + output.debug( + 'baseline', + `Using OAuth for ${organizationSlug}/${projectSlug}` + ); + + // Use the CLI endpoint which accepts OAuth and checks project membership + let apiResponse = await authService.authenticatedRequest( + `/api/cli/${projectSlug}/builds/${buildId}/tdd-baselines`, + { + method: 'GET', + headers: { 'X-Organization': organizationSlug }, + } + ); + + if (!apiResponse) { + throw new Error( + `Build ${buildId} not found or API returned null` + ); + } + + // Process the baselines through tddService + let result = await tddService.processDownloadedBaselines( + apiResponse, + buildId + ); + + sendSuccess(res, { + success: true, + message: `Baselines downloaded from build ${buildId}`, + ...result, + }); + return true; + } catch (oauthError) { + // If OAuth fails with auth error, fall through to other methods + if ( + !oauthError.message?.includes('Not authenticated') && + !oauthError.message?.includes('401') + ) { + throw oauthError; + } + output.debug( + 'baseline', + `OAuth failed, trying other auth methods: ${oauthError.message}` + ); + } + } + + // Fall back to using tddService directly (requires VIZZLY_TOKEN env var) output.info(`Downloading baselines from build ${buildId}...`); let result = await tddService.downloadBaselines( diff --git a/src/tdd/tdd-service.js b/src/tdd/tdd-service.js index a5ad89e6..b954fa62 100644 --- a/src/tdd/tdd-service.js +++ b/src/tdd/tdd-service.js @@ -275,6 +275,25 @@ export class TddService { buildId = null, comparisonId = null ) { + // Destructure dependencies + let { + output, + colors, + getDefaultBranch, + getTddBaselines, + getBuilds, + getComparison, + clearBaselineData, + generateScreenshotSignature, + generateBaselineFilename, + sanitizeScreenshotName, + safePath, + existsSync, + fetchWithTimeout, + writeFileSync, + saveBaselineMetadata, + } = this._deps; + // If no branch specified, detect default branch if (!branch) { branch = await getDefaultBranch(); @@ -672,10 +691,296 @@ export class TddService { } } + /** + * Process already-fetched baseline data (for use when caller handles auth) + * This allows the baseline router to fetch with a project token and pass the response here + * @param {Object} apiResponse - Response from getTddBaselines API call + * @param {string} buildId - Build ID for reference + * @returns {Promise} Baseline data + */ + async processDownloadedBaselines(apiResponse, buildId) { + // Destructure dependencies + let { + output, + colors, + clearBaselineData, + sanitizeScreenshotName, + safePath, + existsSync, + fetchWithTimeout, + writeFileSync, + saveBaselineMetadata, + } = this._deps; + + // Clear local state before downloading + output.info('Clearing local state before downloading baselines...'); + clearBaselineData({ + baselinePath: this.baselinePath, + currentPath: this.currentPath, + diffPath: this.diffPath, + }); + + // Extract signature properties + if ( + apiResponse.signatureProperties && + Array.isArray(apiResponse.signatureProperties) + ) { + this.signatureProperties = apiResponse.signatureProperties; + if (this.signatureProperties.length > 0) { + output.info( + `Using signature properties: ${this.signatureProperties.join(', ')}` + ); + } + } + + let baselineBuild = apiResponse.build; + + if (baselineBuild.status === 'failed') { + output.warn( + `Build ${buildId} is marked as FAILED - falling back to local baselines` + ); + return await this.handleLocalBaselines(); + } else if (baselineBuild.status !== 'completed') { + output.warn( + `Build ${buildId} has status: ${baselineBuild.status} (expected: completed)` + ); + } + + baselineBuild.screenshots = apiResponse.screenshots; + + let buildDetails = baselineBuild; + + if (!buildDetails.screenshots || buildDetails.screenshots.length === 0) { + output.warn('No screenshots found in baseline build'); + return null; + } + + output.info( + `Using baseline from build: ${colors.cyan(baselineBuild.name || 'Unknown')} (${baselineBuild.id || 'Unknown ID'})` + ); + output.info( + `Checking ${colors.cyan(buildDetails.screenshots.length)} baseline screenshots...` + ); + + // Check existing baseline metadata for SHA comparison + let existingBaseline = await this.loadBaseline(); + let existingShaMap = new Map(); + + if (existingBaseline) { + existingBaseline.screenshots.forEach(s => { + if (s.sha256 && s.filename) { + existingShaMap.set(s.filename, s.sha256); + } + }); + } + + // Download screenshots + let downloadedCount = 0; + let skippedCount = 0; + let errorCount = 0; + let batchSize = 5; + + let screenshotsToProcess = []; + for (let screenshot of buildDetails.screenshots) { + let sanitizedName; + try { + sanitizedName = sanitizeScreenshotName(screenshot.name); + } catch (error) { + output.warn( + `Skipping screenshot with invalid name '${screenshot.name}': ${error.message}` + ); + errorCount++; + continue; + } + + let filename = screenshot.filename; + if (!filename) { + output.warn( + `Screenshot ${sanitizedName} has no filename from API - skipping` + ); + errorCount++; + continue; + } + + let imagePath = safePath(this.baselinePath, filename); + + // Check SHA + if (existsSync(imagePath) && screenshot.sha256) { + let storedSha = existingShaMap.get(filename); + if (storedSha === screenshot.sha256) { + downloadedCount++; + skippedCount++; + continue; + } + } + + let downloadUrl = screenshot.original_url || screenshot.url; + if (!downloadUrl) { + output.warn( + `Screenshot ${sanitizedName} has no download URL - skipping` + ); + errorCount++; + continue; + } + + screenshotsToProcess.push({ + screenshot, + sanitizedName, + imagePath, + downloadUrl, + filename, + }); + } + + // Process downloads in batches + if (screenshotsToProcess.length > 0) { + output.info( + `📥 Downloading ${screenshotsToProcess.length} new/updated screenshots...` + ); + + for (let i = 0; i < screenshotsToProcess.length; i += batchSize) { + let batch = screenshotsToProcess.slice(i, i + batchSize); + let batchNum = Math.floor(i / batchSize) + 1; + let totalBatches = Math.ceil(screenshotsToProcess.length / batchSize); + + output.info(`📦 Processing batch ${batchNum}/${totalBatches}`); + + let downloadPromises = batch.map( + async ({ sanitizedName, imagePath, downloadUrl }) => { + try { + let response = await fetchWithTimeout(downloadUrl); + if (!response.ok) { + throw new Error( + `Failed to download ${sanitizedName}: ${response.statusText}` + ); + } + + let arrayBuffer = await response.arrayBuffer(); + let imageBuffer = Buffer.from(arrayBuffer); + writeFileSync(imagePath, imageBuffer); + + return { success: true, name: sanitizedName }; + } catch (error) { + output.warn( + `Failed to download ${sanitizedName}: ${error.message}` + ); + return { + success: false, + name: sanitizedName, + error: error.message, + }; + } + } + ); + + let batchResults = await Promise.all(downloadPromises); + let batchSuccesses = batchResults.filter(r => r.success).length; + let batchFailures = batchResults.filter(r => !r.success).length; + + downloadedCount += batchSuccesses; + errorCount += batchFailures; + } + } + + if (downloadedCount === 0 && skippedCount === 0) { + output.error('No screenshots were successfully downloaded'); + return null; + } + + // Store baseline metadata + this.baselineData = { + buildId: baselineBuild.id, + buildName: baselineBuild.name, + environment: 'test', + branch: null, + threshold: this.threshold, + signatureProperties: this.signatureProperties, + createdAt: new Date().toISOString(), + buildInfo: { + commitSha: baselineBuild.commit_sha, + commitMessage: baselineBuild.commit_message, + approvalStatus: baselineBuild.approval_status, + completedAt: baselineBuild.completed_at, + }, + screenshots: buildDetails.screenshots + .filter(s => s.filename) + .map(s => ({ + name: sanitizeScreenshotName(s.name), + originalName: s.name, + sha256: s.sha256, + id: s.id, + filename: s.filename, + path: safePath(this.baselinePath, s.filename), + browser: s.browser, + viewport_width: s.viewport_width, + originalUrl: s.original_url, + fileSize: s.file_size_bytes, + dimensions: { width: s.width, height: s.height }, + })), + }; + + saveBaselineMetadata(this.baselinePath, this.baselineData); + + // Download hotspots if API key is available (requires SDK auth) + // OAuth-only users won't get hotspots since the hotspot endpoint requires project token + if (this.config.apiKey && buildDetails.screenshots?.length > 0) { + await this.downloadHotspots(buildDetails.screenshots); + } + + // Save baseline build metadata for MCP plugin + let baselineMetadataPath = safePath( + this.workingDir, + '.vizzly', + 'baseline-metadata.json' + ); + writeFileSync( + baselineMetadataPath, + JSON.stringify( + { + buildId: baselineBuild.id, + buildName: baselineBuild.name, + branch: null, + environment: 'test', + commitSha: baselineBuild.commit_sha, + commitMessage: baselineBuild.commit_message, + approvalStatus: baselineBuild.approval_status, + completedAt: baselineBuild.completed_at, + downloadedAt: new Date().toISOString(), + }, + null, + 2 + ) + ); + + // Summary + let actualDownloads = downloadedCount - skippedCount; + if (skippedCount > 0) { + if (actualDownloads === 0) { + output.info(`All ${skippedCount} baselines up-to-date`); + } else { + output.info( + `Downloaded ${actualDownloads} new screenshots, ${skippedCount} already up-to-date` + ); + } + } else { + output.info( + `Downloaded ${downloadedCount}/${buildDetails.screenshots.length} screenshots successfully` + ); + } + + if (errorCount > 0) { + output.warn(`${errorCount} screenshots failed to download`); + } + + return this.baselineData; + } + /** * Download hotspot data for screenshots */ async downloadHotspots(screenshots) { + let { output, getBatchHotspots, saveHotspotMetadata } = this._deps; + if (!this.config.apiKey) { output.debug( 'tdd', diff --git a/tests/server/routers/baseline.test.js b/tests/server/routers/baseline.test.js index 1267ed8d..954a2ddd 100644 --- a/tests/server/routers/baseline.test.js +++ b/tests/server/routers/baseline.test.js @@ -96,6 +96,28 @@ function createMockTddService(options = {}) { if (options.downloadError) throw options.downloadError; return { downloaded: options.downloadCount || 10 }; }, + processDownloadedBaselines: async (_apiResponse, _buildId) => { + if (options.processError) throw options.processError; + return { downloaded: options.downloadCount || 10 }; + }, + }; +} + +/** + * Creates a mock auth service + */ +function _createMockAuthService(options = {}) { + return { + authenticatedRequest: async (_path, _options) => { + if (options.authError) throw options.authError; + return ( + options.response || { + build: { id: 'build-123', status: 'completed' }, + screenshots: [], + signatureProperties: [], + } + ); + }, }; } @@ -446,6 +468,153 @@ describe('server/routers/baseline', () => { let body = res.getParsedBody(); assert.ok(body.error.includes('Download failed')); }); + + it('uses OAuth when organizationSlug and projectSlug provided', async () => { + let authRequestPath = null; + let authRequestHeaders = null; + let mockAuthService = { + authenticatedRequest: async (path, options) => { + authRequestPath = path; + authRequestHeaders = options.headers; + return { + build: { id: 'build-456', status: 'completed' }, + screenshots: [], + signatureProperties: [], + }; + }, + }; + + let handler = createBaselineRouter({ + screenshotHandler: createMockScreenshotHandler(), + tddService: createMockTddService(), + authService: mockAuthService, + }); + let req = createMockRequest('POST', { + buildId: 'build-456', + organizationSlug: 'my-org', + projectSlug: 'my-project', + }); + let res = createMockResponse(); + + await handler(req, res, '/api/baselines/download'); + + assert.strictEqual(res.statusCode, 200); + assert.ok( + authRequestPath.includes('/api/cli/my-project/builds/build-456'), + `Expected OAuth path, got: ${authRequestPath}` + ); + assert.strictEqual(authRequestHeaders['X-Organization'], 'my-org'); + }); + + it('falls back to tddService when OAuth fails with auth error', async () => { + let tddServiceCalled = false; + let mockAuthService = { + authenticatedRequest: async () => { + throw new Error('Not authenticated'); + }, + }; + let mockTddService = { + downloadBaselines: async () => { + tddServiceCalled = true; + return { downloaded: 5 }; + }, + processDownloadedBaselines: async () => { + return { downloaded: 5 }; + }, + }; + + let handler = createBaselineRouter({ + screenshotHandler: createMockScreenshotHandler(), + tddService: mockTddService, + authService: mockAuthService, + }); + let req = createMockRequest('POST', { + buildId: 'build-789', + organizationSlug: 'my-org', + projectSlug: 'my-project', + }); + let res = createMockResponse(); + + await handler(req, res, '/api/baselines/download'); + + assert.strictEqual(res.statusCode, 200); + assert.ok(tddServiceCalled, 'Should fall back to tddService'); + }); + + it('throws non-auth OAuth errors without fallback', async () => { + let mockAuthService = { + authenticatedRequest: async () => { + throw new Error('Build not found'); + }, + }; + let tddServiceCalled = false; + let mockTddService = { + downloadBaselines: async () => { + tddServiceCalled = true; + return { downloaded: 5 }; + }, + processDownloadedBaselines: async () => { + return { downloaded: 5 }; + }, + }; + + let handler = createBaselineRouter({ + screenshotHandler: createMockScreenshotHandler(), + tddService: mockTddService, + authService: mockAuthService, + }); + let req = createMockRequest('POST', { + buildId: 'build-999', + organizationSlug: 'my-org', + projectSlug: 'my-project', + }); + let res = createMockResponse(); + + await handler(req, res, '/api/baselines/download'); + + assert.strictEqual(res.statusCode, 500); + assert.ok( + !tddServiceCalled, + 'Should NOT fall back for non-auth errors' + ); + let body = res.getParsedBody(); + assert.ok(body.error.includes('Build not found')); + }); + + it('skips OAuth when organizationSlug or projectSlug missing', async () => { + let authServiceCalled = false; + let tddServiceCalled = false; + let mockAuthService = { + authenticatedRequest: async () => { + authServiceCalled = true; + return { build: {}, screenshots: [] }; + }, + }; + let mockTddService = { + downloadBaselines: async () => { + tddServiceCalled = true; + return { downloaded: 5 }; + }, + processDownloadedBaselines: async () => { + return { downloaded: 5 }; + }, + }; + + let handler = createBaselineRouter({ + screenshotHandler: createMockScreenshotHandler(), + tddService: mockTddService, + authService: mockAuthService, + }); + // Missing organizationSlug and projectSlug + let req = createMockRequest('POST', { buildId: 'build-123' }); + let res = createMockResponse(); + + await handler(req, res, '/api/baselines/download'); + + assert.strictEqual(res.statusCode, 200); + assert.ok(!authServiceCalled, 'Should NOT use OAuth without slugs'); + assert.ok(tddServiceCalled, 'Should use tddService directly'); + }); }); }); }); diff --git a/tests/services/config-service.test.js b/tests/services/config-service.test.js new file mode 100644 index 00000000..dca2f45b --- /dev/null +++ b/tests/services/config-service.test.js @@ -0,0 +1,381 @@ +import assert from 'node:assert'; +import { + existsSync, + mkdirSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, it } from 'node:test'; +import { createConfigService } from '../../src/services/config-service.js'; + +/** + * Create a unique temporary directory for each test + */ +function createTempDir() { + let dir = join( + tmpdir(), + `vizzly-test-${Date.now()}-${Math.random().toString(36).slice(2)}` + ); + mkdirSync(dir, { recursive: true }); + return dir; +} + +/** + * Clean up temp directory + */ +function cleanupTempDir(dir) { + try { + if (existsSync(dir)) { + rmSync(dir, { recursive: true, force: true }); + } + } catch { + // Ignore cleanup errors + } +} + +describe('services/config-service', () => { + let tempDir; + + beforeEach(() => { + tempDir = createTempDir(); + // Clear any environment variables that might affect tests + delete process.env.VIZZLY_THRESHOLD; + delete process.env.VIZZLY_PORT; + }); + + afterEach(() => { + cleanupTempDir(tempDir); + // Clean up environment variables after each test + delete process.env.VIZZLY_THRESHOLD; + delete process.env.VIZZLY_PORT; + delete process.env.VIZZLY_HOME; + }); + + describe('createConfigService', () => { + it('creates a config service with all methods', () => { + let service = createConfigService({ workingDir: tempDir }); + + assert.ok(service.getConfig, 'Should have getConfig method'); + assert.ok(service.updateConfig, 'Should have updateConfig method'); + assert.ok(service.validateConfig, 'Should have validateConfig method'); + }); + }); + + describe('getConfig - merged', () => { + it('returns default config when no config files exist', async () => { + let service = createConfigService({ workingDir: tempDir }); + + let { config } = await service.getConfig('merged'); + + assert.strictEqual(config.comparison.threshold, 2.0); + assert.strictEqual(config.server.port, 47392); + assert.strictEqual(config.server.timeout, 30000); + }); + + it('merges project config over defaults', async () => { + // Create a vizzly.config.js in temp dir + writeFileSync( + join(tempDir, 'vizzly.config.js'), + `export default { comparison: { threshold: 5.0 } };` + ); + + let service = createConfigService({ workingDir: tempDir }); + let { config, sources } = await service.getConfig('merged'); + + assert.strictEqual(config.comparison.threshold, 5.0); + assert.strictEqual(sources.threshold, 'project'); + }); + }); + + describe('getConfig - project', () => { + it('returns empty config when no project config exists', async () => { + let service = createConfigService({ workingDir: tempDir }); + + let { config, path } = await service.getConfig('project'); + + assert.deepStrictEqual(config, {}); + assert.strictEqual(path, null); + }); + + it('returns project config when it exists', async () => { + writeFileSync( + join(tempDir, 'vizzly.config.js'), + `export default { server: { port: 9999 } };` + ); + + let service = createConfigService({ workingDir: tempDir }); + let { config, path } = await service.getConfig('project'); + + assert.strictEqual(config.server.port, 9999); + assert.ok(path.endsWith('vizzly.config.js')); + }); + }); + + describe('getConfig - global', () => { + it('returns global config path', async () => { + // Use isolated VIZZLY_HOME for this test + let vizzlyHome = join(tempDir, '.vizzly-home'); + mkdirSync(vizzlyHome, { recursive: true }); + process.env.VIZZLY_HOME = vizzlyHome; + + let service = createConfigService({ workingDir: tempDir }); + let { path } = await service.getConfig('global'); + + assert.ok(path.includes('config.json')); + }); + }); + + describe('getConfig - invalid type', () => { + it('throws for unknown config type', async () => { + let service = createConfigService({ workingDir: tempDir }); + + await assert.rejects( + () => service.getConfig('invalid'), + /Unknown config type/ + ); + }); + }); + + describe('updateConfig - project', () => { + it('creates new project config file when none exists', async () => { + let service = createConfigService({ workingDir: tempDir }); + + await service.updateConfig('project', { comparison: { threshold: 4.0 } }); + + let configPath = join(tempDir, 'vizzly.config.js'); + assert.ok(existsSync(configPath), 'Should create config file'); + + let content = readFileSync(configPath, 'utf8'); + assert.ok(content.includes('defineConfig'), 'Should use defineConfig'); + assert.ok(content.includes('4'), 'Should include threshold value'); + }); + + it('updates existing project config', async () => { + // Create initial config + writeFileSync( + join(tempDir, 'vizzly.config.js'), + `export default { comparison: { threshold: 2.0 }, server: { port: 3000 } };` + ); + + let service = createConfigService({ workingDir: tempDir }); + await service.updateConfig('project', { comparison: { threshold: 6.0 } }); + + // Read the file directly to verify write (cosmiconfig may cache) + let content = readFileSync(join(tempDir, 'vizzly.config.js'), 'utf8'); + assert.ok(content.includes('6'), 'Should have updated threshold'); + assert.ok(content.includes('3000'), 'Should preserve port'); + }); + + it('returns success result with path', async () => { + let service = createConfigService({ workingDir: tempDir }); + + let result = await service.updateConfig('project', { + tdd: { openReport: true }, + }); + + assert.strictEqual(result.success, true); + assert.ok(result.path.includes('vizzly.config')); + }); + }); + + describe('updateConfig - global', () => { + it('updates global config settings', async () => { + // Use isolated VIZZLY_HOME for this test + let vizzlyHome = join(tempDir, '.vizzly-home'); + mkdirSync(vizzlyHome, { recursive: true }); + process.env.VIZZLY_HOME = vizzlyHome; + + let service = createConfigService({ workingDir: tempDir }); + + let result = await service.updateConfig('global', { + server: { port: 7777 }, + }); + + assert.strictEqual(result.success, true); + + // Verify the config was saved + let { config } = await service.getConfig('global'); + assert.strictEqual(config.server.port, 7777); + }); + + it('merges with existing global config', async () => { + // Use isolated VIZZLY_HOME + let vizzlyHome = join(tempDir, '.vizzly-home'); + mkdirSync(vizzlyHome, { recursive: true }); + process.env.VIZZLY_HOME = vizzlyHome; + + // Create initial global config + writeFileSync( + join(vizzlyHome, 'config.json'), + JSON.stringify({ settings: { comparison: { threshold: 1.0 } } }) + ); + + let service = createConfigService({ workingDir: tempDir }); + await service.updateConfig('global', { server: { port: 8888 } }); + + let { config } = await service.getConfig('global'); + assert.strictEqual(config.comparison.threshold, 1.0); + assert.strictEqual(config.server.port, 8888); + }); + }); + + describe('updateConfig - invalid type', () => { + it('throws for invalid update type', async () => { + let service = createConfigService({ workingDir: tempDir }); + + await assert.rejects( + () => service.updateConfig('merged', {}), + /Cannot update config type/ + ); + }); + }); + + describe('validateConfig', () => { + it('validates correct config', async () => { + let service = createConfigService({ workingDir: tempDir }); + + let result = await service.validateConfig({ + comparison: { threshold: 5.0 }, + server: { port: 8080, timeout: 10000 }, + }); + + assert.strictEqual(result.valid, true); + assert.strictEqual(result.errors.length, 0); + }); + + it('returns error for negative threshold', async () => { + let service = createConfigService({ workingDir: tempDir }); + + let result = await service.validateConfig({ + comparison: { threshold: -1 }, + }); + + assert.strictEqual(result.valid, false); + assert.ok(result.errors.some(e => e.includes('threshold'))); + }); + + it('returns error for non-number threshold', async () => { + let service = createConfigService({ workingDir: tempDir }); + + let result = await service.validateConfig({ + comparison: { threshold: 'high' }, + }); + + assert.strictEqual(result.valid, false); + assert.ok(result.errors.some(e => e.includes('threshold'))); + }); + + it('returns warning for very high threshold', async () => { + let service = createConfigService({ workingDir: tempDir }); + + let result = await service.validateConfig({ + comparison: { threshold: 150 }, + }); + + assert.strictEqual(result.valid, true); + assert.ok(result.warnings.some(w => w.includes('100'))); + }); + + it('returns error for invalid port', async () => { + let service = createConfigService({ workingDir: tempDir }); + + let result = await service.validateConfig({ + server: { port: 70000 }, + }); + + assert.strictEqual(result.valid, false); + assert.ok(result.errors.some(e => e.includes('port'))); + }); + + it('returns error for non-integer port', async () => { + let service = createConfigService({ workingDir: tempDir }); + + let result = await service.validateConfig({ + server: { port: 8080.5 }, + }); + + assert.strictEqual(result.valid, false); + assert.ok(result.errors.some(e => e.includes('port'))); + }); + + it('returns warning for privileged port', async () => { + let service = createConfigService({ workingDir: tempDir }); + + let result = await service.validateConfig({ + server: { port: 80 }, + }); + + assert.strictEqual(result.valid, true); + assert.ok(result.warnings.some(w => w.includes('1024'))); + }); + + it('returns error for negative timeout', async () => { + let service = createConfigService({ workingDir: tempDir }); + + let result = await service.validateConfig({ + server: { timeout: -1000 }, + }); + + assert.strictEqual(result.valid, false); + assert.ok(result.errors.some(e => e.includes('timeout'))); + }); + + it('returns error for non-integer timeout', async () => { + let service = createConfigService({ workingDir: tempDir }); + + let result = await service.validateConfig({ + server: { timeout: 'slow' }, + }); + + assert.strictEqual(result.valid, false); + assert.ok(result.errors.some(e => e.includes('timeout'))); + }); + + it('validates empty config as valid', async () => { + let service = createConfigService({ workingDir: tempDir }); + + let result = await service.validateConfig({}); + + assert.strictEqual(result.valid, true); + assert.strictEqual(result.errors.length, 0); + }); + }); + + describe('config merging', () => { + it('deep merges nested objects', async () => { + writeFileSync( + join(tempDir, 'vizzly.config.js'), + `export default { + comparison: { threshold: 3.0, minClusterSize: 5 }, + server: { port: 9000 } + };` + ); + + let service = createConfigService({ workingDir: tempDir }); + let { config } = await service.getConfig('merged'); + + // Should have values from both default and project + assert.strictEqual(config.comparison.threshold, 3.0); + assert.strictEqual(config.comparison.minClusterSize, 5); + assert.strictEqual(config.server.port, 9000); + assert.strictEqual(config.server.timeout, 30000); // Default + }); + }); + + describe('config with defineConfig wrapper', () => { + it('handles config with default export', async () => { + writeFileSync( + join(tempDir, 'vizzly.config.js'), + `export default { comparison: { threshold: 8.0 } };` + ); + + let service = createConfigService({ workingDir: tempDir }); + let { config } = await service.getConfig('merged'); + + assert.strictEqual(config.comparison.threshold, 8.0); + }); + }); +}); diff --git a/tests/tdd/tdd-service.test.js b/tests/tdd/tdd-service.test.js index b7a3c21e..61435cd8 100644 --- a/tests/tdd/tdd-service.test.js +++ b/tests/tdd/tdd-service.test.js @@ -1038,6 +1038,61 @@ describe('tdd/tdd-service', () => { }); }); + describe('downloadBaselines', () => { + it('uses injected getDefaultBranch when branch is not specified', async () => { + let getDefaultBranchCalled = false; + let mockDeps = createMockDeps({ + api: { + getDefaultBranch: async () => { + getDefaultBranchCalled = true; + return 'main'; + }, + getBuilds: async () => ({ data: [] }), + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + // Should not throw "getDefaultBranch is not defined" + await service.downloadBaselines('test', null, null, null); + + assert.ok( + getDefaultBranchCalled, + 'Should call injected getDefaultBranch' + ); + }); + + it('uses injected dependencies when downloading by buildId', async () => { + let apiCalls = []; + let mockDeps = createMockDeps({ + api: { + getDefaultBranch: async () => 'main', + getTddBaselines: async (_client, buildId) => { + apiCalls.push({ method: 'getTddBaselines', buildId }); + return { + build: { + id: buildId, + name: 'Test Build', + status: 'completed', + }, + screenshots: [], + signatureProperties: [], + }; + }, + }, + baseline: { + clearBaselineData: () => {}, + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + await service.downloadBaselines('test', null, 'build-123', null); + + let tddCall = apiCalls.find(c => c.method === 'getTddBaselines'); + assert.ok(tddCall, 'Should call injected getTddBaselines'); + assert.strictEqual(tddCall.buildId, 'build-123'); + }); + }); + describe('createNewBaseline', () => { it('creates a new baseline and updates metadata', () => { let writeFileSync = (path, buffer) => { @@ -1179,4 +1234,799 @@ describe('tdd/tdd-service', () => { assert.ok(successCall); }); }); + + describe('processDownloadedBaselines', () => { + it('clears local baseline data before processing', async () => { + let clearCalled = false; + let mockDeps = createMockDeps({ + baseline: { + clearBaselineData: () => { + clearCalled = true; + }, + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'completed' }, + screenshots: [], + signatureProperties: [], + }; + + await service.processDownloadedBaselines(apiResponse, 'build-1'); + + assert.ok(clearCalled, 'Should clear baseline data'); + }); + + it('extracts and stores signature properties from API response', async () => { + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'completed' }, + screenshots: [], + signatureProperties: ['browser', 'viewport_width'], + }; + + await service.processDownloadedBaselines(apiResponse, 'build-1'); + + assert.deepStrictEqual(service.signatureProperties, [ + 'browser', + 'viewport_width', + ]); + }); + + it('returns null and falls back to local baselines when build status is failed', async () => { + let handleLocalBaselinesCalled = false; + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + metadata: { + loadBaselineMetadata: () => { + handleLocalBaselinesCalled = true; + return null; + }, + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'failed' }, + screenshots: [], + }; + + await service.processDownloadedBaselines(apiResponse, 'build-1'); + + // handleLocalBaselines loads baseline metadata + assert.ok(handleLocalBaselinesCalled); + }); + + it('warns when build status is not completed', async () => { + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'pending' }, + screenshots: [], + }; + + await service.processDownloadedBaselines(apiResponse, 'build-1'); + + let warnCall = mockDeps.output.calls.find( + c => c.method === 'warn' && c.args[0].includes('pending') + ); + assert.ok(warnCall, 'Should warn about non-completed status'); + }); + + it('returns null when no screenshots in build', async () => { + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'completed' }, + screenshots: [], + }; + + let result = await service.processDownloadedBaselines( + apiResponse, + 'build-1' + ); + + assert.strictEqual(result, null); + }); + + it('skips screenshots without filename', async () => { + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'completed' }, + screenshots: [ + { name: 'test', original_url: 'http://example.com/1.png' }, + ], + }; + + await service.processDownloadedBaselines(apiResponse, 'build-1'); + + let warnCall = mockDeps.output.calls.find( + c => c.method === 'warn' && c.args[0].includes('no filename') + ); + assert.ok(warnCall, 'Should warn about missing filename'); + }); + + it('skips download when SHA matches existing file', async () => { + let fetchCalled = false; + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + fs: { + existsSync: () => true, + writeFileSync: () => {}, + }, + metadata: { + loadBaselineMetadata: () => ({ + screenshots: [{ filename: 'test_abc.png', sha256: 'matching-sha' }], + }), + saveBaselineMetadata: () => {}, + }, + api: { + fetchWithTimeout: async () => { + fetchCalled = true; + return { ok: true, arrayBuffer: async () => new ArrayBuffer(0) }; + }, + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'completed' }, + screenshots: [ + { + name: 'test', + filename: 'test_abc.png', + sha256: 'matching-sha', + original_url: 'http://example.com/1.png', + }, + ], + }; + + await service.processDownloadedBaselines(apiResponse, 'build-1'); + + assert.ok(!fetchCalled, 'Should not fetch when SHA matches'); + }); + + it('downloads screenshots when SHA differs', async () => { + let fetchCalled = false; + let writtenFiles = []; + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + fs: { + existsSync: () => true, + writeFileSync: (path, buffer) => writtenFiles.push({ path, buffer }), + }, + metadata: { + loadBaselineMetadata: () => ({ + screenshots: [{ filename: 'test_abc.png', sha256: 'old-sha' }], + }), + saveBaselineMetadata: () => {}, + }, + api: { + fetchWithTimeout: async () => { + fetchCalled = true; + return { + ok: true, + arrayBuffer: async () => new ArrayBuffer(10), + }; + }, + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'completed' }, + screenshots: [ + { + name: 'test', + filename: 'test_abc.png', + sha256: 'new-sha', + original_url: 'http://example.com/1.png', + }, + ], + }; + + await service.processDownloadedBaselines(apiResponse, 'build-1'); + + assert.ok(fetchCalled, 'Should fetch when SHA differs'); + assert.strictEqual(writtenFiles.length, 2); // screenshot + metadata + }); + + it('skips screenshots without download URL', async () => { + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + fs: { existsSync: () => false }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'completed' }, + screenshots: [{ name: 'test', filename: 'test.png' }], + }; + + await service.processDownloadedBaselines(apiResponse, 'build-1'); + + let warnCall = mockDeps.output.calls.find( + c => c.method === 'warn' && c.args[0].includes('no download URL') + ); + assert.ok(warnCall, 'Should warn about missing download URL'); + }); + + it('handles download failures gracefully', async () => { + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + fs: { + existsSync: () => false, + writeFileSync: () => {}, + }, + metadata: { + loadBaselineMetadata: () => null, + saveBaselineMetadata: () => {}, + }, + api: { + fetchWithTimeout: async () => { + throw new Error('Network error'); + }, + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'completed' }, + screenshots: [ + { + name: 'test', + filename: 'test.png', + original_url: 'http://example.com/1.png', + }, + ], + }; + + // Should not throw + await service.processDownloadedBaselines(apiResponse, 'build-1'); + + let warnCall = mockDeps.output.calls.find( + c => c.method === 'warn' && c.args[0].includes('Failed to download') + ); + assert.ok(warnCall, 'Should warn about download failure'); + }); + + it('handles non-ok HTTP responses', async () => { + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + fs: { + existsSync: () => false, + writeFileSync: () => {}, + }, + metadata: { + loadBaselineMetadata: () => null, + saveBaselineMetadata: () => {}, + }, + api: { + fetchWithTimeout: async () => ({ + ok: false, + statusText: 'Not Found', + }), + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'completed' }, + screenshots: [ + { + name: 'test', + filename: 'test.png', + original_url: 'http://example.com/1.png', + }, + ], + }; + + await service.processDownloadedBaselines(apiResponse, 'build-1'); + + let warnCall = mockDeps.output.calls.find( + c => c.method === 'warn' && c.args[0].includes('Failed to download') + ); + assert.ok(warnCall, 'Should warn about HTTP error'); + }); + + it('saves baseline metadata after successful downloads', async () => { + let savedMetadata = null; + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + fs: { + existsSync: () => false, + writeFileSync: () => {}, + }, + metadata: { + loadBaselineMetadata: () => null, + saveBaselineMetadata: (_path, data) => { + savedMetadata = data; + }, + }, + api: { + fetchWithTimeout: async () => ({ + ok: true, + arrayBuffer: async () => new ArrayBuffer(10), + }), + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { + id: 'build-1', + name: 'Test Build', + status: 'completed', + commit_sha: 'abc123', + }, + screenshots: [ + { + name: 'test', + filename: 'test.png', + original_url: 'http://example.com/1.png', + sha256: 'sha-value', + }, + ], + }; + + await service.processDownloadedBaselines(apiResponse, 'build-1'); + + assert.ok(savedMetadata, 'Should save baseline metadata'); + assert.strictEqual(savedMetadata.buildId, 'build-1'); + assert.strictEqual(savedMetadata.buildName, 'Test Build'); + assert.ok(savedMetadata.screenshots.length > 0); + }); + + it('downloads hotspots when apiKey is configured', async () => { + let hotspotsCalled = false; + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + fs: { + existsSync: () => false, + writeFileSync: () => {}, + }, + metadata: { + loadBaselineMetadata: () => null, + saveBaselineMetadata: () => {}, + saveHotspotMetadata: () => {}, + }, + api: { + fetchWithTimeout: async () => ({ + ok: true, + arrayBuffer: async () => new ArrayBuffer(10), + }), + getBatchHotspots: async () => { + hotspotsCalled = true; + return { hotspots: {} }; + }, + }, + }); + let service = new TddService( + { apiKey: 'test-key' }, + '/test', + false, + null, + mockDeps + ); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'completed' }, + screenshots: [ + { + name: 'test', + filename: 'test.png', + original_url: 'http://example.com/1.png', + }, + ], + }; + + await service.processDownloadedBaselines(apiResponse, 'build-1'); + + assert.ok(hotspotsCalled, 'Should call hotspots API when apiKey present'); + }); + + it('skips hotspot download when no apiKey configured', async () => { + let hotspotsCalled = false; + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + fs: { + existsSync: () => false, + writeFileSync: () => {}, + }, + metadata: { + loadBaselineMetadata: () => null, + saveBaselineMetadata: () => {}, + }, + api: { + fetchWithTimeout: async () => ({ + ok: true, + arrayBuffer: async () => new ArrayBuffer(10), + }), + getBatchHotspots: async () => { + hotspotsCalled = true; + return { hotspots: {} }; + }, + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'completed' }, + screenshots: [ + { + name: 'test', + filename: 'test.png', + original_url: 'http://example.com/1.png', + }, + ], + }; + + await service.processDownloadedBaselines(apiResponse, 'build-1'); + + assert.ok(!hotspotsCalled, 'Should NOT call hotspots API without apiKey'); + }); + + it('returns null when all downloads fail', async () => { + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + fs: { + existsSync: () => false, + writeFileSync: () => {}, + }, + metadata: { + loadBaselineMetadata: () => null, + saveBaselineMetadata: () => {}, + }, + api: { + fetchWithTimeout: async () => { + throw new Error('Network failure'); + }, + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'completed' }, + screenshots: [ + { + name: 'test', + filename: 'test.png', + original_url: 'http://example.com/1.png', + }, + ], + }; + + let result = await service.processDownloadedBaselines( + apiResponse, + 'build-1' + ); + + assert.strictEqual(result, null); + }); + + it('returns baseline data on successful download', async () => { + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + fs: { + existsSync: () => false, + writeFileSync: () => {}, + }, + metadata: { + loadBaselineMetadata: () => null, + saveBaselineMetadata: () => {}, + }, + api: { + fetchWithTimeout: async () => ({ + ok: true, + arrayBuffer: async () => new ArrayBuffer(10), + }), + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'completed' }, + screenshots: [ + { + name: 'test', + filename: 'test.png', + original_url: 'http://example.com/1.png', + }, + ], + }; + + let result = await service.processDownloadedBaselines( + apiResponse, + 'build-1' + ); + + assert.ok(result, 'Should return baseline data'); + assert.strictEqual(result.buildId, 'build-1'); + assert.strictEqual(result.buildName, 'Test Build'); + }); + + it('processes multiple screenshots in batches', async () => { + let fetchCalls = 0; + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + fs: { + existsSync: () => false, + writeFileSync: () => {}, + }, + metadata: { + loadBaselineMetadata: () => null, + saveBaselineMetadata: () => {}, + }, + api: { + fetchWithTimeout: async () => { + fetchCalls++; + return { + ok: true, + arrayBuffer: async () => new ArrayBuffer(10), + }; + }, + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + // Create 7 screenshots to test batching (batch size is 5) + let screenshots = Array.from({ length: 7 }, (_, i) => ({ + name: `test-${i}`, + filename: `test-${i}.png`, + original_url: `http://example.com/${i}.png`, + })); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'completed' }, + screenshots, + }; + + await service.processDownloadedBaselines(apiResponse, 'build-1'); + + assert.strictEqual(fetchCalls, 7, 'Should fetch all 7 screenshots'); + + // Check batch processing message + let batchCalls = mockDeps.output.calls.filter( + c => c.method === 'info' && c.args[0].includes('batch') + ); + assert.ok(batchCalls.length >= 2, 'Should process in at least 2 batches'); + }); + + it('saves baseline-metadata.json for MCP plugin', async () => { + let writtenFiles = []; + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + fs: { + existsSync: () => false, + writeFileSync: (path, data) => writtenFiles.push({ path, data }), + }, + metadata: { + loadBaselineMetadata: () => null, + saveBaselineMetadata: () => {}, + }, + api: { + fetchWithTimeout: async () => ({ + ok: true, + arrayBuffer: async () => new ArrayBuffer(10), + }), + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { + id: 'build-1', + name: 'Test Build', + status: 'completed', + commit_sha: 'abc123', + approval_status: 'approved', + }, + screenshots: [ + { + name: 'test', + filename: 'test.png', + original_url: 'http://example.com/1.png', + }, + ], + }; + + await service.processDownloadedBaselines(apiResponse, 'build-1'); + + let metadataFile = writtenFiles.find(f => + f.path.includes('baseline-metadata.json') + ); + assert.ok(metadataFile, 'Should write baseline-metadata.json'); + + let metadata = JSON.parse(metadataFile.data); + assert.strictEqual(metadata.buildId, 'build-1'); + assert.strictEqual(metadata.commitSha, 'abc123'); + }); + + it('logs summary with download counts', async () => { + let mockDeps = createMockDeps({ + baseline: { clearBaselineData: () => {} }, + fs: { + existsSync: () => false, + writeFileSync: () => {}, + }, + metadata: { + loadBaselineMetadata: () => null, + saveBaselineMetadata: () => {}, + }, + api: { + fetchWithTimeout: async () => ({ + ok: true, + arrayBuffer: async () => new ArrayBuffer(10), + }), + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + let apiResponse = { + build: { id: 'build-1', name: 'Test Build', status: 'completed' }, + screenshots: [ + { + name: 'test1', + filename: 'test1.png', + original_url: 'http://example.com/1.png', + }, + { + name: 'test2', + filename: 'test2.png', + original_url: 'http://example.com/2.png', + }, + ], + }; + + await service.processDownloadedBaselines(apiResponse, 'build-1'); + + let summaryCall = mockDeps.output.calls.find( + c => c.method === 'info' && c.args[0].includes('Downloaded') + ); + assert.ok(summaryCall, 'Should log download summary'); + }); + }); + + describe('downloadHotspots', () => { + it('skips download when no API key configured', async () => { + let apiCalled = false; + let mockDeps = createMockDeps({ + api: { + getBatchHotspots: async () => { + apiCalled = true; + return { hotspots: {} }; + }, + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + await service.downloadHotspots([{ name: 'test' }]); + + assert.ok(!apiCalled, 'Should not call API without apiKey'); + }); + + it('fetches hotspots for unique screenshot names', async () => { + let requestedNames = null; + let mockDeps = createMockDeps({ + api: { + getBatchHotspots: async (_client, names) => { + requestedNames = names; + return { hotspots: {} }; + }, + }, + metadata: { saveHotspotMetadata: () => {} }, + }); + let service = new TddService( + { apiKey: 'test-key' }, + '/test', + false, + null, + mockDeps + ); + + await service.downloadHotspots([ + { name: 'test1' }, + { name: 'test1' }, // duplicate + { name: 'test2' }, + ]); + + assert.deepStrictEqual(requestedNames, ['test1', 'test2']); + }); + + it('saves hotspot data to disk and memory', async () => { + let savedData = null; + let mockDeps = createMockDeps({ + api: { + getBatchHotspots: async () => ({ + hotspots: { test: { regions: [{ y1: 0, y2: 100 }] } }, + summary: { total: 1 }, + }), + }, + metadata: { + saveHotspotMetadata: (_workingDir, hotspots, summary) => { + savedData = { hotspots, summary }; + }, + }, + }); + let service = new TddService( + { apiKey: 'test-key' }, + '/test', + false, + null, + mockDeps + ); + + await service.downloadHotspots([{ name: 'test' }]); + + assert.ok(savedData, 'Should save hotspot data'); + assert.ok(service.hotspotData, 'Should store in memory'); + assert.deepStrictEqual(service.hotspotData.test.regions, [ + { y1: 0, y2: 100 }, + ]); + }); + + it('handles API errors gracefully', async () => { + let mockDeps = createMockDeps({ + api: { + getBatchHotspots: async () => { + throw new Error('API unavailable'); + }, + }, + }); + let service = new TddService( + { apiKey: 'test-key' }, + '/test', + false, + null, + mockDeps + ); + + // Should not throw + await service.downloadHotspots([{ name: 'test' }]); + + let warnCall = mockDeps.output.calls.find( + c => + c.method === 'warn' && c.args[0].includes('Could not fetch hotspot') + ); + assert.ok(warnCall, 'Should warn about API failure'); + }); + + it('skips when no screenshots provided', async () => { + let apiCalled = false; + let mockDeps = createMockDeps({ + api: { + getBatchHotspots: async () => { + apiCalled = true; + return { hotspots: {} }; + }, + }, + }); + let service = new TddService( + { apiKey: 'test-key' }, + '/test', + false, + null, + mockDeps + ); + + await service.downloadHotspots([]); + + assert.ok(!apiCalled, 'Should not call API with empty screenshots'); + }); + }); });