From 68b6b7d1656680d2a76aecbf19a0b383c5d39be3 Mon Sep 17 00:00:00 2001 From: Shane Brown Date: Fri, 1 Sep 2023 15:26:25 -0600 Subject: [PATCH] Simplify config validation messages --- buildrunner/config.py | 8 +-- buildrunner/validation/config_model.py | 43 +++++++++--- buildrunner/validation/data.py | 95 -------------------------- tests/test_config_validation.py | 83 ++++++++++------------ 4 files changed, 74 insertions(+), 155 deletions(-) delete mode 100644 buildrunner/validation/data.py diff --git a/buildrunner/config.py b/buildrunner/config.py index 977c564c..fa820ff6 100644 --- a/buildrunner/config.py +++ b/buildrunner/config.py @@ -372,10 +372,10 @@ def load_config(self, cfg_file, ctx=None, log_file=True): config = self._reorder_dependency_steps(config) - config_result = validate_config(**config) - if config_result.errors: - raise BuildRunnerConfigurationError('Please fix the following configuration errors:' - f'\n{config_result}') + errors = validate_config(**config) + if errors: + raise BuildRunnerConfigurationError(f'Invalid configuration - {errors.count()} errors found:' + f'\n{errors}') return config diff --git a/buildrunner/validation/config_model.py b/buildrunner/validation/config_model.py index fd630a48..92d5879e 100644 --- a/buildrunner/validation/config_model.py +++ b/buildrunner/validation/config_model.py @@ -11,7 +11,31 @@ # pylint: disable=no-name-in-module from pydantic import BaseModel, validator, ValidationError -from buildrunner.validation.data import ValidationItem, ValidationResult + +class Errors: + """ Error class for storing validation errors """ + class Error: + """ Error class for storing validation error """ + def __init__(self, field: str, message: str): + self.field: str = field + self.message: Union[str, None] = message + + def __init__(self): + self.errors = [] + + def add(self, field: str, message: str): + """ Add an error """ + self.errors.append(self.Error(field, message)) + + def count(self): + """ Return the number of errors """ + return len(self.errors) + + def __str__(self): + return '\n'.join([f' {error.field}: {error.message}' for error in self.errors]) + + def __repr__(self): + return self.__str__() class StepBuild(BaseModel): @@ -145,25 +169,24 @@ def validate_multi_platform_build(mp_push_tags: Set[str]): return values -def _add_validation_errors(result: ValidationResult, exc: ValidationError) -> None: +def _add_validation_errors(exc: ValidationError) -> Errors: + errors = Errors() for error in exc.errors(): loc = [str(item) for item in error["loc"]] - result.add_error(ValidationItem( - message=f'Invalid configuration: {error["msg"]} ({error["type"]})', - field=".".join(loc), - )) + errors.add(field='.'.join(loc), message=f'{error["msg"]} ({error["type"]})') + return errors -def validate_config(**kwargs) -> ValidationResult: +def validate_config(**kwargs) -> Errors: """ Check if the config file is valid Raises: ValueError | pydantic.ValidationError : If the config file is invalid """ - result = ValidationResult() + errors = None try: Config(**kwargs) except ValidationError as exc: - _add_validation_errors(result, exc) - return result + errors = _add_validation_errors(exc) + return errors diff --git a/buildrunner/validation/data.py b/buildrunner/validation/data.py deleted file mode 100644 index 6902db05..00000000 --- a/buildrunner/validation/data.py +++ /dev/null @@ -1,95 +0,0 @@ -""" -Copyright 2023 Adobe -All Rights Reserved. - -NOTICE: Adobe permits you to use, modify, and distribute this file in accordance -with the terms of the Adobe license agreement accompanying it. -""" -from typing import List, Union -# pylint: disable=no-name-in-module -from pydantic import BaseModel - - -class ValidationItem(BaseModel): - """ - Contains a single validation error or warning. - """ - message: str - field: Union[str, None] = None - - -class ValidationResult(BaseModel): - """ - Contains the result of a validation method. - """ - warnings: List[ValidationItem] = [] - errors: List[ValidationItem] = [] - - @staticmethod - def _convert(item: Union[str, ValidationItem]) -> ValidationItem: - if isinstance(item, str): - return ValidationItem(message=item) - return item - - @classmethod - def error(cls, message: Union[str, ValidationItem]) -> 'ValidationResult': - """ - Utility method to create a validation result consisting of a single error. - :param message: the error message - :return: a validation result - """ - return cls(errors=[cls._convert(message)]) - - @classmethod - def warning(cls, message: Union[str, ValidationItem]) -> 'ValidationResult': - """ - Utility method to create a validation result consisting of a single warning. - :param message: the warning message - :return: a validation result - """ - return cls(warnings=[cls._convert(message)]) - - def add_error(self, message: Union[str, ValidationItem]) -> None: - """ - Add an error to the result. - :param message: the error message - :return: None - """ - self.errors.append(self._convert(message)) - - def add_warning(self, message: Union[str, ValidationItem]) -> None: - """ - Add a warning to the result. - :param message: the warning message - :return: None - """ - self.warnings.append(self._convert(message)) - - def merge_result(self, result: 'ValidationResult') -> None: - """ - Merge the results of another validation result into this one. - :param result: the result to merge - :return: None - """ - if result.errors: - self.errors.extend(result.errors) - if result.warnings: - self.warnings.extend(result.warnings) - - def __str__(self) -> str: - message = '' - if self.errors: - errors = ''.join([f' {error.field}: {error.message}\n' for error in self.errors]) - message += f'Errors:\n{errors}' - - if self.warnings: - if message == '': - message += '\n' - - warnings = ''.join([f' {warning.field}: {warning.message}\n' for warning in self.warnings]) - message += f'Warnings:\n{warnings}' - - return message - - def __repr__(self) -> str: - return self.__str__() diff --git a/tests/test_config_validation.py b/tests/test_config_validation.py index cb30e65d..3db47daa 100644 --- a/tests/test_config_validation.py +++ b/tests/test_config_validation.py @@ -1,8 +1,5 @@ -# import buildrunner.validation.config_model as config_model -from buildrunner.validation.config_model import validate_config -from pydantic import ValidationError -import pytest +from buildrunner.validation.config_model import validate_config, Errors def test_valid_version_config(): @@ -10,9 +7,9 @@ def test_valid_version_config(): config = { 'version': 'string' } - result = validate_config(**config) - assert len(result.errors) == 1 - assert len(result.warnings) == 0 + errors = validate_config(**config) + assert isinstance(errors, Errors) + assert errors.count() == 1 # Valid version config = { @@ -20,18 +17,16 @@ def test_valid_version_config(): 'steps': { } } - result = validate_config(**config) - assert len(result.errors) == 0 - assert len(result.warnings) == 0 + errors = validate_config(**config) + assert errors is None # Optional version config = { 'steps': { } } - result = validate_config(**config) - assert len(result.errors) == 0 - assert len(result.warnings) == 0 + errors = validate_config(**config) + assert errors is None def test_platform_and_platforms_invalid(): @@ -56,9 +51,9 @@ def test_platform_and_platforms_invalid(): }, } } - result = validate_config(**config) - assert len(result.errors) == 1 - assert len(result.warnings) == 0 + errors = validate_config(**config) + assert isinstance(errors, Errors) + assert errors.count() == 1 def test_platforms_invalid(): @@ -79,9 +74,9 @@ def test_platforms_invalid(): }, } } - result = validate_config(**config) - assert len(result.errors) == 2 - assert len(result.warnings) == 0 + errors = validate_config(**config) + assert isinstance(errors, Errors) + assert errors.count() == 2 def test_build_is_path(): @@ -92,9 +87,8 @@ def test_build_is_path(): }, } } - result = validate_config(**config) - assert len(result.errors) == 0 - assert len(result.warnings) == 0 + errors = validate_config(**config) + assert errors is None def test_valid_platforms(): @@ -117,9 +111,8 @@ def test_valid_platforms(): }, } } - result = validate_config(**config) - assert len(result.errors) == 0 - assert len(result.warnings) == 0 + errors = validate_config(**config) + assert errors is None def test_duplicate_mp_tags_dictionary_invalid(): @@ -153,9 +146,9 @@ def test_duplicate_mp_tags_dictionary_invalid(): }, } } - result = validate_config(**config) - assert len(result.errors) == 1 - assert len(result.warnings) == 0 + errors = validate_config(**config) + assert isinstance(errors, Errors) + assert errors.count() == 1 def test_duplicate_mp_tags_strings_invalid(): @@ -184,9 +177,9 @@ def test_duplicate_mp_tags_strings_invalid(): }, } } - result = validate_config(**config) - assert len(result.errors) == 1 - assert len(result.warnings) == 0 + errors = validate_config(**config) + assert isinstance(errors, Errors) + assert errors.count() == 1 # Indentical tags in same string format config = { @@ -211,9 +204,9 @@ def test_duplicate_mp_tags_strings_invalid(): }, } } - result = validate_config(**config) - assert len(result.errors) == 1 - assert len(result.warnings) == 0 + errors = validate_config(**config) + assert isinstance(errors, Errors) + assert errors.count() == 1 def test_duplicate_mp_tags_strings_valid(): @@ -240,9 +233,8 @@ def test_duplicate_mp_tags_strings_valid(): }, } } - result = validate_config(**config) - assert len(result.errors) == 0 - assert len(result.warnings) == 0 + errors = validate_config(**config) + assert errors is None def test_duplicate_mp_tags_platform_platforms_invalid(): @@ -266,9 +258,9 @@ def test_duplicate_mp_tags_platform_platforms_invalid(): }, } } - result = validate_config(**config) - assert len(result.errors) == 1 - assert len(result.warnings) == 0 + errors = validate_config(**config) + assert isinstance(errors, Errors) + assert errors.count() == 1 def test_valid_config(): @@ -324,9 +316,8 @@ def test_valid_config(): } } - result = validate_config(**config) - assert len(result.errors) == 0 - assert len(result.warnings) == 0 + errors = validate_config(**config) + assert errors is None def test_multiple_errors(): @@ -354,6 +345,6 @@ def test_multiple_errors(): }, } } - result = validate_config(**config) - assert len(result.errors) == 2 - assert len(result.warnings) == 0 + errors = validate_config(**config) + assert isinstance(errors, Errors) + assert errors.count() == 2