Skip to content

♻️ Refactor TddService.downloadBaselines into smaller functional units #162

@Robdel12

Description

@Robdel12

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:

  1. Branch detection and fallback logic (lines 297-308)
  2. Download by buildId with status validation (lines 313-354)
  3. Download by comparisonId with property extraction (lines 355-424)
  4. Download latest passed build lookup (lines 425-464)
  5. SHA-based deduplication and skip logic (lines 482-530)
  6. Batch downloading with progress tracking (lines 550-598)
  7. Metadata assembly and persistence (lines 605-665)
  8. 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

  • tdd-service.js coverage >= 85%
  • All existing tests pass
  • downloadBaselines() reduced to <100 lines
  • New functions have 100% test coverage
  • No behavior changes (refactor only)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions