[Test Update] ACL_2_8 test module changes for testing ACL override List method #40238
Conversation
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
…mits Adding support for flag forceLegacyListEncoding, useful when writing Attributes with Legacy Write Request Combining prior commits into one im order to merge latest changes WIP: Example of forcing old Write Encoding fixing typehint Update TC_ACL_2_3.py - Added force_legacy_encoding parameter to TC_ACL_2_3 test - Added support for controlling the forceLegacyListEncoding parameter in the TC_ACL_2_3 test through the existing --bool-arg CLI infrastructure. The parameter defaults to True if not specified and can be set via: --bool-arg force_legacy_encoding:false - This allows testing both legacy and non-legacy list encoding behaviors through the test runner. Update TC_ACL_2_3.py Changed expected result for test step 18, as the result should be D_OK_EMPTY if forced legacy encoding, otherwise it should be D_OK_FULL if using the new encoding restyle Updated TC_ACL_2_3 test module: - Changing to establishing a new local function to contain test code - Running the new local function from the main test function, changing the bool value for the force_legacy_encoding var between iterations Restyled by autopep8 clang-tidy fixes: use make_unique to create unique_ptrs making forceLegacyListEncoding default to False Updating TC_ACL_2_8 python3 test module: - Added legacy mode rerun loop to verify that the test will work for both the new and legacy list write methods. - Added test step 11 to show that the test will be rerun after the new list method has completed. - Added resetting the fabrics inbetween the loops of running the new and legacy write list test runs. - This is for easier review of modified ACL_2_6 test module for override PR here: [38693](project-chip#38693) - Test Plan PR Link: *To be added* Restoring minor changes noticed during rebase
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the TC_ACL_2_8 test to support both legacy and new list write methods by running the test twice with different encoding options. The changes include refactoring the test into an internal function, adding logic to handle different event sequences for legacy vs. new encoding, and cleaning up controller states between test runs.
My review focuses on improving code quality and maintainability. I've identified several areas with duplicated code that could be refactored into helper methods to make the test cleaner and easier to maintain. I've also pointed out a couple of minor style issues, such as an unused import and trailing whitespace, that should be addressed.
Referenced style guide elements: Avoid repetition, match prevailing style, remove unused imports.
|
PR #40238: Size comparison from ccf4d20 to 464c821 Full report (4 builds for cc32xx, nrfconnect, stm32)
|
…uggested by Gemini AI
… and improve maintainability: - Unified event extraction and verification logic for both legacy and new encoding modes - Always verify the initial 'added' event outside the encoding-specific logic - Consolidated fabricIndex checks into a single loop for all relevant events - Improved readability and maintainability by reducing duplicated code in steps 9 (TH1) and 10 (TH2)
|
PR #40238: Size comparison from ccf4d20 to 1bf790f Full report (57 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
j-ororke
changed the title
j-ororke
changed the title
Alami-Amine
approved these changes
Jul 24, 2025
|
PR #40238: Size comparison from 17b6939 to bedb31f Full report (57 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40238 +/- ##
=======================================
Coverage 50.89% 50.89%
=======================================
Files 1341 1341
Lines 98435 98435
Branches 12724 12724
=======================================
Hits 50094 50094
Misses 48341 48341 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
PR #40238: Size comparison from b07cc9d to f5f40a1 Full report (57 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
cecille
reviewed
Aug 12, 2025
cecille
approved these changes
Aug 15, 2025
cecille
added
the
sdk-maintainer-approved
dsavitsky-dsr
pushed a commit
to dsavitsky-dsr/connectedhomeip
that referenced
this pull request
Aug 19, 2025
…st method (project-chip#40238) * Rebasing changes from master into TC_ACL_2_8_Update and squashing commits Adding support for flag forceLegacyListEncoding, useful when writing Attributes with Legacy Write Request Combining prior commits into one im order to merge latest changes WIP: Example of forcing old Write Encoding fixing typehint Update TC_ACL_2_3.py - Added force_legacy_encoding parameter to TC_ACL_2_3 test - Added support for controlling the forceLegacyListEncoding parameter in the TC_ACL_2_3 test through the existing --bool-arg CLI infrastructure. The parameter defaults to True if not specified and can be set via: --bool-arg force_legacy_encoding:false - This allows testing both legacy and non-legacy list encoding behaviors through the test runner. Update TC_ACL_2_3.py Changed expected result for test step 18, as the result should be D_OK_EMPTY if forced legacy encoding, otherwise it should be D_OK_FULL if using the new encoding restyle Updated TC_ACL_2_3 test module: - Changing to establishing a new local function to contain test code - Running the new local function from the main test function, changing the bool value for the force_legacy_encoding var between iterations Restyled by autopep8 clang-tidy fixes: use make_unique to create unique_ptrs making forceLegacyListEncoding default to False Updating TC_ACL_2_8 python3 test module: - Added legacy mode rerun loop to verify that the test will work for both the new and legacy list write methods. - Added test step 11 to show that the test will be rerun after the new list method has completed. - Added resetting the fabrics inbetween the loops of running the new and legacy write list test runs. - This is for easier review of modified ACL_2_6 test module for override PR here: [38693](project-chip#38693) - Test Plan PR Link: *To be added* Restoring minor changes noticed during rebase * Update TC_ACL_2_3.py * Update TC_ACL_2_3.py * Stylizer fixes applied * Resolving linting error * Combining teardown th logic between tests into an async function as suggested by Gemini AI * Refactor test steps 9 and 10 in TC_ACL_2_8 to remove code duplication and improve maintainability: - Unified event extraction and verification logic for both legacy and new encoding modes - Always verify the initial 'added' event outside the encoding-specific logic - Consolidated fabricIndex checks into a single loop for all relevant events - Improved readability and maintainability by reducing duplicated code in steps 9 (TH1) and 10 (TH2) * Re-adding test step 11 * Restyled by autopep8 * Updating verbiage for expected results for test steps 9 and 10 --------- Co-authored-by: Alami-Amine <aalami90@gmail.com> Co-authored-by: Restyled.io <commits@restyled.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Updating TC_ACL_2_8 python3 test module:
Related issues
Testing