From f814e945d3e62c87c5f86ef5ac37c5feb733b83d Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Sun, 10 May 2015 21:49:04 +0200
Subject: [PATCH 01/11] tests: Convert ext test to pytests

---
 tests/test_ext.py | 48 ++++++++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/tests/test_ext.py b/tests/test_ext.py
index c58f6b20..7b16df83 100644
--- a/tests/test_ext.py
+++ b/tests/test_ext.py
@@ -1,35 +1,41 @@
 from __future__ import absolute_import, unicode_literals
 
-import unittest
+import pytest
 
 from mopidy import config, ext
 
 
-class ExtensionTest(unittest.TestCase):
+@pytest.fixture
+def extension():
+    return ext.Extension()
 
-    def setUp(self):  # noqa: N802
-        self.ext = ext.Extension()
 
-    def test_dist_name_is_none(self):
-        self.assertIsNone(self.ext.dist_name)
+def test_dist_name_is_none(extension):
+    assert extension.dist_name is None
 
-    def test_ext_name_is_none(self):
-        self.assertIsNone(self.ext.ext_name)
 
-    def test_version_is_none(self):
-        self.assertIsNone(self.ext.version)
+def test_ext_name_is_none(extension):
+    assert extension.ext_name is None
 
-    def test_get_default_config_raises_not_implemented(self):
-        with self.assertRaises(NotImplementedError):
-            self.ext.get_default_config()
 
-    def test_get_config_schema_returns_extension_schema(self):
-        schema = self.ext.get_config_schema()
-        self.assertIsInstance(schema['enabled'], config.Boolean)
+def test_version_is_none(extension):
+    assert extension.version is None
 
-    def test_validate_environment_does_nothing_by_default(self):
-        self.assertIsNone(self.ext.validate_environment())
 
-    def test_setup_raises_not_implemented(self):
-        with self.assertRaises(NotImplementedError):
-            self.ext.setup(None)
+def test_get_default_config_raises_not_implemented(extension):
+    with pytest.raises(NotImplementedError):
+        extension.get_default_config()
+
+
+def test_get_config_schema_returns_extension_schema(extension):
+    schema = extension.get_config_schema()
+    assert isinstance(schema['enabled'], config.Boolean)
+
+
+def test_validate_environment_does_nothing_by_default(extension):
+    assert extension.validate_environment() is None
+
+
+def test_setup_raises_not_implemented(extension):
+    with pytest.raises(NotImplementedError):
+        extension.setup(None)

From c4e18f4218fcf9edba427e8e36c81ec301173d2b Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Sun, 10 May 2015 22:10:02 +0200
Subject: [PATCH 02/11] ext: Add ext.load_extensions tests and basic error
 handling

---
 mopidy/ext.py     | 15 ++++++++-
 tests/test_ext.py | 77 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/mopidy/ext.py b/mopidy/ext.py
index 3122611f..37e91a82 100644
--- a/mopidy/ext.py
+++ b/mopidy/ext.py
@@ -148,8 +148,21 @@ def load_extensions():
     for entry_point in pkg_resources.iter_entry_points('mopidy.ext'):
         logger.debug('Loading entry point: %s', entry_point)
         extension_class = entry_point.load(require=False)
-        extension = extension_class()
+
+        try:
+            if not issubclass(extension_class, Extension):
+                continue  # TODO: log this
+        except TypeError:
+            continue  # TODO: log that extension_class is not a class
+
+        try:
+            extension = extension_class()
+        except Exception:
+            continue  # TODO: log this
+
         extension.entry_point = entry_point
+
+        # TODO: store: (instance, entry_point, command, schema, defaults)
         installed_extensions.append(extension)
         logger.debug(
             'Loaded extension: %s %s', extension.dist_name, extension.version)
diff --git a/tests/test_ext.py b/tests/test_ext.py
index 7b16df83..ab54bc07 100644
--- a/tests/test_ext.py
+++ b/tests/test_ext.py
@@ -1,10 +1,14 @@
 from __future__ import absolute_import, unicode_literals
 
+import mock
+
 import pytest
 
 from mopidy import config, ext
 
 
+# ext.Extension
+
 @pytest.fixture
 def extension():
     return ext.Extension()
@@ -39,3 +43,76 @@ def test_validate_environment_does_nothing_by_default(extension):
 def test_setup_raises_not_implemented(extension):
     with pytest.raises(NotImplementedError):
         extension.setup(None)
+
+
+# ext.load_extensions
+
+class TestExtension(ext.Extension):
+    dist_name = 'Mopidy-Foobar'
+    ext_name = 'foobar'
+    version = '1.2.3'
+
+
+@mock.patch('pkg_resources.iter_entry_points')
+def test_load_extensions_no_extenions(mock_entry_points):
+    mock_entry_points.return_value = []
+    assert [] == ext.load_extensions()
+
+
+@mock.patch('pkg_resources.iter_entry_points')
+def test_load_extensions(mock_entry_points):
+    mock_entry_point = mock.Mock()
+    mock_entry_point.load.return_value = TestExtension
+
+    mock_entry_points.return_value = [mock_entry_point]
+
+    extensions = ext.load_extensions()
+    assert len(extensions) == 1
+    assert isinstance(extensions[0], TestExtension)
+
+
+@mock.patch('pkg_resources.iter_entry_points')
+def test_load_extensions_gets_wrong_class(mock_entry_points):
+
+    class WrongClass(object):
+        pass
+
+    mock_entry_point = mock.Mock()
+    mock_entry_point.load.return_value = WrongClass
+
+    mock_entry_points.return_value = [mock_entry_point]
+
+    assert [] == ext.load_extensions()
+
+
+@mock.patch('pkg_resources.iter_entry_points')
+def test_load_extensions_gets_instance(mock_entry_points):
+    mock_entry_point = mock.Mock()
+    mock_entry_point.load.return_value = TestExtension()
+
+    mock_entry_points.return_value = [mock_entry_point]
+
+    assert [] == ext.load_extensions()
+
+
+@mock.patch('pkg_resources.iter_entry_points')
+def test_load_extensions_creating_instance_fails(mock_entry_points):
+    mock_extension = mock.Mock(spec=ext.Extension)
+    mock_extension.side_effect = Exception
+
+    mock_entry_point = mock.Mock()
+    mock_entry_point.load.return_value = mock_extension
+
+    mock_entry_points.return_value = [mock_entry_point]
+    assert [] == ext.load_extensions()
+
+
+@mock.patch('pkg_resources.iter_entry_points')
+def test_load_extensions_store_entry_point(mock_entry_points):
+    mock_entry_point = mock.Mock()
+    mock_entry_point.load.return_value = TestExtension
+    mock_entry_points.return_value = [mock_entry_point]
+
+    extensions = ext.load_extensions()
+    assert len(extensions) == 1
+    assert extensions[0].entry_point == mock_entry_point

From 5937cdc3b250f762472b3c56cd4eb9c3c1c1a1ff Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Sun, 10 May 2015 23:15:13 +0200
Subject: [PATCH 03/11] ext: Add tests for validate_extension and handle
 validate_environment failures

---
 mopidy/ext.py     |  2 ++
 tests/test_ext.py | 90 +++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 86 insertions(+), 6 deletions(-)

diff --git a/mopidy/ext.py b/mopidy/ext.py
index 37e91a82..625c27a2 100644
--- a/mopidy/ext.py
+++ b/mopidy/ext.py
@@ -211,5 +211,7 @@ def validate_extension(extension):
         logger.info(
             'Disabled extension %s: %s', extension.ext_name, ex.message)
         return False
+    except Exception:
+        return False  # TODO: log
 
     return True
diff --git a/tests/test_ext.py b/tests/test_ext.py
index ab54bc07..72e1e141 100644
--- a/tests/test_ext.py
+++ b/tests/test_ext.py
@@ -2,9 +2,17 @@ from __future__ import absolute_import, unicode_literals
 
 import mock
 
+import pkg_resources
+
 import pytest
 
-from mopidy import config, ext
+from mopidy import config, exceptions, ext
+
+
+class TestExtension(ext.Extension):
+    dist_name = 'Mopidy-Foobar'
+    ext_name = 'foobar'
+    version = '1.2.3'
 
 
 # ext.Extension
@@ -47,11 +55,6 @@ def test_setup_raises_not_implemented(extension):
 
 # ext.load_extensions
 
-class TestExtension(ext.Extension):
-    dist_name = 'Mopidy-Foobar'
-    ext_name = 'foobar'
-    version = '1.2.3'
-
 
 @mock.patch('pkg_resources.iter_entry_points')
 def test_load_extensions_no_extenions(mock_entry_points):
@@ -116,3 +119,78 @@ def test_load_extensions_store_entry_point(mock_entry_points):
     extensions = ext.load_extensions()
     assert len(extensions) == 1
     assert extensions[0].entry_point == mock_entry_point
+
+
+# ext.validate_extension
+
+def test_validate_extension_name_mismatch():
+    ep = mock.Mock()
+    ep.name = 'barfoo'
+
+    extension = TestExtension()
+    extension.entry_point = ep
+
+    assert not ext.validate_extension(extension)
+
+
+def test_validate_extension_distribution_not_found():
+    ep = mock.Mock()
+    ep.name = 'foobar'
+    ep.require.side_effect = pkg_resources.DistributionNotFound
+
+    extension = TestExtension()
+    extension.entry_point = ep
+
+    assert not ext.validate_extension(extension)
+
+
+def test_validate_extension_version_conflict():
+    ep = mock.Mock()
+    ep.name = 'foobar'
+    ep.require.side_effect = pkg_resources.VersionConflict
+
+    extension = TestExtension()
+    extension.entry_point = ep
+
+    assert not ext.validate_extension(extension)
+
+
+def test_validate_extension_exception():
+    ep = mock.Mock()
+    ep.name = 'foobar'
+    ep.require.side_effect = Exception
+
+    extension = TestExtension()
+    extension.entry_point = ep
+
+    # We trust that entry points are well behaved, so exception will bubble.
+    with pytest.raises(Exception):
+        assert not ext.validate_extension(extension)
+
+
+def test_validate_extension_instance_validate_env_ext_error():
+    ep = mock.Mock()
+    ep.name = 'foobar'
+
+    extension = TestExtension()
+    extension.entry_point = ep
+
+    with mock.patch.object(extension, 'validate_environment') as validate:
+        validate.side_effect = exceptions.ExtensionError('error')
+
+        assert not ext.validate_extension(extension)
+        validate.assert_called_once_with()
+
+
+def test_validate_extension_instance_validate_env_exception():
+    ep = mock.Mock()
+    ep.name = 'foobar'
+
+    extension = TestExtension()
+    extension.entry_point = ep
+
+    with mock.patch.object(extension, 'validate_environment') as validate:
+        validate.side_effect = Exception
+
+        assert not ext.validate_extension(extension)
+        validate.assert_called_once_with()

From 5550785146aeb0e9426936c2fc69360f18c76118 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Mon, 11 May 2015 00:28:18 +0200
Subject: [PATCH 04/11] ext: Wrap extension state in a ExtensionData tuple

This allows us to do more of the data loading that might fail safely in the
mopidy.ext module instead of having things spread all over the place.

Note that only minimal changes have been made to __main__ to make things work.
Further refactoring should follow.
---
 mopidy/__main__.py |  23 ++++++----
 mopidy/ext.py      |  34 ++++++++++----
 tests/test_ext.py  | 107 +++++++++++++++++++--------------------------
 3 files changed, 86 insertions(+), 78 deletions(-)

diff --git a/mopidy/__main__.py b/mopidy/__main__.py
index 6584146f..4ec0026c 100644
--- a/mopidy/__main__.py
+++ b/mopidy/__main__.py
@@ -68,19 +68,20 @@ def main():
 
         installed_extensions = ext.load_extensions()
 
-        for extension in installed_extensions:
-            ext_cmd = extension.get_command()
-            if ext_cmd:
-                ext_cmd.set(extension=extension)
-                root_cmd.add_child(extension.ext_name, ext_cmd)
+        for data in installed_extensions:
+            if data.command:
+                data.command.set(extension=data.command)
+                root_cmd.add_child(data.extension.ext_name, data.command)
 
         args = root_cmd.parse(mopidy_args)
 
         create_file_structures_and_config(args, installed_extensions)
         check_old_locations()
 
+        # TODO: make config.load use extension data? or just pass in schema+def
         config, config_errors = config_lib.load(
-            args.config_files, installed_extensions, args.config_overrides)
+            args.config_files, [d.extension for d in installed_extensions],
+            args.config_overrides)
 
         verbosity_level = args.base_verbosity_level
         if args.verbosity_level:
@@ -90,8 +91,11 @@ def main():
 
         extensions = {
             'validate': [], 'config': [], 'disabled': [], 'enabled': []}
-        for extension in installed_extensions:
-            if not ext.validate_extension(extension):
+        for data in installed_extensions:
+            extension = data.extension
+
+            # TODO: factor out all of this to a helper that can be tested
+            if not ext.validate_extension(data.extension, data.entry_point):
                 config[extension.ext_name] = {'enabled': False}
                 config_errors[extension.ext_name] = {
                     'enabled': 'extension disabled by self check.'}
@@ -109,6 +113,9 @@ def main():
             else:
                 extensions['enabled'].append(extension)
 
+        # TODO: convert rest of code to use new ExtensionData
+        installed_extensions = [d.extension for d in installed_extensions]
+
         log_extension_info(installed_extensions, extensions['enabled'])
 
         # Config and deps commands are simply special cased for now.
diff --git a/mopidy/ext.py b/mopidy/ext.py
index 625c27a2..32e06d1c 100644
--- a/mopidy/ext.py
+++ b/mopidy/ext.py
@@ -11,6 +11,12 @@ from mopidy import config as config_lib, exceptions
 logger = logging.getLogger(__name__)
 
 
+_extension_data_fields = ['extension', 'entry_point', 'config_schema',
+                          'config_defaults', 'command']
+
+ExtensionData = collections.namedtuple('ExtensionData', _extension_data_fields)
+
+
 class Extension(object):
 
     """Base class for Mopidy extensions"""
@@ -149,6 +155,8 @@ def load_extensions():
         logger.debug('Loading entry point: %s', entry_point)
         extension_class = entry_point.load(require=False)
 
+        # TODO: start using _extension_error_handling(...) pattern
+
         try:
             if not issubclass(extension_class, Extension):
                 continue  # TODO: log this
@@ -160,19 +168,26 @@ def load_extensions():
         except Exception:
             continue  # TODO: log this
 
-        extension.entry_point = entry_point
+        # TODO: handle exceptions and validate result...
+        config_schema = extension.get_config_schema()
+        default_config = extension.get_default_config()
+        command = extension.get_command()
+
+        installed_extensions.append(ExtensionData(
+            extension, entry_point, config_schema, default_config, command))
+
+        # TODO: call validate_extension here?
+        # TODO: do basic config tests like schema contains enabled?
 
-        # TODO: store: (instance, entry_point, command, schema, defaults)
-        installed_extensions.append(extension)
         logger.debug(
             'Loaded extension: %s %s', extension.dist_name, extension.version)
 
-    names = (e.ext_name for e in installed_extensions)
+    names = (ed.extension.ext_name for ed in installed_extensions)
     logger.debug('Discovered extensions: %s', ', '.join(names))
     return installed_extensions
 
 
-def validate_extension(extension):
+def validate_extension(extension, entry_point):
     """Verify extension's dependencies and environment.
 
     :param extensions: an extension to check
@@ -181,15 +196,15 @@ def validate_extension(extension):
 
     logger.debug('Validating extension: %s', extension.ext_name)
 
-    if extension.ext_name != extension.entry_point.name:
+    if extension.ext_name != entry_point.name:
         logger.warning(
             'Disabled extension %(ep)s: entry point name (%(ep)s) '
             'does not match extension name (%(ext)s)',
-            {'ep': extension.entry_point.name, 'ext': extension.ext_name})
+            {'ep': entry_point.name, 'ext': extension.ext_name})
         return False
 
     try:
-        extension.entry_point.require()
+        entry_point.require()
     except pkg_resources.DistributionNotFound as ex:
         logger.info(
             'Disabled extension %s: Dependency %s not found',
@@ -202,7 +217,8 @@ def validate_extension(extension):
                 'Disabled extension %s: %s required, but found %s at %s',
                 extension.ext_name, required, found, found.location)
         else:
-            logger.info('Disabled extension %s: %s', extension.ext_name, ex)
+            logger.info(
+                'Disabled extension %s: %s', extension.ext_name, ex)
         return False
 
     try:
diff --git a/tests/test_ext.py b/tests/test_ext.py
index 72e1e141..d5dad4b1 100644
--- a/tests/test_ext.py
+++ b/tests/test_ext.py
@@ -8,12 +8,20 @@ import pytest
 
 from mopidy import config, exceptions, ext
 
+from tests import IsA, any_unicode
+
 
 class TestExtension(ext.Extension):
     dist_name = 'Mopidy-Foobar'
     ext_name = 'foobar'
     version = '1.2.3'
 
+    def get_default_config(self):
+        return '[foobar]\nenabled = true'
+
+
+any_testextension = IsA(TestExtension)
+
 
 # ext.Extension
 
@@ -69,9 +77,11 @@ def test_load_extensions(mock_entry_points):
 
     mock_entry_points.return_value = [mock_entry_point]
 
-    extensions = ext.load_extensions()
-    assert len(extensions) == 1
-    assert isinstance(extensions[0], TestExtension)
+    expected = ext.ExtensionData(
+        any_testextension, mock_entry_point, IsA(config.ConfigSchema),
+        any_unicode, None)
+
+    assert ext.load_extensions() == [expected]
 
 
 @mock.patch('pkg_resources.iter_entry_points')
@@ -110,87 +120,62 @@ def test_load_extensions_creating_instance_fails(mock_entry_points):
     assert [] == ext.load_extensions()
 
 
-@mock.patch('pkg_resources.iter_entry_points')
-def test_load_extensions_store_entry_point(mock_entry_points):
-    mock_entry_point = mock.Mock()
-    mock_entry_point.load.return_value = TestExtension
-    mock_entry_points.return_value = [mock_entry_point]
-
-    extensions = ext.load_extensions()
-    assert len(extensions) == 1
-    assert extensions[0].entry_point == mock_entry_point
-
-
 # ext.validate_extension
 
-def test_validate_extension_name_mismatch():
-    ep = mock.Mock()
-    ep.name = 'barfoo'
-
+@pytest.fixture
+def ext_data():
     extension = TestExtension()
-    extension.entry_point = ep
 
-    assert not ext.validate_extension(extension)
+    entry_point = mock.Mock()
+    entry_point.name = extension.ext_name
+
+    schema = extension.get_config_schema()
+    defaults = extension.get_default_config()
+    command = extension.get_command()
+
+    return ext.ExtensionData(extension, entry_point, schema, defaults, command)
 
 
-def test_validate_extension_distribution_not_found():
-    ep = mock.Mock()
-    ep.name = 'foobar'
-    ep.require.side_effect = pkg_resources.DistributionNotFound
-
-    extension = TestExtension()
-    extension.entry_point = ep
-
-    assert not ext.validate_extension(extension)
+def test_validate_extension_name_mismatch(ext_data):
+    ext_data.entry_point.name = 'barfoo'
+    assert not ext.validate_extension(ext_data.extension, ext_data.entry_point)
 
 
-def test_validate_extension_version_conflict():
-    ep = mock.Mock()
-    ep.name = 'foobar'
-    ep.require.side_effect = pkg_resources.VersionConflict
-
-    extension = TestExtension()
-    extension.entry_point = ep
-
-    assert not ext.validate_extension(extension)
+def test_validate_extension_distribution_not_found(ext_data):
+    error = pkg_resources.DistributionNotFound
+    ext_data.entry_point.require.side_effect = error
+    assert not ext.validate_extension(ext_data.extension, ext_data.entry_point)
 
 
-def test_validate_extension_exception():
-    ep = mock.Mock()
-    ep.name = 'foobar'
-    ep.require.side_effect = Exception
+def test_validate_extension_version_conflict(ext_data):
+    ext_data.entry_point.require.side_effect = pkg_resources.VersionConflict
+    assert not ext.validate_extension(ext_data.extension, ext_data.entry_point)
 
-    extension = TestExtension()
-    extension.entry_point = ep
+
+def test_validate_extension_exception(ext_data):
+    ext_data.entry_point.require.side_effect = Exception
 
     # We trust that entry points are well behaved, so exception will bubble.
     with pytest.raises(Exception):
-        assert not ext.validate_extension(extension)
+        assert not ext.validate_extension(
+            ext_data.extension, ext_data.entry_point)
 
 
-def test_validate_extension_instance_validate_env_ext_error():
-    ep = mock.Mock()
-    ep.name = 'foobar'
-
-    extension = TestExtension()
-    extension.entry_point = ep
-
+def test_validate_extension_instance_validate_env_ext_error(ext_data):
+    extension = ext_data.extension
     with mock.patch.object(extension, 'validate_environment') as validate:
         validate.side_effect = exceptions.ExtensionError('error')
 
-        assert not ext.validate_extension(extension)
+        assert not ext.validate_extension(
+            ext_data.extension, ext_data.entry_point)
         validate.assert_called_once_with()
 
 
-def test_validate_extension_instance_validate_env_exception():
-    ep = mock.Mock()
-    ep.name = 'foobar'
-
-    extension = TestExtension()
-    extension.entry_point = ep
-
+def test_validate_extension_instance_validate_env_exception(ext_data):
+    extension = ext_data.extension
     with mock.patch.object(extension, 'validate_environment') as validate:
         validate.side_effect = Exception
 
-        assert not ext.validate_extension(extension)
+        assert not ext.validate_extension(
+            ext_data.extension, ext_data.entry_point)
         validate.assert_called_once_with()

From 8434a22c83871385ca4e5bcacd4bb6118e898450 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Mon, 11 May 2015 20:53:50 +0200
Subject: [PATCH 05/11] ext: Switch to using fixtures for mocking

---
 tests/test_ext.py | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/tests/test_ext.py b/tests/test_ext.py
index d5dad4b1..4d9a4638 100644
--- a/tests/test_ext.py
+++ b/tests/test_ext.py
@@ -63,19 +63,25 @@ def test_setup_raises_not_implemented(extension):
 
 # ext.load_extensions
 
+@pytest.fixture
+def iter_entry_points_mock(request):
+    patcher = mock.patch('pkg_resources.iter_entry_points')
+    iter_entry_points = patcher.start()
+    iter_entry_points.return_value = []
+    request.addfinalizer(patcher.stop)
+    return iter_entry_points
 
-@mock.patch('pkg_resources.iter_entry_points')
-def test_load_extensions_no_extenions(mock_entry_points):
-    mock_entry_points.return_value = []
+
+def test_load_extensions_no_extenions(iter_entry_points_mock):
+    iter_entry_points_mock.return_value = []
     assert [] == ext.load_extensions()
 
 
-@mock.patch('pkg_resources.iter_entry_points')
-def test_load_extensions(mock_entry_points):
+def test_load_extensions(iter_entry_points_mock):
     mock_entry_point = mock.Mock()
     mock_entry_point.load.return_value = TestExtension
 
-    mock_entry_points.return_value = [mock_entry_point]
+    iter_entry_points_mock.return_value = [mock_entry_point]
 
     expected = ext.ExtensionData(
         any_testextension, mock_entry_point, IsA(config.ConfigSchema),
@@ -84,8 +90,7 @@ def test_load_extensions(mock_entry_points):
     assert ext.load_extensions() == [expected]
 
 
-@mock.patch('pkg_resources.iter_entry_points')
-def test_load_extensions_gets_wrong_class(mock_entry_points):
+def test_load_extensions_gets_wrong_class(iter_entry_points_mock):
 
     class WrongClass(object):
         pass
@@ -93,30 +98,29 @@ def test_load_extensions_gets_wrong_class(mock_entry_points):
     mock_entry_point = mock.Mock()
     mock_entry_point.load.return_value = WrongClass
 
-    mock_entry_points.return_value = [mock_entry_point]
+    iter_entry_points_mock.return_value = [mock_entry_point]
 
     assert [] == ext.load_extensions()
 
 
-@mock.patch('pkg_resources.iter_entry_points')
-def test_load_extensions_gets_instance(mock_entry_points):
+def test_load_extensions_gets_instance(iter_entry_points_mock):
     mock_entry_point = mock.Mock()
     mock_entry_point.load.return_value = TestExtension()
 
-    mock_entry_points.return_value = [mock_entry_point]
+    iter_entry_points_mock.return_value = [mock_entry_point]
 
     assert [] == ext.load_extensions()
 
 
-@mock.patch('pkg_resources.iter_entry_points')
-def test_load_extensions_creating_instance_fails(mock_entry_points):
+def test_load_extensions_creating_instance_fails(iter_entry_points_mock):
     mock_extension = mock.Mock(spec=ext.Extension)
     mock_extension.side_effect = Exception
 
     mock_entry_point = mock.Mock()
     mock_entry_point.load.return_value = mock_extension
 
-    mock_entry_points.return_value = [mock_entry_point]
+    iter_entry_points_mock.return_value = [mock_entry_point]
+
     assert [] == ext.load_extensions()
 
 

From 8ed9e5f1e07a492ba45f698c5083bc9dee16edc3 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Mon, 11 May 2015 21:20:37 +0200
Subject: [PATCH 06/11] ext: Catch exceptions in extension helpers

---
 mopidy/ext.py     | 10 ++++++----
 tests/test_ext.py | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/mopidy/ext.py b/mopidy/ext.py
index 32e06d1c..a59eeae1 100644
--- a/mopidy/ext.py
+++ b/mopidy/ext.py
@@ -165,13 +165,15 @@ def load_extensions():
 
         try:
             extension = extension_class()
+            config_schema = extension.get_config_schema()
+            default_config = extension.get_default_config()
         except Exception:
             continue  # TODO: log this
 
-        # TODO: handle exceptions and validate result...
-        config_schema = extension.get_config_schema()
-        default_config = extension.get_default_config()
-        command = extension.get_command()
+        try:
+            command = extension.get_command()
+        except Exception:
+            command = None  # TODO: log this.
 
         installed_extensions.append(ExtensionData(
             extension, entry_point, config_schema, default_config, command))
diff --git a/tests/test_ext.py b/tests/test_ext.py
index 4d9a4638..f6f31c21 100644
--- a/tests/test_ext.py
+++ b/tests/test_ext.py
@@ -124,6 +124,46 @@ def test_load_extensions_creating_instance_fails(iter_entry_points_mock):
     assert [] == ext.load_extensions()
 
 
+def test_load_extensions_get_config_schema_fails(iter_entry_points_mock):
+    mock_entry_point = mock.Mock()
+    mock_entry_point.load.return_value = TestExtension
+
+    iter_entry_points_mock.return_value = [mock_entry_point]
+
+    with mock.patch.object(TestExtension, 'get_config_schema') as get_schema:
+        get_schema.side_effect = Exception
+        assert [] == ext.load_extensions()
+        get_schema.assert_called_once_with()
+
+
+def test_load_extensions_get_default_config_fails(iter_entry_points_mock):
+    mock_entry_point = mock.Mock()
+    mock_entry_point.load.return_value = TestExtension
+
+    iter_entry_points_mock.return_value = [mock_entry_point]
+
+    with mock.patch.object(TestExtension, 'get_default_config') as get_default:
+        get_default.side_effect = Exception
+        assert [] == ext.load_extensions()
+        get_default.assert_called_once_with()
+
+
+def test_load_extensions_get_command_fails(iter_entry_points_mock):
+    mock_entry_point = mock.Mock()
+    mock_entry_point.load.return_value = TestExtension
+
+    iter_entry_points_mock.return_value = [mock_entry_point]
+
+    expected = ext.ExtensionData(
+        any_testextension, mock_entry_point, IsA(config.ConfigSchema),
+        any_unicode, None)
+
+    with mock.patch.object(TestExtension, 'get_command') as get_command:
+        get_command.side_effect = Exception
+        assert [expected] == ext.load_extensions()
+        get_command.assert_called_once_with()
+
+
 # ext.validate_extension
 
 @pytest.fixture

From 4566ddd9ae2b9619b046d4686d13e30c09ff8dfd Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Mon, 11 May 2015 21:29:03 +0200
Subject: [PATCH 07/11] ext: Add exception logging to extension loading

---
 mopidy/ext.py     | 19 +++++++------------
 tests/test_ext.py |  6 +-----
 2 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/mopidy/ext.py b/mopidy/ext.py
index a59eeae1..96e5d5e1 100644
--- a/mopidy/ext.py
+++ b/mopidy/ext.py
@@ -155,32 +155,27 @@ def load_extensions():
         logger.debug('Loading entry point: %s', entry_point)
         extension_class = entry_point.load(require=False)
 
-        # TODO: start using _extension_error_handling(...) pattern
-
         try:
             if not issubclass(extension_class, Extension):
-                continue  # TODO: log this
+                raise TypeError  # issubclass raises TypeError on non-class
         except TypeError:
-            continue  # TODO: log that extension_class is not a class
+            logger.error('Entry point %s did not contain a valid extension'
+                         'class: %r', entry_point.name, extension_class)
+            continue
 
         try:
             extension = extension_class()
             config_schema = extension.get_config_schema()
             default_config = extension.get_default_config()
-        except Exception:
-            continue  # TODO: log this
-
-        try:
             command = extension.get_command()
         except Exception:
-            command = None  # TODO: log this.
+            logger.exception('Setup of extension from entry point %s failed, '
+                             'ignoring extension.', entry_point.name)
+            continue
 
         installed_extensions.append(ExtensionData(
             extension, entry_point, config_schema, default_config, command))
 
-        # TODO: call validate_extension here?
-        # TODO: do basic config tests like schema contains enabled?
-
         logger.debug(
             'Loaded extension: %s %s', extension.dist_name, extension.version)
 
diff --git a/tests/test_ext.py b/tests/test_ext.py
index f6f31c21..b4fa8b9e 100644
--- a/tests/test_ext.py
+++ b/tests/test_ext.py
@@ -154,13 +154,9 @@ def test_load_extensions_get_command_fails(iter_entry_points_mock):
 
     iter_entry_points_mock.return_value = [mock_entry_point]
 
-    expected = ext.ExtensionData(
-        any_testextension, mock_entry_point, IsA(config.ConfigSchema),
-        any_unicode, None)
-
     with mock.patch.object(TestExtension, 'get_command') as get_command:
         get_command.side_effect = Exception
-        assert [expected] == ext.load_extensions()
+        assert [] == ext.load_extensions()
         get_command.assert_called_once_with()
 
 

From 8b6553ec1649987ee27915e5f504536ddf38d12a Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Mon, 11 May 2015 22:30:50 +0200
Subject: [PATCH 08/11] ext: Update validate_extension to
 validate_extension_data

Adds more checks to catch extension errors and importantly tests for exercise
these code paths.
---
 mopidy/__main__.py |  2 +-
 mopidy/ext.py      | 45 ++++++++++++++++++++++++++++++++++-----------
 tests/test_ext.py  | 43 +++++++++++++++++++++++++++++++++----------
 3 files changed, 68 insertions(+), 22 deletions(-)

diff --git a/mopidy/__main__.py b/mopidy/__main__.py
index 4ec0026c..69ba5370 100644
--- a/mopidy/__main__.py
+++ b/mopidy/__main__.py
@@ -95,7 +95,7 @@ def main():
             extension = data.extension
 
             # TODO: factor out all of this to a helper that can be tested
-            if not ext.validate_extension(data.extension, data.entry_point):
+            if not ext.validate_extension_data(data):
                 config[extension.ext_name] = {'enabled': False}
                 config_errors[extension.ext_name] = {
                     'enabled': 'extension disabled by self check.'}
diff --git a/mopidy/ext.py b/mopidy/ext.py
index 96e5d5e1..ab35008a 100644
--- a/mopidy/ext.py
+++ b/mopidy/ext.py
@@ -184,47 +184,70 @@ def load_extensions():
     return installed_extensions
 
 
-def validate_extension(extension, entry_point):
+def validate_extension_data(data):
     """Verify extension's dependencies and environment.
 
     :param extensions: an extension to check
     :returns: if extension should be run
     """
 
-    logger.debug('Validating extension: %s', extension.ext_name)
+    logger.debug('Validating extension: %s', data.extension.ext_name)
 
-    if extension.ext_name != entry_point.name:
+    if data.extension.ext_name != data.entry_point.name:
         logger.warning(
             'Disabled extension %(ep)s: entry point name (%(ep)s) '
             'does not match extension name (%(ext)s)',
-            {'ep': entry_point.name, 'ext': extension.ext_name})
+            {'ep': data.entry_point.name, 'ext': data.extension.ext_name})
         return False
 
     try:
-        entry_point.require()
+        data.entry_point.require()
     except pkg_resources.DistributionNotFound as ex:
         logger.info(
             'Disabled extension %s: Dependency %s not found',
-            extension.ext_name, ex)
+            data.extension.ext_name, ex)
         return False
     except pkg_resources.VersionConflict as ex:
         if len(ex.args) == 2:
             found, required = ex.args
             logger.info(
                 'Disabled extension %s: %s required, but found %s at %s',
-                extension.ext_name, required, found, found.location)
+                data.extension.ext_name, required, found, found.location)
         else:
             logger.info(
-                'Disabled extension %s: %s', extension.ext_name, ex)
+                'Disabled extension %s: %s', data.extension.ext_name, ex)
         return False
 
     try:
-        extension.validate_environment()
+        data.extension.validate_environment()
     except exceptions.ExtensionError as ex:
         logger.info(
-            'Disabled extension %s: %s', extension.ext_name, ex.message)
+            'Disabled extension %s: %s', data.extension.ext_name, ex.message)
         return False
     except Exception:
-        return False  # TODO: log
+        logger.exception('Validating extension %s failed with an exception.',
+                         data.extension.ext_name)
+        return False
+
+    if not data.config_schema:
+        logger.error('Extension %s does not have a config schema, disabling.',
+                     data.extension.ext_name)
+        return False
+    elif not isinstance(data.config_schema.get('enabled'), config_lib.Boolean):
+        logger.error('Extension %s does not have the required "enabled" config'
+                     ' option, disabling.', data.extension.ext_name)
+        return False
+
+    for key, value in data.config_schema.items():
+        if not isinstance(value, config_lib.ConfigValue):
+            logger.error('Extension %s config schema contains an invalid value'
+                         ' for the option "%s", disabling.',
+                         data.extension.ext_name, key)
+            return False
+
+    if not data.config_defaults:
+        logger.error('Extension %s does not have a default config, disabling.',
+                     data.extension.ext_name)
+        return False
 
     return True
diff --git a/tests/test_ext.py b/tests/test_ext.py
index b4fa8b9e..7fe6dba4 100644
--- a/tests/test_ext.py
+++ b/tests/test_ext.py
@@ -160,7 +160,7 @@ def test_load_extensions_get_command_fails(iter_entry_points_mock):
         get_command.assert_called_once_with()
 
 
-# ext.validate_extension
+# ext.validate_extension_data
 
 @pytest.fixture
 def ext_data():
@@ -178,18 +178,18 @@ def ext_data():
 
 def test_validate_extension_name_mismatch(ext_data):
     ext_data.entry_point.name = 'barfoo'
-    assert not ext.validate_extension(ext_data.extension, ext_data.entry_point)
+    assert not ext.validate_extension_data(ext_data)
 
 
 def test_validate_extension_distribution_not_found(ext_data):
     error = pkg_resources.DistributionNotFound
     ext_data.entry_point.require.side_effect = error
-    assert not ext.validate_extension(ext_data.extension, ext_data.entry_point)
+    assert not ext.validate_extension_data(ext_data)
 
 
 def test_validate_extension_version_conflict(ext_data):
     ext_data.entry_point.require.side_effect = pkg_resources.VersionConflict
-    assert not ext.validate_extension(ext_data.extension, ext_data.entry_point)
+    assert not ext.validate_extension_data(ext_data)
 
 
 def test_validate_extension_exception(ext_data):
@@ -197,8 +197,7 @@ def test_validate_extension_exception(ext_data):
 
     # We trust that entry points are well behaved, so exception will bubble.
     with pytest.raises(Exception):
-        assert not ext.validate_extension(
-            ext_data.extension, ext_data.entry_point)
+        assert not ext.validate_extension_data(ext_data)
 
 
 def test_validate_extension_instance_validate_env_ext_error(ext_data):
@@ -206,8 +205,7 @@ def test_validate_extension_instance_validate_env_ext_error(ext_data):
     with mock.patch.object(extension, 'validate_environment') as validate:
         validate.side_effect = exceptions.ExtensionError('error')
 
-        assert not ext.validate_extension(
-            ext_data.extension, ext_data.entry_point)
+        assert not ext.validate_extension_data(ext_data)
         validate.assert_called_once_with()
 
 
@@ -216,6 +214,31 @@ def test_validate_extension_instance_validate_env_exception(ext_data):
     with mock.patch.object(extension, 'validate_environment') as validate:
         validate.side_effect = Exception
 
-        assert not ext.validate_extension(
-            ext_data.extension, ext_data.entry_point)
+        assert not ext.validate_extension_data(ext_data)
         validate.assert_called_once_with()
+
+
+def test_validate_extension_with_missing_schema(ext_data):
+    ext_data = ext_data._replace(config_schema=None)
+    assert not ext.validate_extension_data(ext_data)
+
+
+def test_validate_extension_with_schema_that_is_missing_enabled(ext_data):
+    del ext_data.config_schema['enabled']
+    ext_data.config_schema['baz'] = config.String()
+    assert not ext.validate_extension_data(ext_data)
+
+
+def test_validate_extension_with_schema_with_wrong_types(ext_data):
+    ext_data.config_schema['enabled'] = 123
+    assert not ext.validate_extension_data(ext_data)
+
+
+def test_validate_extension_with_schema_with_invalid_type(ext_data):
+    ext_data.config_schema['baz'] = 123
+    assert not ext.validate_extension_data(ext_data)
+
+
+def test_validate_extension_with_no_defaults(ext_data):
+    ext_data = ext_data._replace(config_defaults=None)
+    assert not ext.validate_extension_data(ext_data)

From dbc3100e9cfa994fb8a87570c089d08f298b1999 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Mon, 11 May 2015 22:47:13 +0200
Subject: [PATCH 09/11] main: Update to use extension_data structure

Updated config and __main__ code to use the new wrapper format and pre-fetched
values.
---
 mopidy/__main__.py        | 25 ++++++++++++-------------
 mopidy/commands.py        |  4 ++--
 mopidy/config/__init__.py | 14 +++++---------
 3 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/mopidy/__main__.py b/mopidy/__main__.py
index 69ba5370..9e1e1c94 100644
--- a/mopidy/__main__.py
+++ b/mopidy/__main__.py
@@ -66,21 +66,22 @@ def main():
         root_cmd.add_child('config', config_cmd)
         root_cmd.add_child('deps', deps_cmd)
 
-        installed_extensions = ext.load_extensions()
+        extensions_data = ext.load_extensions()
 
-        for data in installed_extensions:
-            if data.command:
+        for data in extensions_data:
+            if data.command:  # TODO: check isinstance?
                 data.command.set(extension=data.command)
                 root_cmd.add_child(data.extension.ext_name, data.command)
 
         args = root_cmd.parse(mopidy_args)
 
-        create_file_structures_and_config(args, installed_extensions)
+        create_file_structures_and_config(args, extensions_data)
         check_old_locations()
 
-        # TODO: make config.load use extension data? or just pass in schema+def
         config, config_errors = config_lib.load(
-            args.config_files, [d.extension for d in installed_extensions],
+            args.config_files,
+            [d.config_schema for d in extensions_data],
+            [d.config_defaults for d in extensions_data],
             args.config_overrides)
 
         verbosity_level = args.base_verbosity_level
@@ -91,7 +92,7 @@ def main():
 
         extensions = {
             'validate': [], 'config': [], 'disabled': [], 'enabled': []}
-        for data in installed_extensions:
+        for data in extensions_data:
             extension = data.extension
 
             # TODO: factor out all of this to a helper that can be tested
@@ -113,15 +114,13 @@ def main():
             else:
                 extensions['enabled'].append(extension)
 
-        # TODO: convert rest of code to use new ExtensionData
-        installed_extensions = [d.extension for d in installed_extensions]
-
-        log_extension_info(installed_extensions, extensions['enabled'])
+        log_extension_info([d.extension for d in extensions_data],
+                           extensions['enabled'])
 
         # Config and deps commands are simply special cased for now.
         if args.command == config_cmd:
-            return args.command.run(
-                config, config_errors, installed_extensions)
+            schemas = [d.config_schema for d in extensions_data]
+            return args.command.run(config, config_errors, schemas)
         elif args.command == deps_cmd:
             return args.command.run()
 
diff --git a/mopidy/commands.py b/mopidy/commands.py
index 29564779..24acfb7d 100644
--- a/mopidy/commands.py
+++ b/mopidy/commands.py
@@ -415,8 +415,8 @@ class ConfigCommand(Command):
         super(ConfigCommand, self).__init__()
         self.set(base_verbosity_level=-1)
 
-    def run(self, config, errors, extensions):
-        print(config_lib.format(config, extensions, errors))
+    def run(self, config, errors, schemas):
+        print(config_lib.format(config, schemas, errors))
         return 0
 
 
diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py
index fc6dcb60..3f1f978c 100644
--- a/mopidy/config/__init__.py
+++ b/mopidy/config/__init__.py
@@ -65,24 +65,20 @@ def read(config_file):
         return filehandle.read()
 
 
-def load(files, extensions, overrides):
-    # Helper to get configs, as the rest of our config system should not need
-    # to know about extensions.
+def load(files, ext_schemas, ext_defaults, overrides):
     config_dir = os.path.dirname(__file__)
     defaults = [read(os.path.join(config_dir, 'default.conf'))]
-    defaults.extend(e.get_default_config() for e in extensions)
+    defaults.extend(ext_defaults)
     raw_config = _load(files, defaults, keyring.fetch() + (overrides or []))
 
     schemas = _schemas[:]
-    schemas.extend(e.get_config_schema() for e in extensions)
+    schemas.extend(ext_schemas)
     return _validate(raw_config, schemas)
 
 
-def format(config, extensions, comments=None, display=True):
-    # Helper to format configs, as the rest of our config system should not
-    # need to know about extensions.
+def format(config, ext_schemas, comments=None, display=True):
     schemas = _schemas[:]
-    schemas.extend(e.get_config_schema() for e in extensions)
+    schemas.extend(ext_schemas)
     return _format(config, comments or {}, schemas, display, False)
 
 

From d630a97bc1b0240cc41dbf742414fa2b7fc60b79 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Tue, 12 May 2015 22:00:31 +0200
Subject: [PATCH 10/11] ext: Refactor tests based on review comments

---
 tests/test_ext.py | 293 +++++++++++++++++++++-------------------------
 1 file changed, 136 insertions(+), 157 deletions(-)

diff --git a/tests/test_ext.py b/tests/test_ext.py
index 7fe6dba4..748aebb3 100644
--- a/tests/test_ext.py
+++ b/tests/test_ext.py
@@ -23,222 +23,201 @@ class TestExtension(ext.Extension):
 any_testextension = IsA(TestExtension)
 
 
-# ext.Extension
+class ExtensionTest(object):
 
-@pytest.fixture
-def extension():
-    return ext.Extension()
+    @pytest.fixture
+    def extension(self):
+        return ext.Extension()
 
+    def test_dist_name_is_none(self, extension):
+        assert extension.dist_name is None
 
-def test_dist_name_is_none(extension):
-    assert extension.dist_name is None
+    def test_ext_name_is_none(self, extension):
+        assert extension.ext_name is None
 
+    def test_version_is_none(self, extension):
+        assert extension.version is None
 
-def test_ext_name_is_none(extension):
-    assert extension.ext_name is None
+    def test_get_default_config_raises_not_implemented(self, extension):
+        with pytest.raises(NotImplementedError):
+            extension.get_default_config()
 
+    def test_get_config_schema_returns_extension_schema(self, extension):
+        schema = extension.get_config_schema()
+        assert isinstance(schema['enabled'], config.Boolean)
 
-def test_version_is_none(extension):
-    assert extension.version is None
+    def test_validate_environment_does_nothing_by_default(self, extension):
+        assert extension.validate_environment() is None
 
+    def test_setup_raises_not_implemented(self, extension):
+        with pytest.raises(NotImplementedError):
+            extension.setup(None)
 
-def test_get_default_config_raises_not_implemented(extension):
-    with pytest.raises(NotImplementedError):
-        extension.get_default_config()
 
+class LoadExtensionsTest(object):
 
-def test_get_config_schema_returns_extension_schema(extension):
-    schema = extension.get_config_schema()
-    assert isinstance(schema['enabled'], config.Boolean)
+    @pytest.yield_fixture
+    def iter_entry_points_mock(self, request):
+        patcher = mock.patch('pkg_resources.iter_entry_points')
+        iter_entry_points = patcher.start()
+        iter_entry_points.return_value = []
+        yield iter_entry_points
+        patcher.stop()
 
+    def test_no_extensions(self, iter_entry_points_mock):
+        iter_entry_points_mock.return_value = []
+        assert ext.load_extensions() == []
 
-def test_validate_environment_does_nothing_by_default(extension):
-    assert extension.validate_environment() is None
+    def test_load_extensions(self, iter_entry_points_mock):
+        mock_entry_point = mock.Mock()
+        mock_entry_point.load.return_value = TestExtension
 
+        iter_entry_points_mock.return_value = [mock_entry_point]
 
-def test_setup_raises_not_implemented(extension):
-    with pytest.raises(NotImplementedError):
-        extension.setup(None)
+        expected = ext.ExtensionData(
+            any_testextension, mock_entry_point, IsA(config.ConfigSchema),
+            any_unicode, None)
 
+        assert ext.load_extensions() == [expected]
 
-# ext.load_extensions
+    def test_gets_wrong_class(self, iter_entry_points_mock):
 
-@pytest.fixture
-def iter_entry_points_mock(request):
-    patcher = mock.patch('pkg_resources.iter_entry_points')
-    iter_entry_points = patcher.start()
-    iter_entry_points.return_value = []
-    request.addfinalizer(patcher.stop)
-    return iter_entry_points
+        class WrongClass(object):
+            pass
 
+        mock_entry_point = mock.Mock()
+        mock_entry_point.load.return_value = WrongClass
 
-def test_load_extensions_no_extenions(iter_entry_points_mock):
-    iter_entry_points_mock.return_value = []
-    assert [] == ext.load_extensions()
+        iter_entry_points_mock.return_value = [mock_entry_point]
 
+        assert ext.load_extensions() == []
 
-def test_load_extensions(iter_entry_points_mock):
-    mock_entry_point = mock.Mock()
-    mock_entry_point.load.return_value = TestExtension
+    def test_gets_instance(self, iter_entry_points_mock):
+        mock_entry_point = mock.Mock()
+        mock_entry_point.load.return_value = TestExtension()
 
-    iter_entry_points_mock.return_value = [mock_entry_point]
+        iter_entry_points_mock.return_value = [mock_entry_point]
 
-    expected = ext.ExtensionData(
-        any_testextension, mock_entry_point, IsA(config.ConfigSchema),
-        any_unicode, None)
+        assert ext.load_extensions() == []
 
-    assert ext.load_extensions() == [expected]
+    def test_creating_instance_fails(self, iter_entry_points_mock):
+        mock_extension = mock.Mock(spec=ext.Extension)
+        mock_extension.side_effect = Exception
 
+        mock_entry_point = mock.Mock()
+        mock_entry_point.load.return_value = mock_extension
 
-def test_load_extensions_gets_wrong_class(iter_entry_points_mock):
+        iter_entry_points_mock.return_value = [mock_entry_point]
 
-    class WrongClass(object):
-        pass
+        assert ext.load_extensions() == []
 
-    mock_entry_point = mock.Mock()
-    mock_entry_point.load.return_value = WrongClass
+    def test_get_config_schema_fails(self, iter_entry_points_mock):
+        mock_entry_point = mock.Mock()
+        mock_entry_point.load.return_value = TestExtension
 
-    iter_entry_points_mock.return_value = [mock_entry_point]
+        iter_entry_points_mock.return_value = [mock_entry_point]
 
-    assert [] == ext.load_extensions()
+        with mock.patch.object(TestExtension, 'get_config_schema') as get:
+            get.side_effect = Exception
 
+            assert ext.load_extensions() == []
+            get.assert_called_once_with()
 
-def test_load_extensions_gets_instance(iter_entry_points_mock):
-    mock_entry_point = mock.Mock()
-    mock_entry_point.load.return_value = TestExtension()
+    def test_get_default_config_fails(self, iter_entry_points_mock):
+        mock_entry_point = mock.Mock()
+        mock_entry_point.load.return_value = TestExtension
 
-    iter_entry_points_mock.return_value = [mock_entry_point]
+        iter_entry_points_mock.return_value = [mock_entry_point]
 
-    assert [] == ext.load_extensions()
+        with mock.patch.object(TestExtension, 'get_default_config') as get:
+            get.side_effect = Exception
 
+            assert ext.load_extensions() == []
+            get.assert_called_once_with()
 
-def test_load_extensions_creating_instance_fails(iter_entry_points_mock):
-    mock_extension = mock.Mock(spec=ext.Extension)
-    mock_extension.side_effect = Exception
+    def test_get_command_fails(self, iter_entry_points_mock):
+        mock_entry_point = mock.Mock()
+        mock_entry_point.load.return_value = TestExtension
 
-    mock_entry_point = mock.Mock()
-    mock_entry_point.load.return_value = mock_extension
+        iter_entry_points_mock.return_value = [mock_entry_point]
 
-    iter_entry_points_mock.return_value = [mock_entry_point]
+        with mock.patch.object(TestExtension, 'get_command') as get:
+            get.side_effect = Exception
 
-    assert [] == ext.load_extensions()
+            assert ext.load_extensions() == []
+            get.assert_called_once_with()
 
 
-def test_load_extensions_get_config_schema_fails(iter_entry_points_mock):
-    mock_entry_point = mock.Mock()
-    mock_entry_point.load.return_value = TestExtension
+class ValidateExtensionDataTest(object):
 
-    iter_entry_points_mock.return_value = [mock_entry_point]
+    @pytest.fixture
+    def ext_data(self):
+        extension = TestExtension()
 
-    with mock.patch.object(TestExtension, 'get_config_schema') as get_schema:
-        get_schema.side_effect = Exception
-        assert [] == ext.load_extensions()
-        get_schema.assert_called_once_with()
+        entry_point = mock.Mock()
+        entry_point.name = extension.ext_name
 
+        schema = extension.get_config_schema()
+        defaults = extension.get_default_config()
+        command = extension.get_command()
 
-def test_load_extensions_get_default_config_fails(iter_entry_points_mock):
-    mock_entry_point = mock.Mock()
-    mock_entry_point.load.return_value = TestExtension
+        return ext.ExtensionData(
+            extension, entry_point, schema, defaults, command)
 
-    iter_entry_points_mock.return_value = [mock_entry_point]
-
-    with mock.patch.object(TestExtension, 'get_default_config') as get_default:
-        get_default.side_effect = Exception
-        assert [] == ext.load_extensions()
-        get_default.assert_called_once_with()
-
-
-def test_load_extensions_get_command_fails(iter_entry_points_mock):
-    mock_entry_point = mock.Mock()
-    mock_entry_point.load.return_value = TestExtension
-
-    iter_entry_points_mock.return_value = [mock_entry_point]
-
-    with mock.patch.object(TestExtension, 'get_command') as get_command:
-        get_command.side_effect = Exception
-        assert [] == ext.load_extensions()
-        get_command.assert_called_once_with()
-
-
-# ext.validate_extension_data
-
-@pytest.fixture
-def ext_data():
-    extension = TestExtension()
-
-    entry_point = mock.Mock()
-    entry_point.name = extension.ext_name
-
-    schema = extension.get_config_schema()
-    defaults = extension.get_default_config()
-    command = extension.get_command()
-
-    return ext.ExtensionData(extension, entry_point, schema, defaults, command)
-
-
-def test_validate_extension_name_mismatch(ext_data):
-    ext_data.entry_point.name = 'barfoo'
-    assert not ext.validate_extension_data(ext_data)
-
-
-def test_validate_extension_distribution_not_found(ext_data):
-    error = pkg_resources.DistributionNotFound
-    ext_data.entry_point.require.side_effect = error
-    assert not ext.validate_extension_data(ext_data)
-
-
-def test_validate_extension_version_conflict(ext_data):
-    ext_data.entry_point.require.side_effect = pkg_resources.VersionConflict
-    assert not ext.validate_extension_data(ext_data)
-
-
-def test_validate_extension_exception(ext_data):
-    ext_data.entry_point.require.side_effect = Exception
-
-    # We trust that entry points are well behaved, so exception will bubble.
-    with pytest.raises(Exception):
+    def test_name_mismatch(self, ext_data):
+        ext_data.entry_point.name = 'barfoo'
         assert not ext.validate_extension_data(ext_data)
 
-
-def test_validate_extension_instance_validate_env_ext_error(ext_data):
-    extension = ext_data.extension
-    with mock.patch.object(extension, 'validate_environment') as validate:
-        validate.side_effect = exceptions.ExtensionError('error')
-
+    def test_distribution_not_found(self, ext_data):
+        error = pkg_resources.DistributionNotFound
+        ext_data.entry_point.require.side_effect = error
         assert not ext.validate_extension_data(ext_data)
-        validate.assert_called_once_with()
-
-
-def test_validate_extension_instance_validate_env_exception(ext_data):
-    extension = ext_data.extension
-    with mock.patch.object(extension, 'validate_environment') as validate:
-        validate.side_effect = Exception
 
+    def test_version_conflict(self, ext_data):
+        error = pkg_resources.VersionConflict
+        ext_data.entry_point.require.side_effect = error
         assert not ext.validate_extension_data(ext_data)
-        validate.assert_called_once_with()
 
+    def test_entry_point_require_exception(self, ext_data):
+        ext_data.entry_point.require.side_effect = Exception
 
-def test_validate_extension_with_missing_schema(ext_data):
-    ext_data = ext_data._replace(config_schema=None)
-    assert not ext.validate_extension_data(ext_data)
+        # Hope that entry points are well behaved, so exception will bubble.
+        with pytest.raises(Exception):
+            assert not ext.validate_extension_data(ext_data)
 
+    def test_extenions_validate_environment_error(self, ext_data):
+        extension = ext_data.extension
+        with mock.patch.object(extension, 'validate_environment') as validate:
+            validate.side_effect = exceptions.ExtensionError('error')
 
-def test_validate_extension_with_schema_that_is_missing_enabled(ext_data):
-    del ext_data.config_schema['enabled']
-    ext_data.config_schema['baz'] = config.String()
-    assert not ext.validate_extension_data(ext_data)
+            assert not ext.validate_extension_data(ext_data)
+            validate.assert_called_once_with()
 
+    def test_extenions_validate_environment_exception(self, ext_data):
+        extension = ext_data.extension
+        with mock.patch.object(extension, 'validate_environment') as validate:
+            validate.side_effect = Exception
 
-def test_validate_extension_with_schema_with_wrong_types(ext_data):
-    ext_data.config_schema['enabled'] = 123
-    assert not ext.validate_extension_data(ext_data)
+            assert not ext.validate_extension_data(ext_data)
+            validate.assert_called_once_with()
 
+    def test_missing_schema(self, ext_data):
+        ext_data = ext_data._replace(config_schema=None)
+        assert not ext.validate_extension_data(ext_data)
 
-def test_validate_extension_with_schema_with_invalid_type(ext_data):
-    ext_data.config_schema['baz'] = 123
-    assert not ext.validate_extension_data(ext_data)
+    def test_schema_that_is_missing_enabled(self, ext_data):
+        del ext_data.config_schema['enabled']
+        ext_data.config_schema['baz'] = config.String()
+        assert not ext.validate_extension_data(ext_data)
 
+    def test_schema_with_wrong_types(self, ext_data):
+        ext_data.config_schema['enabled'] = 123
+        assert not ext.validate_extension_data(ext_data)
 
-def test_validate_extension_with_no_defaults(ext_data):
-    ext_data = ext_data._replace(config_defaults=None)
-    assert not ext.validate_extension_data(ext_data)
+    def test_schema_with_invalid_type(self, ext_data):
+        ext_data.config_schema['baz'] = 123
+        assert not ext.validate_extension_data(ext_data)
+
+    def test_no_default_config(self, ext_data):
+        ext_data = ext_data._replace(config_defaults=None)
+        assert not ext.validate_extension_data(ext_data)

From ce3c16de6ecf8150d8d15e2576836eebecfc697c Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Tue, 12 May 2015 22:02:19 +0200
Subject: [PATCH 11/11] startup: Fix mistake in command extension bootstrap
 cleanup

---
 mopidy/__main__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mopidy/__main__.py b/mopidy/__main__.py
index 9e1e1c94..ea1cab6b 100644
--- a/mopidy/__main__.py
+++ b/mopidy/__main__.py
@@ -70,7 +70,7 @@ def main():
 
         for data in extensions_data:
             if data.command:  # TODO: check isinstance?
-                data.command.set(extension=data.command)
+                data.command.set(extension=data.extension)
                 root_cmd.add_child(data.extension.ext_name, data.command)
 
         args = root_cmd.parse(mopidy_args)