Skip to content

Commit

Permalink
Increase strictness of error that are raised by chip-repl runner/adap…
Browse files Browse the repository at this point in the history 
…ter (#25357)

* Increase strictness of error that are raised

* Address PR comments
  • Loading branch information
@tehampson @pull
tehampson authored and pull[bot] committed Dec 14, 2023
1 parent eac45be commit 3107773
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 81 deletions.
4 changes: 2 additions & 2 deletions src/controller/python/chip/yaml/errors.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
124 changes: 45 additions & 79 deletions src/controller/python/chip/yaml/runner.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand All @@ -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
Expand All @@ -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()
Expand All @@ -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:
Expand Down Expand Up @@ -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)
Expand All @@ -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:
Expand Down Expand Up @@ -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)
Expand All @@ -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:
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -616,20 +609,20 @@ 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:
queue_name = stringcase.pascalcase(test_step.attribute)
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:
Expand All @@ -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
Expand All @@ -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':
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand All @@ -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)
Expand All @@ -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.
Expand All @@ -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:
Expand Down

0 comments on commit 3107773

Please sign in to comment.