Repository: mkdocs/mkdocs

Issue:
Broken installed themes should not break MkDocs
As highlighted in [this thread][1], a broken (not importable) installed theme will break MkDocs even if the theme is not being used by the project.

In [the CLI][2] we call [`utils.get_theme_names`][0] and assign the result to a global variable to display in the CLI help. That function wraps [`utils.get_themes`][3], which iterates over the entry points and loads each one. If any one of the modules pointed to by an entry point fails to import, the entire thing crashes. We should fail more gracefully, perhaps with an warning that a theme has a problem and name the specific theme if possible. I suggest a warning unless the theme is the same theme actually being used by MkDocs, in which case an error should be raised, but that would be handled elsewhere anyway.

[0]: https://github.com/mkdocs/mkdocs/blob/master/mkdocs/utils/__init__.py#L406
[1]: https://groups.google.com/forum/#!topic/mkdocs/qEYkyHxYGAw
[2]: https://github.com/mkdocs/mkdocs/blob/master/mkdocs/__main__.py#L78
[3]: https://github.com/mkdocs/mkdocs/blob/master/mkdocs/utils/__init__.py#L379

Discussion:
waylan: I think maybe we need to add some better validation (and graceful errors) for the theme requested by the user. I'm going to hold off merging this until I can add that.
waylan: I've decided the way to improve validation is via a refactor of themes (which had been discussed elsewhere, although I can't find it right now) where we create a `Theme` object for each theme. therefore, I'll merge this as-is.

Patch:
diff --git a/mkdocs/config/config_options.py b/mkdocs/config/config_options.py
index 3e7e2073..7fa3ac6f 100644
--- a/mkdocs/config/config_options.py
+++ b/mkdocs/config/config_options.py
@@ -285,7 +285,7 @@ class ThemeDir(Dir):
 
         package_dir = os.path.abspath(
             os.path.join(os.path.dirname(__file__), '..'))
-        theme_dir = [utils.get_themes()[config['theme']], ]
+        theme_dir = [utils.get_theme_dir(config['theme']), ]
         config['mkdocs_templates'] = os.path.join(package_dir, 'templates')
 
         if config['theme_dir'] is not None:
diff --git a/mkdocs/utils/__init__.py b/mkdocs/utils/__init__.py
index f3134353..f1f88a5f 100644
--- a/mkdocs/utils/__init__.py
+++ b/mkdocs/utils/__init__.py
@@ -376,8 +376,15 @@ def convert_markdown(markdown_source, extensions=None, extension_configs=None):
     return (html_content, table_of_contents, meta)
 
 
+def get_theme_dir(name):
+    """ Return the directory of an installed theme by name. """
+
+    theme = get_themes()[name]
+    return os.path.dirname(os.path.abspath(theme.load().__file__))
+
+
 def get_themes():
-    """Return a dict of theme names and their locations"""
+    """ Return a dict of all installed themes as (name, entry point) pairs. """
 
     themes = {}
     builtins = pkg_resources.get_entry_map(dist='mkdocs', group='mkdocs.themes')
@@ -397,14 +404,11 @@ def get_themes():
 
         themes[theme.name] = theme
 
-    themes = dict((name, os.path.dirname(os.path.abspath(theme.load().__file__)))
-                  for name, theme in themes.items())
-
     return themes
 
 
 def get_theme_names():
-    """Return a list containing all the names of all the builtin themes."""
+    """Return a list of all installed themes by name."""
 
     return get_themes().keys()
 


Test patch:
diff --git a/mkdocs/tests/utils/utils_tests.py b/mkdocs/tests/utils/utils_tests.py
index a92c9631..d96f5cf4 100644
--- a/mkdocs/tests/utils/utils_tests.py
+++ b/mkdocs/tests/utils/utils_tests.py
@@ -114,6 +114,36 @@ class UtilsTests(unittest.TestCase):
             sorted(utils.get_theme_names()),
             ['mkdocs', 'readthedocs'])
 
+    @mock.patch('pkg_resources.iter_entry_points', autospec=True)
+    def test_get_theme_dir(self, mock_iter):
+
+        path = 'some/path'
+
+        theme = mock.Mock()
+        theme.name = 'mkdocs2'
+        theme.dist.key = 'mkdocs2'
+        theme.load().__file__ = os.path.join(path, '__init__.py')
+
+        mock_iter.return_value = iter([theme])
+
+        self.assertEqual(utils.get_theme_dir(theme.name), os.path.abspath(path))
+
+    def test_get_theme_dir_keyerror(self):
+
+        self.assertRaises(KeyError, utils.get_theme_dir, 'nonexistanttheme')
+
+    @mock.patch('pkg_resources.iter_entry_points', autospec=True)
+    def test_get_theme_dir_importerror(self, mock_iter):
+
+        theme = mock.Mock()
+        theme.name = 'mkdocs2'
+        theme.dist.key = 'mkdocs2'
+        theme.load.side_effect = ImportError()
+
+        mock_iter.return_value = iter([theme])
+
+        self.assertRaises(ImportError, utils.get_theme_dir, theme.name)
+
     @mock.patch('pkg_resources.iter_entry_points', autospec=True)
     def test_get_themes_warning(self, mock_iter):