-
Notifications
You must be signed in to change notification settings - Fork 1
Open
Description
Summary
The TddService.downloadBaselines() method in src/tdd/tdd-service.js is ~400 lines with many branches, making it difficult to test and maintain. Current test coverage for tdd-service.js is 77%, below our 85% target, largely due to this method.
Problem
The method handles too many responsibilities:
- Branch detection and fallback logic (lines 297-308)
- Download by buildId with status validation (lines 313-354)
- Download by comparisonId with property extraction (lines 355-424)
- Download latest passed build lookup (lines 425-464)
- SHA-based deduplication and skip logic (lines 482-530)
- Batch downloading with progress tracking (lines 550-598)
- Metadata assembly and persistence (lines 605-665)
- Hotspot downloading (lines 640-665)
This violates single responsibility and makes unit testing impractical.
Proposed Solution
Extract pure functions following the existing pattern in src/tdd/:
src/tdd/
├── core/
│ ├── signature.js # existing
│ ├── hotspot-coverage.js # existing
│ └── baseline-download.js # NEW - pure download logic
├── services/
│ ├── baseline-manager.js # existing
│ └── download-service.js # NEW - orchestration
Functions to extract:
src/tdd/core/baseline-download.js (pure functions):
// Branch resolution
export function resolveBranch(branch, detectedDefault) { ... }
// Build response validation
export function validateBuildResponse(apiResponse, buildId) { ... }
export function shouldFallbackToLocal(buildStatus) { ... }
// Comparison to screenshot conversion
export function extractScreenshotFromComparison(comparison, signatureProps) { ... }
// SHA-based filtering
export function partitionScreenshotsBysha(screenshots, existingShaMap) { ... }
// Metadata assembly
export function buildBaselineMetadata(build, screenshots, options) { ... }src/tdd/services/download-service.js (I/O orchestration):
export async function downloadScreenshotBatch(screenshots, options) { ... }
export async function fetchAndSaveScreenshot(screenshot, destPath, fetch) { ... }Benefits:
- Pure functions are trivially testable
- Each function has single responsibility
- Reduces
TddServiceto thin orchestration layer - Follows existing codebase patterns (
comparison-service.js,result-service.js) - Should bring coverage above 85%
Files to modify
src/tdd/tdd-service.js- Slim downdownloadBaselines()to use extracted functionssrc/tdd/core/baseline-download.js- NEW: Pure functionssrc/tdd/services/download-service.js- NEW: I/O operationstests/tdd/core/baseline-download.test.js- NEW: Unit teststests/tdd/services/download-service.test.js- NEW: Unit tests
Acceptance criteria
-
tdd-service.jscoverage >= 85% - All existing tests pass
-
downloadBaselines()reduced to <100 lines - New functions have 100% test coverage
- No behavior changes (refactor only)
Metadata
Metadata
Assignees
Labels
No labels