Skip to content

Commit

Permalink
Issue #507 - Fix invalid content validation check in base val.py classes
Browse files Browse the repository at this point in the history
Update base Validator.content_val to return a proper default value.
Validation that doesn't include a custom `content_val` implementation
relies on this default implementation during validation checks. By
default, that includes EVR and Limit dictionary checks. The current
implementation incorrect returns a garbage value (None) for the default
implementation.

Documentation of the default `content_val` method has been updated to
elaborate on why it's making the current check and what a more thorough
implementation looks like.

Tangentially related, error handling around schema validation was a bit
lacking. This also includes minor updates around the `schema_val` checks
in `Validator` to better handle and log exceptions related to that.
Additionally, validation is short circuited if failures occur there to
avoid confusing error messages in unrelated `content_val` checks.
  • Loading branch information
MJJoyce committed Jan 5, 2024
1 parent 1fdfd45 commit 4d23009
Showing 1 changed file with 27 additions and 8 deletions.
35 changes: 27 additions & 8 deletions ait/core/val.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,19 +456,27 @@ def validate(self, ymldata=None, messages=None):
"""
content_val_list = []
schema_val = self.schema_val(messages)
try:
schema_val = self.schema_val(messages)
except yaml.YAMLError as e:
log.error(
"Unable to validate file due to YAML error. "
f"Verify that the YAML file is well formed.\n\nError: {e}"
)
return False
except jsonschema.SchemaError as e:
log.error(
"Unable to validate file due to error with schema file. "
f"Verify availability and validity of the schema file.\n\nError: {e}"
)
return False

# Loop through the list of all the tested yaml files
for yaml_file in self.yml_files_to_validate:
log.info(f"Validating: {yaml_file}")
content_val_list.append(self.content_val(yaml_file, messages=messages))

if all(content_val_list): # Test that all tested files returned True
content_val = True
else:
content_val = False

return schema_val and content_val
return schema_val and all(content_val_list)

def schema_val(self, messages=None):
"""Perform validation with processed YAML and Schema"""
Expand Down Expand Up @@ -520,8 +528,19 @@ def schema_val(self, messages=None):
return valid

def content_val(self, yaml_file, ymldata=None, messages=None):
"""Simple base content_val method - needed for unit tests"""
"""Simple base content_val method - needed for unit tests
The base content_val method doesn't check any useful elements of the
loaded file. Instead it just returns whether the file was successfully
loaded by the YAMLProcessor.
Child classes should overwrite this to implement checks for specific
content requirements that cannot be easily encoded in a schema file.
For examples, see :meth:`CmdValidator.content_val` or
:meth:`TlmValidator.content_val`
"""
self._ymlproc = YAMLProcessor(yaml_file, False)
return self._ymlproc.loaded


class CmdValidator(Validator):
Expand Down

0 comments on commit 4d23009

Please sign in to comment.