Fire event when core config is updated#23922
Conversation
balloob
left a comment
balloob
left a comment
There was a problem hiding this comment.
Ok to merge when tests added and last comments addressed
|
|
||
| def set_time_zone(self, time_zone_str: Optional[str]) -> None: | ||
| """Help to set the time zone.""" | ||
| if time_zone_str is None: |
There was a problem hiding this comment.
Let's remove this check and instead not call this function if it's none
| self.time_zone = time_zone | ||
| dt_util.set_default_time_zone(time_zone) | ||
| else: | ||
| _LOGGER.error("Received invalid time zone %s", time_zone_str) |
There was a problem hiding this comment.
Fixed + test added.
|
|
||
| Async friendly. | ||
| """ | ||
| from homeassistant.config import SOURCE_STORAGE |
There was a problem hiding this comment.
Let's store the constant either in const or at top of this file
| mfa_conf)) | ||
|
|
||
| await async_load_ha_core_config(hass) | ||
| await hass.config.load() |
There was a problem hiding this comment.
Can we rename this async_load, in line with other function names that are async.
| await hass.config.update(latitude=50) | ||
|
|
||
| new_core_data = dict(core_data) | ||
| new_core_data['data']['latitude'] = 50 |
There was a problem hiding this comment.
dict on the previous line is not a deep copy of the dictionary. This line is changing the original data.
| async def test_loading_configuration_from_packages(hass): | ||
| """Test loading packages config onto hass object config.""" | ||
| hass.config = mock.Mock() | ||
| hass.config.load.return_value = asyncio.Future() |
There was a problem hiding this comment.
you should be able to drop this. Storage is mocked out in all function based tests.
| with pytest.raises(AssertionError): | ||
| self.config.is_allowed_path(None) | ||
|
|
||
| def test_event_on_update(self): |
There was a problem hiding this comment.
let's not add new tests that are connected to a class.
| def setUp(self): | ||
| """Set up things to be run when tests are started.""" | ||
| self.config = ha.Config() | ||
| self.hass = get_test_home_assistant() |
There was a problem hiding this comment.
Now all tests in this class are getting a HA instance created.
4 participants
Description:
This is a follow-up to #23872
Checklist:
tox. Your PR cannot be merged unless tests passIf the code does not interact with devices: