[Base Wildcard Subscription] Add automatic wildcard subscription validation to MatterBaseTest with C/Q attribute filtering#43274
Draft
j-ororke wants to merge 45 commits intoproject-chip:masterfrom
Draft
Conversation
…dation to MatterBaseTest with C/Q attribute filtering Establishes a per-test background wildcard attribute subscription in MatterBaseTest (started in setup_test, torn down in teardown_test) that validates subscription reporting behaviour across all tests without requiring test authors to add explicit checks. Changes across three modules: spec_parsing.py - Parse changeOmitted and quieterReporting XML quality flags into XmlAttribute.changes_omitted / XmlAttribute.quieter_reporting via new ClusterParser static helpers _is_change_omitted_attribute() and _is_quieter_reporting_attribute() event_attribute_reporting.py - Extend WildcardAttributeSubscriptionHandler (Currently in IDM_4_3 test module PR) with an excluded_attribute_ids frozenset to silently drop C/Q-quality reports - Add _latest_values cache seeded from the priming read (_seed_latest_values_from_priming_read) and updated on every incoming report; exposed via get_latest_value() and latest_values property matter_testing.py - MatterBaseTest.setup_class builds the C/Q exclusion set from the spec DM once per class (_build_cq_excluded_ids) - setup_test starts the subscription (_start_wildcard_subscription); teardown_test shuts it down - verify_attribute_subscription_value() compares a read value against the subscription cache, skipping C/Q attributes automatically - read_single_attribute_check_success() calls the above check automatically (assert_on_error=False) for every successful read on the default controller / DUT node Attributes with Changes Omitted (C) or Quieter Reporting (Q) spec quality flags are excluded from the subscription cache and all comparisons because the spec does not require them to report on every value change.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable enhancement to the Python test framework by adding automatic validation of attribute reports against direct reads via a wildcard subscription. The implementation is well-structured, with clear separation of concerns between spec parsing, the new subscription handler, and the integration into MatterBaseTest.
I have a few suggestions for improvement:
- The most critical one is to change
keepSubscriptions=FalsetoTruewhen starting the wildcard subscription. This will prevent interference with other tests that manage their own subscriptions, addressing the issue noted in the PR description. - Additionally, for critical background tasks like attribute reporting, it's important to let the test fail on unexpected errors rather than catching exceptions, as outlined in repository rules. This has been highlighted as a critical issue. For other exception handling, more specific exception types should be caught.
Overall, this is a great addition that will improve the robustness of the test suite. Thank you for the detailed PR description and for noting the known issue with other subscriptions.
src/python_testing/matter_testing_infrastructure/matter/testing/event_attribute_reporting.py
Show resolved
Hide resolved
src/python_testing/matter_testing_infrastructure/matter/testing/matter_testing.py
Show resolved
Hide resolved
src/python_testing/matter_testing_infrastructure/matter/testing/matter_testing.py
Outdated
Show resolved
Hide resolved
|
PR #43274: Size comparison from faec154 to 25bba16 Full report (6 builds for cc32xx, nrfconnect, realtek, stm32)
|
|
PR #43274: Size comparison from faec154 to dc06974 Full report (6 builds for cc32xx, nrfconnect, realtek, stm32)
|
…g/matter_testing.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
adding import of SpecParsingException from spec_parsing.py
|
PR #43274: Size comparison from faec154 to d199cd6 Full report (6 builds for cc32xx, nrfconnect, realtek, stm32)
|
|
PR #43274: Size comparison from faec154 to 176c5fe Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #43274 +/- ##
=======================================
Coverage 54.32% 54.32%
=======================================
Files 1577 1577
Lines 108271 108271
Branches 13401 13401
=======================================
Hits 58820 58820
Misses 49451 49451 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
PR #43274: Size comparison from 4017426 to e61468e Full report (27 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32)
|
|
PR #43274: Size comparison from 4017426 to bc4551a Full report (27 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32)
|
…ors and resolve ACL test failures from subscription controller The subscription controller was using controller_node_id + 123456, requiring its own ACL entry that was never cleaned up, causing ACL entries to accumulate and breaking ACL tests. Changed to use controller_node_id (same as default_controller) so no extra ACL entry is needed. Also fixed missing super().setup_class() call in TestReadSubscribeAceExistenceErrors and SC_3_6 that left subscription_controller uninitialised.
|
PR #43274: Size comparison from 3e5d368 to 62f8941 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #43274: Size comparison from 4dc9d47 to 9214969 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
…background subscription and new controller initialization for some tests that experience issues with additional ACL entries.
|
PR #43274: Size comparison from 4dc9d47 to 18ab201 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #43274: Size comparison from d93b463 to 03b0c28 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Updating the call to self.verify_attribute_subscription_value() to set assert_on_error back to False in read_single_attribute_check_success() in matter_testing module
|
PR #43274: Size comparison from f4daf7f to 6b62d81 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #43274: Size comparison from a27c039 to f4372e9 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
…atches and add retry loop for value verification Tests that overwrite the DUT ACL remove the subscription controller's Administer entry, causing the DUT to correctly deny subscription report delivery. The validation logic previously flagged this as a DUT bug when it is actually a test framework issue. Changes: - Convert verify_attribute_subscription_value to async - After retry loop exhausts, read the current ACL and check if the subscription controller's entry was removed by the test - If removed, log a warning and skip the error (ACL conflict, not a bug) - If present, proceed with the existing error path (actual DUT bug) - Add get_subscription_acl_entry() helper so tests that write ACLs can opt-in to preserving subscription coverage - Update TC_Breadcrumb_Subscription_Repro.py with ACL conflict test round
|
PR #43274: Size comparison from 56964cc to 9409362 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #43274: Size comparison from 2eff734 to ce79332 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
This was referenced Apr 6, 2026
|
PR #43274: Size comparison from da57c2e to 330714f Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #43274: Size comparison from f403ecc to c0bcf2c Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #43274: Size comparison from 6128265 to 447ca70 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
…syncio error - Add cross-fabric controller skip and fabric-scoped list filtering to verify_attribute_subscription_value to handle multi-fabric test scenarios - Fix teardown_test ACL restore using run_coroutine_threadsafe when the event loop is already running - Align WildcardAttributeSubscriptionHandler.start() default max_interval to 30s to match actual usage - Update docstrings and comments to match current implementation
|
PR #43274: Size comparison from 6128265 to 0edf287 Increases above 0.2%:
Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
…args This test uses fault injection (kFault_ClearInMemoryAllocatedVideoStreams) to clear in-memory stream state inside the read encoder path, bypassing normal attribute mutation APIs and PersistAndNotify. The wildcard subscription cache cannot track these out-of-band changes, causing false mismatch failures.
|
PR #43274: Size comparison from 6128265 to 18a49d9 Increases above 0.2%:
Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #43274: Size comparison from a914062 to 58842c3 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds automatic subscription-based attribute reporting validation to the Matter Python test framework (MatterBaseTest), ensuring all tests verify that subscription reports are consistent with direct reads — without requiring any changes to individual test files.
Root motivation: The Matter spec requires devices to report attribute value changes via subscriptions. Currently, tests validate attribute values by reading them directly but do not cross-check that the same values were (or will be) reported through the subscription path. This gap allows devices with broken subscription reporting to pass all existing tests undetected as mentioned in task 756 created by Cecille.
What changed across three modules:
spec_parsing.py adds:
event_attribute_reporting.py - WildcardAttributeSubscriptionHandler is being extended with:
matter_testing.py - MatterBaseTest gains:
Related issues
Parent Task: 756
Assigned Task: 757
Testing
Verifying with all-clusters-app as the DUT against ACL tests, CADMIN tests, while testing AVSM_2_18-AVSM_2_21 with the camera app: the wildcard subscription starts successfully in setup_test, the _latest_values cache is seeded from the priming read, subsequent reports update the cache, and read_single_attribute_check_success() correctly cross-checks each read value against the cached subscription value.
Confirmed C/Q-quality attributes (e.g. CurrentPositionLiftPercent100ths, CurrentPositionTiltPercent100ths in Window Covering) are excluded from the cache and do not generate false-positive mismatches.
Confirmed that teardown_test cleanly shuts down the subscription between tests with no resource leaks observed in test logs.
Confirmed that the lazy local import of WildcardAttributeSubscriptionHandler inside _start_wildcard_subscription() resolves the circular-import risk between matter_testing.py and event_attribute_reporting.py.
Current Problem Scenario Noticed
Other tests with subscriptions cause issues currently with this method as testing with CADMIN_1_3_4.py results in failure durign test step 9 of CADMIN_1_3 test where it attempts to start a subscription and following error is noticed in log output: