✅ test: Add Phase 1 test coverage for critical paths#7
Merged
SebastienLaurent-CF merged 10 commits intomainfrom Nov 23, 2025
Merged
✅ test: Add Phase 1 test coverage for critical paths#7SebastienLaurent-CF merged 10 commits intomainfrom
SebastienLaurent-CF merged 10 commits intomainfrom
Conversation
Implement comprehensive unit tests for configuration loading and validation, covering happy paths and error cases. Tests validate port ranges, detect conflicts, and ensure proper error handling. Test Coverage (Phase 1): - Config loading and parsing: * Valid configuration with multiple forwards * Invalid ports (out of range, non-numeric) * Port conflicts detection (duplicate local ports) * CUE schema validation errors - Port validation: * Valid range (1-65535) * Edge cases (0, 1, 65535, 65536, 99999, negative) * Non-numeric ports and malformed specifications * Both single ports and local:remote mappings Files Added: - internal/config/config_test.go - Unit tests for config validation - internal/config/testdata/valid.cue - Valid configuration test data - internal/config/testdata/port-conflict.cue - Port conflict test case - internal/config/testdata/invalid-ports.cue - Invalid ports test case - internal/config/testdata/duplicate-names.cue - Duplicate names test case Test Results: - 5 test functions with 13 total test cases - Port validation edge cases: 9 sub-tests - All tests passing (1 skipped for known CUE limitation) Dependencies Added: - github.com/stretchr/testify - Test assertions and utilities Documentation: - Add Testing section to README with commands - Document test status and planned phases - Include coverage report generation instructions Test Commands: ```bash go test ./... # Run all tests go test -v ./internal/config # Verbose config tests go test -cover ./... # Show coverage percentage go test -v -run TestPortValidation ./internal/config # Specific test ``` Planned Next Phases: - Phase 2 (3-5h): Locator implementations, forward names, hot-reload tests - Phase 3 (5-10h): Runner lifecycle, reconnection behavior
Implement comprehensive test coverage for all locator implementations (pod, service, deployment, statefulset, daemonset) with mocked Kubernetes client. Refactor locator interfaces for better testability.
Architecture Changes:
- Create custom KubernetesClient interface hierarchy for cleaner abstraction:
* KubernetesClient (CoreV1() + AppsV1())
* CoreV1Client (Pods() + Services())
* PodClient (Get() + List())
* ServiceClient (Get())
* AppsV1Client (Deployments() + StatefulSets() + DaemonSets())
* ResourceClient (Get() returning interface{})
- Implement MockKubernetesClient supporting all resource types with helper methods
- Add adapters in runner.go to convert real kubernetes.Interface to custom interfaces
- Refactor locator implementations to use custom interfaces instead of kubernetes.Interface
Test Coverage (Phase 2):
- Pod locator (6 tests):
* Pod found and running
* Pod not found error handling
* Pod not running states (pending, failed, succeeded, unknown)
- Service locator (3 tests):
* Service found with running backing pod
* Service not found error handling
* No running pods for service error
- Resource locators (3 tests):
* Deployment, StatefulSet, DaemonSet selector-based discovery
- Locator builder (16 tests):
* Pod format routing (direct name)
* Service formats: svc/, service/, services/
* Deployment formats: dep/, deployment/, deployments/
* StatefulSet formats: sts/, statefulset/, statefulsets/
* DaemonSet formats: ds/, daemonset/, daemonsets/
* Invalid format and too many slashes error handling
Files Added:
- internal/locator/mock_kubernetes.go - Complete mock client implementation (240+ lines)
- internal/locator/locator_test.go - Comprehensive locator tests (350+ lines)
Files Modified:
- internal/locator/locator.go - Define custom interfaces, refactor BuildLocator
- internal/locator/pod.go - Use KubernetesClient interface
- internal/locator/service.go - Use KubernetesClient interface
- internal/locator/selector_based_locator.go - Use KubernetesClient interface with type assertions
- internal/app/runner.go - Add adapter implementations (100+ lines) for real k8s client
- fwkeeper.cue - Example configuration updates
Test Results:
- Total: 38 tests passing
* Phase 1 (config): 7 tests
* Phase 2 (locators): 31 tests (28 locator + 3 builder specific)
- All tests passing, full project builds successfully
Benefits:
- Better interface segregation - clients only depend on methods they use
- Cleaner testing - mock client implements simple interfaces
- Adapter pattern maintains compatibility with real kubernetes.Interface
- Reduced coupling between locator implementations and Kubernetes client API
- Foundation for Phase 3 testing (runner lifecycle, reconnection behavior)
Planned Next Phases:
- Phase 3 (5-10h): Runner lifecycle, reconnection behavior, graceful shutdown
- Phase 4: Integration tests with real Kubernetes connectivity
Implement comprehensive test coverage for runner initialization, lifecycle management, and configuration handling. Tests validate graceful shutdown, context propagation, and internal state management. Test Coverage (Phase 3 - 8 new tests): - Runner lifecycle (4 tests): * Basic startup and initialization * Graceful shutdown without hanging or panics * Context cancellation propagation and handling * Multiple start/stop cycles for clean reuse - Configuration management (4 tests): * Configuration change detection and storage * Empty configuration handling (zero forwarders) * Config path storage and retrieval * Forwarder map initialization and synchronization
…anges Implement comprehensive test coverage for configuration hot-reload, change detection, and dynamic forwarder management. Tests validate configuration state management, mutex protection, and configuration change detection without requiring Kubernetes cluster access. Test Coverage (Phase 4 - 6 new tests): - Configuration change detection (5 sub-tests): * Identical configurations (no change detected) * Namespace changes trigger detection * Resource changes trigger detection * Port additions trigger detection * Port modifications trigger detection - Configuration reload scenarios (5 tests): * Adding new forwarders during reload * Removing existing forwarders * Port configuration changes * Mutex protection during concurrent access * Complex scenarios (add, keep, remove forwarders)
…tate management Implement comprehensive test coverage for configuration reload, forwarder lifecycle management, and file watcher logic. Tests validate state transitions, forwarder cleanup, and path comparison without requiring Kubernetes connectivity or file system integration. Test Coverage (Phase 5 - 8 new tests, 17 sub-tests): - baseName() helper function (6 sub-tests): * Unix absolute paths * Unix relative paths * Windows absolute paths * Filename only * Empty strings * Paths with trailing slashes - Forwarder lifecycle management (2 tests): * stopForwarder() removes entries from maps * stopForwarder() handles non-existent forwarders gracefully - Configuration reload and state management (4 tests): * Config state properly updated during reload * Complex state transitions (add/remove/modify forwarders) * Log configuration preserved across reloads * Multiple sequential reloads work correctly - File watcher path comparison logic (3 sub-tests): * Exact path matches detected * Absolute paths matched correctly * Different files handled correctly
…nfig reload Implement comprehensive test coverage for configuration file loading, file system monitoring, and real-world config reload scenarios. Tests validate config parsing from actual files, error handling for invalid configurations, and file modification detection without requiring live file system watcher integration. Test Coverage (Phase 6 - 8 new tests): - Configuration loading from real files (2 tests): * Single forwarder configuration from file * Multiple forwarders configuration with various port mappings - File modification detection and reload (1 test): * File changes are detected and configuration reloaded correctly - Configuration error handling (2 tests): * Invalid configuration file with bad values * Missing/non-existent configuration file - File path handling and parsing (2 tests): * Config file path parsing (directory extraction from various path formats) * Config path storage in runner with real files - Runner integration with config files (1 test): * Runner initialization with config file
…utdown Implement comprehensive test coverage for graceful shutdown, context cancellation, and signal handling integration. Tests validate shutdown completeness, WaitGroup synchronization, context propagation, and safe multiple shutdown calls without requiring real OS signal handling. Test Coverage (Phase 7 - 10 new tests): - Graceful shutdown behavior (1 test): * Shutdown completes without hanging (timeout protection) - Context cancellation and propagation (2 tests): * Context is cancelled on shutdown * Context integration throughout runner lifecycle - Goroutine and WaitGroup management (3 tests): * Watcher goroutine stops on shutdown * Multiple shutdown calls are safe * WaitGroup synchronization works correctly - Cancel function lifecycle (1 test): * Cancel function exists after start, nil before - Forwarder map persistence (1 test): * Forwarder maps survive shutdown (app manages cleanup) - Logger and logging (2 tests): * Logger accessible during and after shutdown * Shutdown logging doesn't panic
…TERM) Implement comprehensive test coverage for OS signal handling integration. Tests validate signal notification setup, signal channel creation, SIGHUP for configuration reload, SIGTERM for shutdown, and multiple signal handling scenarios without requiring actual OS signal injection. Test Coverage (Phase 8 - 9 new tests): - Signal infrastructure setup (3 tests): * Signal notification can be set up for SIGHUP/SIGTERM * Signal channel creation and cleanup * Signal handling setup and teardown - Signal channel operations (2 tests): * Signal channel buffering for multiple signals * Signal channel creation and send/receive - Runner integration with signals (2 tests): * Signal channel works within runner context * Multiple signal channels can coexist - Signal-specific handlers (2 tests): * SIGHUP configuration reload infrastructure * SIGTERM shutdown infrastructure
…port parsing) Implement 11 comprehensive tests for Forwarder package without Kubernetes dependencies to ensure CI compatibility: **RetryConfig Tests (4 tests)**: - Default exponential backoff configuration validation - Exponential backoff calculation with multiplier and max delay constraints - Max delay enforcement preventing unbounded growth - Jitter option for retry timing variance **Port Parsing & Configuration Tests (7 tests)**: - Port specification parsing (single ports, mapped ports, IPv6 addresses) - PortForwardConfiguration validation with sub-tests: * Single port handling (8080 → local:8080, remote:8080) * Mapped port handling (8080:3000 → local:8080, remote:3000) * Multiple port configurations * Mixed mapped and unmapped ports * Empty port list edge case - Port mapping edge cases (high ports, low ports, port identity mapping) - Forwarder configuration creation and info methods - Custom retry configuration with overrides
Eliminate 9 adapter types that added unnecessary indirection to the codebase. The adapters (kubernetesClientAdapter, coreV1Adapter, podAdapter, serviceAdapter, appsV1Adapter, deploymentAdapter, statefulSetAdapter, daemonSetAdapter) were thin wrappers that simply passed through to kubernetes.Interface without adding value. Changes: - Removed 98 lines of adapter boilerplate from internal/app/runner.go - Simplified locator interfaces to use kubernetes.Interface directly - Updated pod.go, service.go, and selector_based_locator.go type signatures - Refactored locator_test.go to use standard kubernetes/fake.Clientset - Deleted mock_kubernetes.go (243 lines, replaced by fake.Clientset) - Fixed TestRunnerConfigChangeDetection to use fake client for validation Benefits: - 30% reduction in runner.go code - Improved maintainability by removing abstraction layers - Aligns with Kubernetes ecosystem testing standards (fake.Clientset) - Single source of truth: kubernetes.Interface - Zero test regressions: 78/78 tests passing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implement comprehensive unit tests for configuration loading and validation, covering happy paths and error cases. Tests validate port ranges, detect conflicts, and ensure proper error handling.
Test Coverage (Phase 1):
Files Added:
Test Results:
Dependencies Added:
Documentation:
Test Commands:
Planned Next Phases: