Merge pull request #70 from shanejbrown/main

Simplify config validation messages

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
 

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

tests/test_config_validation.py

@@ -1,37 +1,32 @@
 
-# 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():
     #  Invalid version
     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 = {
         'version': 2.0,
         '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