change from boolean checks to handling of exceptions, fix issue where…

… validation would run only for first test

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)