-
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
[Test] Creating TC_CADMIN_1_5 test module #36813
base: master
Are you sure you want to change the base?
Conversation
- Following test steps in Test Plan PR CHIP-Specifications/chip-test-plans#4790 created TC_CADMIN_1_5 test module
Changed Files
|
PR #36813: Size comparison from 7ceaf6f to 631d216 Full report (14 builds for cc13x4_26x4, cc32xx, nrfconnect, qpg, stm32, tizen)
|
PR #36813: Size comparison from 75ab4c9 to 7114742 Full report (10 builds for cc32xx, nrfconnect, qpg, stm32, tizen)
|
- Updating to resolve some commissioning flow errors.
- Resolving linting errors
- Removing as will be no longer needed once python module is functioning
PR #36813: Size comparison from 6706f33 to e721ebb Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
- Changed to using self.SendCommand() for OpenCommissioningWindow in order to use salt and PakeVerifier for commissioning flow.
- Resolving linting errors
PR #36813: Size comparison from 6706f33 to 0bc2b1d Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
- Removing test step 9
PR #36813: Size comparison from 6706f33 to db6e212 Full report (3 builds for cc32xx, stm32)
|
- Resolving linting error
PR #36813: Size comparison from 6706f33 to cbd12a6 Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
src/python_testing/TC_CADMIN_1_5.py
Outdated
|
||
async def CommissionOnNetwork(self, setup_code: int, discriminator: int): | ||
ctx = asserts.assert_raises(ChipStackError) | ||
try: |
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.
This logic is a bit weird - could you just remove this and have the expected error be passed in? Or just use two different functions?
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.
Understood, I can definitely see how the logic used there was a bit odd. I have updated the logic to include the expected_error code value to be passed in and used to assert that the correct error code is being caught during the commissioning attempt for test step 7.
However, for test step 4 while it is a timeout error, we don't get an error code back from the SDK as it appears to be asyncio.exception.CancelledError for timeout of the TH, instead of the one we get back from step 7, which is the ChipStackError for timeout.
Please let me know if the new logic will be better for this scenario!
src/python_testing/TC_CADMIN_1_5.py
Outdated
) | ||
return comm_service | ||
|
||
async def CommissionOnNetwork(self, setup_code: int, discriminator: int): |
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.
Can you change the name to snake case and indicate that this function is expecting an error?
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.
Hi Cecille,
I have updated the local CommissionOnNetwork function to now be named SnakeCase.
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 have also included a comment to inform what the function is used for.
src/python_testing/TC_CADMIN_1_5.py
Outdated
filterType=ChipDeviceCtrl.DiscoveryFilterType.LONG_DISCRIMINATOR, filter=discriminator) | ||
errcode = PyChipError.from_code(ctx.exception.err) | ||
|
||
# Timeout error occurs during step 7 for TH2 due to commissioning window being closed already |
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.
isn't the window closed for step 4 as well? Or is this function attempting to catch EITHER a failure to discover the device or a failure to connect over pase?
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.
Hi Cecille,
Correct the commissioning window is noticed to be closed during both steps 4 and 7.
However, for test step 7 we get an ChipStackError, while for test step 4 we get a asyncio.exception.CancelledError. Both reasons display that it is caused due to timeout error in both instances.
TestStep(15, "TH_CR2 starts a commissioning process with DUT_CE", "Commissioning is successful"), | ||
TestStep(16, "TH_CR1 tries to revoke the commissioning window on DUT_CE using RevokeCommissioning command", | ||
"Verify DUT_CE fails to revoke giving status code 4 (WindowNotOpen) as there was no window open"), | ||
] |
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.
Missed this during the test plan review, but can you also add a step here to remove the TH2 fabric from the device as a cleanup step?
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.
Hi Cecille,
I have added the final test step for this test plan to now being removing TH2 fabric using TH2 fabric ID.
I have also updated the Test Plan PR #4812 to include the new final test step for TC_CADMIN_1_5
"Cluster status must be 4 to pass this step as window should be reported as not open") | ||
|
||
self.step(9) | ||
EcmPakeVerifier = b"hex:d0e8a02db8629e9d172dfd40719c89204ff395651a6a2612839a71469880ec2404687d05cf0642b91242c712b5405b6905070c2a4bd80bdc8437ae5a2aded0cf3de91318d16f0ce9450d1c802cc01f39b8761de87cc7eeeb7f52b51308353da49a" |
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.
where did this come from?
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.
Hi Cecille,
I found the above mentioned EcmPakeVerifier while digging through the yaml test scripts, specifically: Test_TC_THNETDIR_2_3
src/python_testing/TC_CADMIN_1_5.py
Outdated
self.th1 = self.default_controller | ||
th2_certificate_authority = self.certificate_authority_manager.NewCertificateAuthority() | ||
th2_fabric_admin = th2_certificate_authority.NewFabricAdmin(vendorId=0xFFF1, fabricId=self.th1.fabricId + 1) | ||
self.th2 = th2_fabric_admin.NewController(nodeId=2, useTestCommissioner=True) |
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.
do you need the test commissioner?
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.
Hi Cecille,
Sorry, you are correct the test commissioner flag did not need to be set when establishing TH2 in this instance, I have removed it from the code here now.
- Removed test commissioner from establishment of TH2 - Added new step to remove TH2 fabric for final step of this test module - Changed CommissionOnNetwork local function name to SnakeCase - Added comment to indicate that the SnakeCase local function expects an error to occur for steps 4 and 7 - Updated SnakeCase function to include expected_error to include error code expected to be noticed during commissioning attempts during test step 7
- Resolving linting errors
- Resolving further noticed linting errors
Creating CADMIN_1_5 python test module: