Issues 96 - Configuration

- Raising exception for invalid configuration
- Adding more unit tests

mycroft/configuration/__init__.py

@@ -39,22 +39,37 @@ class ConfigurationLoader(object):
     A utility for loading Mycroft configuration files.
     """
 
+    @staticmethod
+    def init_config(config=None):
+        if not config:
+            return {}
+        return config
+
+    @staticmethod
+    def init_locations(locations=None):
+        if not locations:
+            return [DEFAULT_CONFIG, SYSTEM_CONFIG, USER_CONFIG]
+        return locations
+
+    @staticmethod
+    def validate_data(config=None, locations=None):
+        if not (isinstance(config, dict) and isinstance(locations, list)):
+            logger.error("Invalid configuration data type.")
+            logger.error("Locations: %s" % locations)
+            logger.error("Configuration: %s" % config)
+            raise TypeError
+
     @staticmethod
     def load(config=None, locations=None):
         """
         Loads default or specified configuration files
         """
-        if not config:
+        config = ConfigurationLoader.init_config(config)
-            config = {}
+        locations = ConfigurationLoader.init_locations(locations)
+        ConfigurationLoader.validate_data(config, locations)
 
-        if not locations:
+        for location in locations:
-            locations = [DEFAULT_CONFIG, SYSTEM_CONFIG, USER_CONFIG]
+            config = ConfigurationLoader.__load(config, location)
-
-        if isinstance(config, dict) and isinstance(locations, list):
-            for location in locations:
-                config = ConfigurationLoader.__load(config, location)
-        else:
-            logger.debug("Invalid configurations: %s" % locations)
 
         return config
 
@@ -93,14 +108,18 @@ class RemoteConfiguration(object):
         "timezone": "timezone"
     }
 
+    @staticmethod
+    def validate_config(config):
+        if not (config and isinstance(config, dict)):
+            logger.error("Invalid configuration: %s" % config)
+            raise TypeError
+
     @staticmethod
     def load(config=None):
-        if not config or not isinstance(config, dict):
+        RemoteConfiguration.validate_config(config)
-            logger.debug("No valid remote configuration found")
-            return
 
         identity = IdentityManager().get()
-        config_remote = config.get("remote_configuration")
+        config_remote = config.get("remote_configuration", {})
         enabled = str2bool(config_remote.get("enabled", "False"))
 
         if enabled and identity.token:

test/configuration/__init__.py

@@ -1,6 +1,7 @@
 import unittest
 
-from mycroft.configuration import ConfigurationLoader, ConfigurationManager
+from mycroft.configuration import ConfigurationLoader, ConfigurationManager, \
+    DEFAULT_CONFIG, SYSTEM_CONFIG, USER_CONFIG, RemoteConfiguration
 
 __author__ = 'jdorleans'
 
@@ -28,20 +29,42 @@ class AbstractConfigurationTest(unittest.TestCase):
 
 
 class ConfigurationLoaderTest(AbstractConfigurationTest):
+    def test_init_config(self):
+        config = {'a': 'b'}
+        self.assertEquals(ConfigurationLoader.init_config(), {})
+        self.assertEquals(ConfigurationLoader.init_config(config), config)
+
+    def test_init_locations(self):
+        locations = [DEFAULT_CONFIG, SYSTEM_CONFIG, USER_CONFIG]
+        self.assertEquals(ConfigurationLoader.init_locations(), locations)
+
+        locations = ['./mycroft.ini']
+        self.assertEquals(ConfigurationLoader.init_locations(locations),
+                          locations)
+
+    def test_validate_data(self):
+        try:
+            ConfigurationLoader.validate_data({}, [])
+        except TypeError:
+            self.fail()
+
+    def test_validate_data_with_invalid_data(self):
+        self.assertRaises(TypeError, ConfigurationLoader.validate_data)
+
     def test_load(self):
         self.assert_config(ConfigurationLoader.load())
 
-    def test_load_and_override_custom(self):
+    def test_load_with_override_custom(self):
         config = self.create_config('pt-br', 'espeak')
         config = ConfigurationLoader.load(config)
         self.assert_config(config)
 
-    def test_load_and_override_default(self):
+    def test_load_with_override_default(self):
         config = self.create_config()
         config = ConfigurationLoader.load(config, ['./mycroft.ini'])
         self.assert_config(config, 'pt-br', 'espeak')
 
-    def test_load_and_do_not_override_custom(self):
+    def test_load_with_extra_custom(self):
         my_config = {'key': 'value'}
         config = ConfigurationLoader.load(my_config)
         self.assert_config(config)
@@ -51,13 +74,11 @@ class ConfigurationLoaderTest(AbstractConfigurationTest):
         self.assertEquals(value, my_config.get('key'))
 
     def test_load_with_invalid_config_type(self):
-        invalid_type = 'invalid_type'
+        self.assertRaises(TypeError, ConfigurationLoader.load, 'invalid_type')
-        config = ConfigurationLoader.load(invalid_type)
-        self.assertEquals(config, invalid_type)
 
     def test_load_with_invalid_locations_type(self):
-        config = ConfigurationLoader.load(None, './mycroft.ini')
+        self.assertRaises(TypeError, ConfigurationLoader.load,
-        self.assertEquals(config, {})
+                          None, './mycroft.ini')
 
     def test_load_with_invalid_locations_path(self):
         locations = ['./invalid/mycroft.ini', './invalid_mycroft.ini']
@@ -65,15 +86,44 @@ class ConfigurationLoaderTest(AbstractConfigurationTest):
         self.assertEquals(config, {})
 
 
+class RemoteConfigurationTest(AbstractConfigurationTest):
+    def test_validate_config(self):
+        try:
+            RemoteConfiguration.validate_config(self.create_config())
+        except TypeError:
+            self.fail()
+
+    def test_validate_config_with_invalid_config(self):
+        self.assertRaises(TypeError, RemoteConfiguration.validate_config)
+
+    def test_load_without_remote_config(self):
+        config = self.create_config()
+        self.assertEquals(RemoteConfiguration.load(config), config)
+
+
 class ConfigurationManagerTest(AbstractConfigurationTest):
     def test_load_defaults(self):
+        ConfigurationManager.load_defaults()
         self.assert_config(ConfigurationManager.load_defaults())
 
     def test_load_local(self):
+        ConfigurationManager.load_defaults()
         self.assert_config(ConfigurationManager.load_local())
 
+    def test_load_local_with_locations(self):
+        ConfigurationManager.load_defaults()
+        config = ConfigurationManager.load_local(['./mycroft.ini'])
+        self.assert_config(config, 'pt-br', 'espeak')
+
     def test_load_remote(self):
+        ConfigurationManager.load_defaults()
         self.assert_config(ConfigurationManager.load_remote())
 
     def test_get(self):
+        ConfigurationManager.load_defaults()
         self.assert_config(ConfigurationManager.get())
+
+    def test_load_get_with_locations(self):
+        ConfigurationManager.load_defaults()
+        config = ConfigurationManager.get(['./mycroft.ini'])
+        self.assert_config(config, 'pt-br', 'espeak')