From 4c5958058069874a67debae47f7aba17dda65fc2 Mon Sep 17 00:00:00 2001 From: cecille Date: Thu, 15 Feb 2024 12:53:02 -0500 Subject: [PATCH] address review comments --- src/python_testing/TestSpecParsingSupport.py | 29 +++++++++--------- src/python_testing/spec_parsing_support.py | 31 +++++++++++++------- 2 files changed, 35 insertions(+), 25 deletions(-) diff --git a/src/python_testing/TestSpecParsingSupport.py b/src/python_testing/TestSpecParsingSupport.py index 484bf8db8a2d02..bed2edf037a072 100644 --- a/src/python_testing/TestSpecParsingSupport.py +++ b/src/python_testing/TestSpecParsingSupport.py @@ -223,7 +223,7 @@ def test_write_optional(self): def test_derived_clusters(self): clusters: dict[int, XmlCluster] = {} - derived_clusters: dict[str, XmlCluster] = {} + pure_base_clusters: dict[str, XmlCluster] = {} ids_by_name: dict[str, int] = {} problems: list[ProblemNotice] = [] base_cluster_xml = ElementTree.fromstring(BASE_CLUSTER_XML_STR) @@ -231,11 +231,11 @@ def test_derived_clusters(self): expected_global_attrs = [GlobalAttributeIds.FEATURE_MAP_ID, GlobalAttributeIds.ATTRIBUTE_LIST_ID, GlobalAttributeIds.ACCEPTED_COMMAND_LIST_ID, GlobalAttributeIds.GENERATED_COMMAND_LIST_ID, GlobalAttributeIds.CLUSTER_REVISION_ID] - add_cluster_data_from_xml(base_cluster_xml, clusters, derived_clusters, ids_by_name, problems) - add_cluster_data_from_xml(derived_cluster_xml, clusters, derived_clusters, ids_by_name, problems) + add_cluster_data_from_xml(base_cluster_xml, clusters, pure_base_clusters, ids_by_name, problems) + add_cluster_data_from_xml(derived_cluster_xml, clusters, pure_base_clusters, ids_by_name, problems) asserts.assert_equal(len(clusters), 1, "Unexpected number of clusters") - asserts.assert_equal(len(derived_clusters), 1, "Unexpected number of derived clusters") + asserts.assert_equal(len(pure_base_clusters), 1, "Unexpected number of derived clusters") asserts.assert_equal(len(ids_by_name), 1, "Unexpected number of IDs per name") asserts.assert_equal(len(problems), 0, "Unexpected number of problems") asserts.assert_equal(ids_by_name["Test Derived"], 0xFFFF, "Test derived name not added to IDs") @@ -246,22 +246,23 @@ def test_derived_clusters(self): asserts.assert_equal(set(clusters[0xFFFF].accepted_commands.keys()), set([]), "Unexpected accepted commands") asserts.assert_equal(set(clusters[0xFFFF].generated_commands.keys()), set([]), "Unexpected generated commands") - asserts.assert_true("Test Base" in derived_clusters, "Base ID not found in derived clusters") - asserts.assert_equal(set(derived_clusters["Test Base"].attributes.keys()), set( + asserts.assert_true("Test Base" in pure_base_clusters, "Base ID not found in derived clusters") + asserts.assert_equal(set(pure_base_clusters["Test Base"].attributes.keys()), set( [0, 1, 2, 3] + expected_global_attrs), "Unexpected attribute list") - asserts.assert_equal(set(derived_clusters["Test Base"].accepted_commands.keys()), set([0]), "Unexpected accepted commands") - asserts.assert_equal(set(derived_clusters["Test Base"].generated_commands.keys()), + asserts.assert_equal(set(pure_base_clusters["Test Base"].accepted_commands.keys()), + set([0]), "Unexpected accepted commands") + asserts.assert_equal(set(pure_base_clusters["Test Base"].generated_commands.keys()), set([1]), "Unexpected generated commands") - asserts.assert_equal(str(derived_clusters["Test Base"].accepted_commands[0].conformance), + asserts.assert_equal(str(pure_base_clusters["Test Base"].accepted_commands[0].conformance), "M", "Unexpected conformance on base accepted command") - asserts.assert_equal(str(derived_clusters["Test Base"].generated_commands[1].conformance), + asserts.assert_equal(str(pure_base_clusters["Test Base"].generated_commands[1].conformance), "M", "Unexpected conformance on base generated command") - asserts.assert_equal(len(derived_clusters["Test Base"].unknown_commands), + asserts.assert_equal(len(pure_base_clusters["Test Base"].unknown_commands), 0, "Unexpected number of unknown commands in base") asserts.assert_equal(len(clusters[0xFFFF].unknown_commands), 2, "Unexpected number of unknown commands in derived cluster") - combine_derived_clusters_with_base(clusters, derived_clusters, ids_by_name) + combine_derived_clusters_with_base(clusters, pure_base_clusters, ids_by_name) # Ensure the base-only attribute (1) was added to the derived cluster asserts.assert_equal(set(clusters[0xFFFF].attributes.keys()), set( [0, 1, 2, 3] + expected_global_attrs), "Unexpected attribute list") @@ -284,12 +285,12 @@ def test_derived_clusters(self): def test_missing_command_direction(self): clusters: dict[int, XmlCluster] = {} - derived_clusters: dict[str, XmlCluster] = {} + pure_base_clusters: dict[str, XmlCluster] = {} ids_by_name: dict[str, int] = {} problems: list[ProblemNotice] = [] cluster_xml = ElementTree.fromstring(CLUSTER_WITH_UNKNOWN_COMMAND) - add_cluster_data_from_xml(cluster_xml, clusters, derived_clusters, ids_by_name, problems) + add_cluster_data_from_xml(cluster_xml, clusters, pure_base_clusters, ids_by_name, problems) check_clusters_for_unknown_commands(clusters, problems) asserts.assert_equal(len(problems), 1, "Unexpected number of problems found") asserts.assert_equal(problems[0].location.cluster_id, 0xFFFE, "Unexpected problem location (cluster id)") diff --git a/src/python_testing/spec_parsing_support.py b/src/python_testing/spec_parsing_support.py index 878a2fbcf69ed2..3a784178cdb880 100644 --- a/src/python_testing/spec_parsing_support.py +++ b/src/python_testing/spec_parsing_support.py @@ -348,7 +348,7 @@ def parse_attributes(self) -> dict[uint, XmlAttribute]: ), read_access=Clusters.AccessControl.Enums.AccessControlEntryPrivilegeEnum.kView, write_access=Clusters.AccessControl.Enums.AccessControlEntryPrivilegeEnum.kUnknownEnumValue, write_optional=False) return attributes - def get_command_direction(self, element: ElementTree.Element) -> CommandType: + def get_command_type(self, element: ElementTree.Element) -> CommandType: try: if element.attrib['direction'].lower() == 'responsefromserver': return CommandType.GENERATED @@ -360,7 +360,7 @@ def get_command_direction(self, element: ElementTree.Element) -> CommandType: def parse_unknown_commands(self) -> list[XmlCommand]: commands = [] for element, conformance_xml, access_xml in self.command_elements: - if self.get_command_direction(element) != CommandType.UNKNOWN: + if self.get_command_type(element) != CommandType.UNKNOWN: continue code = int(element.attrib['id'], 0) conformance = self.parse_conformance(conformance_xml) @@ -370,7 +370,7 @@ def parse_unknown_commands(self) -> list[XmlCommand]: def parse_commands(self, command_type: CommandType) -> dict[uint, XmlCommand]: commands = {} for element, conformance_xml, access_xml in self.command_elements: - if self.get_command_direction(element) != command_type: + if self.get_command_type(element) != command_type: continue code = int(element.attrib['id'], 0) conformance = self.parse_conformance(conformance_xml) @@ -412,7 +412,16 @@ def get_problems(self) -> list[ProblemNotice]: return self._problems -def add_cluster_data_from_xml(xml: ElementTree.Element, clusters: dict[int, XmlCluster], derived_clusters: dict[str, XmlCluster], ids_by_name: dict[str, int], problems: list[ProblemNotice]) -> None: +def add_cluster_data_from_xml(xml: ElementTree.Element, clusters: dict[int, XmlCluster], pure_base_clusters: dict[str, XmlCluster], ids_by_name: dict[str, int], problems: list[ProblemNotice]) -> None: + ''' Adds cluster data to the supplied dicts as appropriate + + xml: XML element read from from the XML cluster file + clusters: dict of id -> XmlCluster. This function will append new clusters as appropriate to this dict. + pure_base_clusters: dict of base name -> XmlCluster. This data structure is used to hold pure base clusters that don't have + an ID. This function will append new pure base clusters as approrpriate to this dict. + ids_by_name: dict of cluster name -> ID. This function will append new IDs as appropriate to this dict. + problems: list of any problems encountered during spec parsing. This function will append problems as appropriate to this list. + ''' cluster = xml.iter('cluster') for c in cluster: name = c.attrib['name'] @@ -433,7 +442,7 @@ def add_cluster_data_from_xml(xml: ElementTree.Element, clusters: dict[int, XmlC if cluster_id: clusters[cluster_id] = new else: - derived_clusters[name] = new + pure_base_clusters[name] = new def check_clusters_for_unknown_commands(clusters: dict[int, XmlCluster], problems: list[ProblemNotice]): @@ -446,14 +455,14 @@ def check_clusters_for_unknown_commands(clusters: dict[int, XmlCluster], problem def build_xml_clusters() -> tuple[list[XmlCluster], list[ProblemNotice]]: dir = os.path.join(os.path.dirname(os.path.realpath(__file__)), '..', '..', 'data_model', 'clusters') clusters: dict[int, XmlCluster] = {} - derived_clusters: dict[str, XmlCluster] = {} + pure_base_clusters: dict[str, XmlCluster] = {} ids_by_name: dict[str, int] = {} problems: list[ProblemNotice] = [] for xml in glob.glob(f"{dir}/*.xml"): logging.info(f'Parsing file {xml}') tree = ElementTree.parse(f'{xml}') root = tree.getroot() - add_cluster_data_from_xml(root, clusters, derived_clusters, ids_by_name, problems) + add_cluster_data_from_xml(root, clusters, pure_base_clusters, ids_by_name, problems) # There are a few clusters where the conformance columns are listed as desc. These clusters need specific, targeted tests # to properly assess conformance. Here, we list them as Optional to allow these for the general test. Targeted tests are described below. @@ -475,11 +484,11 @@ def remove_problem(location: typing.Union[CommandPathLocation, FeaturePathLocati clusters[action_id].accepted_commands[c].conformance = optional() remove_problem(CommandPathLocation(endpoint_id=0, cluster_id=action_id, command_id=c)) - combine_derived_clusters_with_base(clusters, derived_clusters, ids_by_name) + combine_derived_clusters_with_base(clusters, pure_base_clusters, ids_by_name) for alias_base_name, aliased_clusters in CLUSTER_ALIASES.items(): for id, (alias_name, pics) in aliased_clusters.items(): - base = derived_clusters[alias_base_name] + base = pure_base_clusters[alias_base_name] new = deepcopy(base) new.derived = alias_base_name new.name = alias_name @@ -514,7 +523,7 @@ def remove_problem(location: typing.Union[CommandPathLocation, FeaturePathLocati return clusters, problems -def combine_derived_clusters_with_base(xml_clusters: dict[int, XmlCluster], derived_clusters: dict[str, XmlCluster], ids_by_name: dict[str, int]) -> None: +def combine_derived_clusters_with_base(xml_clusters: dict[int, XmlCluster], pure_base_clusters: dict[str, XmlCluster], ids_by_name: dict[str, int]) -> None: ''' Overrides base elements with the derived cluster values for derived clusters. ''' def combine_attributes(base: dict[uint, XmlAttribute], derived: dict[uint, XmlAttribute], cluster_id: uint) -> dict[uint, XmlAttribute]: @@ -547,7 +556,7 @@ def combine_attributes(base: dict[uint, XmlAttribute], derived: dict[uint, XmlAt if base_name in ids_by_name: base = xml_clusters[ids_by_name[c.derived]] else: - base = derived_clusters[base_name] + base = pure_base_clusters[base_name] feature_map = deepcopy(base.feature_map) feature_map.update(c.feature_map)