58 unit test fixes #2310
Open
58 unit test fixes #2310
Conversation
This file contains hidden or 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
Note how we need only need to simulate the added, init, doConfigure, and infoChanged flow because in those flows we "configure" the buttons by setting various endpoint fields so we know how to handle various button flows. If we were doing the field setting, we could leave the startup messages enabled and use the mock object as is to validate the init sends the correct subscribe and various cap cmd/protocol messages are handled correctly.
|
|
cjswedes
requested review from
aleclorimer,
varzac,
ctowns,
hcarter-775 and
nickolas-deboom
cjswedes
commented
Jul 31, 2025
| local function test_init() | ||
| -- we dont want the integration test framework to generate init/doConfigure, we are doing that here |
There was a problem hiding this comment.
@nickolas-deboom @hcarter-775 @ctowns Most test files will need to be updated with something like this to get the mock devices to go through the added process. This is because often the matter drivers are setting fields and tracking feature support on endpoints during added/init/doConfigure. I would suggest that we make the tests mimick the real added flow where we receive added, then init, and then doConfigure.
cjswedes
commented
Jul 31, 2025
| @@ -73,18 +74,20 @@ local cluster_subscribe_list = { | |||
| } | |||
|
|
|||
| local function test_init() | |||
| -- The startup messages are enabled, so this device will get an init, | |||
| -- and doConfigure (because provisioing_state is TYPED on the device). | |||
| test.mock_device.add_test_device(mock_device) | |||
There was a problem hiding this comment.
We dont need to simulate the whole added/init/doConfigure flow since the driver isn't doing anything special (namely setting fields on the device object); the test framework will queue init and doConfigure automatically, which is why they aren't queued here, but we still are setting expectations on the subscribe and update to provisioning state.
Test Results 68 files 447 suites 0s ⏱️ For more details on these failures, see this check. Results for commit b1242ed. ♻️ This comment has been updated with latest results. |
|
matter-button_coverage.xml
matter-sensor_coverage.xml
matter-switch_coverage.xml
matter-thermostat_coverage.xml
zigbee-contact_coverage.xml
zigbee-humidity-sensor_coverage.xml
zigbee-motion-sensor_coverage.xml
zigbee-power-meter_coverage.xml
zigbee-smoke-detector_coverage.xml
zigbee-sound-sensor_coverage.xml
zigbee-switch_coverage.xml
zigbee-thermostat_coverage.xml
zigbee-water-leak-sensor_coverage.xml
zwave-sensor_coverage.xml
zwave-switch_coverage.xml
zwave-window-treatment_coverage.xml
Minimum allowed coverage is Generated by 🐒 cobertura-action against b1242ed |
cjswedes
commented
Jul 31, 2025
| local subscribe_request = CLUSTER_SUBSCRIBE_LIST[1]:subscribe(mock_device) | ||
| for i, clus in ipairs(CLUSTER_SUBSCRIBE_LIST) do | ||
| if i > 1 then subscribe_request:merge(clus:subscribe(mock_device)) end | ||
| end | ||
| test.socket.matter:__expect_send({mock_device.id, subscribe_request}) | ||
| mock_device:set_field("__ELECTRICAL_TOPOLOGY", {topology = false, tags_on_ep = {}}, {persist = false}) -- since we're assuming this would have happened during device_added in this case. |
There was a problem hiding this comment.
Note how this hack is no longer necessary because it is happening in the actual driver added/init/doConfigure handlers.
cjswedes
commented
Jul 31, 2025
| @@ -249,6 +249,9 @@ local function initialize_mock_device(generic_mock_device, generic_subscribed_at | |||
| test.mock_device.add_test_device(generic_mock_device) | |||
| end | |||
|
|
|||
| -- TODO add tests for configuration using modular profiles | |||
| test.set_rpc_version(7) | |||
There was a problem hiding this comment.
The configure test fail if this is > 7 because then the driver starts using modular profiles. We should add tests for the modular profile usage. cc: @hcarter-775
| @@ -70,6 +70,17 @@ local cluster_subscribe = { | |||
| } | |||
|
|
|||
| local function test_init() | |||
| -- test.socket.device_lifecycle:__queue_receive({ mock_device.id, "added" }) | |||
There was a problem hiding this comment.
Was this intentional?
There was a problem hiding this comment.
This was not intentional. There was a bunch of commits that I squashed into the first commit of this PR, and my guess is this was just something that was carried over. I can update this test.
nickolas-deboom
added a commit
that referenced
this pull request
Jul 31, 2025
This change covers the matter test case updates associated with #2310.
…t temperatureMeasurement - Removed the overloaded handler for `sensor_multilevel_report_handler` for zwave-sensor as it has the same logic as the default handler, and then should be registering for NH - Updated tests from modification above to expect native handler registration messages - Naming fixes for zigbee-switch powerMeter
5 tasks
aleclorimer
reviewed
Jul 31, 2025
| test.disable_startup_messages() | ||
| test.mock_device.add_test_device(mock_device) -- make sure the cache is populated | ||
|
|
||
| -- added sets a bunch of fields on the device, and calls init | ||
| local subscribe_request = CLUSTER_SUBSCRIBE_LIST[1]:subscribe(mock_device) | ||
| for i, clus in ipairs(CLUSTER_SUBSCRIBE_LIST) do | ||
| if i > 1 then subscribe_request:merge(clus:subscribe(mock_device)) end | ||
| end | ||
| test.socket.matter:__expect_send({mock_device.id, subscribe_request}) |
There was a problem hiding this comment.
There was a problem hiding this comment.
Hey Alec, sorry I missed this - yes you are right we do not need to include that double init in the switch driver now that the init event is guaranteed. I have pushed up a PR to remove that init call for hubs running the latest FW, which we can move forward with when these changes roll out: #2339
Once these test fixes go in, I will make the corresponding fixes to the test cases in the other PR as needed.
12 tasks
This commit covers the matter test case updates associated with #2310.
| end | ||
|
|
||
|
|
||
| local function test_init_mcd_unsupported_switch_device_type_old() |
There was a problem hiding this comment.
Was this meant to be removed?
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.
This contains unit test fixes for the following features that are changing in 0.58: