Add missing support for jewish_calendar.omer_count sensor#24958
Conversation
homeassistant
added
has-tests
integration: jewish_calendar
small-pr
|
Hey there @tsvi, mind taking a look at this pull request as its been labeled with a integration ( This is a automatic comment generated by codeowners-mention to help ensure issues and pull requests are seen by the right people. |
tsvi
left a comment
tsvi
left a comment
There was a problem hiding this comment.
Looking good, please update documentation as well.
|
Doc PR is home-assistant/home-assistant.io#9814. |
|
Thanks |
MartinHjelmare
left a comment
MartinHjelmare
left a comment
There was a problem hiding this comment.
The tests for this platform should preferably be rewritten in a future PR. See below comment.
| test_time = time_zone.localize(now) | ||
| self.hass.config.latitude = latitude | ||
| self.hass.config.longitude = longitude | ||
| sensor = JewishCalSensor( |
There was a problem hiding this comment.
The tests will be more robust if we set up the component and platform via setup_component or async_setup_component, let the platform add an entity, mock time passing to have an update then assert that the entity state is correct by getting it from the core state machine.
There was a problem hiding this comment.
Sounds like a great idea. Do you have an example for something of that type?
There was a problem hiding this comment.
Here's an example that does something similar and also calls a service:
https://github.com/home-assistant/home-assistant/blob/25745e9e27a953073cd4a1906978f5c95d323bb8/tests/components/manual/test_alarm_control_panel.py#L75-L107
4 participants
Description:
One sensor that's documented and should be supported is missing from the actual implementation. This PR fixes that.
Related issue (if applicable): n/a
Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>
Checklist:
tox. Your PR cannot be merged unless tests passIf the code does not interact with devices: