Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify config validation messages #70

Merged
merged 1 commit into from
Sep 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions buildrunner/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
43 changes: 33 additions & 10 deletions buildrunner/validation/config_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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
95 changes: 0 additions & 95 deletions buildrunner/validation/data.py

This file was deleted.

83 changes: 37 additions & 46 deletions tests/test_config_validation.py
Original file line number Diff line number Diff line change
@@ -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():
Expand All @@ -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():
Expand All @@ -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():
Expand All @@ -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():
Expand All @@ -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():
Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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 = {
Expand All @@ -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():
Expand All @@ -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():
Expand All @@ -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():
Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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
Loading