[Test] Creating CADMIN 1.15 python test module #35524
Draft
+242
−316
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
Sep 10, 2024
j-ororke
commented
- Creating TC_CADMIN_1_15.py test module following test steps in draft PR from Cecille here: 4636
- Removing test_TC_CADMIN_1_15.yaml as no longer needed once python module is created.
- Creating TC_CADMIN_1_15.py test module following test steps in draft PR from Cecille here: CHIP-Specifications/chip-test-plans#4636 - Removing test_TC_CADMIN_1_15.yaml as no longer needed once python module is created.
|
Review changes with SemanticDiff. Analyzed 1 of 2 files.
|
github-actions
bot
added
app
tests
matter-1.4-te2-script-change
|
PR #35524: Size comparison from d1279a8 to fb1ce98 Full report (77 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #35524: Size comparison from d1279a8 to ff650d5 Full report (12 builds for nrfconnect, nxp, qpg, stm32, tizen)
|
- Fixing linting errors
|
PR #35524: Size comparison from d1279a8 to ed04de6 Full report (12 builds for nrfconnect, nxp, qpg, stm32, tizen)
|
- Made minor linting fixes
|
PR #35524: Size comparison from d1279a8 to cdad862 Full report (77 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
- Updated comment verbage to correct information - Updated and removed some expected results for test steps as necessary
|
PR #35524: Size comparison from d1279a8 to a62f7c6 Increases above 0.2%:
Full report (5 builds for nxp, stm32, tizen)
|
|
PR #35524: Size comparison from d1279a8 to 9aa3455 Increases above 0.2%:
Full report (77 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
cecille
reviewed
Sep 20, 2024
src/python_testing/TC_CADMIN_1_15.py
Outdated
|
|
||
|
|
||
| class TC_CADMIN_1_15(MatterBaseTest): | ||
| async def OpenCommissioningWindow(self, th, expectedErrCode) -> CommissioningParameters: |
There was a problem hiding this comment.
see comment on CADMIN-1.11 - it seems clearer here to have a None error code. Also add the type annotations.
There was a problem hiding this comment.
This should now be resolved, thank you
src/python_testing/TC_CADMIN_1_15.py
Outdated
|
|
||
|
|
||
| class TC_CADMIN_1_15(MatterBaseTest): | ||
| async def OpenCommissioningWindow(self, th, expectedErrCode) -> CommissioningParameters: |
There was a problem hiding this comment.
Given that this function appears in a couple of CADMIN tests, it seems like it would make sense to consolidate. Let's get these all in as-is, and then pull them into a single file with a single implementation of this function. The bonus there is we can run them all in one go and save some time at testing. Well, not through the TH, but in the CI.
| nodeId=self.dut_node_id, setupPinCode=setupPinCode, | ||
| filterType=ChipDeviceCtrl.DiscoveryFilterType.LONG_DISCRIMINATOR, filter=self.discriminator) | ||
|
|
||
| def generate_unique_random_value(self, value): |
There was a problem hiding this comment.
does this function get used anywhere? left over from something else?
There was a problem hiding this comment.
Sorry, I will remove this, it was left over in the template from when TC_CADMIN_1_9 test module was created.
src/python_testing/TC_CADMIN_1_15.py
Outdated
|
|
||
| def steps_TC_CADMIN_1_15(self) -> list[TestStep]: | ||
| return [ | ||
| TestStep(1, "Commissioning, already done", is_commissioning=True), |
There was a problem hiding this comment.
something's wacky with the step numbering here - doesn't quite match the test plan, i don't think. Would you mind double checking?
There was a problem hiding this comment.
There was a problem hiding this comment.
the test steps in the test script should now match the test plan
src/python_testing/TC_CADMIN_1_15.py
Outdated
| asserts.fail("Expected number of fabrics not correct") | ||
|
|
||
| if 2 in fabric_indexes2: | ||
| asserts.fail("fabricIndexes should consist of indexes 1, 3, and 4 at this time") |
There was a problem hiding this comment.
Can you change the assertion comment here? The index numbers aren't guaranteed to start from 1
There was a problem hiding this comment.
Thank you for catching this, I have updated the asertion message to report that the fabric indexes should not contain the fabric_idx_2 set value
| fabric_indexes = [fabric.fabricIndex for fabric in fabrics2] | ||
| if len(fabrics2) != initial_number_of_fabrics + 1: | ||
| # len of fabrics is expected to be 2, if 2 not found then we assert failure | ||
| asserts.fail(f"Expected number of fabrics not correct, should show 2, but instead shows {str(len(fabrics2))}") |
There was a problem hiding this comment.
I don't actually care too much, but you don't need str operator in an f string. less typing.
There was a problem hiding this comment.
Thank you, I will remember and change this!
Added changes from Ceciile to this test module Co-authored-by: C Freeman <[email protected]>
- Added type annotations for OpenCommissioningWindow function - Added None type expectedErrorCode variable - Updated test step numbering to match test plan
- Updated test step 15 failure assertion to not start from number 1, but instead to correctly make sure that fabric_idx_cr2 does not exist in current fabric indexes list
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.