-
Notifications
You must be signed in to change notification settings - Fork 506
58 unit test fixes #2310
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
58 unit test fixes #2310
Conversation
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 69 files 449 suites 0s ⏱️ Results for commit a8c7cb5. ♻️ This comment has been updated with latest results. |
matter-appliance_coverage.xml
matter-button_coverage.xml
matter-energy_coverage.xml
matter-lock_coverage.xml
matter-media_coverage.xml
matter-rvc_coverage.xml
matter-sensor_coverage.xml
matter-switch_coverage.xml
matter-thermostat_coverage.xml
matter-window-covering_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-thermostat_coverage.xml
zwave-window-treatment_coverage.xml
Minimum allowed coverage is Generated by 🐒 cobertura-action against a8c7cb5 |
drivers/SmartThings/matter-switch/src/test/test_matter_multi_button.lua
Outdated
Show resolved
Hide resolved
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
drivers/SmartThings/matter-thermostat/src/test/test_matter_thermo_battery.lua
Outdated
Show resolved
Hide resolved
This change covers the matter test case updates associated with #2310.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
This commit covers the matter test case updates associated with #2310.
drivers/SmartThings/matter-switch/src/test/test_matter_multi_button_switch_mcd.lua
Outdated
Show resolved
Hide resolved
This commit covers the matter test case updates associated with #2310.
6bb6f9f
to
287d0a3
Compare
64dd095
to
1b353be
Compare
This commit covers the matter test case updates associated with #2310.
0ada9bd
to
44ff4e6
Compare
New features include: * Init messages being generated by the hub instead of in the lua libs * Modular profiles * More native handler registration by default
ca76e0c
to
a8c7cb5
Compare
…y/test/58-unit-test-fixes 58 unit test fixes
This contains unit test fixes for the following features that are changing in 0.58: