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
TddService to 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 down downloadBaselines() to use extracted functions
src/tdd/core/baseline-download.js - NEW: Pure functions
src/tdd/services/download-service.js - NEW: I/O operations
tests/tdd/core/baseline-download.test.js - NEW: Unit tests
tests/tdd/services/download-service.test.js - NEW: Unit tests
Acceptance criteria
Summary
The
TddService.downloadBaselines()method insrc/tdd/tdd-service.jsis ~400 lines with many branches, making it difficult to test and maintain. Current test coverage fortdd-service.jsis 77%, below our 85% target, largely due to this method.Problem
The method handles too many responsibilities:
This violates single responsibility and makes unit testing impractical.
Proposed Solution
Extract pure functions following the existing pattern in
src/tdd/:Functions to extract:
src/tdd/core/baseline-download.js(pure functions):src/tdd/services/download-service.js(I/O orchestration):Benefits:
TddServiceto thin orchestration layercomparison-service.js,result-service.js)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 testsAcceptance criteria
tdd-service.jscoverage >= 85%downloadBaselines()reduced to <100 lines