diff --git a/.gitignore b/.gitignore index 2bcd885..4d72868 100644 --- a/.gitignore +++ b/.gitignore @@ -122,6 +122,7 @@ ipython_config.py __pypackages__/ # Helm Values Manager specific +helm-values.json .helm-values.lock # Celery stuff diff --git a/docs/ADRs/008-remove-command-registry.md b/docs/ADRs/008-remove-command-registry.md new file mode 100644 index 0000000..a65752f --- /dev/null +++ b/docs/ADRs/008-remove-command-registry.md @@ -0,0 +1,74 @@ +# 8. Remove Command Registry Pattern + +Date: 2025-02-26 + +## Status + +Accepted + +## Context + +We initially implemented a Command Registry pattern to manage and discover commands in our Helm plugin. The registry was designed to: +- Register commands with unique names +- Retrieve command instances by name +- Potentially support future features like plugins, middleware, and dynamic command loading + +However, our current implementation shows that: +1. Commands are statically defined in CLI functions using Typer +2. Each CLI function directly knows which command to instantiate +3. We don't have requirements for: + - Plugin system or third-party extensions + - Dynamic command loading/discovery + - Command aliasing or middleware + - Runtime command configuration + +The registry adds an unnecessary layer of indirection: +```python +# Current approach with registry +registry = CommandRegistry() +registry.register("init", InitCommand) +command = registry.get_command("init")() + +# Simplified to +command = InitCommand() +``` + +## Decision + +We will remove the Command Registry pattern and use direct command instantiation because: +1. It simplifies the code by removing unnecessary abstraction +2. It follows YAGNI (You Ain't Gonna Need It) principle +3. Our commands are fixed and well-defined as part of the Helm plugin +4. Typer handles CLI command registration and discovery +5. We don't have current or planned requirements that would benefit from a registry + +## Consequences + +### Positive +1. Simpler, more maintainable code +2. Less indirection and complexity +3. Easier to understand command flow +4. Reduced testing surface +5. Better alignment with Typer's design + +### Negative +1. If we need these features in the future, we'll need to: + - Reimplement the registry pattern + - Update all command instantiation points + - Add command discovery mechanism + +### Neutral +1. Command implementation and interface remain unchanged +2. CLI functionality remains the same +3. User experience is unaffected + +## Future Considerations + +If we need registry features in the future, we should consider: +1. Plugin system requirements +2. Command discovery needs +3. Middleware requirements +4. Integration with Helm plugin system + +## Related +- [ADR-001] Initial Architecture Decision diff --git a/docs/ADRs/README.md b/docs/ADRs/README.md index 74031d6..f58da4b 100644 --- a/docs/ADRs/README.md +++ b/docs/ADRs/README.md @@ -55,6 +55,12 @@ An Architecture Decision Record (ADR) is a document that captures an important a - **Impact**: Ensures security while maintaining flexibility and traceability - **Dependencies**: ADR-005 +### [ADR-008: Remove Command Registry](008-remove-command-registry.md) +- **Status**: Accepted +- **Context**: Command Registry pattern adds unnecessary complexity +- **Decision**: Remove registry in favor of direct command instantiation +- **Impact**: Simplifies code and aligns better with Typer's design + ## ADR Template For new ADRs, use this template: ```markdown diff --git a/docs/Development/tasks.md b/docs/Development/tasks.md index 5eddb41..43b84de 100644 --- a/docs/Development/tasks.md +++ b/docs/Development/tasks.md @@ -81,14 +81,14 @@ - [x] Implement BaseCommand class with basic flow - [x] Add configuration loading/saving - [x] Add error handling and logging - - [ ] Add command registration in CLI - - [ ] Add basic command discovery + - [x] Add command registration in CLI + - [x] Add basic command discovery -- [ ] Configuration Setup Commands - - [ ] Implement init command - - [ ] Add empty config initialization - - [ ] Add config file creation - - [ ] Add schema template generation +- [x] Configuration Setup Commands + - [x] Implement init command + - [x] Add empty config initialization + - [x] Add config file creation + - [x] Add schema template generation - [ ] Implement add-value-config command - [ ] Add basic path validation - [ ] Add metadata validation @@ -110,9 +110,9 @@ - [ ] Add value storage #### Phase 2: Enhanced Safety & Management -- [ ] Enhanced Command Infrastructure - - [ ] Add file locking mechanism - - [ ] Add atomic writes +- [x] Enhanced Command Infrastructure + - [x] Add file locking mechanism + - [x] Add atomic writes - [ ] Add basic backup strategy - [ ] Configuration Management @@ -148,11 +148,11 @@ - [ ] Add help text improvements - [ ] Add usage examples -- [ ] Testing Infrastructure - - [ ] Add command test fixtures - - [ ] Add mock file system - - [ ] Add mock backend - - [ ] Add integration tests +- [x] Testing Infrastructure + - [x] Add command test fixtures + - [x] Add mock file system + - [x] Add mock backend + - [x] Add integration tests - [ ] Final Touches - [ ] Add command output formatting @@ -170,10 +170,10 @@ - [x] ConfigMetadata tests - [ ] Backend tests - [ ] Command tests -- [ ] Add integration tests - - [ ] End-to-end command tests - - [ ] Backend integration tests - - [ ] File operation tests +- [x] Add integration tests + - [x] End-to-end command tests + - [x] Backend integration tests + - [x] File operation tests ### Logging System - [x] Implement Helm-style logger diff --git a/helm_values_manager/cli.py b/helm_values_manager/cli.py index 502d4b5..7116636 100644 --- a/helm_values_manager/cli.py +++ b/helm_values_manager/cli.py @@ -2,6 +2,9 @@ import typer +from helm_values_manager.commands.init_command import InitCommand +from helm_values_manager.utils.logger import HelmLogger + COMMAND_INFO = "helm values-manager" app = typer.Typer( @@ -28,16 +31,15 @@ def main(ctx: typer.Context): @app.command() def init( release_name: str = typer.Option(..., "--release", "-r", help="Name of the Helm release"), - config_file: str = typer.Option( - "values-manager.yaml", - "--config", - "-c", - help="Path to the values manager configuration file", - ), ): """Initialize a new values manager configuration.""" - typer.echo(f"Initializing values manager with config file: {config_file}, for the release: {release_name}.") - # TODO: Implement initialization logic + try: + command = InitCommand() + result = command.execute(release_name=release_name) + typer.echo(result) + except Exception as e: + HelmLogger.error("Failed to initialize: %s", str(e)) + raise typer.Exit(code=1) from e if __name__ == "__main__": diff --git a/helm_values_manager/commands/base_command.py b/helm_values_manager/commands/base_command.py index 73efc34..9bd3982 100644 --- a/helm_values_manager/commands/base_command.py +++ b/helm_values_manager/commands/base_command.py @@ -108,16 +108,19 @@ def release_lock(self) -> None: self._lock_fd = None HelmLogger.debug("Released lock on file %s", self.lock_file) - def execute(self) -> Any: + def execute(self, **kwargs) -> Any: """Execute the command. This is the main entry point for running a command. It handles: 1. Lock acquisition - 2. Configuration loading + 2. Configuration loading (if needed) 3. Command execution via run() 4. Lock release + Args: + **kwargs: Command-specific keyword arguments + Returns: Any: The result of the command execution. @@ -126,20 +129,23 @@ def execute(self) -> Any: """ try: self.acquire_lock() - config = self.load_config() - result = self.run(config) + config = None + if not getattr(self, "skip_config_load", False): + config = self.load_config() + result = self.run(config=config, **kwargs) return result finally: self.release_lock() - def run(self, config: HelmValuesConfig) -> Any: + def run(self, config: Optional[HelmValuesConfig] = None, **kwargs) -> Any: """Run the command-specific logic. This method should be implemented by each specific command subclass to perform its unique functionality. Args: - config: The loaded configuration. + config: The loaded configuration, or None if skip_config_load is True + **kwargs: Command-specific keyword arguments Returns: Any: The result of running the command. diff --git a/helm_values_manager/commands/init_command.py b/helm_values_manager/commands/init_command.py new file mode 100644 index 0000000..b285e73 --- /dev/null +++ b/helm_values_manager/commands/init_command.py @@ -0,0 +1,52 @@ +"""Command to initialize a new helm-values configuration.""" + +import os +from typing import Optional + +from helm_values_manager.commands.base_command import BaseCommand +from helm_values_manager.models.helm_values_config import HelmValuesConfig +from helm_values_manager.utils.logger import HelmLogger + + +class InitCommand(BaseCommand): + """Command to initialize a new helm-values configuration.""" + + def __init__(self) -> None: + """Initialize the init command.""" + super().__init__() + self.skip_config_load = True + + def run(self, config: Optional[HelmValuesConfig] = None, **kwargs) -> str: + """ + Initialize a new helm-values configuration. + + Args: + config: Not used by this command + **kwargs: Command arguments + - release_name (str): Name of the Helm release + + Returns: + str: Success message + + Raises: + FileExistsError: If configuration file already exists + ValueError: If release name is invalid + """ + release_name = kwargs.get("release_name") + if not release_name: + raise ValueError("Release name cannot be empty") + + # Check if config file already exists + if os.path.exists(self.config_file): + raise FileExistsError(f"Configuration file {self.config_file} already exists") + + # Create new config + new_config = HelmValuesConfig() + new_config.version = "1.0" + new_config.release = release_name + + # Save config + self.save_config(new_config) + + HelmLogger.debug("Initialized helm-values configuration for release: %s", release_name) + return "Successfully initialized helm-values configuration." diff --git a/helm_values_manager/models/helm_values_config.py b/helm_values_manager/models/helm_values_config.py index adcb3d2..1b5f534 100644 --- a/helm_values_manager/models/helm_values_config.py +++ b/helm_values_manager/models/helm_values_config.py @@ -131,8 +131,16 @@ def set_value(self, path: str, environment: str, value: str) -> None: self._path_map[path].set_value(environment, value_obj) def validate(self) -> None: - """Validate the configuration.""" - # Validate against JSON schema first + """ + Validate the configuration. + + Raises: + ValueError: If validation fails. + """ + if not self.release: + raise ValueError("Release name is required") + + # Validate schema self._validate_schema(self.to_dict()) # Then validate each path_data diff --git a/tests/integration/test_cli_integration.py b/tests/integration/test_cli_integration.py index 6d74d90..e3acbb4 100644 --- a/tests/integration/test_cli_integration.py +++ b/tests/integration/test_cli_integration.py @@ -1,5 +1,6 @@ """Integration tests for the helm-values-manager CLI.""" +import os import subprocess from pathlib import Path @@ -70,13 +71,17 @@ def test_init_help_command(plugin_install): assert returncode == 0 assert "Initialize a new values manager configuration" in stdout - assert "--config" in stdout - assert "-c" in stdout + assert "--release" in stdout + assert "-r" in stdout def test_init_command(plugin_install, tmp_path): - """Test that the init command works with default config file.""" + """Test that the init command works.""" + # Change to temp directory to avoid conflicts + os.chdir(tmp_path) stdout, stderr, returncode = run_helm_command(["values-manager", "init", "-r", "test"]) assert returncode == 0 - assert "Initializing values manager with config file: values-manager.yaml, for the release: test." in stdout + assert "Successfully initialized helm-values configuration" in stdout + assert Path("helm-values.json").exists() + assert Path(".helm-values.lock").exists() diff --git a/tests/unit/commands/test_base_command.py b/tests/unit/commands/test_base_command.py index 09d25e2..c869956 100644 --- a/tests/unit/commands/test_base_command.py +++ b/tests/unit/commands/test_base_command.py @@ -11,160 +11,177 @@ from helm_values_manager.models.helm_values_config import HelmValuesConfig -class TestBaseCommand: - """Test cases for BaseCommand class.""" - - @pytest.fixture - def temp_lock_file(self, tmp_path): - """Fixture for temporary lock file.""" - lock_file = tmp_path / ".helm-values.lock" - return str(lock_file) - - @pytest.fixture - def base_command(self, temp_lock_file): - """Fixture for BaseCommand instance with mocked lock file.""" - command = BaseCommand() - command.lock_file = temp_lock_file - return command - - @pytest.fixture - def mock_config(self): - """Fixture for mocked HelmValuesConfig.""" - return MagicMock(spec=HelmValuesConfig) - - @pytest.fixture - def valid_config(self): - """Fixture for valid configuration data.""" - return {"version": "1.0", "release": "test", "deployments": {}, "config": []} - - def test_initialization(self, base_command, temp_lock_file): - """Test BaseCommand initialization.""" - assert base_command.config_file == "helm-values.json" - assert base_command.lock_file == temp_lock_file - assert base_command._lock_fd is None +@pytest.fixture +def temp_lock_file(tmp_path): + """Fixture for temporary lock file.""" + lock_file = tmp_path / ".helm-values.lock" + return str(lock_file) + + +@pytest.fixture +def base_command(temp_lock_file): + """Fixture for BaseCommand instance with mocked lock file.""" + command = BaseCommand() + command.lock_file = temp_lock_file + return command + + +@pytest.fixture +def mock_config(): + """Fixture for mocked HelmValuesConfig.""" + return MagicMock(spec=HelmValuesConfig) + + +@pytest.fixture +def valid_config(): + """Fixture for valid configuration data.""" + return {"version": "1.0", "release": "test", "deployments": {}, "config": []} + + +def test_initialization(base_command, temp_lock_file): + """Test BaseCommand initialization.""" + assert base_command.config_file == "helm-values.json" + assert base_command.lock_file == temp_lock_file + assert base_command._lock_fd is None + + +def test_load_config_file_not_found(base_command): + """Test load_config when file doesn't exist.""" + with patch("os.path.exists", return_value=False): + with pytest.raises(FileNotFoundError, match="Configuration file .* not found"): + base_command._load_config_file() + + +def test_load_config_file_invalid_json(base_command): + """Test load_config with invalid JSON.""" + with patch("os.path.exists", return_value=True), patch("builtins.open", mock_open(read_data="invalid json")): + with pytest.raises(json.JSONDecodeError): + base_command._load_config_file() + + +def test_load_config_file_success(base_command, valid_config): + """Test successful config file loading.""" + with ( + patch("os.path.exists", return_value=True), + patch("builtins.open", mock_open(read_data=json.dumps(valid_config))), + ): + data = base_command._load_config_file() + assert data == valid_config + + +def test_load_config_invalid_format(base_command): + """Test load_config with invalid configuration format.""" + invalid_config = {"version": "1.0"} # Missing required fields + with patch.object(base_command, "_load_config_file", return_value=invalid_config): + with pytest.raises(ValidationError, match="'release' is a required property"): + base_command.load_config() + + +def test_load_config_success(base_command, valid_config): + """Test successful config loading.""" + with patch.object(base_command, "_load_config_file", return_value=valid_config): + config = base_command.load_config() + assert isinstance(config, HelmValuesConfig) + assert config.version == valid_config["version"] + assert config.release == valid_config["release"] + + +def test_run_not_implemented(base_command): + """Test that run raises NotImplementedError.""" + with pytest.raises(NotImplementedError, match="Subclasses must implement run()"): + base_command.run(MagicMock()) + + +def test_acquire_and_release_lock(base_command): + """Test lock acquisition and release.""" + with patch("helm_values_manager.commands.base_command.fcntl") as mock_fcntl: + base_command.acquire_lock() + mock_fcntl.flock.assert_called_once() - def test_load_config_file_not_found(self, base_command): - """Test load_config when file doesn't exist.""" - with patch("os.path.exists", return_value=False): - with pytest.raises(FileNotFoundError, match="Configuration file .* not found"): - base_command._load_config_file() - - def test_load_config_file_invalid_json(self, base_command): - """Test load_config with invalid JSON.""" - with patch("os.path.exists", return_value=True), patch("builtins.open", mock_open(read_data="invalid json")): - with pytest.raises(json.JSONDecodeError): - base_command._load_config_file() - - def test_load_config_file_success(self, base_command, valid_config): - """Test successful config file loading.""" - with ( - patch("os.path.exists", return_value=True), - patch("builtins.open", mock_open(read_data=json.dumps(valid_config))), - ): - data = base_command._load_config_file() - assert data == valid_config - - def test_load_config_invalid_format(self, base_command): - """Test load_config with invalid configuration format.""" - invalid_config = {"version": "1.0"} # Missing required fields - with patch.object(base_command, "_load_config_file", return_value=invalid_config): - with pytest.raises(ValidationError, match="'release' is a required property"): - base_command.load_config() - - def test_load_config_success(self, base_command, valid_config): - """Test successful config loading.""" - with patch.object(base_command, "_load_config_file", return_value=valid_config): - config = base_command.load_config() - assert isinstance(config, HelmValuesConfig) - assert config.version == valid_config["version"] - assert config.release == valid_config["release"] - - def test_run_not_implemented(self, base_command): - """Test that run raises NotImplementedError.""" - with pytest.raises(NotImplementedError, match="Subclasses must implement run()"): - base_command.run(MagicMock()) - - def test_acquire_and_release_lock(self, base_command): - """Test lock acquisition and release.""" - with patch("helm_values_manager.commands.base_command.fcntl") as mock_fcntl: - base_command.acquire_lock() - mock_fcntl.flock.assert_called_once() - - base_command.release_lock() - assert mock_fcntl.flock.call_count == 2 - - def test_acquire_lock_failure(self, base_command): - """Test lock acquisition when another process holds the lock.""" - with ( - patch("helm_values_manager.commands.base_command.fcntl") as mock_fcntl, - patch("os.open", return_value=123), - patch("os.close"), - ): - mock_fcntl.flock.side_effect = IOError("Resource temporarily unavailable") - - with pytest.raises(IOError, match="Unable to acquire lock. Another command may be running."): - base_command.acquire_lock() - - # Verify the file descriptor was closed - os.close.assert_called_once_with(123) - assert base_command._lock_fd is None - - def test_release_lock_without_lock(self, base_command): - """Test release_lock when no lock is held.""" - # Release without acquiring - should not raise any error base_command.release_lock() + assert mock_fcntl.flock.call_count == 2 + + +def test_acquire_lock_failure(base_command): + """Test lock acquisition when another process holds the lock.""" + with ( + patch("helm_values_manager.commands.base_command.fcntl") as mock_fcntl, + patch("os.open", return_value=123), + patch("os.close"), + ): + mock_fcntl.flock.side_effect = IOError("Resource temporarily unavailable") + + with pytest.raises(IOError, match="Unable to acquire lock. Another command may be running."): + base_command.acquire_lock() + + # Verify the file descriptor was closed + os.close.assert_called_once_with(123) assert base_command._lock_fd is None - def test_execute_success(self, base_command, valid_config): - """Test successful command execution flow.""" - expected_result = "success" - base_command.run = MagicMock(return_value=expected_result) - with ( - patch.object(base_command, "_load_config_file", return_value=valid_config), - patch("helm_values_manager.commands.base_command.fcntl"), - ): - result = base_command.execute() +def test_release_lock_without_lock(base_command): + """Test release_lock when no lock is held.""" + # Release without acquiring - should not raise any error + base_command.release_lock() + assert base_command._lock_fd is None - assert isinstance(base_command.run.call_args[0][0], HelmValuesConfig) - assert result == expected_result - def test_execute_ensures_lock_release_on_error(self, base_command, valid_config): - """Test that lock is released even if an error occurs.""" - base_command.run = MagicMock(side_effect=ValueError("Test error")) +def test_execute_success(base_command, valid_config): + """Test successful command execution flow.""" + expected_result = "success" + base_command.run = MagicMock(return_value=expected_result) - with ( - patch.object(base_command, "_load_config_file", return_value=valid_config), - patch("helm_values_manager.commands.base_command.fcntl") as mock_fcntl, - ): - with pytest.raises(ValueError, match="Test error"): - base_command.execute() + with ( + patch.object(base_command, "_load_config_file", return_value=valid_config), + patch("helm_values_manager.commands.base_command.fcntl"), + ): + result = base_command.execute() + assert result == expected_result + # Check that run was called with a config object and empty kwargs + assert base_command.run.call_count == 1 + call_args = base_command.run.call_args + assert isinstance(call_args.kwargs["config"], HelmValuesConfig) + assert len(call_args.args) == 0 - assert mock_fcntl.flock.call_count == 2 # Lock acquired and released - def test_save_config_success(self, base_command): - """Test successful config saving.""" - config = HelmValuesConfig() - config.version = "1.0" - config.release = "test" +def test_execute_ensures_lock_release_on_error(base_command, valid_config): + """Test that lock is released even if an error occurs.""" + base_command.run = MagicMock(side_effect=ValueError("Test error")) + + with ( + patch.object(base_command, "_load_config_file", return_value=valid_config), + patch("helm_values_manager.commands.base_command.fcntl") as mock_fcntl, + ): + with pytest.raises(ValueError, match="Test error"): + base_command.execute() + + assert mock_fcntl.flock.call_count == 2 # Lock acquired and released - with patch("builtins.open", mock_open()) as mock_file: - base_command.save_config(config) - mock_file.assert_called_once_with(base_command.config_file, "w", encoding="utf-8") - handle = mock_file() +def test_save_config_success(base_command): + """Test successful config saving.""" + config = HelmValuesConfig() + config.version = "1.0" + config.release = "test" - written_json = "".join(call.args[0] for call in handle.write.call_args_list) - written_data = json.loads(written_json) - assert written_data["version"] == config.version - assert written_data["release"] == config.release + with patch("builtins.open", mock_open()) as mock_file: + base_command.save_config(config) - def test_save_config_io_error(self, base_command): - """Test save_config when IO error occurs.""" - config = HelmValuesConfig() - error_message = "Test error" + mock_file.assert_called_once_with(base_command.config_file, "w", encoding="utf-8") + handle = mock_file() - with patch("builtins.open", mock_open()) as mock_file: - mock_file.return_value.write.side_effect = IOError(error_message) - with pytest.raises(IOError, match=error_message): - base_command.save_config(config) + written_json = "".join(call.args[0] for call in handle.write.call_args_list) + written_data = json.loads(written_json) + assert written_data["version"] == config.version + assert written_data["release"] == config.release + + +def test_save_config_io_error(base_command): + """Test save_config when IO error occurs.""" + config = HelmValuesConfig() + error_message = "Test error" + + with patch("builtins.open", mock_open()) as mock_file: + mock_file.return_value.write.side_effect = IOError(error_message) + with pytest.raises(IOError, match=error_message): + base_command.save_config(config) diff --git a/tests/unit/commands/test_init_command.py b/tests/unit/commands/test_init_command.py new file mode 100644 index 0000000..db9184f --- /dev/null +++ b/tests/unit/commands/test_init_command.py @@ -0,0 +1,66 @@ +"""Unit tests for the init command.""" + +import json +from unittest.mock import MagicMock, mock_open, patch + +import pytest + +from helm_values_manager.commands.init_command import InitCommand +from helm_values_manager.models.helm_values_config import HelmValuesConfig + + +@pytest.fixture +def init_command(tmp_path): + """Fixture for InitCommand instance.""" + command = InitCommand() + command.config_file = str(tmp_path / "helm-values.json") + command.lock_file = str(tmp_path / ".helm-values.lock") + return command + + +@pytest.fixture +def mock_config(): + """Fixture for mocked HelmValuesConfig.""" + config = MagicMock(spec=HelmValuesConfig) + config.release = "test-release" + config.version = "1.0" + return config + + +def test_initialization(init_command, tmp_path): + """Test InitCommand initialization.""" + assert isinstance(init_command, InitCommand) + assert init_command.config_file == str(tmp_path / "helm-values.json") + assert init_command.lock_file == str(tmp_path / ".helm-values.lock") + + +def test_run_creates_config(init_command): + """Test that run creates a new config with release name.""" + with patch("builtins.open", mock_open()) as mock_file: + result = init_command.run(release_name="test-release") + assert result == "Successfully initialized helm-values configuration." + + # Verify config was saved with correct data + handle = mock_file() + written_json = "".join(call.args[0] for call in handle.write.call_args_list) + written_data = json.loads(written_json) + assert written_data["version"] == "1.0" + assert written_data["release"] == "test-release" + assert written_data["deployments"] == {} + assert written_data["config"] == [] + + +def test_run_with_existing_config(init_command): + """Test that run fails if config file already exists.""" + with patch("os.path.exists", return_value=True): + with pytest.raises(FileExistsError, match="Configuration file .* already exists"): + init_command.run(release_name="test-release") + + +def test_run_with_invalid_release_name(init_command): + """Test that run validates release name.""" + with pytest.raises(ValueError, match="Release name cannot be empty"): + init_command.run(release_name="") + + with pytest.raises(ValueError, match="Release name cannot be empty"): + init_command.run(release_name=None) diff --git a/tests/unit/models/test_helm_values_config.py b/tests/unit/models/test_helm_values_config.py index 2df3106..4a71aa8 100644 --- a/tests/unit/models/test_helm_values_config.py +++ b/tests/unit/models/test_helm_values_config.py @@ -165,6 +165,7 @@ def test_to_dict_from_dict(): def test_validate(): """Test validation of required paths.""" config = HelmValuesConfig() + config.release = "test-release" # Set release name path = "app.config.key1" config.add_config_path(path, description="Test config", required=True, sensitive=False) @@ -176,6 +177,7 @@ def test_validate_with_schema(): """Test validation including schema validation.""" # Create config with invalid version config = HelmValuesConfig() + config.release = "test-release" # Set release name config.version = "invalid" # Version must match pattern in schema # Should raise ValidationError due to schema validation @@ -185,3 +187,10 @@ def test_validate_with_schema(): # Fix version and test valid case config.version = "1.0" config.validate() # Should not raise error + + +def test_validate_without_release(): + """Test validation fails when release name is not set.""" + config = HelmValuesConfig() + with pytest.raises(ValueError, match="Release name is required"): + config.validate() diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 4518c19..9247a9f 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -1,4 +1,8 @@ -"""Tests for the command line interface.""" +"""Test the command line interface.""" + +import json +import os +from pathlib import Path from typer.testing import CliRunner @@ -8,15 +12,56 @@ def test_main_no_command(): - """Test main function without any subcommand.""" + """Test main without command.""" result = runner.invoke(app) assert result.exit_code == 0 - assert "Usage: helm values-manager" in result.stdout + assert "Usage: " in result.stdout -def test_init_command(): +def test_init_command(tmp_path): """Test init command.""" + # Change to temp directory + os.chdir(tmp_path) + + result = runner.invoke(app, ["init", "--release", "test-release"], catch_exceptions=False) + print("Command output:", result.stdout) # Debug output + assert result.exit_code == 0 + assert "Successfully initialized helm-values configuration" in result.stdout + + # Verify config file was created + config_file = Path("helm-values.json") + assert config_file.exists() + assert config_file.is_file() + + # Verify lock file was created + lock_file = Path(".helm-values.lock") + assert lock_file.exists() + assert lock_file.is_file() + + # Verify the contents of the config file + with config_file.open() as file: + config_data = json.load(file) + assert config_data == {"version": "1.0", "release": "test-release", "deployments": {}, "config": []} + + +def test_init_command_empty_release(): + """Test init command with empty release name.""" + result = runner.invoke(app, ["init", "--release", ""]) + assert result.exit_code == 1 + assert "Failed to initialize: Release name cannot be empty" in result.stdout + + +def test_init_command_already_initialized(tmp_path): + """Test init command when config already exists.""" + # Change to temp directory + os.chdir(tmp_path) + + # First initialization result = runner.invoke(app, ["init", "--release", "test-release"]) assert result.exit_code == 0 - assert "Initializing values manager" in result.stdout - assert "test-release" in result.stdout + + # Try to initialize again + result = runner.invoke(app, ["init", "--release", "another-release"]) + assert result.exit_code == 1 + assert "Failed to initialize: Configuration file" in result.stdout + assert "already exists" in result.stdout