diff --git a/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py b/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py index ef51cbf8c87468..d284f279408786 100644 --- a/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py +++ b/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py @@ -641,111 +641,192 @@ def is_pixit(arg_name: str) -> bool: return arg_name.startswith("PIXIT.") +class PIXITValidationError(Exception): + """Raised when PIXIT validation fails""" + pass + + class PIXITValidator: """Handles validation of PIXIT values against their definitions""" @staticmethod - def is_int_pixit_value_valid(value: str) -> bool: - int(value) - return True + def validate_int_pixit_value(value: str) -> None: + """Validates that a value can be converted to int. + + Args: + value: Value to validate + + Raises: + ValueError: If value cannot be converted to int + """ + try: + int(value) + except ValueError as e: + raise ValueError(f"Invalid integer value: {e}") @staticmethod - def is_bool_pixit_value_valid(value: str) -> bool: - if isinstance(value, str): - value_lower = value.lower() - if value_lower not in ('true', 'false'): - return False - else: - bool(value) - return True + def validate_bool_pixit_value(value: str) -> None: + """Validates that a value represents a valid boolean. + + Args: + value: Value to validate + + Raises: + ValueError: If value is not a valid boolean representation + """ + try: + if isinstance(value, str): + value_lower = value.lower() + if value_lower not in ('true', 'false'): + raise ValueError(f"String value must be 'true' or 'false', got '{value}'") + else: + bool(value) + except ValueError as e: + raise ValueError(f"Invalid boolean value: {e}") @staticmethod - def is_float_pixit_value_valid(value: str) -> bool: - float(value) - return True + def validate_float_pixit_value(value: str) -> None: + """Validates that a value can be converted to float. + + Args: + value: Value to validate + + Raises: + ValueError: If value cannot be converted to float + """ + try: + float(value) + except ValueError as e: + raise ValueError(f"Invalid float value: {e}") @staticmethod - def is_string_pixit_value_valid(value: str) -> bool: - str(value) - return True + def validate_string_pixit_value(value: str) -> None: + """Validates that a value can be converted to string. + + Args: + value: Value to validate + + Raises: + ValueError: If value cannot be converted to string + """ + try: + str(value) + except ValueError as e: + raise ValueError(f"Invalid string value: {e}") @staticmethod - def is_json_pixit_value_valid(value: str) -> bool: - if isinstance(value, str): + def validate_json_pixit_value(value: str) -> None: + """Validates that a value can be parsed as valid JSON. + + Args: + value: Value to validate + + Raises: + ValueError: If value cannot be parsed as valid JSON + """ + try: json.loads(value) - elif not isinstance(value, (dict, list)): - return False - return True + except json.JSONDecodeError as e: + raise ValueError(f"Invalid JSON value: {e}") @staticmethod - def is_hex_pixit_value_valid(value: str) -> bool: - if not isinstance(value, str): - return False - hex_str = value.lower().replace("0x", "") - int(hex_str, 16) - if len(hex_str) % 2 != 0: - return False - return True + def validate_hex_pixit_value(value: str) -> None: + """Validates that a value represents valid hexadecimal data. + + Args: + value: Value to validate. Can include optional "0x" prefix. + + Raises: + ValueError: If value is not valid hexadecimal or has odd number of digits + """ + try: + hex_str = value.lower().replace("0x", "") + int(hex_str, 16) # Validate hex format + + if len(hex_str) % 2 != 0: + raise ValueError(f"Hex string must have even number of digits, got {len(hex_str)} digits") + + except (ValueError, TypeError) as e: + raise ValueError(f"Invalid hex value: {e}") @classmethod - def validate_value(cls, value: Any, pixit_def: PIXITDefinition) -> tuple[bool, Optional[str]]: - """Validate PIXIT value matches its declared type""" + def validate_value(cls, value: Any, pixit_def: PIXITDefinition) -> None]: + """Validate PIXIT value matches its declared type. + + Args: + value: The value to validate + pixit_def: The PIXIT definition containing type and requirements + + Raises: + PIXITValidationError: If validation fails + """ if not PIXITDefinition.is_pixit(pixit_def.name): - return True, None # Skip validation for non-PIXIT args + return # Skip validation for non-PIXIT args if value is None: if pixit_def.required: - return False, f"Required PIXIT {pixit_def.name} ({pixit_def.description}) is missing" - return True, None + raise PIXITValidationError( + f"Required PIXIT {pixit_def.name} ({pixit_def.description}) is missing" + ) + return # Mapping of PIXITType to validation function - type_validators = { - PIXITType.INT: cls.is_int_pixit_value_valid, - PIXITType.BOOL: cls.is_bool_pixit_value_valid, - PIXITType.FLOAT: cls.is_float_pixit_value_valid, - PIXITType.STRING: cls.is_string_pixit_value_valid, - PIXITType.JSON: cls.is_json_pixit_value_valid, - PIXITType.HEX: cls.is_hex_pixit_value_valid, + type_validators= { + PIXITType.INT: cls.validate_int_pixit_value, + PIXITType.BOOL: cls.validate_bool_pixit_value, + PIXITType.FLOAT: cls.validate_float_pixit_value, + PIXITType.STRING: cls.validate_string_pixit_value, + PIXITType.JSON: cls.validate_json_pixit_value, + PIXITType.HEX: cls.validate_hex_pixit_value, } validator = type_validators.get(pixit_def.pixit_type) if not validator: - return False, f"Unknown PIXIT type: {pixit_def.pixit_type}" + raise PIXITValidationError(f"Unknown PIXIT type: {pixit_def.pixit_type}") try: - if validator(value): - return True, None - return False, f"Invalid value for {pixit_def.name}: expected {pixit_def.pixit_type.value}" - except (ValueError, TypeError, json.JSONDecodeError): - return False, f"Invalid value for {pixit_def.name}: {value} (expected {pixit_def.pixit_type.value})" + validator(value) + except (ValueError, TypeError, json.JSONDecodeError) as e: + raise PIXITValidationError( + f"Invalid value for {pixit_def.name}: {value} (expected {pixit_def.pixit_type.value})" + ) from e @classmethod def validate_pixits(cls, pixits: list[PIXITDefinition], - provided_values: dict[str, Any]) -> tuple[bool, str]: - """Validate all PIXITs against provided values""" + provided_values: dict[str, Any]) -> None: + """Validate all PIXITs against provided values. + + Args: + pixits: List of PIXIT definitions to validate + provided_values: Dictionary of provided PIXIT values + + Raises: + PIXITValidationError: If any PIXIT validation fails + """ missing = [] invalid = [] for pixit in pixits: value = provided_values.get(pixit.name) - if value is None: - if pixit.required: - missing.append(f"{pixit.name} ({pixit.description})") + if value is None and pixit.required: + missing.append(f"{pixit.name} ({pixit.description})") continue - - valid, error = cls.validate_value(value, pixit) - if not valid: - invalid.append(error) - + + # Validate non-missing values + if value is not None: + try: + cls.validate_value(value, pixit) + except PIXITValidationError as e: + invalid.append(str(e)) + + # Collect all validation errors if missing or invalid: - error = "" + error_msg = "" if missing: - error += "\nMissing required PIXITs:\n" + "\n".join(missing) + error_msg += "Missing required PIXITs: " + ", ".join(missing) if invalid: - error += "\nInvalid PIXIT values:\n" + "\n".join(invalid) - return False, error - - return True, "" + error_msg += "Invalid PIXIT values: " + ", ".join(invalid) + raise PIXITValidationError(error_msg) @dataclass @@ -1277,8 +1358,15 @@ def setup_test(self): self.step_start_time = datetime.now(timezone.utc) self.step_skipped = False self.failed = False + test_name = self.current_test_info.name + + pixits = self.get_test_pixits(test_name) + validator = PIXITValidator() + valid, error = validator.validate_pixits(pixits, self.matter_test_config.global_test_params) + if not valid: + raise signals.TestFailure(f"PIXIT validation failed for test {test_name}: {error}") + if self.runner_hook and not self.is_commissioning: - test_name = self.current_test_info.name steps = self.get_defined_test_steps(test_name) num_steps = 1 if steps is None else len(steps) filename = inspect.getfile(self.__class__) @@ -2494,15 +2582,7 @@ def get_test_info(test_class: MatterBaseTest, matter_test_config: MatterTestConf def run_tests_no_exit(test_class: MatterBaseTest, matter_test_config: MatterTestConfig, hooks: TestRunnerHooks, default_controller=None, external_stack=None) -> bool: - # Get test info which now includes PIXITs - test_info = get_test_info(test_class, matter_test_config) - - # Validate PIXITs before proceeding - validator = PIXITValidator() - valid, error = validator.validate_pixits(test_info[0].pixits, matter_test_config.global_test_params) - if not valid: - logging.error(f"PIXIT validation failed: {error}") - return False + get_test_info(test_class, matter_test_config) # Load test config file. test_config = generate_mobly_test_config(matter_test_config)