Add tests, fix styling

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):
+    return request_id
-    _get_instance(hass).notify_errors(request_id, error)
 
 
-def request_done(hass, request_id):
+def notify_errors(request_id, error):
-    _get_instance(hass).request_done(request_id)
+    """ 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:
+        """ Validate that the request belongs to this instance. """
-            _LOGGER.error("Invalid configure id received: %s", request_id)
+        return request_id in self._requests
-            return False
-
-        return True

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,

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) {
+    _sortStates: function() {
-      states.sort(function(one, two) {
+      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;
+      if (this.__pushNewState(new_state)) {
-      var stateFound = false;
+        this._sortStates();
-
-      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);
       }
 
       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._pushNewStates(newStates, true);
-
-        this.fire('states-updated');
 
         this._laterFetchStates();
 

homeassistant/components/http/www_static/polymer/more-infos/more-info-configurator.html

@@ -41,7 +41,7 @@
       </p>
 
       <p class='error' hidden?="{{!stateObj.attributes.errors}}">
-        Errors: {{stateObj.attributes.errors}}
+        {{stateObj.attributes.errors}}
       </p>
 
       <p hidden?="{{!stateObj.attributes.description_image}}">

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)

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")

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

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)

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
+    def test_light_profiles(self):
-        os.remove(user_light_file)
+        """ 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')