Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@cecille
cecille committed Feb 15, 2024
1 parent aa348dc commit 4c59580
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 25 deletions.
29 changes: 15 additions & 14 deletions src/python_testing/TestSpecParsingSupport.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,19 +223,19 @@ 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)
derived_cluster_xml = ElementTree.fromstring(DERIVED_CLUSTER_XML_STR)
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")
Expand All @@ -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")
Expand All @@ -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)")
Expand Down
31 changes: 20 additions & 11 deletions src/python_testing/spec_parsing_support.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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']
Expand All @@ -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]):
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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]:
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 4c59580

Please sign in to comment.