-
Notifications
You must be signed in to change notification settings - Fork 75
Open
Description
Problem
The Migration.cleanup method in pkg/controller/plan/migration.go is currently difficult to unit test due to tight coupling with multiple dependencies:
r.kubevirt(KubeVirt interface)r.builder(Builder interface)r.provider(Provider interface)r.destination(DestinationClient interface)
This makes it challenging to write effective unit tests that can verify the cleanup logic without complex integration test setup.
Context
This issue was identified during PR #2517 when implementing the DeleteVmOnFailMigration feature. The team decided to defer the refactoring effort to keep the feature implementation focused.
Backlinks:
- PR: MTV-3165 | Add deleteVmOnFailMigration field to plan #2517
- Discussion: MTV-3165 | Add deleteVmOnFailMigration field to plan #2517 (comment)
Tasks
Phase 1: Refactor for testability
- Extract cleanup logic into smaller, focused functions
- Implement dependency injection pattern for the cleanup method
- Consider using interfaces to abstract external dependencies
- Ensure backward compatibility with existing code
Phase 2: Add comprehensive unit tests
- Test VM deletion logic with
DeleteVmOnFailMigrationflag variations - Test cleanup behavior for successful vs failed migrations
- Test error handling and propagation in cleanup operations
- Test volume populator support scenarios
- Test warm snapshot removal logic
- Achieve high test coverage on cleanup logic paths
Acceptance Criteria
- The cleanup method should be easily unit testable with mocked dependencies
- Comprehensive test suite covering all cleanup scenarios and edge cases
- No regressions in existing cleanup behavior
- Maintain clean, readable code structure
Additional Notes
Integration tests already exist in a separate repository, so this work focuses specifically on unit test coverage.
Metadata
Metadata
Assignees
Labels
No labels