binary_sensor.workday: fix handling of states vs provinces #7162
Conversation
|
@drkp, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fabaff and @BastianPoe to be potential reviewers. |
| @@ -66,15 +66,19 @@ def setup_platform(hass, config, add_devices, discovery_info=None): | |||
| obj_holidays = getattr(holidays, country)(years=year) | |||
|
|
|||
| if province: | |||
| if province not in obj_holidays.PROVINCES and \ | |||
| province not in obj_holidays.STATES: | |||
| year = datetime.datetime.now().year | |||
There was a problem hiding this comment.
This looks like a duplicate. See Line 65.
|
Apart from the change request by @fabaff, the rest seems to be fine! |
|
Good catch, fixed. Thanks! |
|
This should further increase the test coverage (that was something I overlooked when originally creating the tests): Could you please create a test for also covering the remaining two (so far uncovered) lines (see https://coveralls.io/builds/11143076/source?filename=homeassistant%2Fcomponents%2Fbinary_sensor%2Fworkday.py#L78 and https://coveralls.io/builds/11143076/source?filename=homeassistant%2Fcomponents%2Fbinary_sensor%2Fworkday.py#L80)? Thanks! |
Patch from Wolf-Bastian Pöttner
|
100% coverage on this component now. |
|
Thanks! 🐬 |
ghost
added
Description:
The holidays module treats states and provinces differently, so specifying a US state as a province does not work correctly. #7017 is incomplete; it prevents an error on startup when a state is specified, but does not load the correct holidays for that state. This is a fix.
Also added test cases.
Related issue (if applicable): fixes #7015
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>
Example entry for
configuration.yaml(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
toxrun successfully. Your PR cannot be merged unless tests passREQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.If the code does not interact with devices:
toxrun successfully. Your PR cannot be merged unless tests pass