Add some tests around plugin setup()

cardinal/fixtures/fake_plugins/no_setup/plugin.py

@@ -0,0 +1,2 @@
+class TestNoSetupFunctionPlugin(object):
+    pass

cardinal/fixtures/fake_plugins/setup_too_many_arguments/plugin.py

@@ -0,0 +1,5 @@
+class TestSetupTooManyArgumentsPlugin(object):
+    pass
+
+def setup(one, two, three):
+    pass

cardinal/plugins.py

@@ -468,6 +468,8 @@ def load(self, plugins):
                     self.logger.info("Already loaded, reloading: %s" % plugin)
                     reload_flag = True
 
+                    # We don't consider this a failed plugin unless it doesn't
+                    # load correctly
                     try:
                         self._close_plugin_instance(plugin)
                     except Exception:

cardinal/test_plugins.py

@@ -6,6 +6,7 @@
 import pytest
 
 from bot import CardinalBot
+from exceptions import AmbiguousConfigError
 from plugins import PluginManager
 
 DIR_PATH = os.path.dirname(os.path.realpath(__file__))
@@ -55,8 +56,31 @@ def test_load_plugins_nonexistent_plugin(self):
         manager = PluginManager(Mock())
         failed_plugins = manager.load(plugins)
         assert failed_plugins == plugins
+        assert manager.plugins == {}
 
-    def test_load_plugins_valid_plugin(self):
+    def test_load_no_setup_errors(self):
+        name = 'no_setup'
+        plugins = [name]
+
+        manager = PluginManager(Mock(),
+                                _plugin_module_import_prefix='fake_plugins')
+
+        failed_plugins = manager.load(plugins)
+        assert failed_plugins == plugins
+        assert manager.plugins.keys() == []
+
+    def test_load_setup_too_many_arguments_errors(self):
+        name = 'setup_too_many_arguments'
+        plugins = [name]
+
+        manager = PluginManager(Mock(),
+                                _plugin_module_import_prefix='fake_plugins')
+
+        failed_plugins = manager.load(plugins)
+        assert failed_plugins == plugins
+        assert manager.plugins.keys() == []
+
+    def test_load_plugins_valid_plugin_list(self):
         name = 'test_valid_plugin'
         plugins = [name]
 
@@ -65,8 +89,29 @@ def test_load_plugins_valid_plugin(self):
 
         failed_plugins = manager.load(plugins)
         assert failed_plugins == []
+        assert manager.plugins.keys() == plugins
 
+        assert manager.plugins[name]['name'] == name
+        assert inspect.ismodule(manager.plugins[name]['module'])
+        assert isinstance(manager.plugins[name]['instance'],
+                          manager.plugins[name]['module'].TestValidPlugin)
+        assert manager.plugins[name]['commands'] == []
+        assert manager.plugins[name]['callbacks'] == []
+        assert manager.plugins[name]['callback_ids'] == {}
+        assert manager.plugins[name]['config'] is None
+        assert manager.plugins[name]['blacklist'] == []
+
+    def test_load_plugins_valid_plugin_string(self):
+        name = 'test_valid_plugin'
+        plugins = [name]
+
+        manager = PluginManager(Mock(),
+                                _plugin_module_import_prefix='fake_plugins')
+
+        failed_plugins = manager.load(name)
+        assert failed_plugins == []
         assert manager.plugins.keys() == plugins
+
         assert manager.plugins[name]['name'] == name
         assert inspect.ismodule(manager.plugins[name]['module'])
         assert isinstance(manager.plugins[name]['instance'],
@@ -77,6 +122,19 @@ def test_load_plugins_valid_plugin(self):
         assert manager.plugins[name]['config'] is None
         assert manager.plugins[name]['blacklist'] == []
 
+    @patch.object(PluginManager, '_load_plugin_config')
+    def test_load_plugin_ambiguous_config_errors(self, mock):
+        name = 'test_valid_plugin'
+        plugins = [name]
+
+        manager = PluginManager(Mock(),
+                                _plugin_module_import_prefix='fake_plugins')
+        mock.side_effect = AmbiguousConfigError()
+
+        failed_plugins = manager.load(plugins)
+        assert failed_plugins == plugins
+        assert manager.plugins == {}
+
     def test_load_plugins_reload_valid_plugin(self):
         name = 'test_reload_valid_plugin'
         plugins = [name]
@@ -89,9 +147,31 @@ def test_load_plugins_reload_valid_plugin(self):
 
         failed_plugins = manager.load(plugins)
         assert failed_plugins == []
+        assert manager.plugins.keys() == plugins
+
+        failed_plugins = manager.load(plugins)
+        assert failed_plugins == []
+        assert manager.plugins.keys() == plugins
+
+        assert cardinal.reloads == 1
+
+    def test_load_plugins_reload_unclean_close_plugin(self):
+        name = 'test_unclean_close_plugin'
+        plugins = [name]
+
+        cardinal = Mock(CardinalBot)
+        cardinal.reloads = 0
+
+        manager = PluginManager(cardinal,
+                                _plugin_module_import_prefix='fake_plugins')
+
+        failed_plugins = manager.load(plugins)
+        assert failed_plugins == []
+        assert manager.plugins.keys() == plugins
 
         failed_plugins = manager.load(plugins)
         assert failed_plugins == []
+        assert manager.plugins.keys() == plugins
 
         assert cardinal.reloads == 1
 
@@ -113,6 +193,7 @@ def test_unload_plugins_never_loaded_plugin(self):
         failed_plugins = manager.unload(plugins)
 
         assert failed_plugins == plugins
+        assert manager.plugins == {}
 
     def test_unload_plugins_unclean_close_plugin(self):
         name = 'test_unclean_close_plugin'
@@ -123,10 +204,12 @@ def test_unload_plugins_unclean_close_plugin(self):
 
         failed_plugins = manager.load(plugins)
         assert failed_plugins == []
+        assert manager.plugins.keys() == plugins
 
         failed_plugins = manager.unload(plugins)
 
         assert failed_plugins == plugins
+        assert manager.plugins == {}
 
     def test_unload_plugins_valid_plugin(self):
         name = 'test_valid_plugin'
@@ -137,10 +220,12 @@ def test_unload_plugins_valid_plugin(self):
 
         failed_plugins = manager.load(plugins)
         assert failed_plugins == []
+        assert manager.plugins.keys() == plugins
 
         failed_plugins = manager.unload(plugins)
 
         assert failed_plugins == []
+        assert manager.plugins.keys() == []
 
         name = 'test_valid_plugin'
         plugins = [name]
@@ -154,10 +239,12 @@ def test_unload_plugins_clean_close_plugin(self):
 
         failed_plugins = manager.load(plugins)
         assert failed_plugins == []
+        assert manager.plugins.keys() == plugins
 
         failed_plugins = manager.unload(plugins)
 
         assert failed_plugins == []
+        assert manager.plugins.keys() == []
 
     @patch.object(PluginManager, '_unregister_plugin_callbacks')
     def test_unload_plugins_unregister_plugin_callbacks_error(self, mock):
@@ -169,10 +256,12 @@ def test_unload_plugins_unregister_plugin_callbacks_error(self, mock):
 
         failed_plugins = manager.load(plugins)
         assert failed_plugins == []
+        assert manager.plugins.keys() == plugins
 
         mock.side_effect = Exception()
         failed_plugins = manager.unload(plugins)
 
         assert failed_plugins == []
+        assert manager.plugins.keys() == []
 
         mock.assert_called_with(name)