From 236807691a7405b35296a2594dd4c913a72c15a2 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 23 Nov 2022 17:16:48 -0500 Subject: [PATCH] Adding ability to process config variables, saveAs and constraints to yaml python parser (#23599) * Adding ability to process config variables, saveAs and constraints * Rename class * Refactor save as a little * Address PR comments * remove unneeded change in format_converter.py * Address PR comment * Update comment in docstring * Update build file to renamed file * Address PR comment --- src/controller/python/BUILD.gn | 1 + src/controller/python/chip/yaml/parser.py | 279 +++++++++++++++--- .../python/chip/yaml/variable_storage.py | 37 +++ 3 files changed, 277 insertions(+), 40 deletions(-) create mode 100644 src/controller/python/chip/yaml/variable_storage.py diff --git a/src/controller/python/BUILD.gn b/src/controller/python/BUILD.gn index c892038c8998e8..9108c12d01f9a8 100644 --- a/src/controller/python/BUILD.gn +++ b/src/controller/python/BUILD.gn @@ -234,6 +234,7 @@ chip_python_wheel_action("chip-core") { "chip/yaml/errors.py", "chip/yaml/format_converter.py", "chip/yaml/parser.py", + "chip/yaml/variable_storage.py", ] if (chip_controller) { diff --git a/src/controller/python/chip/yaml/parser.py b/src/controller/python/chip/yaml/parser.py index 67fc363ccf12a0..8fc5cd55ec66cd 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,174 @@ from chip.yaml.errors import ParsingError, UnexpectedParsingError from .data_model_lookup import * import chip.yaml.format_converter as Converter +from .variable_storage import VariableStorage _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 _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. + variable_storage: VariableStorage = None + # Top level configuration values for a yaml test. + config_values: dict = None + + +class _ConstraintValue: + '''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._variable_storage.is_key_saved(value): + self._indirect_value_key = value + else: + self._value = Converter.convert_yaml_type( + value, field_type) + + def get_value(self): + '''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, 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') + 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, + context) + self._max_value = _ConstraintValue(constraints.get('maxValue'), field_type, + 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, + context) + + 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 (response & mask) != mask: + return_value = False + + if self._has_masks_clear: + for mask in self._has_masks_clear: + if (response & mask) != 0: + return_value = False + + not_value = self._not_value.get_value() + if not_value and response == not_value: + return_value = False + + return return_value + + +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._variable_storage.save(self._variable_name, value) + + +class _ExpectedResponse: + def __init__(self, value, response_type, context: _ExecutionContext): + self._load_expected_response_in_verify = None + self._expected_response_type = response_type + self._expected_response = None + 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_type, use_from_dict=True) + + def verify(self, response): + if (self._expected_response_type is None): + return True + + if self._load_expected_response_in_verify is not None: + self._expected_response = self._variable_storage.load( + self._load_expected_response_in_verify) + + 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} didn't match " + f"actual object {response}") + return False + + if (self._expected_response != response): + logger.error(f"Expected response {self._expected_response} didn't match " + f"actual object {response}") + return False + return True + + class BaseAction(ABC): '''Interface for a single yaml action that is to be executed.''' @@ -52,13 +215,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, 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. - 'data_model_lookup': Data model lookup to get attribute object. + '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 action to perform for this write attribute. @@ -71,7 +235,8 @@ 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 = context.data_model_lookup.get_command( + self._cluster, self._command_name) if command is None: raise ParsingError( @@ -100,7 +265,8 @@ 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 = 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) @@ -130,13 +296,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, 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. - 'data_model_lookup': Data model lookup to get attribute object. + '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 action to perform for this read attribute. @@ -144,19 +311,22 @@ 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._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._variable_to_save = None - self._cluster_object = 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 = data_model_lookup.get_attribute(self._cluster, self._attribute_name) + self._request_object = context.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 +340,31 @@ 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): + # 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 + # possible corner cases around that (if there are corner cases). raise UnexpectedParsingError(f'ReadAttribute missing expected response. {self.label}') - if 'optional' in item: - self._possibly_unsupported = True + 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: - 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) + self._expected_response = _ExpectedResponse(expected_response_value, + 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, + context) def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): try: @@ -201,29 +384,32 @@ def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): # unsupported, so nothing left to validate. return - # 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] + # 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._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') + # TODO how should we fail the test here? - 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._expected_response is not None: + self._expected_response.verify(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, 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. - 'data_model_lookup': Data model lookup to get attribute object. + '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 action to perform for this write attribute. @@ -234,7 +420,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 = context.data_model_lookup.get_attribute( + self._cluster, self._attribute_name) if attribute is None: raise ParsingError( f'WriteAttribute failed to find cluster:{self._cluster} ' @@ -284,14 +471,25 @@ 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'] - 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'] + + 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) + # 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._data_model_lookup = PreDefinedDataModelLookup() + 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 @@ -301,7 +499,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 +527,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._context) except ParsingError: return None @@ -344,7 +542,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._context) except ParsingError: return None @@ -359,13 +557,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._context) except ParsingError: return None def execute_tests(self, dev_ctrl: ChipDeviceCtrl): '''Executes parsed YAML tests.''' + self._context.variable_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/variable_storage.py b/src/controller/python/chip/yaml/variable_storage.py new file mode 100644 index 00000000000000..d62a7dd82c0ced --- /dev/null +++ b/src/controller/python/chip/yaml/variable_storage.py @@ -0,0 +1,37 @@ +# +# 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 VariableStorage: + '''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): + return self._saved_list.get(key) + + def is_key_saved(self, key) -> bool: + return key in self._saved_list + + def clear(self): + self._saved_list.clear()