From 4f839eabae92c0f4be52fa79455bfa2412dbb0bb Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Tue, 8 Nov 2022 17:13:17 +0000 Subject: [PATCH 1/9] Adding ability to process config variables, saveAs and constraints --- src/controller/python/BUILD.gn | 1 + .../python/chip/yaml/format_converter.py | 14 +- src/controller/python/chip/yaml/parser.py | 219 +++++++++++++++--- .../python/chip/yaml/response_storage.py | 39 ++++ 4 files changed, 238 insertions(+), 35 deletions(-) create mode 100644 src/controller/python/chip/yaml/response_storage.py diff --git a/src/controller/python/BUILD.gn b/src/controller/python/BUILD.gn index af1bcd7fb2a8fd..91cf8e79b051de 100644 --- a/src/controller/python/BUILD.gn +++ b/src/controller/python/BUILD.gn @@ -231,6 +231,7 @@ chip_python_wheel_action("chip-core") { "chip/yaml/errors.py", "chip/yaml/format_converter.py", "chip/yaml/parser.py", + "chip/yaml/response_storage.py", ] if (chip_controller) { diff --git a/src/controller/python/chip/yaml/format_converter.py b/src/controller/python/chip/yaml/format_converter.py index f6402c59706058..ae3e5ed9d97436 100644 --- a/src/controller/python/chip/yaml/format_converter.py +++ b/src/controller/python/chip/yaml/format_converter.py @@ -60,7 +60,7 @@ def convert_name_value_pair_to_dict(arg_values): return ret_value -def convert_yaml_type(field_value, field_type, use_from_dict=False): +def convert_yaml_type(field_value, field_type, config_values, use_from_dict=False): ''' Converts yaml value to expected type. The YAML representation when converted to a Python dictionary does not @@ -71,6 +71,13 @@ def convert_yaml_type(field_value, field_type, use_from_dict=False): ''' origin = typing.get_origin(field_type) + if isinstance(field_value, str) and field_value in config_values: + config_value = config_values[field_value] + if isinstance(config_value, dict) and 'defaultValue' in config_value: + field_value = config_value['defaultValue'] + else: + field_value = config_values[field_value] + if field_value is None: field_value = NullValue @@ -110,7 +117,7 @@ def convert_yaml_type(field_value, field_type, use_from_dict=False): f'Did not find field "{item}" in {str(field_type)}') from None return_field_value[field_descriptor.Label] = convert_yaml_type( - field_value[item], field_descriptor.Type, use_from_dict) + field_value[item], field_descriptor.Type, config_values, use_from_dict) if use_from_dict: return field_type.FromDict(return_field_value) return return_field_value @@ -122,7 +129,8 @@ def convert_yaml_type(field_value, field_type, use_from_dict=False): # The field type passed in is the type of the list element and not list[T]. for idx, item in enumerate(field_value): - field_value[idx] = convert_yaml_type(item, list_element_type, use_from_dict) + field_value[idx] = convert_yaml_type( + item, list_element_type, config_values, use_from_dict) return field_value # YAML conversion treats all numbers as ints. Convert to a uint type if the schema # type indicates so. diff --git a/src/controller/python/chip/yaml/parser.py b/src/controller/python/chip/yaml/parser.py index 67fc363ccf12a0..d451f7a96e5578 100644 --- a/src/controller/python/chip/yaml/parser.py +++ b/src/controller/python/chip/yaml/parser.py @@ -16,9 +16,9 @@ # from abc import ABC, abstractmethod -from dataclasses import field -import typing +from dataclasses import dataclass, field from chip import ChipDeviceCtrl +from chip.clusters.Types import NullValue from chip.tlv import float32 import yaml import stringcase @@ -29,11 +29,119 @@ from chip.yaml.errors import ParsingError, UnexpectedParsingError from .data_model_lookup import * import chip.yaml.format_converter as Converter +from .response_storage import ResponseStorage _SUCCESS_STATUS_CODE = "SUCCESS" +_NODE_ID_DEFAULT = 0x12345 +_ENDPOINT_DETAULT = '' # TODO why is this an empty string +_CLUSTER_DEFAULT = '' +_TIMEOUT_DEFAULT = 90 logger = logging.getLogger('YamlParser') +@dataclass +class _CommonObjects: + # Data model lookup to get python attribute, cluster, command object. + data_model_lookup: DataModelLookup = None + # Where various test action response are stored and loaded from. + response_storage: ResponseStorage = None + # Top level configuration values for a yaml test. + config_values: dict = None + + +class _ConstraintValue: + def __init__(self, value, field_type, common_objects: _CommonObjects): + self._response_storage = common_objects.response_storage + self._load_expected_response_key = None + self._value = None + + if value is None: + # Default values set above is all we need here. + return + + if isinstance(value, str) and self._response_storage.is_key_saved(value): + self._load_expected_response_key = value + else: + self._value = Converter.convert_yaml_type( + value, field_type, common_objects.config_values) + + def get_value(self): + if self._load_expected_response_key: + return self._response_storage.load(self._load_expected_response_key) + return self._value + + +class _Constraints: + def __init__(self, constraints: dict, field_type, common_objects: _CommonObjects): + self._response_storage = common_objects.response_storage + self._has_value = constraints.get('hasValue') + self._type = constraints.get('type') + self._starts_with = constraints.get('startsWith') + self._ends_with = constraints.get('endsWith') + self._is_upper_case = constraints.get('isUpperCase') + self._is_lower_case = constraints.get('isLowerCase') + self._min_value = _ConstraintValue(constraints.get('minValue'), field_type, common_objects) + self._max_value = _ConstraintValue(constraints.get('maxValue'), field_type, common_objects) + self._contains = constraints.get('contains') + self._excludes = constraints.get('excludes') + self._has_masks_set = constraints.get('hasMasksSet') + self._has_masks_clear = constraints.get('hasMasksClear') + self._not_value = _ConstraintValue(constraints.get('notValue'), field_type, common_objects) + + def are_constrains_met(self, response) -> bool: + return_value = True + + if self._has_value: + logger.warn(f'HasValue constraint currently not implemented, forcing failure') + return_value = False + + if self._type: + logger.warn(f'Type constraint currently not implemented, forcing failure') + return_value = False + + if self._starts_with and not response.startswith(self._starts_with): + return_value = False + + if self._ends_with and not response.endswith(self._ends_with): + return_value = False + + if self._is_upper_case and not response.isupper(): + return_value = False + + if self._is_lower_case and not response.islower(): + return_value = False + + min_value = self._min_value.get_value() + if response is not NullValue and min_value and response < min_value: + return_value = False + + max_value = self._max_value.get_value() + if response is not NullValue and max_value and response > max_value: + return_value = False + + if self._contains and not set(self._contains).issubset(response): + return_value = False + + if self._excludes and not set(self._excludes).isdisjoint(response): + return_value = False + + if self._has_masks_set: + for mask in self._has_masks_set: + if not (mask & response): + return_value = False + + if self._has_masks_clear: + for mask in self._has_masks_clear: + if mask & response: + return_value = False + + not_value = self._not_value.get_value() + if not_value and response == not_value: + return_value = False + + return return_value + + class BaseAction(ABC): '''Interface for a single yaml action that is to be executed.''' @@ -52,13 +160,14 @@ def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): class InvokeAction(BaseAction): '''Single invoke action to be executed including validation of response.''' - def __init__(self, item: dict, cluster: str, data_model_lookup: DataModelLookup): + def __init__(self, item: dict, cluster: str, common_objects: _CommonObjects): '''Parse cluster invoke from yaml test configuration. Args: 'item': Dictionary containing single invoke to be parsed. 'cluster': Name of cluster which to invoke action is targeting. - 'data_model_lookup': Data model lookup to get attribute object. + 'common_objects': Contains global common objects such has data model lookup, storage + for device responses and top level test configurations variable. Raises: ParsingError: Raised if there is a benign error, and there is currently no action to perform for this write attribute. @@ -71,7 +180,7 @@ def __init__(self, item: dict, cluster: str, data_model_lookup: DataModelLookup) self._expected_raw_response: dict = field(default_factory=dict) self._expected_response_object = None - command = data_model_lookup.get_command(self._cluster, self._command_name) + command = common_objects.data_model_lookup.get_command(self._cluster, self._command_name) if command is None: raise ParsingError( @@ -85,7 +194,7 @@ def __init__(self, item: dict, cluster: str, data_model_lookup: DataModelLookup) try: request_data = Converter.convert_yaml_type( - request_data_as_dict, type(command_object)) + request_data_as_dict, type(command_object), common_objects.config_values) except ValueError: raise ParsingError('Could not covert yaml type') @@ -100,12 +209,13 @@ def __init__(self, item: dict, cluster: str, data_model_lookup: DataModelLookup) self._expected_raw_response is not None and self._expected_raw_response.get('values')): response_type = stringcase.pascalcase(self._request_object.response_type) - expected_command = data_model_lookup.get_command(self._cluster, response_type) + expected_command = common_objects.data_model_lookup.get_command(self._cluster, + response_type) expected_response_args = self._expected_raw_response['values'] expected_response_data_as_dict = Converter.convert_name_value_pair_to_dict( expected_response_args) expected_response_data = Converter.convert_yaml_type( - expected_response_data_as_dict, expected_command) + expected_response_data_as_dict, expected_command, common_objects.config_values) self._expected_response_object = expected_command.FromDict(expected_response_data) def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): @@ -130,13 +240,14 @@ def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): class ReadAttributeAction(BaseAction): '''Single read attribute action to be executed including validation.''' - def __init__(self, item: dict, cluster: str, data_model_lookup: DataModelLookup): + def __init__(self, item: dict, cluster: str, common_objects: _CommonObjects): '''Parse read attribute action from yaml test configuration. Args: 'item': Dictionary contains single read attribute action to be parsed. 'cluster': Name of cluster read attribute action is targeting. - 'data_model_lookup': Data model lookup to get attribute object. + 'common_objects': Contains global common objects such has data model lookup, storage + for device responses and top level test configurations variable. Raises: ParsingError: Raised if there is a benign error, and there is currently no action to perform for this read attribute. @@ -144,19 +255,24 @@ def __init__(self, item: dict, cluster: str, data_model_lookup: DataModelLookup) ''' super().__init__(item['label']) self._attribute_name = stringcase.pascalcase(item['attribute']) + self._constraints = None self._cluster = cluster self._cluster_object = None + self._load_expected_response_key = None self._request_object = None self._expected_raw_response: dict = field(default_factory=dict) self._expected_response_object: None = None self._possibly_unsupported = False + self._response_storage = common_objects.response_storage + self._save_response_key = None - self._cluster_object = data_model_lookup.get_cluster(self._cluster) + self._cluster_object = common_objects.data_model_lookup.get_cluster(self._cluster) if self._cluster_object is None: raise UnexpectedParsingError( f'ReadAttribute failed to find cluster object:{self._cluster}') - self._request_object = data_model_lookup.get_attribute(self._cluster, self._attribute_name) + self._request_object = common_objects.data_model_lookup.get_attribute(self._cluster, + self._attribute_name) if self._request_object is None: raise ParsingError( f'ReadAttribute failed to find cluster:{self._cluster} ' @@ -170,18 +286,38 @@ def __init__(self, item: dict, cluster: str, data_model_lookup: DataModelLookup) raise UnexpectedParsingError( f'ReadAttribute doesnt have valid attribute_type. {self.label}') + if 'optional' in item: + self._possibly_unsupported = True + self._expected_raw_response = item.get('response') if (self._expected_raw_response is None): raise UnexpectedParsingError(f'ReadAttribute missing expected response. {self.label}') - if 'optional' in item: - self._possibly_unsupported = True + self._save_response_key = self._expected_raw_response.get('saveAs') + if self._save_response_key: + self._response_storage.save(self._save_response_key, None) if 'value' in self._expected_raw_response: self._expected_response_object = self._request_object.attribute_type.Type expected_response_value = self._expected_raw_response['value'] - self._expected_response_data = Converter.convert_yaml_type( - expected_response_value, self._expected_response_object, use_from_dict=True) + + if (isinstance(expected_response_value, str) and + self._response_storage.is_key_saved(expected_response_value)): + # Value provided is actually a key to use with storage to load saved data, since + # this parser, parses all the action first and then runs them we will load the + # saved expected response during validation time as we can rely on a previous + # action to save a value that we are comparing against. + self._load_expected_response_key = expected_response_value + else: + self._expected_response_data = Converter.convert_yaml_type( + expected_response_value, self._expected_response_object, + common_objects.config_values, use_from_dict=True) + + constraints = self._expected_raw_response.get('constraints') + if constraints: + self._constraints = _Constraints(constraints, + self._request_object.attribute_type.Type, + common_objects) def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): try: @@ -201,9 +337,17 @@ def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): # unsupported, so nothing left to validate. return + parsed_resp = resp[endpoint][self._cluster_object][self._request_object] + if self._constraints and not self._constraints.are_constrains_met(parsed_resp): + logger.error(f'Constraints check failed') + # TODO how should we fail the test here? + # TODO: There is likely an issue here with Optional fields since None if (self._expected_response_object is not None): - parsed_resp = resp[endpoint][self._cluster_object][self._request_object] + + if self._load_expected_response_key is not None: + self._expected_response_data = self._response_storage.load( + self._load_expected_response_key) if (self._expected_response_data != parsed_resp): # TODO: It is debatable if this is the right thing to be doing here. This might @@ -213,17 +357,21 @@ def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): logger.error(f'Expected response {self._expected_response_data} didnt match ' f'actual object {parsed_resp}') + if self._save_response_key is not None: + self._response_storage.save(self._save_response_key, parsed_resp) + class WriteAttributeAction(BaseAction): '''Single write attribute action to be executed including validation.''' - def __init__(self, item: dict, cluster: str, data_model_lookup: DataModelLookup): + def __init__(self, item: dict, cluster: str, common_objects: _CommonObjects): '''Parse write attribute action from yaml test configuration. Args: 'item': Dictionary contains single write attribute action to be parsed. 'cluster': Name of cluster write attribute action is targeting. - 'data_model_lookup': Data model lookup to get attribute object. + 'common_objects': Contains global common objects such has data model lookup, storage + for device responses and top level test configurations variable. Raises: ParsingError: Raised if there is a benign error, and there is currently no action to perform for this write attribute. @@ -234,7 +382,8 @@ def __init__(self, item: dict, cluster: str, data_model_lookup: DataModelLookup) self._cluster = cluster self._request_object = None - attribute = data_model_lookup.get_attribute(self._cluster, self._attribute_name) + attribute = common_objects.data_model_lookup.get_attribute( + self._cluster, self._attribute_name) if attribute is None: raise ParsingError( f'WriteAttribute failed to find cluster:{self._cluster} ' @@ -244,7 +393,7 @@ def __init__(self, item: dict, cluster: str, data_model_lookup: DataModelLookup) args = item['arguments']['value'] try: request_data = Converter.convert_yaml_type( - args, attribute.attribute_type.Type) + args, attribute.attribute_type.Type, common_objects.config_values) except ValueError: raise ParsingError('Could not covert yaml type') @@ -285,13 +434,18 @@ def __init__(self, yaml_path: str): raise exc self._name = self._raw_data['name'] - self._node_id = self._raw_data['config']['nodeId'] - self._cluster = self._raw_data['config'].get('cluster') - if self._cluster: - self._cluster = self._cluster.replace(' ', '') - self._endpoint = self._raw_data['config']['endpoint'] + self._config = self._raw_data['config'] + self._config.setdefault('nodeId', _NODE_ID_DEFAULT) + self._config.setdefault('endpoint', _ENDPOINT_DETAULT) + self._config.setdefault('cluster', _CLUSTER_DEFAULT) + # TODO timeout is still not used + self._config.setdefault('timeout', _TIMEOUT_DEFAULT) + + self._config['cluster'] = self._config['cluster'].replace(' ', '').replace('/', '') self._base_action_test_list = [] - self._data_model_lookup = PreDefinedDataModelLookup() + self._common_objects = _CommonObjects(data_model_lookup=PreDefinedDataModelLookup(), + response_storage=ResponseStorage(), + config_values=self._config) for item in self._raw_data['tests']: # This currently behaves differently than the c++ version. We are evaluating if test @@ -301,7 +455,7 @@ def __init__(self, yaml_path: str): continue action = None - cluster = self._cluster + cluster = self._config['cluster'] # Some of the tests contain 'cluster over-rides' that refer to a different # cluster than that specified in 'config'. if (item.get('cluster')): @@ -329,7 +483,7 @@ def _invoke_action_factory(self, item: dict, cluster: str): None if 'item' was not parsed for a known reason that is not fatal. ''' try: - return InvokeAction(item, cluster, self._data_model_lookup) + return InvokeAction(item, cluster, self._common_objects) except ParsingError: return None @@ -344,7 +498,7 @@ def _attribute_read_action_factory(self, item: dict, cluster: str): None if 'item' was not parsed for a known reason that is not fatal. ''' try: - return ReadAttributeAction(item, cluster, self._data_model_lookup) + return ReadAttributeAction(item, cluster, self._common_objects) except ParsingError: return None @@ -359,13 +513,14 @@ def _attribute_write_action_factory(self, item: dict, cluster: str): None if 'item' was not parsed for a known reason that is not fatal. ''' try: - return WriteAttributeAction(item, cluster, self._data_model_lookup) + return WriteAttributeAction(item, cluster, self._common_objects) except ParsingError: return None def execute_tests(self, dev_ctrl: ChipDeviceCtrl): '''Executes parsed YAML tests.''' + self._common_objects.response_storage.clear() for idx, action in enumerate(self._base_action_test_list): logger.info(f'test: {idx} -- Executing{action.label}') - action.run_action(dev_ctrl, self._endpoint, self._node_id) + action.run_action(dev_ctrl, self._config['endpoint'], self._config['nodeId']) diff --git a/src/controller/python/chip/yaml/response_storage.py b/src/controller/python/chip/yaml/response_storage.py new file mode 100644 index 00000000000000..b18e54f277a15f --- /dev/null +++ b/src/controller/python/chip/yaml/response_storage.py @@ -0,0 +1,39 @@ +# +# Copyright (c) 2022 Project CHIP Authors +# All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +class ResponseStorage: + '''Stores key value pairs. + + This is a code readability convience object for saving/loading values. + ''' + + def __init__(self): + self._saved_list = {} + + def save(self, key, value): + self._saved_list[key] = value + + def load(self, key): + if key in self._saved_list: + return self._saved_list[key] + return None + + def is_key_saved(self, key) -> bool: + return key in self._saved_list + + def clear(self): + self._saved_list.clear() From d11dcfd94be2eda6c515786fd7db28d3b580f876 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Mon, 21 Nov 2022 16:45:59 +0000 Subject: [PATCH 2/9] Rename class --- src/controller/python/chip/yaml/parser.py | 72 +++++++++++------------ 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/src/controller/python/chip/yaml/parser.py b/src/controller/python/chip/yaml/parser.py index d451f7a96e5578..9135ed9fd8600b 100644 --- a/src/controller/python/chip/yaml/parser.py +++ b/src/controller/python/chip/yaml/parser.py @@ -40,7 +40,7 @@ @dataclass -class _CommonObjects: +class _ReferenceObjects: # Data model lookup to get python attribute, cluster, command object. data_model_lookup: DataModelLookup = None # Where various test action response are stored and loaded from. @@ -50,8 +50,8 @@ class _CommonObjects: class _ConstraintValue: - def __init__(self, value, field_type, common_objects: _CommonObjects): - self._response_storage = common_objects.response_storage + def __init__(self, value, field_type, reference_objects: _ReferenceObjects): + self._response_storage = reference_objects.response_storage self._load_expected_response_key = None self._value = None @@ -63,7 +63,7 @@ def __init__(self, value, field_type, common_objects: _CommonObjects): self._load_expected_response_key = value else: self._value = Converter.convert_yaml_type( - value, field_type, common_objects.config_values) + value, field_type, reference_objects.config_values) def get_value(self): if self._load_expected_response_key: @@ -72,21 +72,21 @@ def get_value(self): class _Constraints: - def __init__(self, constraints: dict, field_type, common_objects: _CommonObjects): - self._response_storage = common_objects.response_storage + def __init__(self, constraints: dict, field_type, reference_objects: _ReferenceObjects): + self._response_storage = reference_objects.response_storage self._has_value = constraints.get('hasValue') self._type = constraints.get('type') self._starts_with = constraints.get('startsWith') self._ends_with = constraints.get('endsWith') self._is_upper_case = constraints.get('isUpperCase') self._is_lower_case = constraints.get('isLowerCase') - self._min_value = _ConstraintValue(constraints.get('minValue'), field_type, common_objects) - self._max_value = _ConstraintValue(constraints.get('maxValue'), field_type, common_objects) + self._min_value = _ConstraintValue(constraints.get('minValue'), field_type, reference_objects) + self._max_value = _ConstraintValue(constraints.get('maxValue'), field_type, reference_objects) self._contains = constraints.get('contains') self._excludes = constraints.get('excludes') self._has_masks_set = constraints.get('hasMasksSet') self._has_masks_clear = constraints.get('hasMasksClear') - self._not_value = _ConstraintValue(constraints.get('notValue'), field_type, common_objects) + self._not_value = _ConstraintValue(constraints.get('notValue'), field_type, reference_objects) def are_constrains_met(self, response) -> bool: return_value = True @@ -160,13 +160,13 @@ def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): class InvokeAction(BaseAction): '''Single invoke action to be executed including validation of response.''' - def __init__(self, item: dict, cluster: str, common_objects: _CommonObjects): + def __init__(self, item: dict, cluster: str, reference_objects: _ReferenceObjects): '''Parse cluster invoke from yaml test configuration. Args: 'item': Dictionary containing single invoke to be parsed. 'cluster': Name of cluster which to invoke action is targeting. - 'common_objects': Contains global common objects such has data model lookup, storage + 'reference_objects': Contains global common objects such has data model lookup, storage for device responses and top level test configurations variable. Raises: ParsingError: Raised if there is a benign error, and there is currently no @@ -180,7 +180,7 @@ def __init__(self, item: dict, cluster: str, common_objects: _CommonObjects): self._expected_raw_response: dict = field(default_factory=dict) self._expected_response_object = None - command = common_objects.data_model_lookup.get_command(self._cluster, self._command_name) + command = reference_objects.data_model_lookup.get_command(self._cluster, self._command_name) if command is None: raise ParsingError( @@ -194,7 +194,7 @@ def __init__(self, item: dict, cluster: str, common_objects: _CommonObjects): try: request_data = Converter.convert_yaml_type( - request_data_as_dict, type(command_object), common_objects.config_values) + request_data_as_dict, type(command_object), reference_objects.config_values) except ValueError: raise ParsingError('Could not covert yaml type') @@ -209,13 +209,13 @@ def __init__(self, item: dict, cluster: str, common_objects: _CommonObjects): self._expected_raw_response is not None and self._expected_raw_response.get('values')): response_type = stringcase.pascalcase(self._request_object.response_type) - expected_command = common_objects.data_model_lookup.get_command(self._cluster, - response_type) + expected_command = reference_objects.data_model_lookup.get_command(self._cluster, + response_type) expected_response_args = self._expected_raw_response['values'] expected_response_data_as_dict = Converter.convert_name_value_pair_to_dict( expected_response_args) expected_response_data = Converter.convert_yaml_type( - expected_response_data_as_dict, expected_command, common_objects.config_values) + expected_response_data_as_dict, expected_command, reference_objects.config_values) self._expected_response_object = expected_command.FromDict(expected_response_data) def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): @@ -240,13 +240,13 @@ def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): class ReadAttributeAction(BaseAction): '''Single read attribute action to be executed including validation.''' - def __init__(self, item: dict, cluster: str, common_objects: _CommonObjects): + def __init__(self, item: dict, cluster: str, reference_objects: _ReferenceObjects): '''Parse read attribute action from yaml test configuration. Args: 'item': Dictionary contains single read attribute action to be parsed. 'cluster': Name of cluster read attribute action is targeting. - 'common_objects': Contains global common objects such has data model lookup, storage + 'reference_objects': Contains global common objects such has data model lookup, storage for device responses and top level test configurations variable. Raises: ParsingError: Raised if there is a benign error, and there is currently no @@ -263,16 +263,16 @@ def __init__(self, item: dict, cluster: str, common_objects: _CommonObjects): self._expected_raw_response: dict = field(default_factory=dict) self._expected_response_object: None = None self._possibly_unsupported = False - self._response_storage = common_objects.response_storage + self._response_storage = reference_objects.response_storage self._save_response_key = None - self._cluster_object = common_objects.data_model_lookup.get_cluster(self._cluster) + self._cluster_object = reference_objects.data_model_lookup.get_cluster(self._cluster) if self._cluster_object is None: raise UnexpectedParsingError( f'ReadAttribute failed to find cluster object:{self._cluster}') - self._request_object = common_objects.data_model_lookup.get_attribute(self._cluster, - self._attribute_name) + self._request_object = reference_objects.data_model_lookup.get_attribute(self._cluster, + self._attribute_name) if self._request_object is None: raise ParsingError( f'ReadAttribute failed to find cluster:{self._cluster} ' @@ -311,13 +311,13 @@ def __init__(self, item: dict, cluster: str, common_objects: _CommonObjects): else: self._expected_response_data = Converter.convert_yaml_type( expected_response_value, self._expected_response_object, - common_objects.config_values, use_from_dict=True) + reference_objects.config_values, use_from_dict=True) constraints = self._expected_raw_response.get('constraints') if constraints: self._constraints = _Constraints(constraints, self._request_object.attribute_type.Type, - common_objects) + reference_objects) def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): try: @@ -364,13 +364,13 @@ def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): class WriteAttributeAction(BaseAction): '''Single write attribute action to be executed including validation.''' - def __init__(self, item: dict, cluster: str, common_objects: _CommonObjects): + def __init__(self, item: dict, cluster: str, reference_objects: _ReferenceObjects): '''Parse write attribute action from yaml test configuration. Args: 'item': Dictionary contains single write attribute action to be parsed. 'cluster': Name of cluster write attribute action is targeting. - 'common_objects': Contains global common objects such has data model lookup, storage + 'reference_objects': Contains global common objects such has data model lookup, storage for device responses and top level test configurations variable. Raises: ParsingError: Raised if there is a benign error, and there is currently no @@ -382,7 +382,7 @@ def __init__(self, item: dict, cluster: str, common_objects: _CommonObjects): self._cluster = cluster self._request_object = None - attribute = common_objects.data_model_lookup.get_attribute( + attribute = reference_objects.data_model_lookup.get_attribute( self._cluster, self._attribute_name) if attribute is None: raise ParsingError( @@ -393,7 +393,7 @@ def __init__(self, item: dict, cluster: str, common_objects: _CommonObjects): args = item['arguments']['value'] try: request_data = Converter.convert_yaml_type( - args, attribute.attribute_type.Type, common_objects.config_values) + args, attribute.attribute_type.Type, reference_objects.config_values) except ValueError: raise ParsingError('Could not covert yaml type') @@ -438,14 +438,14 @@ def __init__(self, yaml_path: str): self._config.setdefault('nodeId', _NODE_ID_DEFAULT) self._config.setdefault('endpoint', _ENDPOINT_DETAULT) self._config.setdefault('cluster', _CLUSTER_DEFAULT) - # TODO timeout is still not used + # TODO timeout is currently not used self._config.setdefault('timeout', _TIMEOUT_DEFAULT) self._config['cluster'] = self._config['cluster'].replace(' ', '').replace('/', '') self._base_action_test_list = [] - self._common_objects = _CommonObjects(data_model_lookup=PreDefinedDataModelLookup(), - response_storage=ResponseStorage(), - config_values=self._config) + self._reference_objects = _ReferenceObjects(data_model_lookup=PreDefinedDataModelLookup(), + response_storage=ResponseStorage(), + config_values=self._config) for item in self._raw_data['tests']: # This currently behaves differently than the c++ version. We are evaluating if test @@ -483,7 +483,7 @@ def _invoke_action_factory(self, item: dict, cluster: str): None if 'item' was not parsed for a known reason that is not fatal. ''' try: - return InvokeAction(item, cluster, self._common_objects) + return InvokeAction(item, cluster, self._reference_objects) except ParsingError: return None @@ -498,7 +498,7 @@ def _attribute_read_action_factory(self, item: dict, cluster: str): None if 'item' was not parsed for a known reason that is not fatal. ''' try: - return ReadAttributeAction(item, cluster, self._common_objects) + return ReadAttributeAction(item, cluster, self._reference_objects) except ParsingError: return None @@ -513,13 +513,13 @@ def _attribute_write_action_factory(self, item: dict, cluster: str): None if 'item' was not parsed for a known reason that is not fatal. ''' try: - return WriteAttributeAction(item, cluster, self._common_objects) + return WriteAttributeAction(item, cluster, self._reference_objects) except ParsingError: return None def execute_tests(self, dev_ctrl: ChipDeviceCtrl): '''Executes parsed YAML tests.''' - self._common_objects.response_storage.clear() + self._reference_objects.response_storage.clear() for idx, action in enumerate(self._base_action_test_list): logger.info(f'test: {idx} -- Executing{action.label}') From 69b945d9009cb2b8c7de98ab7fdac6ec2511fd4b Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Mon, 21 Nov 2022 22:34:17 +0000 Subject: [PATCH 3/9] Refactor save as a little --- src/controller/python/chip/yaml/parser.py | 114 ++++++++++++++-------- 1 file changed, 72 insertions(+), 42 deletions(-) diff --git a/src/controller/python/chip/yaml/parser.py b/src/controller/python/chip/yaml/parser.py index 9135ed9fd8600b..aa323cfc92545e 100644 --- a/src/controller/python/chip/yaml/parser.py +++ b/src/controller/python/chip/yaml/parser.py @@ -80,13 +80,16 @@ def __init__(self, constraints: dict, field_type, reference_objects: _ReferenceO self._ends_with = constraints.get('endsWith') self._is_upper_case = constraints.get('isUpperCase') self._is_lower_case = constraints.get('isLowerCase') - self._min_value = _ConstraintValue(constraints.get('minValue'), field_type, reference_objects) - self._max_value = _ConstraintValue(constraints.get('maxValue'), field_type, reference_objects) + self._min_value = _ConstraintValue(constraints.get('minValue'), field_type, + reference_objects) + self._max_value = _ConstraintValue(constraints.get('maxValue'), field_type, + reference_objects) self._contains = constraints.get('contains') self._excludes = constraints.get('excludes') self._has_masks_set = constraints.get('hasMasksSet') self._has_masks_clear = constraints.get('hasMasksClear') - self._not_value = _ConstraintValue(constraints.get('notValue'), field_type, reference_objects) + self._not_value = _ConstraintValue(constraints.get('notValue'), field_type, + reference_objects) def are_constrains_met(self, response) -> bool: return_value = True @@ -142,6 +145,54 @@ def are_constrains_met(self, response) -> bool: return return_value +class _SaveAs: + def __init__(self, save_as_key: str, response_storage: ResponseStorage): + self._save_as_key = save_as_key + self._response_storage = response_storage + self._response_storage.save(self._save_as_key, None) + + def save_response(self, value): + self._response_storage.save(self._save_as_key, value) + + @classmethod + def extract_save_as_property(cls, items: dict, response_storage: ResponseStorage): + save_as_key = items.get('saveAs') + if not save_as_key: + return None + return cls(save_as_key, response_storage) + + +class _ExpectedResponse: + def __init__(self, value, response_object, reference_objects: _ReferenceObjects): + self._load_expected_response_in_verify = None + self._expected_response_object = response_object + self._expected_response = None + self._response_storage = reference_objects.response_storage + if isinstance(value, str) and self._response_storage.is_key_saved(value): + self._load_expected_response_in_verify = value + else: + self._expected_response = Converter.convert_yaml_type( + value, response_object, reference_objects.config_values, use_from_dict=True) + + def verify(self, response): + if (self._expected_response_object is None): + return True + + if self._load_expected_response_in_verify is not None: + self._expected_response = self._response_storage.load( + self._load_expected_response_in_verify) + + if (self._expected_response != response): + # TODO: It is debatable if this is the right thing to be doing here. This might + # need a follow up cleanup. + if (self._expected_response_object != float32 or + not math.isclose(self._expected_response, response, rel_tol=1e-6)): + logger.error(f'Expected response {self._expected_response} didnt match ' + f'actual object {response}') + return False + return True + + class BaseAction(ABC): '''Interface for a single yaml action that is to be executed.''' @@ -180,7 +231,8 @@ def __init__(self, item: dict, cluster: str, reference_objects: _ReferenceObject self._expected_raw_response: dict = field(default_factory=dict) self._expected_response_object = None - command = reference_objects.data_model_lookup.get_command(self._cluster, self._command_name) + command = reference_objects.data_model_lookup.get_command( + self._cluster, self._command_name) if command is None: raise ParsingError( @@ -258,21 +310,20 @@ def __init__(self, item: dict, cluster: str, reference_objects: _ReferenceObject self._constraints = None self._cluster = cluster self._cluster_object = None - self._load_expected_response_key = None self._request_object = None self._expected_raw_response: dict = field(default_factory=dict) - self._expected_response_object: None = None + self._expected_response: _ExpectedResponse = None self._possibly_unsupported = False self._response_storage = reference_objects.response_storage - self._save_response_key = None + self._save_as = None self._cluster_object = reference_objects.data_model_lookup.get_cluster(self._cluster) if self._cluster_object is None: raise UnexpectedParsingError( f'ReadAttribute failed to find cluster object:{self._cluster}') - self._request_object = reference_objects.data_model_lookup.get_attribute(self._cluster, - self._attribute_name) + self._request_object = reference_objects.data_model_lookup.get_attribute( + self._cluster, self._attribute_name) if self._request_object is None: raise ParsingError( f'ReadAttribute failed to find cluster:{self._cluster} ' @@ -293,25 +344,15 @@ def __init__(self, item: dict, cluster: str, reference_objects: _ReferenceObject if (self._expected_raw_response is None): raise UnexpectedParsingError(f'ReadAttribute missing expected response. {self.label}') - self._save_response_key = self._expected_raw_response.get('saveAs') - if self._save_response_key: - self._response_storage.save(self._save_response_key, None) + self._save_as = _SaveAs.extract_save_as_property( + self._expected_raw_response, reference_objects.response_storage) if 'value' in self._expected_raw_response: - self._expected_response_object = self._request_object.attribute_type.Type + expected_response_object = self._request_object.attribute_type.Type expected_response_value = self._expected_raw_response['value'] - - if (isinstance(expected_response_value, str) and - self._response_storage.is_key_saved(expected_response_value)): - # Value provided is actually a key to use with storage to load saved data, since - # this parser, parses all the action first and then runs them we will load the - # saved expected response during validation time as we can rely on a previous - # action to save a value that we are comparing against. - self._load_expected_response_key = expected_response_value - else: - self._expected_response_data = Converter.convert_yaml_type( - expected_response_value, self._expected_response_object, - reference_objects.config_values, use_from_dict=True) + self._expected_response = _ExpectedResponse(expected_response_value, + expected_response_object, + reference_objects) constraints = self._expected_raw_response.get('constraints') if constraints: @@ -338,27 +379,16 @@ def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): return parsed_resp = resp[endpoint][self._cluster_object][self._request_object] + + if self._save_as is not None: + self._save_as.save_response(parsed_resp) + if self._constraints and not self._constraints.are_constrains_met(parsed_resp): logger.error(f'Constraints check failed') # TODO how should we fail the test here? - # TODO: There is likely an issue here with Optional fields since None - if (self._expected_response_object is not None): - - if self._load_expected_response_key is not None: - self._expected_response_data = self._response_storage.load( - self._load_expected_response_key) - - if (self._expected_response_data != parsed_resp): - # TODO: It is debatable if this is the right thing to be doing here. This might - # need a follow up cleanup. - if (self._expected_response_object != float32 or - not math.isclose(self._expected_response_data, parsed_resp, rel_tol=1e-6)): - logger.error(f'Expected response {self._expected_response_data} didnt match ' - f'actual object {parsed_resp}') - - if self._save_response_key is not None: - self._response_storage.save(self._save_response_key, parsed_resp) + if self._expected_response is not None: + self._expected_response.verify(parsed_resp) class WriteAttributeAction(BaseAction): From f4c9cc84b6af14676e4fa05795c583401aaf24c8 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 23 Nov 2022 18:00:52 +0000 Subject: [PATCH 4/9] Address PR comments --- .../python/chip/yaml/format_converter.py | 13 +- src/controller/python/chip/yaml/parser.py | 161 ++++++++++-------- ...esponse_storage.py => variable_storage.py} | 6 +- 3 files changed, 91 insertions(+), 89 deletions(-) rename src/controller/python/chip/yaml/{response_storage.py => variable_storage.py} (89%) diff --git a/src/controller/python/chip/yaml/format_converter.py b/src/controller/python/chip/yaml/format_converter.py index ae3e5ed9d97436..ef4c0dd9b85dd7 100644 --- a/src/controller/python/chip/yaml/format_converter.py +++ b/src/controller/python/chip/yaml/format_converter.py @@ -60,7 +60,7 @@ def convert_name_value_pair_to_dict(arg_values): return ret_value -def convert_yaml_type(field_value, field_type, config_values, use_from_dict=False): +def convert_yaml_type(field_value, field_type, use_from_dict=False): ''' Converts yaml value to expected type. The YAML representation when converted to a Python dictionary does not @@ -71,13 +71,6 @@ def convert_yaml_type(field_value, field_type, config_values, use_from_dict=Fals ''' origin = typing.get_origin(field_type) - if isinstance(field_value, str) and field_value in config_values: - config_value = config_values[field_value] - if isinstance(config_value, dict) and 'defaultValue' in config_value: - field_value = config_value['defaultValue'] - else: - field_value = config_values[field_value] - if field_value is None: field_value = NullValue @@ -117,7 +110,7 @@ def convert_yaml_type(field_value, field_type, config_values, use_from_dict=Fals f'Did not find field "{item}" in {str(field_type)}') from None return_field_value[field_descriptor.Label] = convert_yaml_type( - field_value[item], field_descriptor.Type, config_values, use_from_dict) + field_value[item], field_descriptor.Type, use_from_dict) if use_from_dict: return field_type.FromDict(return_field_value) return return_field_value @@ -130,7 +123,7 @@ def convert_yaml_type(field_value, field_type, config_values, use_from_dict=Fals # The field type passed in is the type of the list element and not list[T]. for idx, item in enumerate(field_value): field_value[idx] = convert_yaml_type( - item, list_element_type, config_values, use_from_dict) + item, list_element_type, use_from_dict) return field_value # YAML conversion treats all numbers as ints. Convert to a uint type if the schema # type indicates so. diff --git a/src/controller/python/chip/yaml/parser.py b/src/controller/python/chip/yaml/parser.py index aa323cfc92545e..dc0cef01ee4828 100644 --- a/src/controller/python/chip/yaml/parser.py +++ b/src/controller/python/chip/yaml/parser.py @@ -29,7 +29,7 @@ from chip.yaml.errors import ParsingError, UnexpectedParsingError from .data_model_lookup import * import chip.yaml.format_converter as Converter -from .response_storage import ResponseStorage +from .variable_storage import VariableStorage _SUCCESS_STATUS_CODE = "SUCCESS" _NODE_ID_DEFAULT = 0x12345 @@ -40,40 +40,49 @@ @dataclass -class _ReferenceObjects: +class _ExecutionContext: + ''' Objects that is commonly passed around this file that are vital to test execution.''' # Data model lookup to get python attribute, cluster, command object. data_model_lookup: DataModelLookup = None # Where various test action response are stored and loaded from. - response_storage: ResponseStorage = None + variable_storage: VariableStorage = None # Top level configuration values for a yaml test. config_values: dict = None class _ConstraintValue: - def __init__(self, value, field_type, reference_objects: _ReferenceObjects): - self._response_storage = reference_objects.response_storage - self._load_expected_response_key = None + '''Constraints that are numeric primitive data types''' + + def __init__(self, value, field_type, context: _ExecutionContext): + self._variable_storage = context.variable_storage + # When not none _indirect_value_key is binding a name to the constraint value, and the + # actual value can only be looked-up dynamically, which is why this is a key name. + self._indirect_value_key = None self._value = None if value is None: # Default values set above is all we need here. return - if isinstance(value, str) and self._response_storage.is_key_saved(value): - self._load_expected_response_key = value + if isinstance(value, str) and self._variable_storage.is_key_saved(value): + self._indirect_value_key = value else: self._value = Converter.convert_yaml_type( - value, field_type, reference_objects.config_values) + value, field_type) def get_value(self): - if self._load_expected_response_key: - return self._response_storage.load(self._load_expected_response_key) + '''Gets the current value of the constraint. + + This method accounts for getting the runtime saved value from DUT previous responses. + ''' + if self._indirect_value_key: + return self._variable_storage.load(self._indirect_value_key) return self._value class _Constraints: - def __init__(self, constraints: dict, field_type, reference_objects: _ReferenceObjects): - self._response_storage = reference_objects.response_storage + def __init__(self, constraints: dict, field_type, context: _ExecutionContext): + self._variable_storage = context.variable_storage self._has_value = constraints.get('hasValue') self._type = constraints.get('type') self._starts_with = constraints.get('startsWith') @@ -81,15 +90,15 @@ def __init__(self, constraints: dict, field_type, reference_objects: _ReferenceO self._is_upper_case = constraints.get('isUpperCase') self._is_lower_case = constraints.get('isLowerCase') self._min_value = _ConstraintValue(constraints.get('minValue'), field_type, - reference_objects) + context) self._max_value = _ConstraintValue(constraints.get('maxValue'), field_type, - reference_objects) + context) self._contains = constraints.get('contains') self._excludes = constraints.get('excludes') self._has_masks_set = constraints.get('hasMasksSet') self._has_masks_clear = constraints.get('hasMasksClear') self._not_value = _ConstraintValue(constraints.get('notValue'), field_type, - reference_objects) + context) def are_constrains_met(self, response) -> bool: return_value = True @@ -130,12 +139,12 @@ def are_constrains_met(self, response) -> bool: if self._has_masks_set: for mask in self._has_masks_set: - if not (mask & response): + if (response & mask) != mask: return_value = False if self._has_masks_clear: for mask in self._has_masks_clear: - if mask & response: + if (response & mask) != 0: return_value = False not_value = self._not_value.get_value() @@ -145,51 +154,46 @@ def are_constrains_met(self, response) -> bool: return return_value -class _SaveAs: - def __init__(self, save_as_key: str, response_storage: ResponseStorage): - self._save_as_key = save_as_key - self._response_storage = response_storage - self._response_storage.save(self._save_as_key, None) +class _VariableToSave: + def __init__(self, variable_name: str, variable_storage: VariableStorage): + self._variable_name = variable_name + self._variable_storage = variable_storage + self._variable_storage.save(self._variable_name, None) def save_response(self, value): - self._response_storage.save(self._save_as_key, value) - - @classmethod - def extract_save_as_property(cls, items: dict, response_storage: ResponseStorage): - save_as_key = items.get('saveAs') - if not save_as_key: - return None - return cls(save_as_key, response_storage) + self._variable_storage.save(self._variable_name, value) class _ExpectedResponse: - def __init__(self, value, response_object, reference_objects: _ReferenceObjects): + def __init__(self, value, response_type, context: _ExecutionContext): self._load_expected_response_in_verify = None - self._expected_response_object = response_object + self._expected_response_type = response_type self._expected_response = None - self._response_storage = reference_objects.response_storage - if isinstance(value, str) and self._response_storage.is_key_saved(value): + self._variable_storage = context.variable_storage + if isinstance(value, str) and self._variable_storage.is_key_saved(value): self._load_expected_response_in_verify = value else: self._expected_response = Converter.convert_yaml_type( - value, response_object, reference_objects.config_values, use_from_dict=True) + value, response_type, use_from_dict=True) def verify(self, response): - if (self._expected_response_object is None): + if (self._expected_response_type is None): return True if self._load_expected_response_in_verify is not None: - self._expected_response = self._response_storage.load( + self._expected_response = self._variable_storage.load( self._load_expected_response_in_verify) - if (self._expected_response != response): - # TODO: It is debatable if this is the right thing to be doing here. This might - # need a follow up cleanup. - if (self._expected_response_object != float32 or - not math.isclose(self._expected_response, response, rel_tol=1e-6)): + if isinstance(self._expected_response_type, float32): + if not math.isclose(self._expected_response, response, rel_tol=1e-6): logger.error(f'Expected response {self._expected_response} didnt match ' f'actual object {response}') return False + + if (self._expected_response != response): + logger.error(f'Expected response {self._expected_response} didnt match ' + f'actual object {response}') + return False return True @@ -211,13 +215,13 @@ def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): class InvokeAction(BaseAction): '''Single invoke action to be executed including validation of response.''' - def __init__(self, item: dict, cluster: str, reference_objects: _ReferenceObjects): + def __init__(self, item: dict, cluster: str, context: _ExecutionContext): '''Parse cluster invoke from yaml test configuration. Args: 'item': Dictionary containing single invoke to be parsed. 'cluster': Name of cluster which to invoke action is targeting. - 'reference_objects': Contains global common objects such has data model lookup, storage + 'context': Contains global common objects such has data model lookup, storage for device responses and top level test configurations variable. Raises: ParsingError: Raised if there is a benign error, and there is currently no @@ -231,7 +235,7 @@ def __init__(self, item: dict, cluster: str, reference_objects: _ReferenceObject self._expected_raw_response: dict = field(default_factory=dict) self._expected_response_object = None - command = reference_objects.data_model_lookup.get_command( + command = context.data_model_lookup.get_command( self._cluster, self._command_name) if command is None: @@ -246,7 +250,7 @@ def __init__(self, item: dict, cluster: str, reference_objects: _ReferenceObject try: request_data = Converter.convert_yaml_type( - request_data_as_dict, type(command_object), reference_objects.config_values) + request_data_as_dict, type(command_object)) except ValueError: raise ParsingError('Could not covert yaml type') @@ -261,13 +265,13 @@ def __init__(self, item: dict, cluster: str, reference_objects: _ReferenceObject self._expected_raw_response is not None and self._expected_raw_response.get('values')): response_type = stringcase.pascalcase(self._request_object.response_type) - expected_command = reference_objects.data_model_lookup.get_command(self._cluster, - response_type) + expected_command = context.data_model_lookup.get_command(self._cluster, + response_type) expected_response_args = self._expected_raw_response['values'] expected_response_data_as_dict = Converter.convert_name_value_pair_to_dict( expected_response_args) expected_response_data = Converter.convert_yaml_type( - expected_response_data_as_dict, expected_command, reference_objects.config_values) + expected_response_data_as_dict, expected_command) self._expected_response_object = expected_command.FromDict(expected_response_data) def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): @@ -292,13 +296,13 @@ def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): class ReadAttributeAction(BaseAction): '''Single read attribute action to be executed including validation.''' - def __init__(self, item: dict, cluster: str, reference_objects: _ReferenceObjects): + def __init__(self, item: dict, cluster: str, context: _ExecutionContext): '''Parse read attribute action from yaml test configuration. Args: 'item': Dictionary contains single read attribute action to be parsed. 'cluster': Name of cluster read attribute action is targeting. - 'reference_objects': Contains global common objects such has data model lookup, storage + 'context': Contains global common objects such has data model lookup, storage for device responses and top level test configurations variable. Raises: ParsingError: Raised if there is a benign error, and there is currently no @@ -314,15 +318,14 @@ def __init__(self, item: dict, cluster: str, reference_objects: _ReferenceObject self._expected_raw_response: dict = field(default_factory=dict) self._expected_response: _ExpectedResponse = None self._possibly_unsupported = False - self._response_storage = reference_objects.response_storage - self._save_as = None + self._variable_to_save = None - self._cluster_object = reference_objects.data_model_lookup.get_cluster(self._cluster) + self._cluster_object = context.data_model_lookup.get_cluster(self._cluster) if self._cluster_object is None: raise UnexpectedParsingError( f'ReadAttribute failed to find cluster object:{self._cluster}') - self._request_object = reference_objects.data_model_lookup.get_attribute( + self._request_object = context.data_model_lookup.get_attribute( self._cluster, self._attribute_name) if self._request_object is None: raise ParsingError( @@ -344,21 +347,21 @@ def __init__(self, item: dict, cluster: str, reference_objects: _ReferenceObject if (self._expected_raw_response is None): raise UnexpectedParsingError(f'ReadAttribute missing expected response. {self.label}') - self._save_as = _SaveAs.extract_save_as_property( - self._expected_raw_response, reference_objects.response_storage) + variable_name = self._expected_raw_response.get('saveAs') + if variable_name: + self._variable_to_save = _VariableToSave(variable_name, context.variable_storage) if 'value' in self._expected_raw_response: - expected_response_object = self._request_object.attribute_type.Type expected_response_value = self._expected_raw_response['value'] self._expected_response = _ExpectedResponse(expected_response_value, - expected_response_object, - reference_objects) + self._request_object.attribute_type.Type, + context) constraints = self._expected_raw_response.get('constraints') if constraints: self._constraints = _Constraints(constraints, self._request_object.attribute_type.Type, - reference_objects) + context) def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): try: @@ -378,10 +381,12 @@ def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): # unsupported, so nothing left to validate. return + # TODO Currently there are no checks that this indexing won't fail. Need to add some + # initial validity checks. Coming soon an a future PR. parsed_resp = resp[endpoint][self._cluster_object][self._request_object] - if self._save_as is not None: - self._save_as.save_response(parsed_resp) + if self._variable_to_save is not None: + self._variable_to_save.save_response(parsed_resp) if self._constraints and not self._constraints.are_constrains_met(parsed_resp): logger.error(f'Constraints check failed') @@ -394,13 +399,13 @@ def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): class WriteAttributeAction(BaseAction): '''Single write attribute action to be executed including validation.''' - def __init__(self, item: dict, cluster: str, reference_objects: _ReferenceObjects): + def __init__(self, item: dict, cluster: str, context: _ExecutionContext): '''Parse write attribute action from yaml test configuration. Args: 'item': Dictionary contains single write attribute action to be parsed. 'cluster': Name of cluster write attribute action is targeting. - 'reference_objects': Contains global common objects such has data model lookup, storage + 'context': Contains global common objects such has data model lookup, storage for device responses and top level test configurations variable. Raises: ParsingError: Raised if there is a benign error, and there is currently no @@ -412,7 +417,7 @@ def __init__(self, item: dict, cluster: str, reference_objects: _ReferenceObject self._cluster = cluster self._request_object = None - attribute = reference_objects.data_model_lookup.get_attribute( + attribute = context.data_model_lookup.get_attribute( self._cluster, self._attribute_name) if attribute is None: raise ParsingError( @@ -423,7 +428,7 @@ def __init__(self, item: dict, cluster: str, reference_objects: _ReferenceObject args = item['arguments']['value'] try: request_data = Converter.convert_yaml_type( - args, attribute.attribute_type.Type, reference_objects.config_values) + args, attribute.attribute_type.Type) except ValueError: raise ParsingError('Could not covert yaml type') @@ -463,8 +468,14 @@ def __init__(self, yaml_path: str): except yaml.YAMLError as exc: raise exc + if 'name' not in self._raw_data: + raise UnexpectedParsingError("YAML expected to have 'name'") self._name = self._raw_data['name'] + + if 'config' not in self._raw_data: + raise UnexpectedParsingError("YAML expected to have 'config'") self._config = self._raw_data['config'] + self._config.setdefault('nodeId', _NODE_ID_DEFAULT) self._config.setdefault('endpoint', _ENDPOINT_DETAULT) self._config.setdefault('cluster', _CLUSTER_DEFAULT) @@ -473,9 +484,9 @@ def __init__(self, yaml_path: str): self._config['cluster'] = self._config['cluster'].replace(' ', '').replace('/', '') self._base_action_test_list = [] - self._reference_objects = _ReferenceObjects(data_model_lookup=PreDefinedDataModelLookup(), - response_storage=ResponseStorage(), - config_values=self._config) + self._context = _ExecutionContext(data_model_lookup=PreDefinedDataModelLookup(), + variable_storage=VariableStorage(), + config_values=self._config) for item in self._raw_data['tests']: # This currently behaves differently than the c++ version. We are evaluating if test @@ -513,7 +524,7 @@ def _invoke_action_factory(self, item: dict, cluster: str): None if 'item' was not parsed for a known reason that is not fatal. ''' try: - return InvokeAction(item, cluster, self._reference_objects) + return InvokeAction(item, cluster, self._context) except ParsingError: return None @@ -528,7 +539,7 @@ def _attribute_read_action_factory(self, item: dict, cluster: str): None if 'item' was not parsed for a known reason that is not fatal. ''' try: - return ReadAttributeAction(item, cluster, self._reference_objects) + return ReadAttributeAction(item, cluster, self._context) except ParsingError: return None @@ -543,13 +554,13 @@ def _attribute_write_action_factory(self, item: dict, cluster: str): None if 'item' was not parsed for a known reason that is not fatal. ''' try: - return WriteAttributeAction(item, cluster, self._reference_objects) + return WriteAttributeAction(item, cluster, self._context) except ParsingError: return None def execute_tests(self, dev_ctrl: ChipDeviceCtrl): '''Executes parsed YAML tests.''' - self._reference_objects.response_storage.clear() + self._context.variable_storage.clear() for idx, action in enumerate(self._base_action_test_list): logger.info(f'test: {idx} -- Executing{action.label}') diff --git a/src/controller/python/chip/yaml/response_storage.py b/src/controller/python/chip/yaml/variable_storage.py similarity index 89% rename from src/controller/python/chip/yaml/response_storage.py rename to src/controller/python/chip/yaml/variable_storage.py index b18e54f277a15f..d62a7dd82c0ced 100644 --- a/src/controller/python/chip/yaml/response_storage.py +++ b/src/controller/python/chip/yaml/variable_storage.py @@ -15,7 +15,7 @@ # limitations under the License. # -class ResponseStorage: +class VariableStorage: '''Stores key value pairs. This is a code readability convience object for saving/loading values. @@ -28,9 +28,7 @@ def save(self, key, value): self._saved_list[key] = value def load(self, key): - if key in self._saved_list: - return self._saved_list[key] - return None + return self._saved_list.get(key) def is_key_saved(self, key) -> bool: return key in self._saved_list From 389e47827fa79ebb6b064438ca32cd3e8a4c1618 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 23 Nov 2022 18:16:42 +0000 Subject: [PATCH 5/9] remove unneeded change in format_converter.py --- src/controller/python/chip/yaml/format_converter.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/controller/python/chip/yaml/format_converter.py b/src/controller/python/chip/yaml/format_converter.py index ef4c0dd9b85dd7..f6402c59706058 100644 --- a/src/controller/python/chip/yaml/format_converter.py +++ b/src/controller/python/chip/yaml/format_converter.py @@ -122,8 +122,7 @@ def convert_yaml_type(field_value, field_type, use_from_dict=False): # The field type passed in is the type of the list element and not list[T]. for idx, item in enumerate(field_value): - field_value[idx] = convert_yaml_type( - item, list_element_type, use_from_dict) + field_value[idx] = convert_yaml_type(item, list_element_type, use_from_dict) return field_value # YAML conversion treats all numbers as ints. Convert to a uint type if the schema # type indicates so. From db9a0222ac817c745cea9a57e32ea11a92421bb1 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 23 Nov 2022 18:20:56 +0000 Subject: [PATCH 6/9] Address PR comment --- src/controller/python/chip/yaml/parser.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/controller/python/chip/yaml/parser.py b/src/controller/python/chip/yaml/parser.py index dc0cef01ee4828..d2ae0859fecf87 100644 --- a/src/controller/python/chip/yaml/parser.py +++ b/src/controller/python/chip/yaml/parser.py @@ -345,6 +345,9 @@ def __init__(self, item: dict, cluster: str, context: _ExecutionContext): self._expected_raw_response = item.get('response') if (self._expected_raw_response is None): + # TODO actually if response is missing is typically just means that we need to confirm + # that we got a successful response. This will be implemented later to consider all + # poossibly corner cases (if there are corner cases) around that. raise UnexpectedParsingError(f'ReadAttribute missing expected response. {self.label}') variable_name = self._expected_raw_response.get('saveAs') From aacfbc9724ac0dc166991abf37db7091b0c1c459 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 23 Nov 2022 18:25:33 +0000 Subject: [PATCH 7/9] Update comment in docstring --- src/controller/python/chip/yaml/parser.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/controller/python/chip/yaml/parser.py b/src/controller/python/chip/yaml/parser.py index d2ae0859fecf87..32ed95f24f729c 100644 --- a/src/controller/python/chip/yaml/parser.py +++ b/src/controller/python/chip/yaml/parser.py @@ -221,7 +221,7 @@ def __init__(self, item: dict, cluster: str, context: _ExecutionContext): Args: 'item': Dictionary containing single invoke to be parsed. 'cluster': Name of cluster which to invoke action is targeting. - 'context': Contains global common objects such has data model lookup, storage + 'context': Contains test-wide common objects such as DataModelLookup instance, storage for device responses and top level test configurations variable. Raises: ParsingError: Raised if there is a benign error, and there is currently no @@ -302,7 +302,7 @@ def __init__(self, item: dict, cluster: str, context: _ExecutionContext): Args: 'item': Dictionary contains single read attribute action to be parsed. 'cluster': Name of cluster read attribute action is targeting. - 'context': Contains global common objects such has data model lookup, storage + 'context': Contains test-wide common objects such as DataModelLookup instance, storage for device responses and top level test configurations variable. Raises: ParsingError: Raised if there is a benign error, and there is currently no @@ -408,7 +408,7 @@ def __init__(self, item: dict, cluster: str, context: _ExecutionContext): Args: 'item': Dictionary contains single write attribute action to be parsed. 'cluster': Name of cluster write attribute action is targeting. - 'context': Contains global common objects such has data model lookup, storage + 'context': Contains test-wide common objects such as DataModelLookup instance, storage for device responses and top level test configurations variable. Raises: ParsingError: Raised if there is a benign error, and there is currently no From 9277ceb3654ca5dcaff11dce19ebaab6f46b224a Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 23 Nov 2022 18:50:14 +0000 Subject: [PATCH 8/9] Update build file to renamed file --- src/controller/python/BUILD.gn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controller/python/BUILD.gn b/src/controller/python/BUILD.gn index 91cf8e79b051de..a00a9e3d8756ce 100644 --- a/src/controller/python/BUILD.gn +++ b/src/controller/python/BUILD.gn @@ -231,7 +231,7 @@ chip_python_wheel_action("chip-core") { "chip/yaml/errors.py", "chip/yaml/format_converter.py", "chip/yaml/parser.py", - "chip/yaml/response_storage.py", + "chip/yaml/variable_storage.py", ] if (chip_controller) { From 9d09a68bcc555a929bc3f3ad89d954353868a185 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 23 Nov 2022 20:16:45 +0000 Subject: [PATCH 9/9] Address PR comment --- src/controller/python/chip/yaml/parser.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/controller/python/chip/yaml/parser.py b/src/controller/python/chip/yaml/parser.py index 32ed95f24f729c..8fc5cd55ec66cd 100644 --- a/src/controller/python/chip/yaml/parser.py +++ b/src/controller/python/chip/yaml/parser.py @@ -186,13 +186,13 @@ def verify(self, response): if isinstance(self._expected_response_type, float32): if not math.isclose(self._expected_response, response, rel_tol=1e-6): - logger.error(f'Expected response {self._expected_response} didnt match ' - f'actual object {response}') + logger.error(f"Expected response {self._expected_response} didn't match " + f"actual object {response}") return False if (self._expected_response != response): - logger.error(f'Expected response {self._expected_response} didnt match ' - f'actual object {response}') + logger.error(f"Expected response {self._expected_response} didn't match " + f"actual object {response}") return False return True @@ -345,9 +345,9 @@ def __init__(self, item: dict, cluster: str, context: _ExecutionContext): self._expected_raw_response = item.get('response') if (self._expected_raw_response is None): - # TODO actually if response is missing is typically just means that we need to confirm + # TODO actually if response is missing it typically means that we need to confirm # that we got a successful response. This will be implemented later to consider all - # poossibly corner cases (if there are corner cases) around that. + # possible corner cases around that (if there are corner cases). raise UnexpectedParsingError(f'ReadAttribute missing expected response. {self.label}') variable_name = self._expected_raw_response.get('saveAs')