From 4ac4c79ab11cbd2a919d278b017d0652bef3fe5c Mon Sep 17 00:00:00 2001 From: "Daniel W. Anner" Date: Fri, 14 Jul 2023 15:38:14 -0400 Subject: [PATCH] Updating comments and error messages for better readability and validation (#1457) --- tests/definitions_test.py | 20 +++++++++++++++----- tests/device_types.py | 20 ++++---------------- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/tests/definitions_test.py b/tests/definitions_test.py index e0cc547c1..e478993a8 100644 --- a/tests/definitions_test.py +++ b/tests/definitions_test.py @@ -49,6 +49,9 @@ def _get_image_files(): return file_list def _decimal_file_handler(uri): + """ + Handler to work with floating decimals that fail normal validation. + """ with urlopen(uri) as url: result = json.loads(url.read().decode("utf-8"), parse_float=decimal.Decimal) return result @@ -68,14 +71,14 @@ def test_definitions(file_path, schema): """ Validate each definition file using the provided JSON schema and check for duplicate entries. """ - # Check file extension + # Check file extension. Only .yml or .yaml files are supported. assert file_path.split('.')[-1] in ('yaml', 'yml'), f"Invalid file extension: {file_path}" # Read file with open(file_path) as definition_file: content = definition_file.read() - # Check for trailing newline + # Check for trailing newline. YAML files must end with an emtpy newline. assert content.endswith('\n'), "Missing trailing newline" # Load YAML data from file @@ -88,23 +91,28 @@ def test_definitions(file_path, schema): schema, handlers={"file": _decimal_file_handler}, ) + # Validate definition against schema Draft4Validator(schema, resolver=resolver).validate(definition) except ValidationError as e: + # Schema validation failure. Ensure you are following the proper format. pytest.fail(f"{file_path} failed validation: {e}", False) # Identify if the definition is for a Device or Module if "device-types" in file_path: + # A device this_device = DeviceType(definition, file_path) else: + # A module this_device = ModuleType(definition, file_path) - # Verify the slug is valid if the definition is a Device + # Verify the slug is valid, only if the definition type is a Device if this_device.isDevice: assert this_device.verify_slug(), pytest.fail(this_device.failureMessage, False) + # Verify the filename is valid. Must either be the model or part_number. assert verify_filename(this_device), pytest.fail(this_device.failureMessage, False) - # Check for duplicate components + # Check for duplicate components within the definition for component_type in COMPONENT_TYPES: known_names = set() defined_components = definition.get(component_type, []) @@ -114,7 +122,7 @@ def test_definitions(file_path, schema): pytest.fail(f'Duplicate entry "{name}" in {component_type} list', False) known_names.add(name) - # Check for empty quotes + # Check for empty quotes and fail if found def iterdict(var): for dict_value in var.values(): if isinstance(dict_value, dict): @@ -141,12 +149,14 @@ def test_definitions(file_path, schema): elif len(manufacturer_images)>2: pytest.fail(f'More than 2 images found for device with slug {this_device.get_slug()}: {manufacturer_images}', False) + # If front_image is True, verify that a front image exists if(definition.get('front_image')): front_image = [image_path.split('/')[2] for image_path in manufacturer_images if os.path.basename(image_path).split('.')[1] == 'front'] if not front_image: pytest.fail(f'{file_path} has front_image set to True but no matching image found for device ({manufacturer_images})', False) + # If rear_image is True, verify that a front image exists if(definition.get('rear_image')): rear_image = [image_path.split('/')[2] for image_path in manufacturer_images if os.path.basename(image_path).split('.')[1] == 'rear'] diff --git a/tests/device_types.py b/tests/device_types.py index 1c9fd2a93..380ae0af6 100644 --- a/tests/device_types.py +++ b/tests/device_types.py @@ -17,9 +17,6 @@ class DeviceType: self._slug_part_number = self._slugify_part_number() self.failureMessage = None - def get_manufacturer(self): - return self.manufacturer - def _slugify_manufacturer(self): return self.manufacturer.casefold().replace(" ", "-").replace("sfp+", "sfpp").replace("poe+", "poep").replace("-+", "-plus-").replace("+", "-plus").replace("_", "-").replace("!", "").replace("/", "-").replace(",", "").replace("'", "").replace("*", "-").replace("&", "and") @@ -28,18 +25,12 @@ class DeviceType: return self.slug return None - def get_model(self): - return self.model - def _slugify_model(self): slugified = self.model.casefold().replace(" ", "-").replace("sfp+", "sfpp").replace("poe+", "poep").replace("-+", "-plus").replace("+", "-plus-").replace("_", "-").replace("&", "-and-").replace("!", "").replace("/", "-").replace(",", "").replace("'", "").replace("*", "-") if slugified.endswith("-"): slugified = slugified[:-1] return slugified - def get_part_number(self): - return self.part_number - def _slugify_part_number(self): slugified = self.part_number.casefold().replace(" ", "-").replace("-+", "-plus").replace("+", "-plus-").replace("_", "-").replace("&", "-and-").replace("!", "").replace("/", "-").replace(",", "").replace("'", "").replace("*", "-") if slugified.endswith("-"): @@ -52,17 +43,17 @@ class DeviceType: def verify_slug(self): # Verify the slug is unique, and not already known if self.slug in KNOWN_SLUGS: - self.failureMessage = f'{self.file_path} device type has duplicate slug "{self.slug}"' + self.failureMessage = f'{self.file_path} has a duplicate slug "{self.slug}"' return False # Verify the manufacturer is appended to the slug if not self.slug.startswith(self._slug_manufacturer): - self.failureMessage = f'{self.file_path} device type has slug "{self.slug}" which does not start with manufacturer "{self.manufacturer}"' + self.failureMessage = f'{self.file_path} contains slug "{self.slug}". Does not start with manufacturer: "{self.manufacturer.casefold()}-"' return False # Verify the slug ends with either the model or part number if not (self.slug.endswith(self._slug_model) or self.slug.endswith(self._slug_part_number)): - self.failureMessage = f'{self.file_path} has slug "{self.slug}". Does not end with the model "{self._slug_model}" or part number "{self._slug_part_number}"' + self.failureMessage = f'{self.file_path} has slug "{self.slug}". Does not end with the model "{self._slug_model}" or part_number "{self._slug_part_number}"' return False # Add the slug to the list of known slugs @@ -82,9 +73,6 @@ class ModuleType: self.part_number = definition.get('part_number', "") self._slug_part_number = self._slugify_part_number() - def get_manufacturer(self): - return self.manufacturer - def get_filepath(self): return self.file_path @@ -105,7 +93,7 @@ def verify_filename(device: (DeviceType or ModuleType)): filename = tail.rsplit(".", 1)[0].casefold() if not (filename == device._slug_model or filename == device._slug_part_number or filename == device.part_number.casefold()): - device.failureMessage = f'{device.file_path} file is not either the model "{device._slug_model}" or part_number "{device.part_number} / {device._slug_part_number}"' + device.failureMessage = f'{device.file_path} file name is invalid. Must be either the model "{device._slug_model}" or part_number "{device.part_number} / {device._slug_part_number}"' return False return True \ No newline at end of file