From d0704125e9648c6f5a432359f1e57599e7288271 Mon Sep 17 00:00:00 2001 From: Kenneth Belitzky Date: Sat, 27 Jun 2026 15:05:35 -0300 Subject: [PATCH] fix: report validate config errors cleanly --- structkit/commands/validate.py | 81 ++++++++++++++++++++++------------ tests/test_commands_more.py | 57 ++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 28 deletions(-) diff --git a/structkit/commands/validate.py b/structkit/commands/validate.py index 64668e8..8370828 100644 --- a/structkit/commands/validate.py +++ b/structkit/commands/validate.py @@ -2,6 +2,11 @@ from dotenv import load_dotenv from structkit.commands import Command + +class ValidationConfigError(ValueError): + """Expected structure validation error that should be shown without a traceback.""" + + load_dotenv() @@ -16,17 +21,37 @@ def __init__(self, parser): def execute(self, args): self.logger.info(f"Validating {args.yaml_file}") - with open(args.yaml_file, 'r') as f: - config = yaml.safe_load(f) + try: + with open(args.yaml_file, 'r') as f: + config = yaml.safe_load(f) + except FileNotFoundError: + self.logger.error(f"❗ File not found: {args.yaml_file}") + raise SystemExit(1) from None + except yaml.YAMLError as exc: + self.logger.error(f"❗ Invalid YAML in {args.yaml_file}: {exc}") + raise SystemExit(1) from None + except OSError as exc: + self.logger.error(f"❗ Failed to read {args.yaml_file}: {exc}") + raise SystemExit(1) from None + + try: + if not isinstance(config, dict): + raise ValidationConfigError("Top-level YAML content must be a mapping.") + + self._validate_config(config) + except ValidationConfigError as exc: + self.logger.error(f"❗ Invalid structure config: {exc}") + raise SystemExit(1) from None + def _validate_config(self, config): # Validate pre_hooks and post_hooks if present for hook_key in ["pre_hooks", "post_hooks"]: if hook_key in config: if not isinstance(config[hook_key], list): - raise ValueError(f"The '{hook_key}' key must be a list of shell commands (strings).") + raise ValidationConfigError(f"The '{hook_key}' key must be a list of shell commands (strings).") for cmd in config[hook_key]: if not isinstance(cmd, str): - raise ValueError(f"Each item in '{hook_key}' must be a string (shell command).") + raise ValidationConfigError(f"Each item in '{hook_key}' must be a string (shell command).") if 'structure' in config and 'files' in config: self.logger.warning("Both 'structure' and 'files' keys exist. Prioritizing 'structure'.") @@ -48,21 +73,21 @@ def execute(self, args): # module_name: my_module_two def _validate_folders_config(self, folders): if not isinstance(folders, list): - raise ValueError("The 'folders' key must be a list.") + raise ValidationConfigError("The 'folders' key must be a list.") for item in folders: if not isinstance(item, dict): - raise ValueError("Each item in the 'folders' list must be a dictionary.") + raise ValidationConfigError("Each item in the 'folders' list must be a dictionary.") for name, content in item.items(): if not isinstance(name, str): - raise ValueError("Each name in the 'folders' item must be a string.") + raise ValidationConfigError("Each name in the 'folders' item must be a string.") if not isinstance(content, dict): - raise ValueError(f"The content of '{name}' must be a dictionary.") + raise ValidationConfigError(f"The content of '{name}' must be a dictionary.") if 'struct' not in content: - raise ValueError(f"Dictionary item '{name}' must contain a 'struct' key.") + raise ValidationConfigError(f"Dictionary item '{name}' must contain a 'struct' key.") if not isinstance(content['struct'], str) and not isinstance(content['struct'], list): - raise ValueError(f"The 'struct' value for '{name}' must be a string.") + raise ValidationConfigError(f"The 'struct' value for '{name}' must be a string.") if 'with' in content and not isinstance(content['with'], dict): - raise ValueError(f"The 'with' value for '{name}' must be a dictionary.") + raise ValidationConfigError(f"The 'with' value for '{name}' must be a dictionary.") # Validate the 'variables' key in the configuration file # variables should be defined as a list of dictionaries @@ -79,48 +104,48 @@ def _validate_folders_config(self, folders): # help: The name of the project def _validate_variables_config(self, variables): if not isinstance(variables, list): - raise ValueError("The 'variables' key must be a list.") + raise ValidationConfigError("The 'variables' key must be a list.") for item in variables: if not isinstance(item, dict): - raise ValueError("Each item in the 'variables' list must be a dictionary.") + raise ValidationConfigError("Each item in the 'variables' list must be a dictionary.") for name, content in item.items(): if not isinstance(name, str): - raise ValueError("Each name in the 'variables' item must be a string.") + raise ValidationConfigError("Each name in the 'variables' item must be a string.") if not isinstance(content, dict): - raise ValueError(f"The content of '{name}' must be a dictionary.") + raise ValidationConfigError(f"The content of '{name}' must be a dictionary.") if 'type' not in content: - raise ValueError(f"Dictionary item '{name}' must contain a 'type' key.") + raise ValidationConfigError(f"Dictionary item '{name}' must contain a 'type' key.") if content['type'] not in ['string', 'number', 'boolean']: - raise ValueError(f"Invalid type for '{name}'. Must be 'string', 'number' or 'boolean'.") + raise ValidationConfigError(f"Invalid type for '{name}'. Must be 'string', 'number' or 'boolean'.") if 'default' in content and content['type'] == 'boolean' and not isinstance(content['default'], bool): - raise ValueError(f"Invalid default value for '{name}'. Must be a boolean.") + raise ValidationConfigError(f"Invalid default value for '{name}'. Must be a boolean.") def _validate_structure_config(self, structure): if not isinstance(structure, list): - raise ValueError("The 'structure' key must be a list.") + raise ValidationConfigError("The 'structure' key must be a list.") for item in structure: if not isinstance(item, dict): - raise ValueError("Each item in the 'structure' list must be a dictionary.") + raise ValidationConfigError("Each item in the 'structure' list must be a dictionary.") for name, content in item.items(): if not isinstance(name, str): - raise ValueError("Each name in the 'structure' item must be a string.") + raise ValidationConfigError("Each name in the 'structure' item must be a string.") if isinstance(content, dict): # Check that any of the keys 'content', 'file' or 'prompt' is present if 'content' not in content and 'file' not in content and 'user_prompt' not in content: - raise ValueError(f"Dictionary item '{name}' must contain either 'content' or 'file' or 'user_prompt' key.") + raise ValidationConfigError(f"Dictionary item '{name}' must contain either 'content' or 'file' or 'user_prompt' key.") # Check if 'file' key is present and its value is a string if 'file' in content and not isinstance(content['file'], str): - raise ValueError(f"The 'file' value for '{name}' must be a string.") + raise ValidationConfigError(f"The 'file' value for '{name}' must be a string.") # Check if 'permissions' key is present and its value is a string if 'permissions' in content and not isinstance(content['permissions'], str): - raise ValueError(f"The 'permissions' value for '{name}' must be a string.") + raise ValidationConfigError(f"The 'permissions' value for '{name}' must be a string.") # Check if 'prompt' key is present and its value is a string if 'prompt' in content and not isinstance(content['prompt'], str): - raise ValueError(f"The 'prompt' value for '{name}' must be a string.") + raise ValidationConfigError(f"The 'prompt' value for '{name}' must be a string.") if 'skip' in content and not isinstance(content['skip'], bool): - raise ValueError(f"The 'skip' value for '{name}' must be a string.") + raise ValidationConfigError(f"The 'skip' value for '{name}' must be a string.") if 'skip_if_exists' in content and not isinstance(content['skip_if_exists'], bool): - raise ValueError(f"The 'skip_if_exists' value for '{name}' must be a string.") + raise ValidationConfigError(f"The 'skip_if_exists' value for '{name}' must be a string.") elif not isinstance(content, str): - raise ValueError(f"The content of '{name}' must be a string or dictionary.") + raise ValidationConfigError(f"The content of '{name}' must be a string or dictionary.") self.logger.info("Configuration validation passed.") diff --git a/tests/test_commands_more.py b/tests/test_commands_more.py index 12399c3..824926d 100644 --- a/tests/test_commands_more.py +++ b/tests/test_commands_more.py @@ -187,6 +187,63 @@ async def fake_start(args): mock_start.assert_called_once() +def test_validate_missing_file_reports_clean_error(parser, tmp_path, caplog): + command = ValidateCommand(parser) + missing = tmp_path / 'missing.yaml' + args = parser.parse_args([str(missing)]) + + with pytest.raises(SystemExit) as excinfo: + command.execute(args) + + assert excinfo.value.code == 1 + assert f"File not found: {missing}" in caplog.text + assert "Traceback" not in caplog.text + + +def test_validate_invalid_yaml_reports_clean_error(parser, tmp_path, caplog): + command = ValidateCommand(parser) + invalid = tmp_path / 'invalid.yaml' + invalid.write_text('files: [\n') + args = parser.parse_args([str(invalid)]) + + with pytest.raises(SystemExit) as excinfo: + command.execute(args) + + assert excinfo.value.code == 1 + assert f"Invalid YAML in {invalid}" in caplog.text + assert "while parsing" in caplog.text + assert "Traceback" not in caplog.text + + +def test_validate_invalid_config_reports_clean_error(parser, tmp_path, caplog): + command = ValidateCommand(parser) + invalid_config = tmp_path / 'invalid-config.yaml' + invalid_config.write_text('files:\n - out.txt: {}\n') + args = parser.parse_args([str(invalid_config)]) + + with pytest.raises(SystemExit) as excinfo: + command.execute(args) + + assert excinfo.value.code == 1 + assert "Invalid structure config" in caplog.text + assert "Dictionary item 'out.txt' must contain" in caplog.text + assert "Traceback" not in caplog.text + + +def test_validate_top_level_non_mapping_reports_clean_error(parser, tmp_path, caplog): + command = ValidateCommand(parser) + invalid_config = tmp_path / 'list.yaml' + invalid_config.write_text('- item\n') + args = parser.parse_args([str(invalid_config)]) + + with pytest.raises(SystemExit) as excinfo: + command.execute(args) + + assert excinfo.value.code == 1 + assert "Top-level YAML content must be a mapping." in caplog.text + assert "Traceback" not in caplog.text + + # ValidateCommand error-path tests on helpers def test_validate_structure_config_errors(parser):