bluet / bluet/syspkg

Snap CommandRunner Migration (Issue #20 Part 2)

Open
#28 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
17
Forks
8
PR merge metrics
No merged PRs in 30d

Description

## Summary
Migrate Snap package manager from direct `exec.Command` calls to unified CommandRunner interface for consistent, testable command execution architecture.

**This is Part 2 of Issue #20 - CommandRunner Pattern Implementation**

## Background
Following successful APT CommandRunner migration (Issue #27), implement the same architectural patterns for Snap package manager to achieve full consistency across the codebase.

## Current State
- **Snap**: Uses direct `exec.Command` calls
- **APT**: ✅ Uses CommandRunner (Issue #27 completed)
- **YUM**: ✅ Uses CommandRunner (already implemented)
- **Flatpak**: Uses direct `exec.Command` calls (Issue #20-3)

## Scope
- **Snap package manager migration** from `exec.Command` to CommandRunner
- **Constructor standardization** following APT/YUM patterns
- **Dependency injection** for comprehensive testing capabilities
- **Test coverage expansion** (currently 0% coverage for Snap)

## Technical Implementation

### CommandRunner Integration
- Replace all `exec.Command` calls with CommandRunner interface
- Implement automatic LC_ALL=C handling for consistent English output
- Add built-in interactive mode support via `RunInteractive()`
- Enable dependency injection for testing

### Constructor Pattern
Following APT/YUM standardization:
- `NewPackageManager()` - Production use with default CommandRunner
- `NewPackageManagerWithCustomRunner()` - Testing use with mock CommandRunner

### Testing Infrastructure
- Implement comprehensive mock-based testing
- Add environment variable tracking capabilities
- Create cross-platform test scenarios
- Achieve significant test coverage improvement (currently 0%)

## Benefits

### Architecture Benefits
- ✅ **Consistent interface** - Same CommandRunner pattern across all package managers
- ✅ **Automatic LC_ALL=C** - No manual environment setup needed
- ✅ **Built-in interactive support** - Proper stdin/stdout/stderr handling
- ✅ **Unified testing approach** - Same mocking patterns across codebase

### Testing Benefits
- ✅ **Simple mocking** - Map-based command mocking vs complex setup
- ✅ **Environment testing** - Built-in environment variable verification
- ✅ **Cross-platform testing** - Same tests work across all environments
- ✅ **Comprehensive coverage** - All command execution paths testable

## Acceptance Criteria
- [ ] Snap migrated from direct exec.Command to CommandRunner
- [ ] Constructor naming follows APT/YUM patterns
- [ ] Dependency injection implemented for testing
- [ ] Comprehensive test coverage added (target: >80%)
- [ ] LC_ALL=C automatically handled
- [ ] Interactive mode works consistently
- [ ] All existing functionality preserved
- [ ] Documentation updated

## Priority
**Medium Priority** - Important architectural consistency improvement, but lower priority than core APT/YUM functionality.

## Dependencies
- Requires Issue #27 (APT CommandRunner Migration) completion
- Should follow same patterns established in APT migration

## Related Issues
- Part of Issue #20 - CommandRunner Pattern Implementation
- Preceded by Issue #27 (APT CommandRunner Migration)
- Followed by Issue #20-3 (Flatpak CommandRunner Migration)

## Implementation Notes
- Follow proven patterns from APT migration (Issue #27)
- Leverage existing CommandRunner interface and MockCommandRunner
- Maintain backward compatibility during migration
- Use established testing frameworks and patterns

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.