TC-CGEN-2.2 python test #37262
Open
TC-CGEN-2.2 python test #37262
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
|
Review changes with |
… 44 in TC_CGEN_2_2
|
Currently exploring two improvements for the test: - Handling the waits: The test has long waits, especially in the failsafe process. One option being explored is to minimize these waits in CI by arming the failsafe with a time=0 command, which would trigger the failsafe immediately without unnecessary delays. |
|
PR #37262: Size comparison from b082219 to 36442c5 Full report (3 builds for cc32xx, stm32)
|
|
PR #37262: Size comparison from 3044eeb to f2664b2 Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
cecille
reviewed
Jan 31, 2025
|
Final version. Below are the highlighted updates, changes, and refactoring: Note: The script has passed CI job checkpoints, including Lint code base, Restyled, etc.
|
cecille
reviewed
Feb 12, 2025
src/python_testing/TC_CGEN_2_2.py
Outdated
| logger.info(f'Step #6: The updated size of the num_trusted_roots_original list: {trusted_root_list_original_size_updated}') | ||
|
|
||
| self.step(7) | ||
| if self.is_pics_sdk_ci_only: |
There was a problem hiding this comment.
I think this might actually be fine for ALL devices - you're still seeing the failsafe expire on its timer vs. forcing it to 0 explicitly. ie, it might make sense to just adjust the test to set this to 1s before waiting.
There was a problem hiding this comment.
In the CI flow, the failsafe expiration time is explicitly set to 1 second before invoking the set_failsafe_timer function, ensuring the timer expires as expected.
Would like to make any adjustments to this approach?
There was a problem hiding this comment.
yes, the test is just trying to ensure that the dut times out correctly vs. being forced to expire. We don't have to wait out the original time even in cert - you can set to this 1s for both CI and for real devices and the test results are still valid because the expiration happens as a result of a timer, even if the timer is small. Just add a test step to the test plan, and it cuts down the wait time for this test during both CI and certification.
There was a problem hiding this comment.
Fixed: To manage timeout behavior effectively, CI and Cert tests handled a bit differently: CI uses the PIXIT.CGEN.FailsafeExpiryLengthSeconds value for dynamic timeout adjustment based on the environment, while Cert uses the FAILSAFE_EXPIRATION_SECONDS constant for flexibility in setting longer timeouts if needed.
src/python_testing/TC_CGEN_2_2.py
Outdated
| async def test_TC_CGEN_2_2(self): | ||
|
|
||
| # PIXIT.CGEN.FailsafeExpiryLengthSeconds: | ||
| # Timeout used in test steps to verify failsafe. Must be less than DUT MaxCumulativeFailsafeSeconds |
There was a problem hiding this comment.
Can you hook this up to a user parameter? https://project-chip.github.io/connectedhomeip-doc/testing/python.html#pics-and-pixits. that will let testers adjust this if required.
There was a problem hiding this comment.
Hooked failsafe_expiration_seconds to user parameter PIXIT.CGEN.FailsafeExpiryLengthSeconds for flexible configuration based on CI or Cert flow
There was a problem hiding this comment.
Can you please add a default here? And a check that it's less than MaxCumulativeFailsafeSeconds
There was a problem hiding this comment.
Fixed: Added FAILSAFE_EXPIRATION_SECONDS constant value, for flexibility to adjust the timeout with longer waits if needed it on cert testing
| logger.info('Step #15 - TH2 successfully establish PASE session completed') | ||
|
|
||
| self.step(16) | ||
| logger.info('Step #16 - TH2 Generating a new CSR to update the root certificate...') |
There was a problem hiding this comment.
I meant the process of generating a new cert. But actually, as long as you're always adding a cert that's not the one on the device, you should be able to re-use the first certificate instead of doing this dance every time.
…f the expected exception is not raised
jtrejoespinoza-grid
approved these changes
Feb 13, 2025
…N.FailsafeExpiryLengthSeconds for CI or Cert flow flexibility.
|
PR #37262: Size comparison from ecb3c14 to 722f4e2 Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #37262: Size comparison from ecb3c14 to b1bb5a4 Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
cecille
reviewed
Feb 25, 2025
src/python_testing/TC_CGEN_2_2.py
Outdated
| async def test_TC_CGEN_2_2(self): | ||
|
|
||
| # PIXIT.CGEN.FailsafeExpiryLengthSeconds: | ||
| # Timeout used in test steps to verify failsafe. Must be less than DUT MaxCumulativeFailsafeSeconds |
There was a problem hiding this comment.
Can you please add a default here? And a check that it's less than MaxCumulativeFailsafeSeconds
src/python_testing/TC_CGEN_2_2.py
Outdated
| logger.info(f'Step #6: The updated size of the num_trusted_roots_original list: {trusted_root_list_original_size_updated}') | ||
|
|
||
| self.step(7) | ||
| if self.is_pics_sdk_ci_only: |
There was a problem hiding this comment.
yes, the test is just trying to ensure that the dut times out correctly vs. being forced to expire. We don't have to wait out the original time even in cert - you can set to this 1s for both CI and for real devices and the test results are still valid because the expiration happens as a result of a timer, even if the timer is small. Just add a test step to the test plan, and it cuts down the wait time for this test during both CI and certification.
| # Verify that failsafe_timeout_less_than_max is less than max_fail_safe | ||
| asserts.assert_less(failsafe_timeout_less_than_max, maxFailsafe) | ||
|
|
||
| if self.is_pics_sdk_ci_only: |
There was a problem hiding this comment.
I'm not sure I see the difference here between the CI and the cert test.
There was a problem hiding this comment.
To manage timeout behavior more effectively, we approach CI and Cert tests differently. Both are currently set to 1 second, but they handle timeout a bit differently:
-
CI:
PIXIT.CGEN.FailsafeExpiryLengthSecondsPIXIT value, to adjust the timeout duration based on the CI and specs req. -
[FIXED] Cert:
FAILSAFE_EXPIRATION_SECONDSconstant value, for flexibility to adjust the timeout with longer waits if needed it.
The goal is to maintain flexibility to adjust the timeouts as needed, depending on the type of testing.
… handle timeout more effectively
juandediosg
commented
Feb 26, 2025
… and create constant RUN_TEST_ to define the run test type.
cecille
reviewed
Mar 3, 2025
src/python_testing/TC_CGEN_2_2.py
Outdated
| failsafe_expiration_seconds = self.matter_test_config.global_test_params['PIXIT.CGEN.FailsafeExpiryLengthSeconds'] | ||
| else: | ||
| run_type = self.RUN_TEST_CERT | ||
| failsafe_expiration_seconds = self.CERT_FAILSAFE_EXPIRATION_TIME_SECONDS |
There was a problem hiding this comment.
This wasn't quite what I meant. This failsafe needs to be adjustable for cert. In fact, it's MORE important that it's adjustable for certification because this test will be run against devices that have variable network technologies and conditions. There should be a reasonable default.
So for BOTH CI and certification, you can use
default_failsafe = 20
failsafe_expiration_seconds = self.user_params.get(“PIXIT.CGEN.FailsafeExpiryLengthSeconds”, default_failsafe)
The point was that step 8 is attempting to ensure that the failsafe times out on its own (ie, that the DUT is setting the timer correctly). The time THERE doesn't matter. So instead of step 7 reading
"TH1 waits for PIXIT.CGEN.FailsafeExpiryLengthSeconds to ensure the failsafe timer has expired"
Change that to
"TH1 sends an ArmFailSafe command to the DUT with ExpiryLengthSeconds field set to 1 and the Breadcrumb value as 2. TH1 then waits 1.5 seconds to ensure the failsafe timer has expired"
There was a problem hiding this comment.
Fixed: I have updated the failsafe timer logic for both CI and certification environments: for both CI and certification, the default expiration time is 20 seconds, but it is adjustable through user parameters.
| run_type = self.RUN_TEST_CI | ||
| failsafe_expiration_seconds = self.matter_test_config.global_test_params['PIXIT.CGEN.FailsafeExpiryLengthSeconds'] | ||
| else: | ||
| run_type = self.RUN_TEST_CERT |
There was a problem hiding this comment.
This gets set a bunch of times during the test, but it won't change past this point. (ie, pics are fixed for the duration of the test)
…ation and default value of 20 seconds
juandediosg
commented
Mar 4, 2025
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.
Description
This PR is focused on implementing and validating the TC-CGEN-2.2
Fixes: project-chip/matter-test-scripts#404
Test plan: https://github.com/CHIP-Specifications/chip-test-plans/blob/master/src/cluster/General_Commissioning.adoc
Purpose
This test case validates the behavior of the failsafe timer during the commissioning process, focusing on its expiration, reset behavior, and interactions with commissioning-related commands. The test ensures that the failsafe behaves as expected in various scenarios and has been adapted to bypass waiting for the failsafe to expire, optimizing test execution in CI environments. This approach simulates real-world scenarios such as network misconfigurations, incorrect credentials, expired certificates, and other issues that might occur during commissioning.
Notes
This PR includes a workaround for the waits related to the failsafe timer expiration. As specified in the test plan, the original behavior expected a wait for the failsafe expiry. However, to improve CI testing times and avoid unnecessary delays, the failsafe timer is set to
0seconds using the functionexpire_failsafe_timer().Testing
To run the automated TC-CGEN-2.2 test, follow these steps:
python3 src/python_testing/TC_CGEN_2_2.py --commissioning-method on-network --qr-code MT:-24J0AFN00KA0648G00 --int-arg PIXIT.CGEN.FailsafeExpiryLengthSeconds:20To simulate CI
python3 src/python_testing/TC_CGEN_2_2.py --commissioning-method on-network --qr-code MT:-24J0AFN00KA0648G00 --PICS src/app/tests/suites/certification/ci-pics-values --int-arg PIXIT.CGEN.FailsafeExpiryLengthSeconds:1