[TC_CLTRL-3.1] Added new python test case for Calibrate based on test plan #38299
Conversation
|
|
|
PR #38299: Size comparison from 2e4f22a to 7902086 Full report (10 builds for cc32xx, nrfconnect, qpg, stm32, tizen)
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new Python test case for the Calibrate command based on the test plan for ClosureControl.
- Introduces a dedicated test file implementing multiple test steps for calibrating a DUT
- Implements error handling and reporting for various test steps related to attribute reads and command sends
Comments suppressed due to low confidence (3)
src/python_testing/TC_CLCTRL_3_1.py:105
- The sleep function is used without an import and may block the event loop in an async context; consider using asyncio.sleep or importing time.sleep appropriately.
sleep(2)
src/python_testing/TC_CLCTRL_3_1.py:296
- The variable 'e' is referenced outside of the corresponding except block and may not be defined if no exception is thrown, which could lead to a NameError.
if e.status == Status.INVALID_IN_STATE:
src/python_testing/TC_CLCTRL_3_1.py:281
- [nitpick] A duplicate status check for mainstate is present; consider removing the redundant block to streamline the verification step.
if not type_matches(mainstate.status, Status.SUCCESS):
|
This test script will be validated once the app is finished and added to the CI at the same time. |
|
PR #38299: Size comparison from eeba970 to ffccc86 Full report (3 builds for cc32xx, stm32)
|
| def steps_TC_CLCTRL_3_1(self) -> list[TestStep]: | ||
| steps = [ | ||
| TestStep(1, "Commission DUT to TH (can be skipped if done in a preceding test).", is_commissioning=True), | ||
| TestStep("2a", "TH sends command Calibrate to DUT"), |
There was a problem hiding this comment.
The test plan has a requirements to set the MainState to either SetupRequired or Stop. How is this translated to the test steps?
There was a problem hiding this comment.
@mkardous-silabs I did this draft as solution for missing steps. Please check this and then I change the script like this.
https://github.com/CHIP-Specifications/chip-test-plans/pull/5110
| # STEP 2b: after 1 seconds, TH reads from the DUT the MainState attribute | ||
| self.step("2b") | ||
|
|
||
| sleep(1) |
There was a problem hiding this comment.
Its written in the testplan. See Draft for Testplan change.
src/python_testing/TC_CLCTRL_3_1.py
Outdated
| if e.status == Status.UNSUPPORTED_CLUSTER: | ||
| logging.info("Test step skipped") | ||
| return |
There was a problem hiding this comment.
I think silently failing is the best approach; if we are executing this test and the cluster is not supported, something is definetly wrong either in the pics or the device.
There was a problem hiding this comment.
I changed it to a assert check like the other positions.
IMO it is horrible for a tester if you don't know why it is fail.
|
PR #38299: Size comparison from f28fd9a to 701bc62 Full report (3 builds for cc32xx, stm32)
|
|
PR #38299: Size comparison from f28fd9a to 3dd2de4 Full report (3 builds for cc32xx, stm32)
|
| await self.send_command(endpoint=endpoint, cluster=Clusters.Objects.ClosureControl, command=Clusters.ClosureControl.Commands.Calibrate) | ||
| except InteractionModelError as e: | ||
| asserts.assert_equal( | ||
| e.status, Status.Success, "Unexpected error returned") |
There was a problem hiding this comment.
success is not an exception - this will never happen.
exceptions will propagate on failure - there is no need to try/catch.
the logic below is off: the e access is outside of the except block, so it works on undefined variables.
This test needs to be actually executed before being placed as a PR
There was a problem hiding this comment.
Please complete the Testing section to at least say that you can run it manually. Right now I see no proof it can run (and linter already stops all runs because it does not pass linting).
Marking as changes requested since it did not seem to progress in several weeks.
|
This PR is added in the PR #39015. |
This PR adds python script for testing the Testcase TC-CLCTRL-3.1. This script should test the Calibrate command from the calibration feature of the ClosureControl Cluster.
Testing