From 78bafc49de382e4d9492c4fa90d9f461626e359a Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Jun 21 2017 15:44:37 +0000 Subject: [PATCH 1/7] fix StringType and itervalues in plugin and cli --- diff --git a/cli/koji b/cli/koji index fdb2a1b..4959f3b 100755 --- a/cli/koji +++ b/cli/koji @@ -51,7 +51,7 @@ def register_plugin(plugin): Handlers are functions marked with one of the decorators defined in koji.plugin """ - for v in vars(plugin).itervalues(): + for v in six.itervalues(vars(plugin)): if isinstance(v, (types.ClassType, types.TypeType)): #skip classes continue 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 From 9c749f051e0407e2780c18b0da8e0c5c70e4c84b Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Jun 21 2017 15:44:37 +0000 Subject: [PATCH 2/7] use six.class_types instead of ClassType, TypeType in cli --- diff --git a/cli/koji b/cli/koji index 4959f3b..0f75927 100755 --- a/cli/koji +++ b/cli/koji @@ -52,7 +52,7 @@ def register_plugin(plugin): Handlers are functions marked with one of the decorators defined in koji.plugin """ for v in six.itervalues(vars(plugin)): - if isinstance(v, (types.ClassType, types.TypeType)): + if isinstance(v, six.class_types): #skip classes continue if callable(v): From 35c13986c46e90925591789b1d36310d2a0facb3 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jun 21 2017 16:09:36 +0000 Subject: [PATCH 3/7] some unit tests for koji.plugins --- diff --git a/tests/test_lib/test_plugin.py b/tests/test_lib/test_plugin.py new file mode 100644 index 0000000..87c9f8a --- /dev/null +++ b/tests/test_lib/test_plugin.py @@ -0,0 +1,71 @@ +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]) From 6692b7e8d95fc6b3a0f7d443bb5bfb12985d4292 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jun 21 2017 16:49:35 +0000 Subject: [PATCH 4/7] unit tests for callback handling --- diff --git a/tests/test_lib/test_plugin.py b/tests/test_lib/test_plugin.py index 87c9f8a..fbf91ca 100644 --- a/tests/test_lib/test_plugin.py +++ b/tests/test_lib/test_plugin.py @@ -1,3 +1,4 @@ +import copy import mock import unittest @@ -69,3 +70,71 @@ class TestCallbackDecorators(unittest.TestCase): 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) + From 92b974a16aedbf18b05040e3a19a5bf47cf6ebd2 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jun 21 2017 17:18:06 +0000 Subject: [PATCH 5/7] unit tests for PluginTracker --- diff --git a/tests/test_lib/test_plugin.py b/tests/test_lib/test_plugin.py index fbf91ca..d3fc19d 100644 --- a/tests/test_lib/test_plugin.py +++ b/tests/test_lib/test_plugin.py @@ -138,3 +138,63 @@ class TestCallbacks(unittest.TestCase): 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!') + + 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']) + + @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() From 360ecd14f4a4c27c06f226c9a541da8631dcc1a1 Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: May 08 2017 01:49:05 +0000 Subject: [PATCH 6/7] extend unit tests for PluginTracker --- diff --git a/tests/test_lib/test_plugin.py b/tests/test_lib/test_plugin.py index d3fc19d..02376af 100644 --- a/tests/test_lib/test_plugin.py +++ b/tests/test_lib/test_plugin.py @@ -175,6 +175,12 @@ class TestPluginTracker(unittest.TestCase): 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): @@ -189,6 +195,11 @@ class TestPluginTracker(unittest.TestCase): 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') From 916fbd778a00504b4323307edb6d5e4bc76a68d5 Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Jun 22 2017 05:36:04 +0000 Subject: [PATCH 7/7] unit tests for cli.load_plugins --- diff --git a/cli/koji b/cli/koji index 0f75927..0a79ca3 100755 --- a/cli/koji +++ b/cli/koji @@ -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/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'))