From 34653985c02593e44a53940a4676337dd479ec43 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Tue, 28 Feb 2023 09:11:07 -0500 Subject: [PATCH] Increase strictness of error that are raised by chip-repl runner/adapter (#25357) * Increase strictness of error that are raised * Address PR comments --- src/controller/python/chip/yaml/errors.py | 4 +- src/controller/python/chip/yaml/runner.py | 124 ++++++++-------------- 2 files changed, 47 insertions(+), 81 deletions(-) diff --git a/src/controller/python/chip/yaml/errors.py b/src/controller/python/chip/yaml/errors.py index 092128f5b8d90e..a96360bbd9ef98 100644 --- a/src/controller/python/chip/yaml/errors.py +++ b/src/controller/python/chip/yaml/errors.py @@ -15,12 +15,12 @@ # limitations under the License. # -class ParsingError(ValueError): +class ActionCreationError(Exception): def __init__(self, message): super().__init__(message) -class UnexpectedParsingError(ValueError): +class UnexpectedActionCreationError(Exception): def __init__(self, message): super().__init__(message) diff --git a/src/controller/python/chip/yaml/runner.py b/src/controller/python/chip/yaml/runner.py index 0e75b595e02061..5cf58f16ee94dd 100644 --- a/src/controller/python/chip/yaml/runner.py +++ b/src/controller/python/chip/yaml/runner.py @@ -29,7 +29,7 @@ from chip.clusters.Attribute import (AttributeStatus, EventReadResult, SubscriptionTransaction, TypedAttributePath, ValueDecodeFailure) from chip.exceptions import ChipStackError -from chip.yaml.errors import ParsingError, UnexpectedParsingError +from chip.yaml.errors import ActionCreationError, UnexpectedActionCreationError from matter_yamltests.pseudo_clusters.clusters.delay_commands import DelayCommands from matter_yamltests.pseudo_clusters.clusters.log_commands import LogCommands from matter_yamltests.pseudo_clusters.clusters.system_commands import SystemCommands @@ -125,7 +125,7 @@ def __init__(self, test_step): super().__init__(test_step) self._test_step = test_step if not _PSEUDO_CLUSTERS.supports(test_step): - raise ParsingError(f'Default cluster {test_step.cluster} {test_step.command}, not supported') + raise ActionCreationError(f'Default cluster {test_step.cluster} {test_step.command}, not supported') def run_action(self, dev_ctrl: ChipDeviceController) -> _ActionResult: resp = asyncio.run(_PSEUDO_CLUSTERS.execute(self._test_step)) @@ -143,9 +143,11 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): 'cluster': Name of cluster which to invoke action is targeting. 'context': Contains test-wide common objects such as DataModelLookup instance. Raises: - ParsingError: Raised if there is a benign error, and there is currently no - action to perform for this write attribute. - UnexpectedParsingError: Raised if there is an unexpected parsing error. + ActionCreationError: Raised if there is a benign error. This occurs when we + cannot find the action to invoke for the provided cluster. When this happens + it is expected that the action to invoke and the provided cluster is an action + to be invoked on a pseudo cluster. + UnexpectedActionCreationError: Raised if there is an unexpected parsing error. ''' super().__init__(test_step) self._busy_wait_ms = test_step.busy_wait_ms @@ -159,13 +161,14 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): self._group_id = test_step.group_id if self._node_id is None and self._group_id is None: - raise UnexpectedParsingError( + raise UnexpectedActionCreationError( 'Both node_id and group_id are None, at least one needs to be provided') command = context.data_model_lookup.get_command(self._cluster, self._command_name) if command is None: - raise ParsingError( + # If we have not found a command it could me that it is a pseudo cluster command. + raise ActionCreationError( f'Failed to find cluster:{self._cluster} Command:{self._command_name}') command_object = command() @@ -177,9 +180,7 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): request_data = Converter.convert_to_data_model_type( request_data_as_dict, type(command_object)) except ValueError: - # TODO after allowing out of bounds enums to be written this should be changed to - # UnexpectedParsingError. - raise ParsingError('Could not covert yaml type') + raise UnexpectedActionCreationError('Could not covert yaml type') self._request_object = command_object.FromDict(request_data) else: @@ -214,9 +215,7 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): 'cluster': Name of cluster read attribute action is targeting. 'context': Contains test-wide common objects such as DataModelLookup instance. Raises: - ParsingError: Raised if there is a benign error, and there is currently no - action to perform for this read attribute. - UnexpectedParsingError: Raised if there is an unexpected parsing error. + UnexpectedActionCreationError: Raised if there is an unexpected parsing error. ''' super().__init__(test_step) self._attribute_name = stringcase.pascalcase(test_step.attribute) @@ -232,22 +231,22 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): self._cluster_object = context.data_model_lookup.get_cluster(self._cluster) if self._cluster_object is None: - raise UnexpectedParsingError( + raise UnexpectedActionCreationError( f'ReadAttribute failed to find cluster object:{self._cluster}') self._request_object = context.data_model_lookup.get_attribute( self._cluster, self._attribute_name) if self._request_object is None: - raise ParsingError( + raise UnexpectedActionCreationError( f'ReadAttribute failed to find cluster:{self._cluster} ' f'Attribute:{self._attribute_name}') if test_step.arguments: - raise UnexpectedParsingError( + raise UnexpectedActionCreationError( f'ReadAttribute should not contain arguments. {self.label}') if self._request_object.attribute_type is None: - raise UnexpectedParsingError( + raise UnexpectedActionCreationError( f'ReadAttribute doesnt have valid attribute_type. {self.label}') def run_action(self, dev_ctrl: ChipDeviceController) -> _ActionResult: @@ -293,7 +292,7 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): 'cluster': Name of cluster read event action is targeting. 'context': Contains test-wide common objects such as DataModelLookup instance. Raises: - UnexpectedParsingError: Raised if there is an unexpected parsing error. + UnexpectedActionCreationError: Raised if there is an unexpected parsing error. ''' super().__init__(test_step) self._event_name = stringcase.pascalcase(test_step.event) @@ -311,11 +310,11 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): self._request_object = context.data_model_lookup.get_event(self._cluster, self._event_name) if self._request_object is None: - raise UnexpectedParsingError( + raise UnexpectedActionCreationError( f'ReadEvent failed to find cluster:{self._cluster} Event:{self._event_name}') if test_step.arguments: - raise UnexpectedParsingError( + raise UnexpectedActionCreationError( f'ReadEvent should not contain arguments. {self.label}') def run_action(self, dev_ctrl: ChipDeviceController) -> _ActionResult: @@ -424,19 +423,17 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): 'cluster': Name of cluster write attribute action is targeting. 'context': Contains test-wide common objects such as DataModelLookup instance. Raises: - ParsingError: Raised if there is a benign error, and there is currently no - action to perform for this write attribute. - UnexpectedParsingError: Raised if there is an unexpected parsing error. + UnexpectedActionCreationError: Raised if there is an unexpected parsing error. ''' super().__init__(test_step, cluster, context) self._context = context if test_step.min_interval is None: - raise UnexpectedParsingError( + raise UnexpectedActionCreationError( f'SubscribeAttribute action does not have min_interval {self.label}') self._min_interval = test_step.min_interval if test_step.max_interval is None: - raise UnexpectedParsingError( + raise UnexpectedActionCreationError( f'SubscribeAttribute action does not have max_interval {self.label}') self._max_interval = test_step.max_interval @@ -479,19 +476,17 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): 'cluster': Name of cluster subscribe event action is targeting. 'context': Contains test-wide common objects such as DataModelLookup instance. Raises: - ParsingError: Raised if there is a benign error, and there is currently no - action to perform for this subscribe event. - UnexpectedParsingError: Raised if there is an unexpected parsing error. + UnexpectedActionCreationError: Raised if there is an unexpected parsing error. ''' super().__init__(test_step, cluster, context) self._context = context if test_step.min_interval is None: - raise UnexpectedParsingError( + raise UnexpectedActionCreationError( f'SubscribeEvent action does not have min_interval {self.label}') self._min_interval = test_step.min_interval if test_step.max_interval is None: - raise UnexpectedParsingError( + raise UnexpectedActionCreationError( f'SubscribeEvent action does not have max_interval {self.label}') self._max_interval = test_step.max_interval @@ -536,9 +531,7 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): 'cluster': Name of cluster write attribute action is targeting. 'context': Contains test-wide common objects such as DataModelLookup instance. Raises: - ParsingError: Raised if there is a benign error, and there is currently no - action to perform for this write attribute. - UnexpectedParsingError: Raised if there is an unexpected parsing error. + UnexpectedActionCreationError: Raised if there is an unexpected parsing error. ''' super().__init__(test_step) self._attribute_name = stringcase.pascalcase(test_step.attribute) @@ -551,29 +544,29 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext): self._request_object = None if self._node_id is None and self._group_id is None: - raise UnexpectedParsingError( + raise UnexpectedActionCreationError( 'Both node_id and group_id are None, at least one needs to be provided') attribute = context.data_model_lookup.get_attribute( self._cluster, self._attribute_name) if attribute is None: - raise ParsingError( + raise UnexpectedActionCreationError( f'WriteAttribute failed to find cluster:{self._cluster} ' f'Attribute:{self._attribute_name}') if not test_step.arguments: - raise UnexpectedParsingError(f'WriteAttribute action does have arguments {self.label}') + raise UnexpectedActionCreationError(f'WriteAttribute action does have arguments {self.label}') args = test_step.arguments['values'] if len(args) != 1: - raise UnexpectedParsingError(f'WriteAttribute is trying to write multiple values') + raise UnexpectedActionCreationError(f'WriteAttribute is trying to write multiple values') request_data_as_dict = args[0] try: # TODO this is an ugly hack request_data = Converter.convert_to_data_model_type( request_data_as_dict['value'], attribute.attribute_type.Type) except ValueError: - raise ParsingError('Could not covert yaml type') + raise UnexpectedActionCreationError('Could not covert yaml type') # Create a cluster object for the request from the provided YAML data. self._request_object = attribute(request_data) @@ -616,7 +609,7 @@ def __init__(self, test_step, context: _ExecutionContext): 'test_step': Step containing information required to run wait for report action. 'context': Contains test-wide common objects such as DataModelLookup instance. Raises: - UnexpectedParsingError: Raised if the expected queue does not exist. + UnexpectedActionCreationError: Raised if the expected queue does not exist. ''' super().__init__(test_step) if test_step.attribute is not None: @@ -624,12 +617,12 @@ def __init__(self, test_step, context: _ExecutionContext): elif test_step.event is not None: queue_name = stringcase.pascalcase(test_step.event) else: - raise UnexpectedParsingError( + raise UnexpectedActionCreationError( f'WaitForReport needs to wait on either attribute or event, neither were provided') self._output_queue = context.subscription_callback_result_queue.get(queue_name, None) if self._output_queue is None: - raise UnexpectedParsingError(f'Could not find output queue') + raise UnexpectedActionCreationError(f'Could not find output queue') def run_action(self, dev_ctrl: ChipDeviceController) -> _ActionResult: try: @@ -654,7 +647,7 @@ def __init__(self, test_step): Args: 'test_step': Step containing information required to run wait for report action. Raises: - UnexpectedParsingError: Raised if the expected queue does not exist. + UnexpectedActionCreationError: Raised if the expected queue does not exist. ''' super().__init__(test_step) self._command = test_step.command @@ -667,7 +660,7 @@ def __init__(self, test_step): self._setup_payload = request_data_as_dict['payload'] self._node_id = request_data_as_dict['nodeId'] else: - raise UnexpectedParsingError(f'Unexpected CommisionerCommand {test_step.command}') + raise UnexpectedActionCreationError(f'Unexpected CommisionerCommand {test_step.command}') def run_action(self, dev_ctrl: ChipDeviceController) -> _ActionResult: if self._command == 'GetCommissionerNodeId': @@ -714,7 +707,7 @@ def _filter_for_step(test_step) -> (discovery.FilterType, any): if test_step.command == 'FindCommissionableByVendorId': return discovery.FilterType.VENDOR_ID, filter - raise UnexpectedParsingError(f'Invalid command: {test_step.command}') + raise UnexpectedActionCreationError(f'Invalid command: {test_step.command}') def __init__(self, test_step): super().__init__(test_step) @@ -776,7 +769,7 @@ def _invoke_action_factory(self, test_step, cluster: str): ''' try: return InvokeAction(test_step, cluster, self._context) - except ParsingError: + except ActionCreationError: return None def _attribute_read_action_factory(self, test_step, cluster: str): @@ -788,10 +781,7 @@ def _attribute_read_action_factory(self, test_step, cluster: str): Returns: ReadAttributeAction if 'test_step' is a valid read attribute to be executed. ''' - try: - return ReadAttributeAction(test_step, cluster, self._context) - except ParsingError: - return None + return ReadAttributeAction(test_step, cluster, self._context) def _event_read_action_factory(self, test_step, cluster: str): return ReadEventAction(test_step, cluster, self._context) @@ -807,13 +797,7 @@ def _attribute_subscribe_action_factory(self, test_step, cluster: str): None if we were unable to use the provided 'test_step' for a known reason that is not fatal to test execution. ''' - try: - return SubscribeAttributeAction(test_step, cluster, self._context) - except ParsingError: - # TODO For now, ParsingErrors are largely issues that will be addressed soon. Once this - # runner has matched parity of the codegen YAML test, this exception should be - # propogated. - return None + return SubscribeAttributeAction(test_step, cluster, self._context) def _attribute_subscribe_event_factory(self, test_step, cluster: str): '''Creates subscribe event command from TestStep provided. @@ -837,39 +821,21 @@ def _attribute_write_action_factory(self, test_step, cluster: str): None if we were unable to use the provided 'test_step' for a known reason that is not fatal to test execution. ''' - try: - return WriteAttributeAction(test_step, cluster, self._context) - except ParsingError: - return None + return WriteAttributeAction(test_step, cluster, self._context) def _wait_for_commissionee_action_factory(self, test_step): - try: - return WaitForCommissioneeAction(test_step) - except ParsingError: - # TODO For now, ParsingErrors are largely issues that will be addressed soon. Once this - # runner has matched parity of the codegen YAML test, this exception should be - # propogated. - return None + return WaitForCommissioneeAction(test_step) def _wait_for_report_action_factory(self, test_step): - try: - return WaitForReportAction(test_step, self._context) - except ParsingError: - # TODO For now, ParsingErrors are largely issues that will be addressed soon. Once this - # runner has matched parity of the codegen YAML test, this exception should be - # propogated. - return None + return WaitForReportAction(test_step, self._context) def _commissioner_command_action_factory(self, test_step): - try: - return CommissionerCommandAction(test_step) - except ParsingError: - return None + return CommissionerCommandAction(test_step) def _default_pseudo_cluster(self, test_step): try: return DefaultPseudoCluster(test_step) - except ParsingError: + except ActionCreationError: return None def encode(self, request) -> BaseAction: