MACL followup: addressing review comments in last PR

project-chip#35413 (comment)

Author: cecille

src/python_testing/TC_ACL_2_11.py

@@ -105,11 +105,11 @@ async def test_TC_ACL_2_11(self):
         self.step(1)
 
         wildcard_read = (await dev_ctrl.Read(self.dut_node_id, [()]))
-        has_arl, has_carl = arls_populated(wildcard_read.tlvAttributes)
+        arl_data = arls_populated(wildcard_read.tlvAttributes)
         asserts.assert_true(
-            has_arl, "ARL attribute must contain at least one restriction to run this test. Please follow manufacturer-specific steps to add access restrictions and re-run this test")
+            arl_data.have_arl, "ARL attribute must contain at least one restriction to run this test. Please follow manufacturer-specific steps to add access restrictions and re-run this test")
         asserts.assert_true(
-            has_carl, "CommissioningARL attribute must contain at least one restriction to run this test. Please follow manufacturer-specific steps to add access restrictions and re-run this test")
+            arl_data.have_carl, "CommissioningARL attribute must contain at least one restriction to run this test. Please follow manufacturer-specific steps to add access restrictions and re-run this test")
 
         self.step(2)
         await self.read_single_attribute_check_success(

src/python_testing/TestConformanceTest.py

@@ -251,33 +251,33 @@ async def test_macl_restrictions(self):
         self.endpoints_tlv = {0: root, 1: nim}
 
         # device with no macl
-        have_arl, have_carl = arls_populated(self.endpoints_tlv)
-        asserts.assert_false(have_arl, "Unexpected ARL found")
-        asserts.assert_false(have_carl, "Unexpected CommissioningARL found")
+        arl_data = arls_populated(self.endpoints_tlv)
+        asserts.assert_false(arl_data.have_arl, "Unexpected ARL found")
+        asserts.assert_false(arl_data.have_carl, "Unexpected CommissioningARL found")
 
         # device with unpopulated macl
         self.add_macl(root)
-        have_arl, have_carl = arls_populated(self.endpoints_tlv)
-        asserts.assert_false(have_arl, "Unexpected ARL found")
-        asserts.assert_false(have_carl, "Unexpected CommissioningARL found")
+        arl_data = arls_populated(self.endpoints_tlv)
+        asserts.assert_false(arl_data.have_arl, "Unexpected ARL found")
+        asserts.assert_false(arl_data.have_carl, "Unexpected CommissioningARL found")
 
         # device with populated ARL
         self.add_macl(root, populate_arl=True)
-        have_arl, have_carl = arls_populated(self.endpoints_tlv)
-        asserts.assert_true(have_arl, "Did not find expected ARL")
-        asserts.assert_false(have_carl, "Unexpected CommissioningARL found")
+        arl_data = arls_populated(self.endpoints_tlv)
+        asserts.assert_true(arl_data.have_arl, "Did not find expected ARL")
+        asserts.assert_false(arl_data.have_carl, "Unexpected CommissioningARL found")
 
         # device with populated commissioning ARL
         self.add_macl(root, populate_commissioning_arl=True)
-        have_arl, have_carl = arls_populated(self.endpoints_tlv)
-        asserts.assert_false(have_arl, "Unexpected ARL found")
-        asserts.assert_true(have_carl, "Did not find expected Commissioning ARL")
+        arl_data = arls_populated(self.endpoints_tlv)
+        asserts.assert_false(arl_data.have_arl, "Unexpected ARL found")
+        asserts.assert_true(arl_data.have_carl, "Did not find expected Commissioning ARL")
 
         # device with both
         self.add_macl(root, populate_arl=True, populate_commissioning_arl=True)
-        have_arl, have_carl = arls_populated(self.endpoints_tlv)
-        asserts.assert_true(have_arl, "Did not find expected ARL")
-        asserts.assert_true(have_carl, "Did not find expected Commissioning ARL")
+        arl_data = arls_populated(self.endpoints_tlv)
+        asserts.assert_true(arl_data.have_arl, "Did not find expected ARL")
+        asserts.assert_true(arl_data.have_carl, "Did not find expected Commissioning ARL")
 
 
 if __name__ == "__main__":

src/python_testing/basic_composition_support.py

@@ -23,6 +23,7 @@
 import pathlib
 import sys
 import typing
+from dataclasses import dataclass
 from pprint import pformat, pprint
 from typing import Any, Optional
 
@@ -33,21 +34,27 @@
 from mobly import asserts
 
 
-def arls_populated(tlv_data: dict[int, Any]) -> tuple[bool, bool]:
+@dataclass
+class ArlData:
+    have_arl: bool
+    have_carl: bool
+
+
+def arls_populated(tlv_data: dict[int, Any]) -> ArlData:
     """ Returns a tuple indicating if the ARL and CommissioningARL are populated.
         Requires a wildcard read of the device TLV.
     """
     # ACL is always on endpoint 0
     if 0 not in tlv_data or Clusters.AccessControl.id not in tlv_data[0]:
-        return (False, False)
+        return ArlData(have_arl=False, have_carl=False)
     # Both attributes are mandatory for this feature, so if one doesn't exist, neither should the other.
     if Clusters.AccessControl.Attributes.Arl.attribute_id not in tlv_data[0][Clusters.AccessControl.id][Clusters.AccessControl.Attributes.AttributeList.attribute_id]:
-        return (False, False)
+        return ArlData(have_arl=False, have_carl=False)
 
     have_arl = tlv_data[0][Clusters.AccessControl.id][Clusters.AccessControl.Attributes.Arl.attribute_id]
     have_carl = tlv_data[0][Clusters.AccessControl.id][Clusters.AccessControl.Attributes.CommissioningARL.attribute_id]
 
-    return (have_arl, have_carl)
+    return ArlData(have_arl=have_arl, have_carl=have_carl)
 
 
 def MatterTlvToJson(tlv_data: dict[int, Any]) -> dict[str, Any]:
@@ -187,11 +194,11 @@ async def setup_class_helper(self, allow_pase: bool = True):
         logging.info("Start of actual tests")
         logging.info("###########################################################")
 
-        have_arl, have_carl = arls_populated(self.endpoints_tlv)
+        arl_data = arls_populated(self.endpoints_tlv)
         asserts.assert_false(
-            have_arl, "ARL cannot be populated for this test - Please follow manufacturer-specific steps to remove the access restrictions and re-run this test")
+            arl_data.have_arl, "ARL cannot be populated for this test - Please follow manufacturer-specific steps to remove the access restrictions and re-run this test")
         asserts.assert_false(
-            have_carl, "CommissioningARL cannot be populated for this test - Please follow manufacturer-specific steps to remove the access restrictions and re-run this test")
+            arl_data.have_carl, "CommissioningARL cannot be populated for this test - Please follow manufacturer-specific steps to remove the access restrictions and re-run this test")
 
     def get_test_name(self) -> str:
         """Return the function name of the caller. Used to create logging entries."""