Skip to content

TESTING_MAINTENANCE_REPORT.md

Latest commit

 

History

History
434 lines (313 loc) · 12.1 KB

TESTING_MAINTENANCE_REPORT.md

File metadata and controls

434 lines (313 loc) · 12.1 KB
Raw

InfraFoundry Testing & Maintenance Analysis

Report Date: 2025-12-01 Version: 1.0 Codebase Size: 113 source files (15,702 LOC), 36 test files (9,455 LOC) Test Count: 449 tests collected

Executive Summary

This comprehensive analysis evaluates InfraFoundry's testing, testability, error handling, global dependencies, and refactoring opportunities. The codebase demonstrates solid engineering in many areas (custom exceptions, event system, plugin architecture) but has significant gaps in test coverage and opportunities for improved testability.

Key Findings:

  • Test Coverage: ~32% of source files have dedicated tests
  • Critical Gaps: 21 CLI command modules untested, orchestrator workflows only integration tested
  • Complexity Hotspots: 3 files >500 lines with god class patterns
  • Error Handling: Excellent exception hierarchy, but 50+ overly broad catches
  • Global State: Minimal issues; one singleton registry, scattered env var usage
  • Code Duplication: High in provider terraform generation (~150 lines duplicated)

1. Test Coverage Analysis

Overview Statistics

Source Files:     113
Test Files:       36
Coverage Ratio:   31.9%
Test Lines:       9,455
Source Lines:     15,702
Test Count:       449 tests

Test Directory Structure

tests/
├── conftest.py           # Shared fixtures (pytest configuration)
├── unit/                 # 26 test files
│   ├── test_cli.py
│   ├── test_config.py
│   ├── test_orchestrator.py
│   ├── test_provider_proxmox.py
│   ├── test_provider_opnsense.py
│   ├── test_provider_kubernetes.py
│   ├── test_events.py
│   ├── test_state.py
│   ├── test_policy.py
│   ├── test_credential_loader.py
│   ├── test_notifications.py
│   ├── test_secrets.py
│   ├── test_validation.py
│   ├── test_pyinfra_runner.py
│   └── ... (12 more)
└── integration/          # 6 test files
    ├── test_cli_execution.py
    ├── test_orchestrator_workflows.py
    ├── test_end_to_end.py
    └── ... (3 more)

Well-Tested Modules ✓

Module Test File Status
core/config/config_manager.py unit/test_config.py ✓ Comprehensive
core/orchestrator.py unit/test_orchestrator.py ✓ Good coverage
core/events.py unit/test_events.py ✓ Comprehensive
core/state/state_manager.py unit/test_state.py ✓ Good coverage
core/policy/engine.py unit/test_policy.py ✓ Comprehensive
core/credential_loader/credential_loader.py unit/test_credential_loader.py ✓ Good coverage
core/notifications/manager.py unit/test_notifications.py ✓ Good coverage
core/secrets/secret_manager.py unit/test_secrets.py ✓ Comprehensive
core/validation.py unit/test_validation.py ✓ Good coverage
core/runners/pyinfra_runner.py unit/test_pyinfra_runner.py ✓ Good coverage
providers/proxmox/__init__.py unit/test_provider_proxmox.py ✓ Integration tests
providers/opnsense/__init__.py unit/test_provider_opnsense.py ✓ Integration tests
providers/kubernetes/__init__.py unit/test_provider_kubernetes.py ✓ Integration tests

Critical Testing Gaps ✗

Note: Specific tasks to address these gaps have been moved to Project To-Do & Roadmap and are tracked via GitHub Issues.

The analysis identified significant gaps in:

  1. CLI Commands (21 modules)
  2. Orchestration Workflows
  3. Deployment Executor
  4. Runners (Terraform, Ansible, Pulumi)
  5. Validators (Proxmox, OPNsense)

2. Testability Issues

2.1 High-Complexity Functions

Functions with high cyclomatic complexity are harder to test and maintain.

orchestrator_workflows.py

PlanOrchestrator.plan() (lines 309-406)

  • Lines: 98
  • Complexity: Very High
  • Issues:
    • Takes 4 parameters, calls 15+ methods
    • Deeply nested (6+ levels)
    • Mixes file I/O, console output, state management, secret handling
    • Multiple error handling paths
    • Complex conditional branches

ValidationOrchestrator.validate() (lines 122-252)

  • Lines: 130+
  • Complexity: Very High
  • Issues:
    • Multiple nested loops and conditionals
    • Mixes validation logic with console output
    • Direct provider access

deployment_executor.py

apply_serial() (lines 75-138)

  • Lines: 64
  • Issues:
    • Hardcoded provider order: ["opnsense", "proxmox", "kubernetes"]
    • Multiple responsibilities: filtering, iteration, execution
    • Console output mixed with business logic

apply_parallel() (lines 140-224)

  • Lines: 85
  • Issues:
    • ThreadPoolExecutor with complex error handling
    • Mixed concerns: threading, console output, state updates
    • Error aggregation logic embedded

orchestrator.py

Orchestrator.init() (lines 52-169)

  • Lines: 117 (!)
  • Complexity: Extreme
  • Issues:
    • Initializes 15+ attributes
    • Creates 9 sub-orchestrators
    • Sets up notification handlers
    • Registers runners
    • Mixed initialization concerns

Provider Validators

ProxmoxValidator.validate_references() (estimated lines 100-400)

  • Lines: ~300
  • Issues:
    • Validates templates, networks, storage, snippets all in one method
    • Multiple API calls
    • Complex conditionals
    • Hard to test individual validation rules

OPNsenseValidator.validate_references()

  • Same issues as ProxmoxValidator

2.2 Tight Coupling

Orchestrator Dependencies

Issues:

  • Hard to construct for testing
  • Requires mocking 7+ dependencies
  • No interface abstraction

Direct Database Access

Issues:

  • Direct SQLAlchemy engine creation
  • No interface/protocol for mocking
  • Hard to test without real database

Provider Registration Pattern

Issues:

  • Global import-based registration
  • Hard to test provider discovery
  • Side effects in CLI entry point

2.3 Hard-to-Test Patterns

Direct Subprocess Calls (26 occurrences)

Issues:

  • Cannot test without real terraform/ansible/etc installed
  • Cannot test error conditions without triggering real failures
  • Slow tests
  • No subprocess abstraction layer

Direct HTTP Requests (4 files)

Issues:

  • Tests make real HTTP calls or require complex mocking
  • Cannot test network errors without real network issues
  • No retry logic testing

Direct File I/O (11 files with open())

Issues:

  • Tests require real files on disk
  • Cannot test error conditions (permissions, disk full, etc.)
  • Requires test fixtures

Console Output Mixed with Logic (90 occurrences)

Issues:

  • Unit tests are noisy
  • Hard to assert on output
  • Mixed concerns

2.4 Classes with Many Dependencies (God Classes)

Orchestrator Class

File: core/orchestrator.py (562 lines)

Responsibilities:

  1. Provider registry management
  2. Resource loading coordination
  3. Dependency graph management
  4. Policy checking coordination
  5. State management coordination
  6. Event management coordination
  7. Notification setup
  8. Workflow delegation to sub-orchestrators

Issues:

  • Violates Single Responsibility Principle
  • Hard to test (requires 7 constructor dependencies)
  • Hard to maintain (changes affect multiple concerns)

PlanOrchestrator & ApplyOrchestrator

File: core/orchestrator_workflows.py (900+ lines combined)

Issues:

  • 11 dependencies injected (7 as callbacks!)
  • Hard to construct for testing
  • Callbacks typed as Callable[...] - weak typing
  • Mixes console output, state updates, secret handling, validation

ProxmoxValidator & OPNsenseValidator

Files:

  • providers/proxmox/validator.py (608 lines)
  • providers/opnsense/validator.py (455 lines)

Issues:

  • Single validate_references() method is 200-300 lines
  • Does template validation, network validation, storage validation, snippet validation
  • Multiple API clients injected
  • Violates Single Responsibility Principle

3. Error Handling Analysis

3.1 Good Practices ✓

Custom Exception Hierarchy

File: core/exceptions.py (332 lines)

Excellent exception design with specific exceptions for different scenarios and proper inheritance structure.

Centralized CLI Error Handling

File: cli/decorators.py (lines 76-85)

Decorator wraps all CLI commands and converts internal exceptions to Click exceptions.

3.2 Issues Found ✗

Bare Exception Handling (1 occurrence)

File: providers/opnsense/services/isc_dhcp.py (line 43)

Issues:

  • Catches all exceptions including KeyboardInterrupt, SystemExit
  • Silently returns empty data on any error
  • No logging of what went wrong
  • Hides real problems

Priority: P0 - Critical fix

Overly Broad Exception Catching (50+ occurrences)

Issues:

  • Catches more than intended (SystemExit, MemoryError, etc.)
  • Makes debugging harder
  • Hides underlying issues

Missing Error Handling

1. No Timeout Handling in Subprocess Calls (26 occurrences)

Risk:

  • Terraform/Ansible commands can hang indefinitely
  • No way to detect stuck operations
  • CLI appears frozen

Priority: P1 - High

2. File Operations Without Proper Error Handling

Missing:

  • FileNotFoundError handling
  • PermissionError handling
  • YAML parse error handling
  • Schema validation

Priority: P2 - Medium

3. API Client Missing Retry Logic

Missing:

  • Exponential backoff for transient failures
  • Retry logic for 5xx errors
  • Circuit breaker pattern
  • Rate limiting

Priority: P2 - Medium

Error Logging Inconsistency

Issues:

  • Inconsistent across codebase (Logger vs Console vs Traceback printing)
  • Console output not captured in logs
  • Tracebacks printed to console (user-facing)

4. Global Dependencies Analysis

4.1 Module-Level Singletons

Runner Registry

File: core/runner_registry.py (lines 74-118)

Issues:

  • Global mutable state
  • Makes parallel testing difficult (tests can interfere)
  • No way to reset registry between tests
  • Functions at module level wrap instance methods

Impact: Medium

4.2 Environment Variable Dependencies

Scattered throughout codebase

Issues:

  • No centralized environment configuration
  • Hard to test (must set actual env vars)
  • No validation of env var values
  • No documentation of required vs optional

Impact: Medium (testing pain)

4.3 Import-Based Registration

File: cli/main.py (lines 86-121)

Issues:

  • Global side effects on import
  • Plugin discovery happens in CLI entry point
  • Hard to test provider registration independently
  • Order-dependent

Impact: Low (but makes CLI entry point complex)

4.4 Good News ✓

No Shared Mutable State Found:

  • No module-level caches
  • No global configuration dictionaries
  • No shared collections being mutated
  • Most state is in instance attributes
  • Event system uses proper pub/sub pattern

No Database Connection Pooling Issues:

  • SQLAlchemy engine created per StateManager instance
  • No connection leaks found

No Thread-Local Storage Abuse:

  • Threading only in deployment_executor.py
  • Properly scoped to function execution

5. Refactoring Opportunities

Note: Specific refactoring tasks have been moved to Project To-Do & Roadmap and are tracked via GitHub Issues.

5.1 Code Duplication

The analysis identified high duplication in:

  1. Provider Terraform Generation Patterns (~150 lines)
  2. Resource-Specific Terraform Generation (~150 lines)
  3. Console Output Patterns (~90 occurrences)
  4. Validation Patterns (~300 lines)

5.2 God Classes

The analysis identified these classes as candidates for refactoring:

  1. Orchestrator Class (18 attributes, 8+ responsibilities)
  2. PlanOrchestrator & ApplyOrchestrator (11 dependencies, mixed concerns)
  3. ProxmoxValidator & OPNsenseValidator (1,063 lines of validation code)

5.3 Feature Envy

Identified in DeploymentExecutor (using provider internals) and Orchestrators (accessing StateManager internals).

5.4 Long Parameter Lists

Identified in PlanOrchestrator.__init__, OPNsenseClient.request, and various Orchestrator methods.

Back to Table of Contents