From 733dbc4b72bba5ab0e2e673c41e185a258ebd521 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jun 22 2017 16:00:32 +0000 Subject: PR#475 fix StringType and itervalues in plugin and cli Merges #475 https://pagure.io/koji/pull-request/475 --- diff --git a/cli/koji b/cli/koji index fdb2a1b..0a79ca3 100755 --- a/cli/koji +++ b/cli/koji @@ -51,8 +51,8 @@ def register_plugin(plugin): Handlers are functions marked with one of the decorators defined in koji.plugin """ - for v in vars(plugin).itervalues(): - if isinstance(v, (types.ClassType, types.TypeType)): + for v in six.itervalues(vars(plugin)): + if isinstance(v, six.class_types): #skip classes continue if callable(v): @@ -65,16 +65,14 @@ def register_plugin(plugin): globals()[name] = v -def load_plugins(options): +def load_plugins(options, path): """Load plugins specified by our configuration plus system plugins. Order is that system plugins are first, so they can be overriden by user-specified ones with same name.""" logger = logging.getLogger('koji.plugins') - syspath = '%s/lib/python%s.%s/site-packages/koji_cli_plugins' % \ - (sys.prefix, sys.version_info.major, sys.version_info.minor) - if os.path.exists(syspath): - tracker = koji.plugin.PluginTracker(path=syspath) - for name in sorted(os.listdir(syspath)): + if os.path.exists(path): + tracker = koji.plugin.PluginTracker(path=path) + for name in sorted(os.listdir(path)): if not name.endswith('.py'): continue name = name[:-3] @@ -165,7 +163,9 @@ def get_options(): else: warn("Warning: The pkgurl option is obsolete, please use topurl instead") - load_plugins(options) + plugins_path = '%s/lib/python%s.%s/site-packages/koji_cli_plugins' % \ + (sys.prefix, sys.version_info.major, sys.version_info.minor) + load_plugins(options, plugins_path) if options.help_commands: list_commands() diff --git a/koji/plugin.py b/koji/plugin.py index e3ee49c..0f874ef 100644 --- a/koji/plugin.py +++ b/koji/plugin.py @@ -24,6 +24,7 @@ import koji import logging import sys import traceback +import six # the available callback hooks and a list # of functions to be called for each event @@ -91,7 +92,7 @@ class PluginTracker(object): return self.plugins.get(name) def pathlist(self, path): - if isinstance(path, basestring): + if isinstance(path, six.string_types): return [path] else: return path diff --git a/tests/test_cli/data/plugins/not_plugin.omg b/tests/test_cli/data/plugins/not_plugin.omg new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/tests/test_cli/data/plugins/not_plugin.omg diff --git a/tests/test_cli/data/plugins/plugin1.py b/tests/test_cli/data/plugins/plugin1.py new file mode 100644 index 0000000..3e405f5 --- /dev/null +++ b/tests/test_cli/data/plugins/plugin1.py @@ -0,0 +1,19 @@ +from koji.plugin import export_cli, export_as + +@export_as('foobar') +@export_cli +def foo(): + pass + +@export_cli +def foo2(): + pass + +def foo3(): + pass + +foo4 = 'foo4' + +class bar(): + pass + diff --git a/tests/test_cli/data/plugins/plugin2.py b/tests/test_cli/data/plugins/plugin2.py new file mode 100644 index 0000000..58648d6 --- /dev/null +++ b/tests/test_cli/data/plugins/plugin2.py @@ -0,0 +1 @@ +sth = 123 \ No newline at end of file diff --git a/tests/test_cli/test_load_plugins.py b/tests/test_cli/test_load_plugins.py new file mode 100644 index 0000000..411ee96 --- /dev/null +++ b/tests/test_cli/test_load_plugins.py @@ -0,0 +1,20 @@ +from __future__ import absolute_import +import mock +import os +import unittest + +from . import loadcli +cli = loadcli.cli + + +class TestLoadPlugins(unittest.TestCase): + + @mock.patch('logging.getLogger') + def test_load_plugins(self, getLogger): + options = mock.MagicMock() + cli.load_plugins(options, os.path.dirname(__file__) + '/data/plugins') + self.assertTrue(callable(cli.foobar)) + self.assertTrue(callable(cli.foo2)) + self.assertFalse(hasattr(cli, 'foo3')) + self.assertFalse(hasattr(cli, 'foo4')) + self.assertFalse(hasattr(cli, 'sth')) diff --git a/tests/test_lib/test_plugin.py b/tests/test_lib/test_plugin.py new file mode 100644 index 0000000..02376af --- /dev/null +++ b/tests/test_lib/test_plugin.py @@ -0,0 +1,211 @@ +import copy +import mock +import unittest + +import koji +import koji.plugin + + +class TestCallbackDecorators(unittest.TestCase): + + def test_callback_decorator(self): + def myfunc(a, b, c=None): + return [a,b,c] + callbacks = ('preImport', 'postImport') + newfunc = koji.plugin.callback(*callbacks)(myfunc) + self.assertEqual(newfunc.callbacks, callbacks) + self.assertEqual(newfunc(1, 2), [1, 2, None]) + self.assertEqual(newfunc(1, 2, 3), [1, 2, 3]) + + def test_ignore_error_decorator(self): + def myfunc(a, b, c=None): + return [a,b,c] + newfunc = koji.plugin.ignore_error(myfunc) + self.assertEqual(newfunc.failure_is_an_option, True) + self.assertEqual(newfunc(1, 2), [1, 2, None]) + self.assertEqual(newfunc(1, 2, 3), [1, 2, 3]) + + def test_export_decorator(self): + def myfunc(a, b, c=None): + return [a,b,c] + newfunc = koji.plugin.export(myfunc) + self.assertEqual(newfunc.exported, True) + self.assertEqual(newfunc(1, 2), [1, 2, None]) + self.assertEqual(newfunc(1, 2, 3), [1, 2, 3]) + + def test_export_cli_decorator(self): + def myfunc(a, b, c=None): + return [a,b,c] + newfunc = koji.plugin.export_cli(myfunc) + self.assertEqual(newfunc.exported_cli, True) + self.assertEqual(newfunc(1, 2), [1, 2, None]) + self.assertEqual(newfunc(1, 2, 3), [1, 2, 3]) + + def test_export_as_decorator(self): + def myfunc(a, b, c=None): + return [a,b,c] + alias = "ALIAS" + newfunc = koji.plugin.export_as(alias)(myfunc) + self.assertEqual(newfunc.exported, True) + self.assertEqual(newfunc.export_alias, alias) + self.assertEqual(newfunc(1, 2), [1, 2, None]) + self.assertEqual(newfunc(1, 2, 3), [1, 2, 3]) + + def test_export_in_decorator_with_alias(self): + def myfunc(a, b, c=None): + return [a,b,c] + newfunc = koji.plugin.export_in('MODULE', 'ALIAS')(myfunc) + self.assertEqual(newfunc.exported, True) + self.assertEqual(newfunc.export_alias, 'MODULE.ALIAS') + self.assertEqual(newfunc.export_module, 'MODULE') + self.assertEqual(newfunc(1, 2), [1, 2, None]) + self.assertEqual(newfunc(1, 2, 3), [1, 2, 3]) + + def test_export_in_decorator_no_alias(self): + def myfunc(a, b, c=None): + return [a,b,c] + newfunc = koji.plugin.export_in('MODULE')(myfunc) + self.assertEqual(newfunc.exported, True) + self.assertEqual(newfunc.export_alias, 'MODULE.myfunc') + self.assertEqual(newfunc.export_module, 'MODULE') + self.assertEqual(newfunc(1, 2), [1, 2, None]) + self.assertEqual(newfunc(1, 2, 3), [1, 2, 3]) + + +class TestError(Exception): + """Raised by a test callback defined below""" + pass + + +class TestCallbacks(unittest.TestCase): + + def setUp(self): + self.orig_callbacks = koji.plugin.callbacks + koji.plugin.callbacks = copy.deepcopy(koji.plugin.callbacks) + self.callbacks = [] + + def tearDown(self): + koji.plugin.callbacks = self.orig_callbacks + + def callback(self, cbtype, *args, **kwargs): + self.callbacks.append([cbtype, args, kwargs]) + + def error_callback(self, cbtype, *args, **kwargs): + raise TestError + + @koji.plugin.ignore_error + def safe_error_callback(self, cbtype, *args, **kwargs): + self.callbacks.append([cbtype, args, kwargs]) + raise TestError + + def test_simple_callback(self): + args = ('hello',) + kwargs = {'world': 1} + cbtype = 'preTag' + koji.plugin.register_callback(cbtype, self.callback) + koji.plugin.run_callbacks(cbtype, *args, **kwargs) + self.assertEqual(len(self.callbacks), 1) + self.assertEqual(self.callbacks[0], [cbtype, args, kwargs]) + + def test_error_callback(self): + args = ('hello',) + kwargs = {'world': 1} + cbtype = 'preTag' + koji.plugin.register_callback(cbtype, self.error_callback) + with self.assertRaises(koji.CallbackError): + koji.plugin.run_callbacks(cbtype, *args, **kwargs) + + @mock.patch('logging.getLogger') + def test_safe_error_callback(self, getLogger): + args = ('hello',) + kwargs = {'world': 1} + cbtype = 'preTag' + koji.plugin.register_callback(cbtype, self.safe_error_callback) + koji.plugin.run_callbacks(cbtype, *args, **kwargs) + self.assertEqual(len(self.callbacks), 1) + self.assertEqual(self.callbacks[0], [cbtype, args, kwargs]) + getLogger.assert_called_once() + getLogger.return_value.warn.assert_called_once() + + def test_bad_callback(self): + args = ('hello',) + kwargs = {'world': 1} + with self.assertRaises(koji.PluginError): + koji.plugin.register_callback('badtype', self.callback) + with self.assertRaises(koji.PluginError): + koji.plugin.register_callback('preImport', "not a function") + self.assertEqual(koji.plugin.callbacks, self.orig_callbacks) + with self.assertRaises(koji.PluginError): + koji.plugin.run_callbacks('badtype', *args, **kwargs) + + +class TestPluginTracker(unittest.TestCase): + + def setUp(self): + self.find_module = mock.patch('imp.find_module').start() + self.modfile = mock.MagicMock() + self.modpath = mock.MagicMock() + self.moddesc = mock.MagicMock() + self.find_module.return_value = (self.modfile, self.modpath, + self.moddesc) + self.load_module = mock.patch('imp.load_module').start() + + def tearDown(self): + mock.patch.stopall() + + def test_tracked_plugin(self): + tracker = koji.plugin.PluginTracker(path='/MODPATH') + self.load_module.return_value = 'MODULE!' + tracker.load('hello') + self.assertEqual(tracker.get('hello'), 'MODULE!') + self.find_module.assert_called_once_with('hello', ['/MODPATH']) + + def test_plugin_reload(self): + tracker = koji.plugin.PluginTracker(path='/MODPATH') + self.load_module.return_value = 'MODULE!' + tracker.load('hello') + self.assertEqual(tracker.get('hello'), 'MODULE!') + + # should not reload if we don't ask + self.load_module.return_value = 'DUPLICATE!' + tracker.load('hello') + self.assertEqual(tracker.get('hello'), 'MODULE!') + + # should reload if we do ask + tracker.load('hello', reload=True) + self.assertEqual(tracker.get('hello'), 'DUPLICATE!') + + # should throw exception if not reloading and duplicate in sys.modules + module_mock = mock.MagicMock() + with mock.patch.dict('sys.modules', _koji_plugin__dup_module=module_mock): + with self.assertRaises(koji.PluginError): + tracker.load('dup_module') + + def test_no_plugin_path(self): + tracker = koji.plugin.PluginTracker() + with self.assertRaises(koji.PluginError): + tracker.load('hello') + self.load_module.assert_not_called() + self.assertEqual(tracker.get('hello'), None) + + def test_plugin_path_list(self): + tracker = koji.plugin.PluginTracker(path='/MODPATH') + self.load_module.return_value = 'MODULE!' + tracker.load('hello', path=['/PATH1', '/PATH2']) + self.assertEqual(tracker.get('hello'), 'MODULE!') + self.find_module.assert_called_once_with('hello', ['/PATH1', '/PATH2']) + + self.find_module.reset_mock() + tracker.load('hey', path='/PATH1') + self.assertEqual(tracker.get('hey'), 'MODULE!') + self.find_module.assert_called_once_with('hey', ['/PATH1']) + + @mock.patch('logging.getLogger') + def test_bad_plugin(self, getLogger): + tracker = koji.plugin.PluginTracker(path='/MODPATH') + self.load_module.side_effect = TestError + with self.assertRaises(TestError): + tracker.load('hello') + self.assertEqual(tracker.get('hello'), None) + getLogger.assert_called_once() + getLogger.return_value.error.assert_called_once()