Repository: opsdroid/opsdroid

Issue:
Change default modules directory
Currently the default modules directory location is `./modules`. 

This makes a few assumptions:  
* Current directory is in the python path 
* There are no other python modules in the current directory
* There are no other modules named `modules`
* Current directory is writable

A better default location may be `~/.opsdroid/modules/opsdroid-modules`. This would be created if it doesn't exist and `~/.opsdroid/modules` could be added to the python path without fear of collision as `opsdroid-modules` is less generic. As it is in the home directory we can be fairly sure it is writable. 

Also when a user specifies a custom modules directory it should still be suffixed with `/opsdroid-modules` and the custom directory should be added to the python path. 

Discussion:
coveralls: 
[![Coverage Status](https://coveralls.io/builds/11771359/badge)](https://coveralls.io/builds/11771359)

Coverage increased (+0.02%) to 95.876% when pulling **7372d076c962767f4d418fd0e84f96a6a2f02aa6 on jacobtomlinson:new-module-path** into **b6a6b803844b36e49146b468dc104be1e49235a5 on opsdroid:dev**.


Patch:
diff --git a/opsdroid/const.py b/opsdroid/const.py
index be722ef..c12d2ae 100644
--- a/opsdroid/const.py
+++ b/opsdroid/const.py
@@ -4,5 +4,6 @@ __version__ = "0.7.1"
 
 LOG_FILENAME = 'output.log'
 DEFAULT_GIT_URL = "https://github.com/opsdroid/"
-MODULES_DIRECTORY = "modules"
+MODULES_DIRECTORY = "opsdroid-modules"
+DEFAULT_MODULES_PATH = "~/.opsdroid/modules"
 DEFAULT_MODULE_BRANCH = "master"
diff --git a/opsdroid/loader.py b/opsdroid/loader.py
index e494cda..34d1330 100644
--- a/opsdroid/loader.py
+++ b/opsdroid/loader.py
@@ -8,7 +8,8 @@ import subprocess
 import importlib
 import yaml
 from opsdroid.const import (
-    DEFAULT_GIT_URL, MODULES_DIRECTORY, DEFAULT_MODULE_BRANCH)
+    DEFAULT_GIT_URL, MODULES_DIRECTORY, DEFAULT_MODULES_PATH,
+    DEFAULT_MODULE_BRANCH)
 
 
 _LOGGER = logging.getLogger(__name__)
@@ -20,7 +21,7 @@ class Loader:
     def __init__(self, opsdroid):
         """Create object with opsdroid instance."""
         self.opsdroid = opsdroid
-        self.modules_directory = MODULES_DIRECTORY
+        self.modules_directory = None
         self.current_import_config = None
         _LOGGER.debug("Loaded loader")
 
@@ -122,16 +123,26 @@ class Loader:
         except FileNotFoundError as error:
             self.opsdroid.critical(str(error), 1)
 
+    def setup_modules_directory(self, config):
+        """Create and configure the modules directory."""
+        module_path = os.path.expanduser(
+            config.get("module-path", DEFAULT_MODULES_PATH))
+        sys.path.append(module_path)
+
+        if not os.path.isdir(module_path):
+            os.makedirs(module_path, exist_ok=True)
+
+        self.modules_directory = os.path.join(module_path, MODULES_DIRECTORY)
+
+        # Create modules directory if doesn't exist
+        if not os.path.isdir(self.modules_directory):
+            os.makedirs(self.modules_directory)
+
     def load_modules_from_config(self, config):
         """Load all module types based on config."""
         _LOGGER.debug("Loading modules from config")
 
-        if "module-path" in config:
-            sys.path.append(config["module-path"])
-            if not os.path.isdir(config["module-path"]):
-                os.makedirs(config["module-path"], exist_ok=True)
-            self.modules_directory = os.path.join(config["module-path"],
-                                                  self.modules_directory)
+        self.setup_modules_directory(config)
 
         connectors, databases, skills = None, None, None
 
@@ -159,10 +170,6 @@ class Loader:
         _LOGGER.debug("Loading " + modules_type + " modules")
         loaded_modules = []
 
-        # Create modules directory if doesn't exist
-        if not os.path.isdir(self.modules_directory):
-            os.makedirs(self.modules_directory)
-
         for module in modules:
 
             # Set up module config


Test patch:
diff --git a/tests/test_loader.py b/tests/test_loader.py
index 1724997..ae83378 100644
--- a/tests/test_loader.py
+++ b/tests/test_loader.py
@@ -153,20 +153,10 @@ class TestLoader(unittest.TestCase):
         with mock.patch.object(loader, '_install_module') as mockinstall, \
                 mock.patch.object(loader, 'import_module',
                                   mockedmodule) as mockimport:
+            loader.setup_modules_directory({})
             loader._load_modules(modules_type, modules)
-            mockinstall.assert_called_with({
-                'branch': 'master',
-                'module_path': 'modules.test.testmodule',
-                'name': 'testmodule',
-                'type': modules_type,
-                'install_path': 'modules/test/testmodule'})
-            mockimport.assert_called_with({
-                'module_path': 'modules.test.testmodule',
-                'name': 'testmodule',
-                'type': modules_type,
-                'branch': 'master',
-                'install_path':
-                'modules/test/testmodule'})
+            assert mockinstall.call_count
+            assert mockimport.call_count
 
     def test_install_existing_module(self):
         opsdroid, loader = self.setup()