Skip to content

Fix ledvance unit tests #2918

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.

Sign up for GitHub

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

Merged
cjswedes merged 2 commits into from
Apr 22, 2026
+9 −2
Merged

Fix ledvance unit tests #2918

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
84a60d4
Fix ledvance unit tests
cjswedes Apr 22, 2026
1918c71
Merge branch 'main' into fix/ledvance-test
cjswedes Apr 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions drivers/SmartThings/zigbee-switch/src/test/test_ledvance_metering_plug.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ local mock_device = test.mock_device.build_test_zigbee_device(
zigbee_test_utils.prepare_zigbee_env_info()

local function test_init()
test.disable_startup_messages()
test.mock_device.add_test_device(mock_device)
end

Expand All @@ -37,7 +38,10 @@ test.register_coroutine_test(
test.wait_for_events()
assert(mock_device:get_field(zigbee_constants.SIMPLE_METERING_MULTIPLIER_KEY) == 1)
assert(mock_device:get_field(zigbee_constants.SIMPLE_METERING_DIVISOR_KEY) == 100)
end
end,
{
min_api_version = 15
Copy link
Copy Markdown
Contributor

@aleclorimer aleclorimer Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this tested against 15? HC 0.59 is running lua-libs v17.

Copy link
Copy Markdown
Contributor Author

@cjswedes cjswedes Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put it at 15 because that is when the disable_startup_messages was included in the test framework, so in theory it should work all the way back to version 15 if we ever include those earlier versions

Copy link
Copy Markdown
Contributor Author

@cjswedes cjswedes Apr 22, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing against 15 has unrelated failures due to stateless step capabilities in the driver. New capabilities in a driver are a separate issue that would need to be dealt with if we use those older api versions. Testing on 17 does pass since the capabilities are present

Copy link
Copy Markdown
Contributor

@hcarter-775 hcarter-775 Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if it makes sense to do backwards compat tests with the artificial limitation of capabilities not being generated, which is a UT only situation. To me, this should be worked around by the jenkins test framework rather than the devs, since new capabilities will continue to be made.

Copy link
Copy Markdown
Contributor Author

@cjswedes cjswedes Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we ever include tests for lua libs <17 we will need to address that. I agree that a work around would be best done in jenkins when running the tests. For now I am opting for setting the min_api_version to what it should be, but I can change to 17 if people want.

Copy link
Copy Markdown
Contributor

@hcarter-775 hcarter-775 Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm wondering what will happen when we add statelessWindowStep (or whatever) in the next release and roll that out to the drivers. Will that cause everything to fail?

Copy link
Copy Markdown
Contributor

@aleclorimer aleclorimer Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ill look into updating jenkins to be using the latest capabilities. IDRC if we do 15 or 17 as we likely aren't going to be adding 58 to our testing matrix in the future. The only way we'd go below 17 is if we wanted more support into the Cell hubs back on 52.

Copy link
Copy Markdown
Contributor

@hcarter-775 hcarter-775 Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should support 58, right? We still have some tv hubs on 58, no? Calling this out since HQ caught a 58 bug in one of my PRs about a month ago.

Copy link
Copy Markdown
Contributor Author

@cjswedes cjswedes Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

60 is almost fully released to TVs, so there is not much need to include it. 60 will be the lowest FW version out there (besides the ST Wifi hubs on 52).

}
)

test.register_coroutine_test(
Expand All @@ -49,7 +53,10 @@ test.register_coroutine_test(
test.wait_for_events()
assert(mock_device:get_field(zigbee_constants.SIMPLE_METERING_MULTIPLIER_KEY) == 5)
assert(mock_device:get_field(zigbee_constants.SIMPLE_METERING_DIVISOR_KEY) == 1000)
end
end,
{
min_api_version = 15
}
)

test.run_registered_tests()
Loading