Re: [PATCH] Create a common base class for autoqa tests
On Fri, 2010-07-09 at 16:12 +0200, Josef Skladanka wrote:
Hello gang,
while I was playing around with ResultsDB, I've come to some issues,
which bothered me to the level I wrote a fix :)
1) Common base class for the tests
-----------------------------------
This part is fine - nothing to talk about here, really.
[patch 0002]
For the email's sake in current 'send email to results list' model, the
postprocess fuction implements the email-sending. Summary is the subject
and highlights + outputs are body. The self.mail_to list is filled in
the tests which have the "inform package owners" fuctionality.
This part is fine too.
2) ExceptionCatcher decorator
-----------------------------
[patch 0001 - lib/python/decorators.py]
What the heck is this?!? Well, you remember, that the 'postprocess'
function is called only if the run_once ends well (i.e. no exception is
thrown from the run_once). This, of course, is absolutely not OK, if one
wants to have some common "out" point - e.g. call the postprocess function.
This decorator then watches over the decorated function, and if
exception is raised, the decorator stores it, calls the
'exception_happened' function (which is given as parameter to the
decorator, see the docstring for more detail), and then re-raises the
exception.
I definitely like the decorator idea, but I have some questions about
the implementation. It's interesting that you chose to implement this as
a class, rather than a function - although decorators-with-args are kind
of a brain-buster either way, really. Heh.
As I understand from reading the code, it seems you want to pass a class
method (like 'self.run_once_failed') to trigger if an exception is
raised, which is a good idea. But it seems like you could do this just
by doing, say:
def initialize_failed(self):
...
@ExceptionCatcher(initialize_failed)
def initialize(self, config, **kwargs):
self.config = config_loader(config, self.tmpdir)
...
i.e. pass the function directly, rather than using the tricky
manipulation of string-name-to-callable.
Given that the first argument will be a callable, it seems like you
could define the decorator in the more traditional way, something like
this:
def ExceptionCatcher(on_exception, *oe_args, **oe_kwargs):
'''docs go here'''
def _wrap(f): # this wraps the function we're decorating
def _inner(*args, **kwargs):
exc_info = None
try:
r = f(*args, **kwargs) # decorated function
except:
exc_info = sys.exc_info() # save exc info
finally:
if exc_info is not None:
on_exception(*oe_args, **oe_kwargs)
raise exc_info[1], None, exc_info[2]
return r
return _inner
return _wrap
(see http://www.kylev.com/2009/05/28/a-brief-python-decorator-primer/
for examples of the traditional double-function way of doing decorators
with arguments)
Would something like that work, or would that mess up autotest's
introspection?
3) Tests & templates rewritten
------------------------------
This is (of course) reasonable, given the other patches. We'll want to
test it out a bit to be sure, though.
-w