Checkbox

Merge lp:~zyga/checkbox/better-plugins into lp:checkbox

  1. better-plugins
  2. Merge into trunk
Proposed by Zygmunt Krynicki
Status: Merged
Approved by: Zygmunt Krynicki
Approved revision: 2583
Merged at revision: 2573
Proposed branch: lp:~zyga/checkbox/better-plugins
Merge into: lp:checkbox
Diff against target: 697 lines (+425/-51)
2 files modified
plainbox/plainbox/impl/secure/plugins.py (+127/-22)
plainbox/plainbox/impl/secure/test_plugins.py (+298/-29)
To merge this branch: bzr merge lp:~zyga/checkbox/better-plugins
Reviewer Review Type Date Requested Status
Zygmunt Krynicki (community) Approve
Brendan Donegan (community) Approve
Review via email: mp+199324@code.launchpad.net

Description of the change

5d891e0 plainbox:secure:plugins: make IPlugInCollection a proper interface
77a2935 plainbox:secure:plugins: add tests for PlugIn
1d869c8 plainbox:secure:plugins: add tests for PlugInCollectionBase
c501ccb plainbox:secure:plugins: add docstrings for PkgResourcesPlugInCollectionTests
2566d72 plainbox:secure:plugins: add docstrings for PlugIn
6a57cef plainbox:secure:plugins: add docstrings for PlugInCollectionBase
541c2f2 plainbox:secure:plugins: add a way to track problems in plugin collection
5e0fd54 plainbox:secure:plugins: add IPlugInCollection.get_all_plugin_objects()
aba51ed plainbox:secure:plugins: add a way to pass extra arguments to wrappers
0fc1159 plainbox:secure:plugins: handle OSError in FSPlugInCollection
4a317ea plainbox:secure:plugins: support multiple extensions
a9244ca plainbox:secure:plugins: unify fake_open() functions

Assorted improvements to plugins

To post a comment you must log in.
Revision history for this message
Zygmunt Krynicki (zyga) wrote :

I'll work on making sure new features are tested but please do send other comments

review: Needs Fixing
Revision history for this message
Daniel Manrique (roadmr) wrote :

Looks OK to me, this is pretty foundational and I'd like to see how this is used, but as a starting point it's fine. Also looking at upcoming tests would make it easier to see how it all fits together.

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

I have all the usage code written now. I'll post it tomorrow in the
load-all-jobs branch.

On Tue, Dec 17, 2013 at 10:33 PM, Daniel Manrique
<email address hidden> wrote:
> Looks OK to me, this is pretty foundational and I'd like to see how this is used, but as a starting point it's fine. Also looking at upcoming tests would make it easier to see how it all fits together.
> --
> https://code.launchpad.net/~zkrynicki/checkbox/better-plugins/+merge/199324
> You are the owner of lp:~zkrynicki/checkbox/better-plugins.

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

Added lumps of tests, fixed some iffy bugs :-)

Revision history for this message
Brendan Donegan (brendan-donegan) wrote :

Looks good, +1

review: Approve
Revision history for this message
Daniel Manrique (roadmr) wrote :
Download full text (6.3 KiB)

The attempt to merge lp:~zkrynicki/checkbox/better-plugins into lp:checkbox failed. Below is the output from the failed tests.

[precise] Bringing VM 'up'
[precise] (timing) 8.91user 3.90system 5:45.47elapsed 3%CPU (0avgtext+0avgdata 20960maxresident)k
[precise] (timing) 0inputs+256outputs (0major+168550minor)pagefaults 0swaps
[precise] Starting tests...
[precise] Checkbox GUI build: PASS
[precise] (timing) 1.12user 0.48system 1:03.72elapsed 2%CPU (0avgtext+0avgdata 20240maxresident)k
[precise] (timing) 7336inputs+72outputs (84major+47364minor)pagefaults 0swaps
[precise] CheckBox test suite: PASS
[precise] (timing) 0.76user 0.30system 0:37.35elapsed 2%CPU (0avgtext+0avgdata 20216maxresident)k
[precise] (timing) 0inputs+40outputs (0major+49209minor)pagefaults 0swaps
[precise] (timing) 0.73user 0.32system 0:05.73elapsed 18%CPU (0avgtext+0avgdata 20628maxresident)k
[precise] (timing) 0inputs+16outputs (0major+48876minor)pagefaults 0swaps
[precise] PlainBox test suite: FAIL
[precise] stdout: http://paste.ubuntu.com/6604998/
[precise] stderr: http://paste.ubuntu.com/6604999/
[precise] (timing) Command exited with non-zero status 1
[precise] (timing) 0.98user 0.30system 0:13.85elapsed 9%CPU (0avgtext+0avgdata 22184maxresident)k
[precise] (timing) 0inputs+592outputs (0major+48140minor)pagefaults 0swaps
[precise] PlainBox documentation build: PASS
[precise] (timing) 0.93user 0.29system 0:25.96elapsed 4%CPU (0avgtext+0avgdata 23568maxresident)k
[precise] (timing) 0inputs+968outputs (0major+48440minor)pagefaults 0swaps
[precise] CheckBoxNG test suite: FAIL
[precise] stdout: http://paste.ubuntu.com/6605000/
[precise] stderr: http://paste.ubuntu.com/6605001/
[precise] (timing) Command exited with non-zero status 1
[precise] (timing) 0.80user 0.28system 0:06.72elapsed 16%CPU (0avgtext+0avgdata 19704maxresident)k
[precise] (timing) 0inputs+24outputs (0major+47404minor)pagefaults 0swaps
[precise] Integration tests: FAIL
[precise] stdout:
[precise] stderr: http://paste.ubuntu.com/6605002/
[precise] (timing) Command exited with non-zero status 1
[precise] (timing) 0.72user 0.29system 0:05.49elapsed 18%CPU (0avgtext+0avgdata 20464maxresident)k
[precise] (timing) 0inputs+8outputs (0major+47416minor)pagefaults 0swaps
[precise] Destroying VM
[quantal] Bringing VM 'up'
[quantal] (timing) 8.06user 3.67system 4:40.75elapsed 4%CPU (0avgtext+0avgdata 21280maxresident)k
[quantal] (timing) 16inputs+232outputs (1major+202643minor)pagefaults 0swaps
[quantal] Starting tests...
[quantal] Checkbox GUI build: PASS
[quantal] (timing) 0.84user 0.27system 1:08.02elapsed 1%CPU (0avgtext+0avgdata 19808maxresident)k
[quantal] (timing) 0inputs+64outputs (0major+47283minor)pagefaults 0swaps
[quantal] CheckBox test suite: PASS
[quantal] (timing) 0.81user 0.31system 0:41.79elapsed 2%CPU (0avgtext+0avgdata 19808maxresident)k
[quantal] (timing) 0inputs+40outputs (0major+47410minor)pagefaults 0swaps
[quantal] (timing) 0.72user 0.29system 0:08.18elapsed 12%CPU (0avgtext+0avgdata 20632maxresident)k
[quantal] (timing) 0inputs+16outputs (0major+47353minor)pagefaults 0swaps
[quantal...

Read more...

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

Ah, so collections.abc is python3.3 only, I've read the docs for that module and it seems it used to be a part of collections directly so I've changed references refrences ``collections.abc.Sequence`` to just ``collections.Sequence``.

Trying again?

review: Approve
Revision history for this message
Daniel Manrique (roadmr) wrote :
Download full text (5.6 KiB)

The attempt to merge lp:~zkrynicki/checkbox/better-plugins into lp:checkbox failed. Below is the output from the failed tests.

[precise] Bringing VM 'up'
[precise] (timing) 8.83user 3.86system 5:45.82elapsed 3%CPU (0avgtext+0avgdata 21460maxresident)k
[precise] (timing) 8inputs+240outputs (1major+170203minor)pagefaults 0swaps
[precise] Starting tests...
[precise] Checkbox GUI build: PASS
[precise] (timing) 0.81user 0.26system 1:03.47elapsed 1%CPU (0avgtext+0avgdata 19940maxresident)k
[precise] (timing) 24inputs+56outputs (0major+47531minor)pagefaults 0swaps
[precise] CheckBox test suite: PASS
[precise] (timing) 0.83user 0.29system 0:37.48elapsed 3%CPU (0avgtext+0avgdata 19936maxresident)k
[precise] (timing) 0inputs+48outputs (0major+47353minor)pagefaults 0swaps
[precise] (timing) 0.74user 0.28system 0:05.68elapsed 18%CPU (0avgtext+0avgdata 20156maxresident)k
[precise] (timing) 0inputs+16outputs (0major+49065minor)pagefaults 0swaps
[precise] PlainBox test suite: FAIL
[precise] stdout: http://paste.ubuntu.com/6605282/
[precise] stderr: http://paste.ubuntu.com/6605283/
[precise] (timing) Command exited with non-zero status 1
[precise] (timing) 1.05user 0.28system 0:16.62elapsed 8%CPU (0avgtext+0avgdata 20672maxresident)k
[precise] (timing) 0inputs+312outputs (0major+47685minor)pagefaults 0swaps
[precise] PlainBox documentation build: PASS
[precise] (timing) 0.78user 0.30system 0:33.72elapsed 3%CPU (0avgtext+0avgdata 20688maxresident)k
[precise] (timing) 0inputs+32outputs (0major+47361minor)pagefaults 0swaps
[precise] CheckBoxNG test suite: PASS
[precise] (timing) 0.75user 0.27system 0:07.30elapsed 14%CPU (0avgtext+0avgdata 20524maxresident)k
[precise] (timing) 0inputs+16outputs (0major+47277minor)pagefaults 0swaps
[precise] Integration tests: PASS
[precise] (timing) 0.80user 0.32system 0:07.68elapsed 14%CPU (0avgtext+0avgdata 20132maxresident)k
[precise] (timing) 0inputs+8outputs (0major+47029minor)pagefaults 0swaps
[precise] Destroying VM
[quantal] Bringing VM 'up'
[quantal] (timing) 8.29user 3.74system 4:40.67elapsed 4%CPU (0avgtext+0avgdata 21004maxresident)k
[quantal] (timing) 0inputs+224outputs (0major+205394minor)pagefaults 0swaps
[quantal] Starting tests...
[quantal] Checkbox GUI build: PASS
[quantal] (timing) 0.79user 0.28system 1:08.13elapsed 1%CPU (0avgtext+0avgdata 20308maxresident)k
[quantal] (timing) 792inputs+56outputs (19major+47278minor)pagefaults 0swaps
[quantal] CheckBox test suite: PASS
[quantal] (timing) 1.06user 0.46system 0:41.21elapsed 3%CPU (0avgtext+0avgdata 19932maxresident)k
[quantal] (timing) 4576inputs+40outputs (49major+47199minor)pagefaults 0swaps
[quantal] (timing) 0.79user 0.25system 0:08.35elapsed 12%CPU (0avgtext+0avgdata 20300maxresident)k
[quantal] (timing) 0inputs+16outputs (0major+47371minor)pagefaults 0swaps
[quantal] PlainBox test suite: FAIL
[quantal] stdout: http://paste.ubuntu.com/6605322/
[quantal] stderr: http://paste.ubuntu.com/6605323/
[quantal] (timing) Command exited with non-zero status 1
[quantal] (timing) 1.09user 0.31system 0:23.71elapsed 5%CPU (0avgtext+0avgdata 2067...

Read more...

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

So a small genuine problem that I think was masked by python3.3 and its new exception class hierarchy. I've corrected that (it still passes on 3.3) so let's see again.

review: Approve
Revision history for this message
Daniel Manrique (roadmr) wrote :
Download full text (5.6 KiB)

The attempt to merge lp:~zkrynicki/checkbox/better-plugins into lp:checkbox failed. Below is the output from the failed tests.

[precise] Bringing VM 'up'
[precise] (timing) 9.20user 4.04system 6:06.51elapsed 3%CPU (0avgtext+0avgdata 22112maxresident)k
[precise] (timing) 8inputs+240outputs (1major+180941minor)pagefaults 0swaps
[precise] Starting tests...
[precise] Checkbox GUI build: PASS
[precise] (timing) 1.02user 0.46system 1:15.98elapsed 1%CPU (0avgtext+0avgdata 20528maxresident)k
[precise] (timing) 5368inputs+72outputs (68major+49017minor)pagefaults 0swaps
[precise] CheckBox test suite: PASS
[precise] (timing) 0.78user 0.40system 0:38.82elapsed 3%CPU (0avgtext+0avgdata 19940maxresident)k
[precise] (timing) 0inputs+40outputs (0major+47310minor)pagefaults 0swaps
[precise] (timing) 0.79user 0.30system 0:05.80elapsed 18%CPU (0avgtext+0avgdata 19960maxresident)k
[precise] (timing) 0inputs+16outputs (0major+47450minor)pagefaults 0swaps
[precise] PlainBox test suite: FAIL
[precise] stdout: http://paste.ubuntu.com/6605582/
[precise] stderr: http://paste.ubuntu.com/6605583/
[precise] (timing) Command exited with non-zero status 1
[precise] (timing) 1.10user 0.32system 0:17.36elapsed 8%CPU (0avgtext+0avgdata 20664maxresident)k
[precise] (timing) 0inputs+320outputs (0major+47572minor)pagefaults 0swaps
[precise] PlainBox documentation build: PASS
[precise] (timing) 0.81user 0.30system 0:35.62elapsed 3%CPU (0avgtext+0avgdata 20520maxresident)k
[precise] (timing) 0inputs+32outputs (0major+47491minor)pagefaults 0swaps
[precise] CheckBoxNG test suite: PASS
[precise] (timing) 0.77user 0.29system 0:07.41elapsed 14%CPU (0avgtext+0avgdata 20060maxresident)k
[precise] (timing) 0inputs+16outputs (0major+49839minor)pagefaults 0swaps
[precise] Integration tests: PASS
[precise] (timing) 0.76user 0.28system 0:07.66elapsed 13%CPU (0avgtext+0avgdata 20492maxresident)k
[precise] (timing) 0inputs+8outputs (0major+47469minor)pagefaults 0swaps
[precise] Destroying VM
[quantal] Bringing VM 'up'
[quantal] (timing) 8.22user 3.85system 4:38.93elapsed 4%CPU (0avgtext+0avgdata 21020maxresident)k
[quantal] (timing) 16inputs+224outputs (0major+206523minor)pagefaults 0swaps
[quantal] Starting tests...
[quantal] Checkbox GUI build: PASS
[quantal] (timing) 0.87user 0.34system 1:08.38elapsed 1%CPU (0avgtext+0avgdata 20456maxresident)k
[quantal] (timing) 1840inputs+64outputs (16major+47067minor)pagefaults 0swaps
[quantal] CheckBox test suite: PASS
[quantal] (timing) 0.78user 0.29system 0:40.66elapsed 2%CPU (0avgtext+0avgdata 20604maxresident)k
[quantal] (timing) 0inputs+40outputs (0major+47333minor)pagefaults 0swaps
[quantal] (timing) 0.77user 0.26system 0:08.31elapsed 12%CPU (0avgtext+0avgdata 20608maxresident)k
[quantal] (timing) 0inputs+16outputs (0major+47372minor)pagefaults 0swaps
[quantal] PlainBox test suite: FAIL
[quantal] stdout: http://paste.ubuntu.com/6605637/
[quantal] stderr: http://paste.ubuntu.com/6605638/
[quantal] (timing) Command exited with non-zero status 1
[quantal] (timing) 1.14user 0.25system 0:23.65elapsed 5%CPU (0avgtext+0avgdata 198...

Read more...

lp:~zyga/checkbox/better-plugins updated
2581. By Zygmunt Krynicki

plainbox:secure:plugins: handle OSError in FSPlugInCollection

This patch makes FSPlugInCollection handle both OSError and IOError
since IOError can happen in read (this was already handled) but OSError
can happen on open().

Signed-off-by: Zygmunt Krynicki <email address hidden>

2582. By Zygmunt Krynicki

plainbox:secure:plugins: support multiple extensions

This patch adds support for multiple file extensions in
FsPlugInCollection. By passing any sequence-like object instead of a
plain string any of the extensions contained therein are accepted.

Signed-off-by: Zygmunt Krynicki <email address hidden>

2583. By Zygmunt Krynicki

plainbox:secure:plugins: unify fake_open() functions

Signed-off-by: Zygmunt Krynicki <email address hidden>

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

The ping-pong continues, apparenetly there was a separate problem in FsPlugInCollection that dind't handle OSError. It now does so let's try again.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plainbox/plainbox/impl/secure/plugins.py'
2--- plainbox/plainbox/impl/secure/plugins.py 2013-12-16 20:39:40 +0000
3+++ plainbox/plainbox/impl/secure/plugins.py 2013-12-20 13:53:55 +0000
4@@ -98,14 +98,20 @@
5
6 @property
7 def plugin_name(self):
8+ """
9+ plugin name, arbitrary string
10+ """
11 return self._name
12
13 @property
14 def plugin_object(self):
15+ """
16+ plugin object, arbitrary object
17+ """
18 return self._obj
19
20
21-class IPlugInCollection:
22+class IPlugInCollection(metaclass=abc.ABCMeta):
23 """
24 A collection of IPlugIn objects.
25 """
26@@ -129,11 +135,26 @@
27 """
28
29 @abc.abstractmethod
30+ def get_all_plugin_objects(self):
31+ """
32+ Get an list of plug-in objects
33+
34+ This is a shortcut that gives fastest access to a list of
35+ :attr:`IPlugIn.plugin_object` of each loaded plugin.
36+ """
37+
38+ @abc.abstractmethod
39 def get_all_items(self):
40 """
41 Get an iterator to a sequence of (name, plug-in)
42 """
43
44+ @abc.abstractproperty
45+ def problem_list(self):
46+ """
47+ List of problems encountered while loading plugins
48+ """
49+
50 @abc.abstractmethod
51 def load(self):
52 """
53@@ -146,15 +167,18 @@
54
55 @abc.abstractmethod
56 @contextlib.contextmanager
57- def fake_plugins(self, plugins):
58+ def fake_plugins(self, plugins, problem_list=None):
59 """
60 Context manager for using fake list of plugins
61
62- :param plugins: list of PlugIn-alike objects
63+ :param plugins:
64+ list of PlugIn-alike objects
65+ :param problem_list:
66+ list of problems (exceptions)
67
68- The provided list of plugins overrides any previously loaded
69- plugins and prevent loading any other, real, plugins. After
70- the context manager exits the previous state is restored.
71+ The provided list of plugins and exceptions overrides any previously
72+ loaded plugins and prevent loading any other, real, plugins. After the
73+ context manager exits the previous state is restored.
74 """
75
76
77@@ -164,7 +188,8 @@
78 PlugInCollection implemenetations.
79 """
80
81- def __init__(self, load=False, wrapper=PlugIn):
82+ def __init__(self, load=False, wrapper=PlugIn, *wrapper_args,
83+ **wrapper_kwargs):
84 """
85 Initialize a collection of plug-ins
86
87@@ -172,68 +197,129 @@
88 if true, load all of the plug-ins now
89 :param wrapper:
90 wrapper class for all loaded objects, defaults to :class:`PlugIn`
91+ :param wrapper_args:
92+ additional arguments passed to each instantiated wrapper
93+ :param wrapper_kwargs:
94+ additional keyword arguments passed to each instantiated wrapper
95 """
96 self._wrapper = wrapper
97+ self._wrapper_args = wrapper_args
98+ self._wrapper_kwargs = wrapper_kwargs
99 self._plugins = collections.OrderedDict()
100 self._loaded = False
101 self._mocked_objects = None
102+ self._problem_list = []
103 if load:
104 self.load()
105
106 def get_by_name(self, name):
107 """
108 Get the specified plug-in (by name)
109+
110+ :param name:
111+ name of the plugin to locate
112+ :returns:
113+ :class:`PlugIn` like object associated with the name
114+ :raises KeyError:
115+ if the specified name cannot be found
116 """
117 return self._plugins[name]
118
119 def get_all_names(self):
120 """
121- Get an iterator to a sequence of plug-in names
122+ Get a list of all the plug-in names
123+
124+ :returns:
125+ a list of plugin names
126 """
127 return list(self._plugins.keys())
128
129 def get_all_plugins(self):
130 """
131- Get an iterator to a sequence plug-ins
132+ Get a list of all the plug-ins
133+
134+ :returns:
135+ a list of plugin objects
136 """
137 return list(self._plugins.values())
138
139+ def get_all_plugin_objects(self):
140+ """
141+ Get an list of plug-in objects
142+ """
143+ return [plugin.plugin_object for plugin in self._plugins.values()]
144+
145 def get_all_items(self):
146 """
147- Get an iterator to a sequence of (name, plug-in)
148+ Get a list of all the pairs of plugin name and plugin
149+
150+ :returns:
151+ a list of tuples (plugin.plugin_name, plugin)
152 """
153 return list(self._plugins.items())
154
155+ @property
156+ def problem_list(self):
157+ """
158+ List of problems encountered while loading plugins
159+ """
160+ return self._problem_list
161+
162 @contextlib.contextmanager
163- def fake_plugins(self, plugins):
164+ def fake_plugins(self, plugins, problem_list=None):
165 """
166 Context manager for using fake list of plugins
167
168- :param plugins: list of PlugIn-alike objects
169+ :param plugins:
170+ list of PlugIn-alike objects
171+ :param problem_list:
172+ list of problems (exceptions)
173
174 The provided list of plugins overrides any previously loaded
175 plugins and prevent loading any other, real, plugins. After
176 the context manager exits the previous state is restored.
177 """
178 old_loaded = self._loaded
179+ old_problem_list = self._problem_list
180 old_plugins = self._plugins
181 self._loaded = True
182 self._plugins = collections.OrderedDict([
183 (plugin.plugin_name, plugin)
184 for plugin in sorted(
185 plugins, key=lambda plugin: plugin.plugin_name)])
186+ if problem_list is None:
187+ problem_list = []
188+ self._problem_list = problem_list
189 try:
190 yield
191 finally:
192 self._loaded = old_loaded
193 self._plugins = old_plugins
194+ self._problem_list = old_problem_list
195
196 def wrap_and_add_plugin(self, plugin_name, plugin_obj):
197+ """
198+ Internal method of PlugInCollectionBase.
199+
200+ :param plugin_name:
201+ plugin name, some arbitrary string
202+ :param plugin_obj:
203+ plugin object, some arbitrary object.
204+
205+ This method prepares a wrapper (PlugIn subclass instance) for the
206+ specified plugin name/object by attempting to instantiate the wrapper
207+ class. If a PlugInError exception is raised then it is added to the
208+ problem_list and the corresponding plugin is not added to the
209+ collection of plugins.
210+ """
211 try:
212- wrapper = self._wrapper(plugin_name, plugin_obj)
213+ wrapper = self._wrapper(
214+ plugin_name, plugin_obj,
215+ *self._wrapper_args, **self._wrapper_kwargs)
216 except PlugInError as exc:
217 logger.warning(
218 "Unable to prepare plugin %s: %s", plugin_name, exc)
219+ self._problem_list.append(exc)
220 else:
221 self._plugins[plugin_name] = wrapper
222
223@@ -248,7 +334,8 @@
224 may be adjusted if required.
225 """
226
227- def __init__(self, namespace, load=False, wrapper=PlugIn):
228+ def __init__(self, namespace, load=False, wrapper=PlugIn, *wrapper_args,
229+ **wrapper_kwargs):
230 """
231 Initialize a collection of plug-ins from the specified name-space.
232
233@@ -258,9 +345,13 @@
234 if true, load all of the plug-ins now
235 :param wrapper:
236 wrapper class for all loaded objects, defaults to :class:`PlugIn`
237+ :param wrapper_args:
238+ additional arguments passed to each instantiated wrapper
239+ :param wrapper_kwargs:
240+ additional keyword arguments passed to each instantiated wrapper
241 """
242 self._namespace = namespace
243- super(PkgResourcesPlugInCollection, self).__init__(load, wrapper)
244+ super().__init__(load, wrapper, *wrapper_args, **wrapper_kwargs)
245
246 def load(self):
247 """
248@@ -280,8 +371,9 @@
249 for entry_point in sorted(iterator, key=lambda ep: ep.name):
250 try:
251 obj = entry_point.load()
252- except ImportError:
253+ except ImportError as exc:
254 logger.exception("Unable to import %s", entry_point)
255+ self._problem_list.append(exc)
256 else:
257 self.wrap_and_add_plugin(entry_point.name, obj)
258
259@@ -307,7 +399,8 @@
260 each plugin is the text read from the plugin file.
261 """
262
263- def __init__(self, path, ext, load=False, wrapper=PlugIn):
264+ def __init__(self, path, ext, load=False, wrapper=PlugIn, *wrapper_args,
265+ **wrapper_kwargs):
266 """
267 Initialize a collection of plug-ins from the specified name-space.
268
269@@ -316,15 +409,19 @@
270 directory entries. Each directory is searched for (not recursively)
271 for plugins.
272 :param ext:
273- extension of each plugin definition file
274+ extension of each plugin definition file (or a list of those)
275 :param load:
276 if true, load all of the plug-ins now
277 :param wrapper:
278 wrapper class for all loaded objects, defaults to :class:`PlugIn`
279+ :param wrapper_args:
280+ additional arguments passed to each instantiated wrapper
281+ :param wrapper_kwargs:
282+ additional keyword arguments passed to each instantiated wrapper
283 """
284 self._path = path
285 self._ext = ext
286- super(FsPlugInCollection, self).__init__(load, wrapper)
287+ super().__init__(load, wrapper, *wrapper_args, **wrapper_kwargs)
288
289 def load(self):
290 """
291@@ -341,8 +438,9 @@
292 try:
293 with open(filename, encoding='UTF-8') as stream:
294 text = stream.read()
295- except IOError as exc:
296+ except (OSError, IOError) as exc:
297 logger.error("Unable to load %r: %s", filename, str(exc))
298+ self._problem_list.append(exc)
299 else:
300 self.wrap_and_add_plugin(filename, text)
301
302@@ -362,8 +460,15 @@
303 # Look at each file there
304 for entry in entries:
305 # Skip files with other extensions
306- if not entry.endswith(self._ext):
307- continue
308+ if isinstance(self._ext, str):
309+ if not entry.endswith(self._ext):
310+ continue
311+ elif isinstance(self._ext, collections.Sequence):
312+ for ext in self._ext:
313+ if entry.endswith(ext):
314+ break
315+ else:
316+ continue
317 info_file = os.path.join(path, entry)
318 # Skip all non-files
319 if not os.path.isfile(info_file):
320
321=== modified file 'plainbox/plainbox/impl/secure/test_plugins.py'
322--- plainbox/plainbox/impl/secure/test_plugins.py 2013-11-27 20:40:19 +0000
323+++ plainbox/plainbox/impl/secure/test_plugins.py 2013-12-20 13:53:55 +0000
324@@ -25,29 +25,254 @@
325 """
326
327 from unittest import TestCase
328+import collections
329 import os
330
331 from plainbox.impl.secure.plugins import FsPlugInCollection
332 from plainbox.impl.secure.plugins import IPlugIn, PlugIn
333 from plainbox.impl.secure.plugins import PkgResourcesPlugInCollection
334+from plainbox.impl.secure.plugins import PlugInCollectionBase
335+from plainbox.impl.secure.plugins import PlugInError
336 from plainbox.vendor import mock
337
338
339 class PlugInTests(TestCase):
340-
341- def test_smoke(self):
342- obj = mock.Mock()
343- # Create a plug-in
344- plugin = PlugIn("name", obj)
345- # Ensure that plugin name and plugin object are okay
346- self.assertEqual(plugin.plugin_name, "name")
347- self.assertEqual(plugin.plugin_object, obj)
348+ """
349+ Tests for PlugIn class
350+ """
351+
352+ NAME = "name"
353+ OBJ = mock.Mock(name="obj")
354+
355+ def setUp(self):
356+ self.plugin = PlugIn(self.NAME, self.OBJ)
357+
358+ def test_property_name(self):
359+ """
360+ verify that PlugIn.plugin_name getter works
361+ """
362+ self.assertEqual(self.plugin.plugin_name, self.NAME)
363+
364+ def test_property_object(self):
365+ """
366+ verify that PlugIn.plugin_object getter works
367+ """
368+ self.assertEqual(self.plugin.plugin_object, self.OBJ)
369+
370+ def test_repr(self):
371+ """
372+ verify that repr for PlugIn works
373+ """
374+ self.assertEqual(repr(self.plugin), "<PlugIn plugin_name:'name'>")
375
376 def test_base_cls(self):
377+ """
378+ verify that PlugIn inherits IPlugIn
379+ """
380 self.assertTrue(issubclass(PlugIn, IPlugIn))
381
382
383+class DummyPlugInCollection(PlugInCollectionBase):
384+ """
385+ A dummy, concrete subclass of PlugInCollectionBase
386+ """
387+
388+ def load(self):
389+ """
390+ dummy implementation of load()
391+
392+ :raises NotImplementedError:
393+ always raised
394+ """
395+ raise NotImplementedError("this is a dummy method")
396+
397+
398+class PlugInCollectionBaseTests(TestCase):
399+ """
400+ Tests for PlugInCollectionBase class.
401+
402+ Since this is an abstract class we're creating a concrete subclass with
403+ dummy implementation of the load() method.
404+ """
405+
406+ def setUp(self):
407+ self.col = DummyPlugInCollection()
408+ self.plug1 = PlugIn("name1", "obj1")
409+ self.plug2 = PlugIn("name2", "obj2")
410+
411+ @mock.patch.object(DummyPlugInCollection, "load")
412+ def test_auto_loading(self, mock_col):
413+ """
414+ verify that PlugInCollectionBase.load() is called when load=True is
415+ passed to the initializer.
416+ """
417+ col = DummyPlugInCollection(load=True)
418+ col.load.assert_called()
419+
420+ def test_defaults(self):
421+ """
422+ verify what defaults are passed to the initializer or set internally
423+ """
424+ self.assertEqual(self.col._wrapper, PlugIn)
425+ self.assertEqual(self.col._plugins, collections.OrderedDict())
426+ self.assertEqual(self.col._loaded, False)
427+ self.assertEqual(self.col._mocked_objects, None)
428+ self.assertEqual(self.col._problem_list, [])
429+
430+ def test_get_by_name__typical(self):
431+ """
432+ verify that PlugInCollectionBase.get_by_name() works
433+ """
434+ with self.col.fake_plugins([self.plug1]):
435+ self.assertEqual(
436+ self.col.get_by_name(self.plug1.plugin_name), self.plug1)
437+
438+ def test_get_by_name__missing(self):
439+ """
440+ check how PlugInCollectionBase.get_by_name() behaves when there is no
441+ match for the given name.
442+ """
443+ with self.assertRaises(KeyError), self.col.fake_plugins([]):
444+ self.col.get_by_name(self.plug1.plugin_name)
445+
446+ def test_get_all_names(self):
447+ """
448+ verify that PlugInCollectionBase.get_all_names() works
449+ """
450+ with self.col.fake_plugins([self.plug1, self.plug2]):
451+ self.assertEqual(
452+ self.col.get_all_names(),
453+ [self.plug1.plugin_name, self.plug2.plugin_name])
454+
455+ def test_get_all_plugins(self):
456+ """
457+ verify that PlugInCollectionBase.get_all_plugins() works
458+ """
459+ with self.col.fake_plugins([self.plug1, self.plug2]):
460+ self.assertEqual(
461+ self.col.get_all_plugins(), [self.plug1, self.plug2])
462+
463+ def test_get_all_plugin_objects(self):
464+ """
465+ verify that PlugInCollectionBase.get_all_plugin_objects() works
466+ """
467+ with self.col.fake_plugins([self.plug1, self.plug2]):
468+ self.assertEqual(
469+ self.col.get_all_plugin_objects(),
470+ [self.plug1.plugin_object, self.plug2.plugin_object])
471+
472+ def test_get_items(self):
473+ """
474+ verify that PlugInCollectionBase.get_all_items() works
475+ """
476+ with self.col.fake_plugins([self.plug1, self.plug2]):
477+ self.assertEqual(
478+ self.col.get_all_items(),
479+ [(self.plug1.plugin_name, self.plug1),
480+ (self.plug2.plugin_name, self.plug2)])
481+
482+ def test_problem_list(self):
483+ """
484+ verify that PlugInCollectionBase.problem_list works
485+ """
486+ self.assertIs(self.col.problem_list, self.col._problem_list)
487+
488+ def test_fake_plugins(self):
489+ """
490+ verify that PlugInCollectionBase.fake_plugins() works
491+ """
492+ # create a canary object we'll check for below
493+ canary = object()
494+ # store it to all the attributes we expect to see changed by
495+ # fake_plugins()
496+ self.col._loaded = canary
497+ self.col._plugins = canary
498+ self.col._problems = canary
499+ # use fake_plugins() with some plugins we have
500+ fake_plugins = [self.plug1, self.plug2]
501+ with self.col.fake_plugins(fake_plugins):
502+ # ensure that we don't have canaries here
503+ self.assertEqual(self.col._loaded, True)
504+ self.assertEqual(self.col._plugins, collections.OrderedDict([
505+ (self.plug1.plugin_name, self.plug1),
506+ (self.plug2.plugin_name, self.plug2)]))
507+ self.assertEqual(self.col._problem_list, [])
508+ # ensure that we see canaries outside of the context manager
509+ self.assertEqual(self.col._loaded, canary)
510+ self.assertEqual(self.col._plugins, canary)
511+ self.assertEqual(self.col._problems, canary)
512+
513+ def test_fake_plugins__with_problem_list(self):
514+ """
515+ verify that PlugInCollectionBase.fake_plugins() works when called with
516+ the optional problem list.
517+ """
518+ # create a canary object we'll check for below
519+ canary = object()
520+ # store it to all the attributes we expect to see changed by
521+ # fake_plugins()
522+ self.col._loaded = canary
523+ self.col._plugins = canary
524+ self.col._problems = canary
525+ # use fake_plugins() with some plugins we have
526+ fake_plugins = [self.plug1, self.plug2]
527+ fake_problems = [PlugInError("just testing")]
528+ with self.col.fake_plugins(fake_plugins, fake_problems):
529+ # ensure that we don't have canaries here
530+ self.assertEqual(self.col._loaded, True)
531+ self.assertEqual(self.col._plugins, collections.OrderedDict([
532+ (self.plug1.plugin_name, self.plug1),
533+ (self.plug2.plugin_name, self.plug2)]))
534+ self.assertEqual(self.col._problem_list, fake_problems)
535+ # ensure that we see canaries outside of the context manager
536+ self.assertEqual(self.col._loaded, canary)
537+ self.assertEqual(self.col._plugins, canary)
538+ self.assertEqual(self.col._problems, canary)
539+
540+ def test_wrap_and_add_plugin__normal(self):
541+ """
542+ verify that PlugInCollectionBase.wrap_and_add_plugin() works
543+ """
544+ self.col.wrap_and_add_plugin("new-name", "new-obj")
545+ self.assertIn("new-name", self.col._plugins)
546+ self.assertEqual(
547+ self.col._plugins["new-name"].plugin_name, "new-name")
548+ self.assertEqual(
549+ self.col._plugins["new-name"].plugin_object, "new-obj")
550+
551+ def test_wrap_and_add_plugin__problem(self):
552+ """
553+ verify that PlugInCollectionBase.wrap_and_add_plugin() works when a
554+ problem occurs.
555+ """
556+ with mock.patch.object(self.col, "_wrapper") as mock_wrapper:
557+ mock_wrapper.side_effect = PlugInError
558+ self.col.wrap_and_add_plugin("new-name", "new-obj")
559+ mock_wrapper.assert_called_with("new-name", "new-obj")
560+ self.assertIsInstance(self.col.problem_list[0], PlugInError)
561+ self.assertNotIn("new-name", self.col._plugins)
562+
563+ def test_extra_wrapper_args(self):
564+ """
565+ verify that PlugInCollectionBase passes extra arguments to the wrapper
566+ """
567+ class TestPlugIn(PlugIn):
568+
569+ def __init__(self, name, obj, *args, **kwargs):
570+ super().__init__(name, obj)
571+ self.args = args
572+ self.kwargs = kwargs
573+ col = DummyPlugInCollection(
574+ False, TestPlugIn, 1, 2, 3, some="argument")
575+ col.wrap_and_add_plugin("name", "obj")
576+ self.assertEqual(col._plugins["name"].args, (1, 2, 3))
577+ self.assertEqual(col._plugins["name"].kwargs, {"some": "argument"})
578+
579+
580 class PkgResourcesPlugInCollectionTests(TestCase):
581+ """
582+ Tests for PlugInCollectionBase class
583+ """
584
585 _NAMESPACE = "namespace"
586
587@@ -114,23 +339,8 @@
588 # Ensure that an exception was logged
589 mock_logger.exception.assert_called_with(
590 "Unable to import %s", mock_ep2)
591-
592- def test_fake_plugins(self):
593- # Create a mocked entry plugin
594- plug1 = PlugIn("ep1", "obj1")
595- plug2 = PlugIn("ep2", "obj2")
596- # With fake plugins
597- with self.col.fake_plugins([plug1, plug2]):
598- # Check that plugins are correct
599- self.assertIs(self.col.get_by_name('ep1'), plug1)
600- self.assertIs(self.col.get_by_name('ep2'), plug2)
601- # Access all plugins
602- self.assertEqual(self.col.get_all_plugins(), [plug1, plug2])
603- # Access all plugin names
604- self.assertEqual(self.col.get_all_names(), ['ep1', 'ep2'])
605- # Access all pairs (name, plugin)
606- self.assertEqual(self.col.get_all_items(),
607- [('ep1', plug1), ('ep2', plug2)])
608+ # Ensure that the error was collected
609+ self.assertIsInstance(self.col.problem_list[0], ImportError)
610
611
612 class FsPlugInCollectionTests(TestCase):
613@@ -190,9 +400,8 @@
614 return not os.path.basename(path).startswith('dir.')
615
616 def fake_open(path, encoding=None, mode=None):
617- m = mock.Mock()
618- m.__enter__ = mock.Mock()
619- m.__exit__ = mock.Mock()
620+ m = mock.MagicMock(name='opened file {!r}'.format(path))
621+ m.__enter__.return_value = m
622 if path == os.path.join(self._P1, 'foo.plugin'):
623 m.read.return_value = "foo"
624 return m
625@@ -200,7 +409,7 @@
626 m.read.return_value = "bar"
627 return m
628 elif path == os.path.join(self._P1, 'noperm.plugin'):
629- raise IOError("You cannot open this file")
630+ raise OSError("You cannot open this file")
631 else:
632 raise IOError("Unexpected file: {}".format(path))
633 mock_listdir.side_effect = fake_listdir
634@@ -239,3 +448,63 @@
635 'Unable to load %r: %s',
636 '/system/providers/noperm.plugin',
637 'You cannot open this file')
638+ # Ensure that all of the errors are collected
639+ # Using repr() since OSError seems hard to compare correctly
640+ self.assertEqual(
641+ repr(self.col.problem_list[0]),
642+ repr(OSError('You cannot open this file')))
643+
644+ @mock.patch('plainbox.impl.secure.plugins.logger')
645+ @mock.patch('builtins.open')
646+ @mock.patch('os.path.isfile')
647+ @mock.patch('os.listdir')
648+ def test_load__two_extensions(self, mock_listdir, mock_isfile, mock_open,
649+ mock_logger):
650+ """
651+ verify that FsPlugInCollection works with multiple extensions
652+ """
653+ mock_listdir.return_value = ["foo.txt", "bar.txt.in"]
654+ mock_isfile.return_value = True
655+
656+ def fake_open(path, encoding=None, mode=None):
657+ m = mock.MagicMock(name='opened file {!r}'.format(path))
658+ m.read.return_value = "text"
659+ m.__enter__.return_value = m
660+ return m
661+ mock_open.side_effect = fake_open
662+ # Create a collection that looks for both extensions
663+ col = FsPlugInCollection(self._P1, (".txt", ".txt.in"))
664+ # Load everything
665+ col.load()
666+ # Ensure that we actually tried to look at the filesystem
667+ self.assertEqual(
668+ mock_listdir.call_args_list, [
669+ ((self._P1, ), {}),
670+ ])
671+ # Ensure that we actually tried to check if things are files
672+ self.assertEqual(
673+ mock_isfile.call_args_list, [
674+ ((os.path.join(self._P1, 'foo.txt'),), {}),
675+ ((os.path.join(self._P1, 'bar.txt.in'),), {}),
676+ ])
677+ # Ensure that we actually tried to open some files
678+ self.assertEqual(
679+ mock_open.call_args_list, [
680+ ((os.path.join(self._P1, 'bar.txt.in'),),
681+ {'encoding': 'UTF-8'}),
682+ ((os.path.join(self._P1, 'foo.txt'),),
683+ {'encoding': 'UTF-8'}),
684+ ])
685+ # Ensure that no exception was logged
686+ mock_logger.error.assert_not_called()
687+ # Ensure that everything was okay
688+ self.assertEqual(col.problem_list, [])
689+ # Ensure that both files got added
690+ self.assertEqual(
691+ col.get_by_name(
692+ os.path.join(self._P1, "foo.txt")
693+ ).plugin_object, "text")
694+ self.assertEqual(
695+ col.get_by_name(
696+ os.path.join(self._P1, "bar.txt.in")
697+ ).plugin_object, "text")

Subscribers

People subscribed via source and target branches