[Fix] Possible fix for matter-test-scripts issue #227: Remove PICS from OPSTATE tests #34290
+433
−185
Conversation
This file contains 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
Contributor
j-ororke
commented
Jul 10, 2024
j-ororke
commented
- Removed PICS checks in TC_OpstateCommon.py module and replaced with attribute and command checks from connected endpoint during tests.
- Removed PICS checks and replaced with attribute and command checks from endpoint during tests.
|
PR #34290: Size comparison from 3c96d5b to d10612e Full report (11 builds for cc32xx, mbed, nrfconnect, qpg, stm32, tizen)
|
- Removed some variables that were no longer needed
|
PR #34290: Size comparison from 3c96d5b to 832661a Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
2 tasks
- Removed automatable PICS checks and replaced with attributes available to be gathered from endpoint.
|
PR #34290: Size comparison from 3c96d5b to 53f6afb Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
- Adding back in missing time import and test runner comments into test module
- Replaced input() with wait_for_user_input() in test script - Added back in short sleep to script, not sure why it got removed.
|
PR #34290: Size comparison from 3c96d5b to e30aefa Full report (3 builds for cc32xx, stm32)
|
|
PR #34290: Size comparison from 3c96d5b to 084a115 Increases above 0.2%:
Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
- Had to remove "test_step" variable that was attempting to be called in wait_for_user_input() as it was not being defined earlier in the test module.
|
PR #34290: Size comparison from 3c96d5b to 3c2ba60 Full report (11 builds for cc32xx, mbed, nrfconnect, qpg, stm32, tizen)
|
- Minor change to remove unneeded f-string from wait_for_user_input().
|
PR #34290: Size comparison from 3c96d5b to ec31380 Increases above 0.2%:
Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
pullapprove
bot
requested review from
andy31415,
andyg-apple,
anush-apple,
arkq,
axelnxp,
bauerschwan,
bzbarsky-apple,
carol-apple,
cecille and
chapongatien
- Resolving linting error
|
PR #34290: Size comparison from 4c69001 to b9df025 Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #34290: Size comparison from 38ad07d to a435556 Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
cecille
reviewed
Dec 4, 2024
| @@ -1451,6 +1455,26 @@ def pics_guard(self, pics_condition: bool): | |||
| self.mark_current_step_skipped() | |||
| return pics_condition | |||
|
|
|||
| async def attributes_guard(self, endpoint: int, attribute: str): | |||
There was a problem hiding this comment.
Suggested change
| async def attributes_guard(self, endpoint: int, attribute: str): | |
| async def attribute_guard(self, endpoint: int, attribute: ClusterObjects.ClusterAttributeDescriptor): |
WDYT about wrapping the async part into a asyncio.run so that the caller doesn't need to await this? I think it might be less error prone for authors.
There was a problem hiding this comment.
I have added the above code and think that is much better than it was previously. 👍
There was a problem hiding this comment.
I think the problem of implementing the asyncio.run() presviously in place of the await here is that I found it was inside an event loop, so if we ran it from the attribute_guard(), then it complained that we can't do that since asyncio.run() has to be run outside of an active event loop. I had also tried with replacing the await in the test itself, but it also complained there as well.
However, recollecting what you mentioned during our teams call on tuesday, you had mentioned to do the assignment of the self.global_wildcard var after the self.is_commissioning was done, thus I have now moved the assignment of the self.global_wildcard var to the setup_test() instead of the setup_class() in matter testing support module, then put it in the if statement for if not self.is_commissioning and self.runner_hook, this allows us to do the assignment of self.global_wildcard at the start of the test runs and allows us to do it in a asyncio.run().
Please let me know if this will be acceptable.
There was a problem hiding this comment.
My apologies, while moving the self.global_wildcard var to the setup_test() in matter_testing support module worked on some tests, it unfortunately did not work on some others, you had mentioned this was a concern during the call as well, but I had not noticed this during initial testing.
I have reverted the code back to a prior version in order to get it to work on all the tests.
…matter_testing.py Adding coding change from Cecille! Co-authored-by: C Freeman <[email protected]>
|
PR #34290: Size comparison from 38ad07d to d18eaa6 Full report (19 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, nrfconnect, qpg, stm32, tizen)
|
- Updated to using attribute_guard function in place of attributes_guard.
|
PR #34290: Size comparison from 38ad07d to 06c7306 Full report (14 builds for cc13x4_26x4, cc32xx, nrfconnect, qpg, stm32, tizen)
|
|
PR #34290: Size comparison from 38ad07d to c5906ab Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
j-ororke
force-pushed
the
remove_PICS_from_OPSTATE_tests
branch
from
c5906ab to
06c7306
Compare
|
PR #34290: Size comparison from ca772d7 to 06c7306 Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #34290: Size comparison from 84fb78f to b5a9905 Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
- Updated matter_testing support module to include new command_guard() and feature_guard() - Created standalone test to show that guard functionality works for CASE, PASE, and no factory reset commissioning - Added TC_TestAttrAvail to slow tests as it takes ~30 seconds to run the tests
|
PR #34290: Size comparison from 84fb78f to e57caad Full report (5 builds for cc32xx, stm32, tizen)
|
- Updated to using new command_guard() for OPSTATE tests
|
PR #34290: Size comparison from 84fb78f to c21d11b Full report (3 builds for cc32xx, stm32)
|
- Resolving linting errors
|
PR #34290: Size comparison from 84fb78f to b1a3e06 Full report (14 builds for cc13x4_26x4, cc32xx, nrfconnect, qpg, stm32, tizen)
|
|
PR #34290: Size comparison from 5d42d92 to 7e22b21 Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
cecille
approved these changes
Dec 11, 2024
src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py
Outdated
Show resolved
Hide resolved
| @@ -97,6 +97,7 @@ slow_tests: | |||
| - { name: TC_PS_2_3.py, duration: 30 seconds } | |||
| - { name: TC_RR_1_1.py, duration: 25 seconds } | |||
| - { name: TC_SWTCH.py, duration: 1 minute } | |||
| - { name: TC_TestAttrAvail.py, duration: 30 seconds } | |||
There was a problem hiding this comment.
Does this test actually take that long?
There was a problem hiding this comment.
This takes a bit of time, maybe not entirely 30 seconds, but due to the commissioning for runs 1 and 2 it does take some time to complete.
…matter_testing.py Adding commi from Cecille to the code here to help better clarify and explain reasoning for this coding change. Co-authored-by: C Freeman <[email protected]>
|
PR #34290: Size comparison from 5d42d92 to 360e51c Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
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.