-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Fix] Proposed resolution of issue #230: remove PICS from timesync tests #33953
[Fix] Proposed resolution of issue #230: remove PICS from timesync tests #33953
Conversation
* Removed PICS from TC_TIMESYNC_2_1 and TC_TIMESYNC_2_2 in python_testing folder * Added using the wait_for_user_input() to allow user to inform if certain features are present on DUT
PR #33953: Size comparison from 4d5e2ee to 0902e50 Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
* Now utilizes Clusters.TimeSynchronization.Bitmaps.Feature to check supported features on DUT in place of PICS or waiting for user input.
module as mentioned in restyled.io check.
Updated method for this fix after being provided some further information from Cecille on Tuesday:
|
from TC_TIMESYNC_2_1 class as no longer needed with recent changes implemented.
- Restored pics_TC_TIMESYNC_2_1 and pics_TC_TIMESYNC_2_2 functions - Added if check for timesource attribute ID for test step 3 in TC_TIMESYNC_2_1 test module - Updated endpoint variable to statically be set to 0
- Changed method for getting attribute_list -- now using get_single_attribute_expect_success() from matter_testing_support module - Updated time source test step 5 to include if check in TC_TIMESYNC_2_2 test module to match with TC_TIMESYNC_2_1
PR #33953: Size comparison from 2908685 to 82929a9 Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good to approve this with the last nits addressed.
- Changed class name value assigned to time_cluster variable - Updated timesync_attr_list and timesource_attr_id variables to contain time_cluster in value in place of Clusters.TimeSynchronization
Adding these suggestions from Cecille after testing in local dev env Co-authored-by: C Freeman <[email protected]>
PR #33953: Size comparison from 2908685 to 3ec7bbc 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)
|
PR #33953: Size comparison from 70744ed to bcbdddb Full report (85 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
… timesync tests (project-chip#33953) * Created proposed resolution of Github issue project-chip#230: * Removed PICS from TC_TIMESYNC_2_1 and TC_TIMESYNC_2_2 in python_testing folder * Added using the wait_for_user_input() to allow user to inform if certain features are present on DUT * Updated TC_TIMESYNC_2_1 test: * Now utilizes Clusters.TimeSynchronization.Bitmaps.Feature to check supported features on DUT in place of PICS or waiting for user input. * Restyled by autopep8 * Applied stylization fix to TC_TIMESYNC_2_2 test module as mentioned in restyled.io check. * Restyled by autopep8 * Removed unnecessary pics_TC_TIMESYNC_2_1() from TC_TIMESYNC_2_1 class as no longer needed with recent changes implemented. * Updates to TC_TIMESYNC_2_1 and 2_2 test modules: - Restored pics_TC_TIMESYNC_2_1 and pics_TC_TIMESYNC_2_2 functions - Added if check for timesource attribute ID for test step 3 in TC_TIMESYNC_2_1 test module - Updated endpoint variable to statically be set to 0 * Restyled by autopep8 * Updated TC_TIMESYNC_2_1 and TC_TIMESYNC_2_2: - Changed method for getting attribute_list -- now using get_single_attribute_expect_success() from matter_testing_support module - Updated time source test step 5 to include if check in TC_TIMESYNC_2_2 test module to match with TC_TIMESYNC_2_1 * Restyled by autopep8 * Updated TC_TIMESYNC_2_2 with suggestions from Cecille - Changed class name value assigned to time_cluster variable - Updated timesync_attr_list and timesource_attr_id variables to contain time_cluster in value in place of Clusters.TimeSynchronization * Apply suggestions from code review from Cecille Adding these suggestions from Cecille after testing in local dev env Co-authored-by: C Freeman <[email protected]> --------- Co-authored-by: Restyled.io <[email protected]> Co-authored-by: C Freeman <[email protected]>
Possible fix for issue 230: remove PICS from timesync tests: