[Fix] Update IDM_3_2 test step 3 adding attribute filter to resolve writing attribute type of wrong type#43540
Open
j-ororke wants to merge 5 commits intoproject-chip:masterfrom
Open
Conversation
…ot fail due to finding an unsupported attribute to test that is of incorrect type
…congested test environments.
![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 addresses a TypeError in the TC_IDM_3_2 test case, which occurred when attempting to write to an unsupported attribute of a complex type. The fix refactors the logic to first compile a list of all unsupported attribute candidates. It then iterates through this list, attempting to write a 0 value to each, and uses a try-except block to gracefully handle TypeError for attributes (like structs or lists) that cannot be serialized with a simple integer. The test proceeds upon finding a serializable attribute to verify the expected UNSUPPORTED_ATTRIBUTE error. If no such attribute is found, the test step is now correctly skipped. This change makes the test more robust and effectively resolves the reported issue.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #43540 +/- ##
=======================================
Coverage 54.07% 54.07%
=======================================
Files 1548 1548
Lines 106701 106701
Branches 13309 13309
=======================================
Hits 57699 57699
Misses 49002 49002 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
|
Additionally ran test in TH CLI, please see screenshot of result below: |
|
PR #43540: Size comparison from d93b463 to 31699e5 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Contributor
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This pull request effectively resolves a TypeError in test step 3 of TC_IDM_3_2.py by introducing a more robust method for finding and testing unsupported attributes. The logic now gracefully handles attributes that are not serializable with a default value, which is a good improvement. The increased timeout is also a sensible addition given the more exhaustive search. My only concern is a small, out-of-scope change affecting steps 5 and 6, which would be better placed in a separate PR to keep changes focused.
Removed skips from test step 5 and 6 as not needed since skip is handled by the attribute_guard()
|
PR #43540: Size comparison from e39dcc1 to a7d7340 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
Fixes a TypeError: issubclass() arg 1 must be a class crash in test step 3 as reported in issue 43494.
The fix builds a flat list of all unsupported attribute candidates and iterates through them, skipping any that raise TypeError during serialization, until a candidate that can be successfully written is found. The test assertion remains the same: the DUT must respond with UNSUPPORTED_ATTRIBUTE.
Related issues
Fixes #43494
Testing
Verified by running using following command in WSL:
Verified test runs correctly in the TH UI:
