Skip to content

Commit d2a7180

Browse files
[Test Teardown] Cleanup coverage extended to DUTs commissioned during the test (not by the test runner) (project-chip#72906)
* improve logging * remove unused var * clarify comments * fix ci fail * fix ci
1 parent 535a8f2 commit d2a7180

2 files changed

Lines changed: 46 additions & 39 deletions

File tree

src/python_testing/matter_testing_infrastructure/matter/testing/basic_composition.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,10 @@ def log_test_start():
180180
LOGGER.info("###########################################################")
181181

182182
if self.test_from_file:
183+
# File-mode runs have no DUT: skip the pre-test DUT-state capture used by
184+
# teardown cleanup and the background wildcard subscription, both gated on
185+
# requires_dut in setup_test.
186+
self.requires_dut = False
183187
cache = JsonToMatterTlv(self.test_from_file)
184188
self.endpoints = cache.GetUpdatedAttributeCache()
185189
self.endpoints_tlv = cache.attributeTLVCache

src/python_testing/matter_testing_infrastructure/matter/testing/matter_testing.py

Lines changed: 42 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -527,13 +527,15 @@ class MatterBaseTest(base_test.BaseTestClass):
527527
Wildcard subscription (see setup_test), a SEPARATE concern from device classification:
528528
529529
* Set class attribute requires_dut = False for tests that do not interact with a
530-
real DUT (e.g. parser/conformance unit tests under test_testing/). Such tests
531-
will skip the background wildcard subscription so they don't try to subscribe to a
532-
device that isn't there. Default is True. This flag is independent of the
533-
device-STATE markers (MatterTestCommissionedDevice / MatterTestUncommissionedDevice /
534-
MatterTestCommissioner); do not derive one from the other. The sole exception is
535-
CertificationUnitTestNoDevice, which sets requires_dut = False on the base since a
536-
no-device test can never use the subscription.
530+
DUT (e.g. parser/conformance unit tests under test_testing/). Such tests
531+
will skip the background wildcard subscription and the pre-test DUT-state capture,
532+
so they don't attempt network operations against a device that isn't there.
533+
Default is True. This flag is independent of the device-STATE markers
534+
(MatterTestCommissionedDevice / MatterTestUncommissionedDevice / MatterTestCommissioner);
535+
do not derive one from the other. The sole exception is CertificationUnitTestNoDevice,
536+
which sets requires_dut = False on the base since a no-device test can never use
537+
the subscription.
538+
537539
* Set class attribute disable_wildcard_subscription = True to skip the background
538540
wildcard subscription and its ACL side effects — same effect as --no-wildcard-subscription.
539541
* When a wildcard subscription is active, read_single_attribute_check_success compares
@@ -567,9 +569,6 @@ def __init__(self, *args):
567569
# Pre-test snapshot of the DUT's fabric identities
568570
self._original_fabrics = None
569571
self._framework_cleanup_done = False
570-
# Set to True by commission_devices() on success; gates the per-test ACL read in
571-
# setup_test so unit tests (which never commission) incur zero network overhead.
572-
self._dut_confirmed_available = False
573572
# Prevents double-execution when the override calls super().teardown_test()
574573
# and __init_subclass__ also calls it afterward.
575574
self._teardown_ran = False
@@ -1172,10 +1171,12 @@ async def _purge_scenes(self) -> None:
11721171
LOGGER.info("[CLN] wildcard not available, skipping scene cleanup")
11731172
return
11741173

1174+
found_any = False
11751175
for endpoint_id in self.stored_global_wildcard.attributes:
11761176
if not _has_cluster(wildcard=self.stored_global_wildcard, endpoint=endpoint_id,
11771177
cluster=Clusters.ScenesManagement): # type: ignore[arg-type]
11781178
continue
1179+
found_any = True
11791180
if not _has_cluster(wildcard=self.stored_global_wildcard, endpoint=endpoint_id,
11801181
cluster=Clusters.Groups): # type: ignore[arg-type]
11811182
continue
@@ -1199,6 +1200,8 @@ async def _purge_scenes(self) -> None:
11991200
LOGGER.info("[CLN] scenes cleared on endpoint %d", endpoint_id)
12001201
except Exception as e: # DUT may be unreachable or the group may have been removed by the test
12011202
LOGGER.warning("[CLN] scene removal failed on endpoint %d: %s", endpoint_id, e)
1203+
if not found_any:
1204+
LOGGER.info("[CLN] ScenesManagement cluster not present on any endpoint, skipping scene cleanup")
12021205

12031206
async def _purge_group_memberships(self) -> None:
12041207
"""Removes all group memberships from the DUT's group table.
@@ -1459,33 +1462,36 @@ def setup_test(self):
14591462
self._framework_cleanup_done = False
14601463
self.cleanup_config = TestCleanupConfig()
14611464
self._validate_test_parameters()
1462-
# Capture the ACL before the test runs so _reset_acls_to_default can restore it
1463-
# in teardown_class. Skip when the DUT is not known to be available: unit tests
1464-
# never commission a device so _dut_confirmed_available stays False, and
1465-
# commissioning_method is None, eliminating any network overhead for them.
1466-
# For runner-commissioned tests commissioning_method is set; for in-test
1467-
# commissioning the flag is set by commission_devices() on success.
1468-
# is_commissioning is True for CommissionDeviceTest, where the DUT is not yet
1469-
# on the fabric, an operational read there would send CASE Sigma1 to an
1470-
# uncommissioned device, triggering unexpected DUT behaviour.
1471-
dut_expected = (
1472-
not self.is_commissioning
1473-
and (
1474-
self._dut_confirmed_available
1475-
or self.matter_test_config.commissioning_method is not None
1476-
)
1477-
)
1465+
# Capture the ACL so _reset_acls_to_default can restore it during framework cleanup.
1466+
# Captured when:
1467+
# - the DUT is commissioned (runner or in-test)
1468+
# Skipped when:
1469+
# - requires_dut is False (unit tests, file mode)
1470+
# - is_commissioning is True (DUT not on the fabric yet)
1471+
# - the probe fails (PASE-only connection, or DUT absent/unreachable)
1472+
dut_expected = not self.is_commissioning and self.requires_dut
14781473
if dut_expected:
14791474
try:
1480-
self._original_acl = self.event_loop.run_until_complete(
1481-
self.read_single_attribute_check_success(
1482-
cluster=Clusters.AccessControl,
1483-
attribute=Clusters.AccessControl.Attributes.Acl,
1484-
endpoint=0
1485-
)
1486-
)
1487-
except Exception:
1475+
self.event_loop.run_until_complete(
1476+
self.default_controller.GetConnectedDevice(
1477+
nodeId=self.dut_node_id, allowPASE=False, timeoutMs=5000))
1478+
except Exception as e:
1479+
LOGGER.info("[CLN] No CASE session to the DUT (not commissioned, or unreachable), "
1480+
"skipping pre-test ACL capture: %s", e)
14881481
self._original_acl = None
1482+
else:
1483+
try:
1484+
self._original_acl = self.event_loop.run_until_complete(
1485+
self.read_single_attribute_check_success(
1486+
cluster=Clusters.AccessControl,
1487+
attribute=Clusters.AccessControl.Attributes.Acl,
1488+
endpoint=0
1489+
)
1490+
)
1491+
LOGGER.info("[CLN] Pre-test ACL captured (%d entries)", len(self._original_acl))
1492+
except Exception as e:
1493+
LOGGER.warning("[CLN] Pre-test ACL capture failed, teardown will skip ACL restore: %s", e)
1494+
self._original_acl = None
14891495

14901496
# Capture the pre-existing fabrics _remove_extra_fabrics uses this snapshot so that
14911497
# only fabrics added during the test are removed
@@ -1505,7 +1511,7 @@ def setup_test(self):
15051511

15061512
if self.runner_hook and not self.is_commissioning:
15071513
# Start the background wildcard subscription only for tests that interact with a
1508-
# real DUT (requires_dut = True, the default) and unless the test has opted out
1514+
# DUT (requires_dut = True, the default) and unless the test has opted out
15091515
# via --no-wildcard-subscription or disable_wildcard_subscription = True on
15101516
# the test class (e.g. tests that directly manipulate the ACL or tests that count
15111517
# the TH entries).
@@ -2287,10 +2293,7 @@ async def commission_devices(self) -> bool:
22872293
thread_ba_port=self.matter_test_config.thread_ba_port,
22882294
)
22892295

2290-
result = await commission_devices(dev_ctrl, dut_node_ids, setup_payloads, commissioning_info)
2291-
if result:
2292-
self._dut_confirmed_available = True
2293-
return result
2296+
return await commission_devices(dev_ctrl, dut_node_ids, setup_payloads, commissioning_info)
22942297

22952298
async def commission_ntl_device(self, setup_payload: SetupPayload) -> bool:
22962299
"""Commission a single DUT devices over NTL.

0 commit comments

Comments
 (0)