TC-CGEN-2.2 python test #37262
TC-CGEN-2.2 python test #37262
Conversation
|
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)
|
src/python_testing/TC_CGEN_2_2.py
Outdated
| @@ -0,0 +1,796 @@ | |||
| # | |||
| # Copyright (c) 2022 Project CHIP Authors | |||
There was a problem hiding this comment.
| # Copyright (c) 2022 Project CHIP Authors | |
| # Copyright (c) 2025 Project CHIP Authors |
There was a problem hiding this comment.
Fixed: Line updated
src/python_testing/TC_CGEN_2_2.py
Outdated
|
|
||
|
|
||
| class TC_CGEN_2_2(MatterBaseTest): | ||
| async def FindAndEstablishPase(self, longDiscriminator: int, setupPinCode: int, nodeid: int, dev_ctrl: ChipDeviceCtrl = None): |
There was a problem hiding this comment.
Can this be replaced with the FindOrEstablishPASESession function from ChipDeviceCtrl.py to avoid duplication?
There was a problem hiding this comment.
Fixed: Replaced with FindOrEstablishPASESession function to avoid duplication
src/python_testing/TC_CGEN_2_2.py
Outdated
| def steps_TC_CGEN_2_2(self) -> list[TestStep]: | ||
| steps = [ | ||
| TestStep(0, "Commissioning, already done", is_commissioning=True), | ||
| TestStep(1, """TH1 reads the TrustedRootCertificates attribute from the Node Operational Credentials cluster |
There was a problem hiding this comment.
None of these are multiline, so I think single quotes are fine here.
src/python_testing/TC_CGEN_2_2.py
Outdated
| async def test_TC_CGEN_2_2(self): | ||
| cluster_opcreds = Clusters.OperationalCredentials | ||
| cluster_cgen = Clusters.GeneralCommissioning | ||
| # maxFailsafe_tmp = 60 |
There was a problem hiding this comment.
| # maxFailsafe_tmp = 60 |
There was a problem hiding this comment.
Fixed: Line removed.
src/python_testing/TC_CGEN_2_2.py
Outdated
|
|
||
| self.step(0) | ||
|
|
||
| # Read the Spteps |
There was a problem hiding this comment.
| # Read the Spteps | |
| # Read the Steps |
There was a problem hiding this comment.
Fixed: Line updated.
src/python_testing/TC_CGEN_2_2.py
Outdated
| logger.info("Step 38 skipped: Saving Tstart (current wall time clock) is bypassed due to failsafe expiration workaround.") | ||
|
|
||
| self.step(39) | ||
| trusted_root_list_original_updated = await self.read_single_attribute_check_success( |
There was a problem hiding this comment.
this isn't from the test plan...
There was a problem hiding this comment.
Fixed: Function removed as it wasn't part of the test plan.
src/python_testing/TC_CGEN_2_2.py
Outdated
| logger.info( | ||
| f'Step #39 - The updated size of the num_trusted_roots_original list: {trusted_root_list_original_size_updated}') | ||
|
|
||
| # Flow generates a new TrustedRootCertificate - Request CSR (Certificate Signing Request) and update NOC (Node Operational Certificate) |
There was a problem hiding this comment.
can re-use the previously generated one.
There was a problem hiding this comment.
Fixed: Reused cert created in step 5.
src/python_testing/TC_CGEN_2_2.py
Outdated
| "Step #31 - Unexpected number of entries in the TrustedRootCertificates table after update") | ||
|
|
||
| self.step(41) | ||
| # Step 41 - Skipped |
There was a problem hiding this comment.
keep this in for cert testing, skip for CI.
There was a problem hiding this comment.
Fixed: Implemented CI and Cert test flows
src/python_testing/TC_CGEN_2_2.py
Outdated
| # which immediately expires the failsafe timer (sets it to 0), bypassing the natural wait and speeding up the test execution. | ||
|
|
||
| # This function bypasses the wait for the FailsafeTimer to expire for TH1 as originally defined in the test plan. | ||
| resp = await self.expire_failsafe_timer(dev_ctrl=self.default_controller, node_id=self.dut_node_id) |
There was a problem hiding this comment.
Fixed: Implemented CI and Cert test flows
src/python_testing/TC_CGEN_2_2.py
Outdated
| cmd = Clusters.OperationalCredentials.Commands.RemoveFabric(fabricIndex=fabric_idx) | ||
| await self.send_single_cmd(dev_ctrl=TH2, node_id=newNodeId+1, cmd=cmd) | ||
|
|
||
| # The expected number of root certificates should be 1 after removing the fabric |
There was a problem hiding this comment.
| # The expected number of root certificates should be 1 after removing the fabric | |
| # The expected number of root certificates should be numTrustedRootsOriginal after removing the fabric |
There was a problem hiding this comment.
Fixed: Comment updated.
|
Final version. Below are the highlighted updates, changes, and refactoring: Note: The script has passed CI job checkpoints, including Lint code base, Restyled, etc.
|
src/python_testing/TC_CGEN_2_2.py
Outdated
| cluster_opcreds = Clusters.OperationalCredentials | ||
| cluster_cgen = Clusters.GeneralCommissioning | ||
|
|
||
| async def expire_failsafe_timer(self, dev_ctrl, node_id, expiration_time_seconds): |
There was a problem hiding this comment.
naming - normally setting the failsafe to 0 is "expiring" or "disarming" the failsafe timer. Any non-zero value is normally "arming" the failsafe timer for that time. a failsafe of 0 is treated as an immediate disarm.
There was a problem hiding this comment.
The function was designed based on your previous suggestion, which recommended a short duration of 1 second to test the expiration of the failsafe timer. To facilitate testing in continuous integration (CI) environments, I hardcoded the 1-second expiration time to speed up and improve the efficiency of the tests. However, the function remains flexible, allowing the expiration_time_seconds to be set to any value, making it easy to adjust for various testing needs or scenarios.
Additionally, to clarify the function's purpose, I have renamed it to set_failsafe_timer, which better indicates that this function is used to configure the failsafe timer.
| response: The response from the command sent to the device. | ||
| ''' | ||
| # Buffer for latency | ||
| buffer_latency = .5 |
There was a problem hiding this comment.
If you use 0 to disarm, you shouldn't need a buffer here.
There was a problem hiding this comment.
The function was designed to always arm and expire the failsafe timer with a positive expiration time. To ensure accurate expiration even with short durations like 1 second, I added a latency buffer to account for potential delays in CI environments. This helps ensure the timer expires correctly, considering possible network or processing latencies
src/python_testing/TC_CGEN_2_2.py
Outdated
| if c_time is None: | ||
| c_time = time.time() | ||
|
|
||
| formatted_time = ( |
There was a problem hiding this comment.
I'm guessing this is for logging, but does having the UTC time help here, or would it make more sense to have the number of elapsed ms (or us or whatever) since t_start?
There was a problem hiding this comment.
I have removed the function that formatted the UTC time, and now I am logging the elapsed_time in milliseconds since t_start in steps 41 and 43 for better precision and relevance to the test flow.
| 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.
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
| 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.
| logger.info(f'Step #20 - TH1 Original size of the trusted_root list: {trusted_roots_list_size}') | ||
|
|
||
| self.step(21) | ||
| # Commissioning stage numbers - we should find a better way to match these to the C++ code |
There was a problem hiding this comment.
It looks like the folks doing some of the new features plumbed through the functions required to do this using the commissioning parameters. Can you take a look at CGEN-2.5 (I James used it a few places). https://github.com/project-chip/connectedhomeip/blob/master/src/python_testing/TC_CGEN_2_5.py#L88. That will make this much neater, and I think it means you don't need to use the test commissioner.
There was a problem hiding this comment.
Currently, it is not possible to implement the function self.commission_devices() due to structural limitations. The use of CommissionDeviceTest conflicts with the requirement of having only one subclass of MatterBaseTest in the main file. Although the function is implemented in TC_CGEN_2_5.py, the test fails because the function cannot be found. Therefore, maintaining the current implementation is the most viable option for now.
…f the expected exception is not raised
…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)
|
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
| 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
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