From cdbcc844cf3f538375fa8067351db70afcf4f9e7 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Mon, 19 Jan 2015 21:39:24 -0800 Subject: [PATCH] Add tests, fix styling --- homeassistant/components/configurator.py | 75 +++++++---- homeassistant/components/demo.py | 5 +- .../polymer/home-assistant-api.html | 91 ++++++++++---- .../more-infos/more-info-configurator.html | 2 +- homeassistant/components/light/hue.py | 11 +- homeassistant/helpers.py | 1 + tests/config/custom_components/light/test.py | 7 +- tests/test_component_configurator.py | 118 ++++++++++++++++++ tests/test_component_light.py | 15 ++- 9 files changed, 265 insertions(+), 60 deletions(-) create mode 100644 tests/test_component_configurator.py diff --git a/homeassistant/components/configurator.py b/homeassistant/components/configurator.py index 8b465abb9e9..3ece10d4d40 100644 --- a/homeassistant/components/configurator.py +++ b/homeassistant/components/configurator.py @@ -1,7 +1,18 @@ +""" +homeassistant.components.configurator +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +A component to allow pieces of code to request configuration from the user. + +Initiate a request by calling the `request_config` method with a callback. +This will return a request id that has to be used for future calls. +A callback has to be provided to `request_config` which will be called when +the user has submitted configuration information. +""" import logging +import threading from homeassistant.helpers import generate_entity_id -from homeassistant.const import EVENT_TIME_CHANGED DOMAIN = "configurator" DEPENDENCIES = [] @@ -19,30 +30,50 @@ ATTR_SUBMIT_CAPTION = "submit_caption" ATTR_FIELDS = "fields" ATTR_ERRORS = "errors" +_REQUESTS = {} _INSTANCES = {} _LOGGER = logging.getLogger(__name__) +# pylint: disable=too-many-arguments def request_config( hass, name, callback, description=None, description_image=None, submit_caption=None, fields=None): """ Create a new request for config. Will return an ID to be used for sequent calls. """ - return _get_instance(hass).request_config( + instance = _get_instance(hass) + + request_id = instance.request_config( name, callback, description, description_image, submit_caption, fields) + _REQUESTS[request_id] = instance -def notify_errors(hass, request_id, error): - _get_instance(hass).notify_errors(request_id, error) + return request_id -def request_done(hass, request_id): - _get_instance(hass).request_done(request_id) +def notify_errors(request_id, error): + """ Add errors to a config request. """ + try: + _REQUESTS[request_id].notify_errors(request_id, error) + except KeyError: + # If request_id does not exist + pass +def request_done(request_id): + """ Mark a config request as done. """ + try: + _REQUESTS.pop(request_id).request_done(request_id) + except KeyError: + # If request_id does not exist + pass + + +# pylint: disable=unused-argument def setup(hass, config): + """ Set up Configurator. """ return True @@ -51,7 +82,6 @@ def _get_instance(hass): try: return _INSTANCES[hass] except KeyError: - print("Creating instance") _INSTANCES[hass] = Configurator(hass) if DOMAIN not in hass.components: @@ -61,6 +91,10 @@ def _get_instance(hass): class Configurator(object): + """ + Class to keep track of current configuration requests. + """ + def __init__(self, hass): self.hass = hass self._cur_id = 0 @@ -68,6 +102,7 @@ class Configurator(object): hass.services.register( DOMAIN, SERVICE_CONFIGURE, self.handle_service_call) + # pylint: disable=too-many-arguments def request_config( self, name, callback, description, description_image, submit_caption, fields): @@ -120,25 +155,26 @@ class Configurator(object): entity_id = self._requests.pop(request_id)[0] - # If we remove the state right away, it will not be passed down - # with the service request (limitation current design). - # Instead we will set it to configured right away and remove it soon. - def deferred_remove(event): - self.hass.states.remove(entity_id) - - self.hass.bus.listen_once(EVENT_TIME_CHANGED, deferred_remove) - self.hass.states.set(entity_id, STATE_CONFIGURED) + # If we remove the state right away, it will not be included with + # the result fo the service call (limitation current design). + # Instead we will set it to configured to give as feedback but delete + # it shortly after so that it is deleted when the client updates. + threading.Timer( + .001, lambda: self.hass.states.remove(entity_id)).start() + def handle_service_call(self, call): + """ Handle a configure service call. """ request_id = call.data.get(ATTR_CONFIGURE_ID) if not self._validate_request_id(request_id): return + # pylint: disable=unused-variable entity_id, fields, callback = self._requests[request_id] - # TODO field validation? + # field validation goes here? callback(call.data.get(ATTR_FIELDS, {})) @@ -148,8 +184,5 @@ class Configurator(object): return "{}-{}".format(id(self), self._cur_id) def _validate_request_id(self, request_id): - if request_id not in self._requests: - _LOGGER.error("Invalid configure id received: %s", request_id) - return False - - return True + """ Validate that the request belongs to this instance. """ + return request_id in self._requests diff --git a/homeassistant/components/demo.py b/homeassistant/components/demo.py index 62dafffa9d6..cfb37895561 100644 --- a/homeassistant/components/demo.py +++ b/homeassistant/components/demo.py @@ -174,6 +174,7 @@ def setup(hass, config): configurator_ids = [] + # pylint: disable=unused-argument def hue_configuration_callback(data): """ Fake callback, mark config as done. """ time.sleep(2) @@ -181,12 +182,12 @@ def setup(hass, config): # First time it is called, pretend it failed. if len(configurator_ids) == 1: configurator.notify_errors( - hass, configurator_ids[0], + configurator_ids[0], "Failed to register, please try again.") configurator_ids.append(0) else: - configurator.request_done(hass, configurator_ids[0]) + configurator.request_done(configurator_ids[0]) request_id = configurator.request_config( hass, "Philips Hue", hue_configuration_callback, diff --git a/homeassistant/components/http/www_static/polymer/home-assistant-api.html b/homeassistant/components/http/www_static/polymer/home-assistant-api.html index 94be4e0629e..aeffc74cf1a 100644 --- a/homeassistant/components/http/www_static/polymer/home-assistant-api.html +++ b/homeassistant/components/http/www_static/polymer/home-assistant-api.html @@ -137,6 +137,14 @@ }, // local methods + removeState: function(entityId) { + var state = this.getState(entityId); + + if (state !== null) { + this.states.splice(this.states.indexOf(state), 1); + } + }, + getState: function(entityId) { var found = this.states.filter(function(state) { return state.entity_id == entityId; @@ -158,6 +166,11 @@ return states; }, + getEntityIDs: function() { + return this.states.map( + function(state) { return state.entity_id; }); + }, + hasService: function(domain, service) { var found = this.services.filter(function(serv) { return serv.domain == domain && serv.services.indexOf(service) !== -1; @@ -179,8 +192,8 @@ this.stateUpdateTimeout = setTimeout(this.fetchStates.bind(this), 60000); }, - _sortStates: function(states) { - states.sort(function(one, two) { + _sortStates: function() { + this.states.sort(function(one, two) { if (one.entity_id > two.entity_id) { return 1; } else if (one.entity_id < two.entity_id) { @@ -191,32 +204,62 @@ }); }, + /** + * Pushes a new state to the state machine. + * Will resort the states after a push and fire states-updated event. + */ _pushNewState: function(new_state) { - var state; - var stateFound = false; - - for(var i = 0; i < this.states.length; i++) { - if(this.states[i].entity_id == new_state.entity_id) { - state = this.states[i]; - state.attributes = new_state.attributes; - state.last_changed = new_state.last_changed; - state.state = new_state.state; - - stateFound = true; - break; - } - } - - if(!stateFound) { - this.states.push(new State(new_state, this)); - this._sortStates(this.states); + if (this.__pushNewState(new_state)) { + this._sortStates(); } this.fire('states-updated'); }, - _pushNewStates: function(new_states) { - new_states.forEach(this._pushNewState.bind(this)); + /** + * Creates or updates a state. Returns if a new state was added. + */ + __pushNewState: function(new_state) { + var curState = this.getState(new_state.entity_id); + + if (curState === null) { + this.states.push(new State(new_state, this)); + + return true; + } else { + curState.attributes = new_state.attributes; + curState.last_changed = new_state.last_changed; + curState.state = new_state.state; + + return false; + } + }, + + _pushNewStates: function(newStates, removeNonPresent) { + removeNonPresent = !!removeNonPresent; + var currentEntityIds = removeNonPresent ? this.getEntityIDs() : []; + + var hasNew = newStates.reduce(function(hasNew, newState) { + var isNewState = this.__pushNewState(newState); + + if (isNewState) { + return true; + } else if(removeNonPresent) { + currentEntityIds.splice(currentEntityIds.indexOf(newState.entity_id), 1); + } + + return hasNew; + }.bind(this), false); + + currentEntityIds.forEach(function(entityId) { + this.removeState(entityId); + }.bind(this)); + + if (hasNew) { + this._sortStates(); + } + + this.fire('states-updated'); }, // call api methods @@ -236,9 +279,7 @@ fetchStates: function(onSuccess, onError) { var successStatesUpdate = function(newStates) { - this._pushNewStates(newStates); - - this.fire('states-updated'); + this._pushNewStates(newStates, true); this._laterFetchStates(); diff --git a/homeassistant/components/http/www_static/polymer/more-infos/more-info-configurator.html b/homeassistant/components/http/www_static/polymer/more-infos/more-info-configurator.html index 517b1cec296..5b9ace3d13e 100644 --- a/homeassistant/components/http/www_static/polymer/more-infos/more-info-configurator.html +++ b/homeassistant/components/http/www_static/polymer/more-infos/more-info-configurator.html @@ -41,7 +41,7 @@

- Errors: {{stateObj.attributes.errors}} + {{stateObj.attributes.errors}}

diff --git a/homeassistant/components/light/hue.py b/homeassistant/components/light/hue.py index 981a19f1301..8bbe528c6c3 100644 --- a/homeassistant/components/light/hue.py +++ b/homeassistant/components/light/hue.py @@ -26,7 +26,8 @@ _LOGGER = logging.getLogger(__name__) def setup_platform(hass, config, add_devices_callback, discovery_info=None): """ Gets the Hue lights. """ try: - import phue + # pylint: disable=unused-variable + import phue # noqa except ImportError: _LOGGER.exception("Error while importing dependency phue.") @@ -45,6 +46,7 @@ def setup_platform(hass, config, add_devices_callback, discovery_info=None): def setup_bridge(host, hass, add_devices_callback): + """ Setup a phue bridge based on host parameter. """ import phue try: @@ -68,7 +70,7 @@ def setup_bridge(host, hass, add_devices_callback): configurator = get_component('configurator') - configurator.request_done(hass, request_id) + configurator.request_done(request_id) lights = {} @@ -108,13 +110,14 @@ def request_configuration(host, hass, add_devices_callback): """ Request configuration steps from the user. """ configurator = get_component('configurator') - # If this method called while we are configuring, means we got an error + # We got an error if this method is called while we are configuring if host in _CONFIGURING: configurator.notify_errors( - hass, _CONFIGURING[host], "Failed to register, please try again.") + _CONFIGURING[host], "Failed to register, please try again.") return + # pylint: disable=unused-argument def hue_configuration_callback(data): """ Actions to do when our configuration callback is called. """ setup_bridge(host, hass, add_devices_callback) diff --git a/homeassistant/helpers.py b/homeassistant/helpers.py index d2f5f20ee99..4fbffaf1df7 100644 --- a/homeassistant/helpers.py +++ b/homeassistant/helpers.py @@ -13,6 +13,7 @@ from homeassistant.util import ensure_unique_string, slugify def generate_entity_id(entity_id_format, name, current_ids=None, hass=None): + """ Generate a unique entity ID based on given entity IDs or used ids. """ if current_ids is None: if hass is None: raise RuntimeError("Missing required parameter currentids or hass") diff --git a/tests/config/custom_components/light/test.py b/tests/config/custom_components/light/test.py index 9b4ebcac2b1..c557fc8575a 100644 --- a/tests/config/custom_components/light/test.py +++ b/tests/config/custom_components/light/test.py @@ -24,6 +24,11 @@ def init(empty=False): ] -def get_lights(hass, config): +def setup_platform(hass, config, add_devices_callback, discovery_info=None): """ Returns mock devices. """ + add_devices_callback(DEVICES) + + +def get_lights(): + """ Helper method to get current light objects. """ return DEVICES diff --git a/tests/test_component_configurator.py b/tests/test_component_configurator.py new file mode 100644 index 00000000000..a6b1ccf1acf --- /dev/null +++ b/tests/test_component_configurator.py @@ -0,0 +1,118 @@ +""" +tests.test_component_configurator +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Tests Configurator component. +""" +# pylint: disable=too-many-public-methods,protected-access +import unittest +import time + +import homeassistant as ha +import homeassistant.components.configurator as configurator + + +class TestConfigurator(unittest.TestCase): + """ Test the chromecast module. """ + + def setUp(self): # pylint: disable=invalid-name + self.hass = ha.HomeAssistant() + + def tearDown(self): # pylint: disable=invalid-name + """ Stop down stuff we started. """ + self.hass.stop() + + def test_request_least_info(self): + """ Test request config with least amount of data. """ + + request_id = configurator.request_config( + self.hass, "Test Request", lambda _: None) + + self.assertEqual( + 1, len(self.hass.services.services.get(configurator.DOMAIN, [])), + "No new service registered") + + states = self.hass.states.all() + + self.assertEqual(1, len(states), "Expected a new state registered") + + state = states[0] + + self.assertEqual(configurator.STATE_CONFIGURE, state.state) + self.assertEqual( + request_id, state.attributes.get(configurator.ATTR_CONFIGURE_ID)) + + def test_request_all_info(self): + """ Test request config with all possible info. """ + + values = [ + "config_description", "config image url", + "config submit caption", []] + + keys = [ + configurator.ATTR_DESCRIPTION, configurator.ATTR_DESCRIPTION_IMAGE, + configurator.ATTR_SUBMIT_CAPTION, configurator.ATTR_FIELDS] + + exp_attr = dict(zip(keys, values)) + + exp_attr[configurator.ATTR_CONFIGURE_ID] = configurator.request_config( + self.hass, "Test Request", lambda _: None, + *values) + + states = self.hass.states.all() + + self.assertEqual(1, len(states)) + + state = states[0] + + self.assertEqual(configurator.STATE_CONFIGURE, state.state) + self.assertEqual(exp_attr, state.attributes) + + def test_callback_called_on_configure(self): + """ Test if our callback gets called when configure service called. """ + calls = [] + + request_id = configurator.request_config( + self.hass, "Test Request", lambda _: calls.append(1)) + + self.hass.services.call( + configurator.DOMAIN, configurator.SERVICE_CONFIGURE, + {configurator.ATTR_CONFIGURE_ID: request_id}) + + self.hass.pool.block_till_done() + + self.assertEqual(1, len(calls), "Callback not called") + + def test_state_change_on_notify_errors(self): + """ Test state change on notify errors. """ + request_id = configurator.request_config( + self.hass, "Test Request", lambda _: None) + + error = "Oh no bad bad bad" + + configurator.notify_errors(request_id, error) + + state = self.hass.states.all()[0] + + self.assertEqual(error, state.attributes.get(configurator.ATTR_ERRORS)) + + def test_notify_errors_fail_silently_on_bad_request_id(self): + """ Test if notify errors fails silently with a bad request id. """ + configurator.notify_errors(2015, "Try this error") + + def test_request_done_works(self): + """ Test if calling request done works. """ + request_id = configurator.request_config( + self.hass, "Test Request", lambda _: None) + + configurator.request_done(request_id) + + self.assertEqual(1, len(self.hass.states.all())) + + time.sleep(.02) + + self.assertEqual(0, len(self.hass.states.all())) + + def test_request_done_fail_silently_on_bad_request_id(self): + """ Test that request_done fails silently with a bad request id. """ + configurator.request_done(2016) diff --git a/tests/test_component_light.py b/tests/test_component_light.py index c5db2f37299..c9264a37e40 100644 --- a/tests/test_component_light.py +++ b/tests/test_component_light.py @@ -8,7 +8,6 @@ Tests switch component. import unittest import os -import homeassistant as ha import homeassistant.loader as loader import homeassistant.util as util from homeassistant.const import ( @@ -104,7 +103,7 @@ class TestLight(unittest.TestCase): self.assertTrue( light.setup(self.hass, {light.DOMAIN: {CONF_TYPE: 'test'}})) - dev1, dev2, dev3 = platform.get_lights(None, None) + dev1, dev2, dev3 = platform.get_lights() # Test init self.assertTrue(light.is_on(self.hass, dev1.entity_id)) @@ -214,7 +213,7 @@ class TestLight(unittest.TestCase): light.ATTR_XY_COLOR: [prof_x, prof_y]}, data) - def test_light_profiles(self): + def test_broken_light_profiles(self): """ Test light profiles. """ platform = loader.get_component('light.test') platform.init() @@ -230,8 +229,12 @@ class TestLight(unittest.TestCase): self.hass, {light.DOMAIN: {CONF_TYPE: 'test'}} )) - # Clean up broken file - os.remove(user_light_file) + def test_light_profiles(self): + """ Test light profiles. """ + platform = loader.get_component('light.test') + platform.init() + + user_light_file = self.hass.get_config_path(light.LIGHT_PROFILES_FILE) with open(user_light_file, 'w') as user_file: user_file.write('id,x,y,brightness\n') @@ -241,7 +244,7 @@ class TestLight(unittest.TestCase): self.hass, {light.DOMAIN: {CONF_TYPE: 'test'}} )) - dev1, dev2, dev3 = platform.get_lights(None, None) + dev1, dev2, dev3 = platform.get_lights() light.turn_on(self.hass, dev1.entity_id, profile='test')