This is the remaining part of the multihook patchset. Now all raised exceptions should be reported as CRASHED, together with a short exception description in the summary. All tests were modified not to use exceptions for ending correctly.
All this code is also available in multitests branch, so it's enough to just merge the whole branch.
Josef Skladanka (1): ExceptionCatcher hands over the Exception object
Kamil Páral (2): raise exception only when you want to report the result as CRASHED be more concise when run_once fails, let's have shorter email subjects
lib/python/decorators.py | 4 ++-- lib/python/test.py | 22 ++++++++++++---------- tests/rats_install/rats_install.py | 2 -- tests/rats_sanity/rats_sanity.py | 4 +--- tests/repoclosure/repoclosure.py | 3 --- tests/upgradepath/upgradepath.py | 15 ++------------- 6 files changed, 17 insertions(+), 33 deletions(-)
From: Josef Skladanka jskladan@redhat.com
ExceptionCatcher calls the 'on_exception' method with a parameter exc = exc_info[1] - i.e. the Exception object of the raised Exception, so the methods called can read the exception info. --- lib/python/decorators.py | 4 ++-- lib/python/test.py | 22 ++++++++++++---------- 2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/lib/python/decorators.py b/lib/python/decorators.py index 727d0d5..0b920b4 100644 --- a/lib/python/decorators.py +++ b/lib/python/decorators.py @@ -76,10 +76,10 @@ class ExceptionCatcher(object): #if the 'on_exception' is a name of 'class member function' #then call the f_self.on_exception(oe_args, oe_kwargs) if self.oe_is_self_method: - f_self.__getattribute__(self.oe)(*self.oe_a, **self.oe_k) + f_self.__getattribute__(self.oe)(exc = exc_info[1], *self.oe_a, **self.oe_k) #'on_exception' is pointer to function else: - self.oe(*self.oe_a, **self.oe_k) + self.oe(exc = exc_info[1], *self.oe_a, **self.oe_k) #re-raise the stored exception raise exc_info[1], None, exc_info[2] return f_result diff --git a/lib/python/test.py b/lib/python/test.py index 15d6d16..20b35cd 100644 --- a/lib/python/test.py +++ b/lib/python/test.py @@ -43,21 +43,23 @@ class AutoQATest(test.test, object): def run_once(self, **kwargs): pass
- def initialize_failed(self): - if self.result is None: - self.result = "CRASHED" - if self.summary == "": - self.summary = "%s: Initialize failed" % self.__class__.__name__ + def initialize_failed(self, exc = None): + self.result = "CRASHED" + if exc is not None: + self.summary = "Initialize failed: %s: %s" % (exc.__class__.__name__, exc) + else: + self.summary = "Initialize failed: Exception: Unknown exception" try: self.postprocess_iteration() except: pass
- def run_once_failed(self): - if self.result is None: - self.result = "CRASHED" - if self.summary == "": - self.summary = "%s: Run_once failed" % self.__class__.__name__ + def run_once_failed(self, exc = None): + self.result = "CRASHED" + if exc is not None: + self.summary = "Run_once failed: %s: %s" % (exc.__class__.__name__, exc) + else: + self.summary = "Run_once failed: Exception: Unknown exception" try: self.postprocess_iteration() except:
We will start using exceptions only when something goes wrong and we want to report the test result as CRASHED. Otherwise we will just set self.result and end the test normally. --- tests/rats_install/rats_install.py | 2 -- tests/rats_sanity/rats_sanity.py | 4 +--- tests/repoclosure/repoclosure.py | 3 --- tests/upgradepath/upgradepath.py | 15 ++------------- 4 files changed, 3 insertions(+), 21 deletions(-)
diff --git a/tests/rats_install/rats_install.py b/tests/rats_install/rats_install.py index 21c95de..e4050d2 100644 --- a/tests/rats_install/rats_install.py +++ b/tests/rats_install/rats_install.py @@ -90,5 +90,3 @@ class rats_install(AutoQATest): except Exception, e: print "Failed to send results to irb: %s" % str(e)
- if self.result == 'FAILED': - raise error.TestFail diff --git a/tests/rats_sanity/rats_sanity.py b/tests/rats_sanity/rats_sanity.py index 8ae7081..3f1e915 100644 --- a/tests/rats_sanity/rats_sanity.py +++ b/tests/rats_sanity/rats_sanity.py @@ -49,7 +49,7 @@ class rats_sanity(AutoQATest): try: out = utils.system_output(cmd + " 2>&1", retain_output=True) except error.CmdError, e: - result = 'FAILED' + self.result = 'FAILED' out = e.result_obj.stdout
if self.result is None: @@ -73,5 +73,3 @@ class rats_sanity(AutoQATest): except Exception, e: print "Failed to send results to irb: %s" % str(e)
- if self.result == 'FAILED': - raise error.TestFail diff --git a/tests/repoclosure/repoclosure.py b/tests/repoclosure/repoclosure.py index f3ffb8a..c0e4838 100644 --- a/tests/repoclosure/repoclosure.py +++ b/tests/repoclosure/repoclosure.py @@ -19,7 +19,6 @@
import autoqa.util from autotest_lib.client.bin import utils -from autotest_lib.client.common_lib import error from autoqa.test import AutoQATest from autoqa.decorators import ExceptionCatcher
@@ -58,5 +57,3 @@ class repoclosure(AutoQATest): self.outputs = out
- if unresolved_count: - raise error.TestFail, "%u packages with unresolved deps" % unresolved_count diff --git a/tests/upgradepath/upgradepath.py b/tests/upgradepath/upgradepath.py index 257eddc..dfbbb81 100755 --- a/tests/upgradepath/upgradepath.py +++ b/tests/upgradepath/upgradepath.py @@ -31,7 +31,6 @@ from autoqa.decorators import ExceptionCatcher from autotest_lib.client.bin import utils from autotest_lib.client.bin import test from autotest_lib.client.bin.test_config import config_loader -from autotest_lib.client.common_lib import error
class upgradepath(AutoQATest): @@ -82,15 +81,8 @@ class upgradepath(AutoQATest): reponames = [reponame for reponame in autoqa.koji_utils.repoinfo.repos() if not reponame.endswith('-testing')] repotags = [autoqa.koji_utils.repoinfo.getrepo(reponame)['tag'] for reponame in reponames] repotags.sort() - try: - current_tag = repotags.index(kojitag) - except ValueError: - self.test_result = 'FAIL' - msg = "ERROR: Entered bad kojitag" - print msg - self.log.append(msg) - self.envr_list.add(matching_build['envr']) - raise error.TestFail + assert kojitag in repotags, 'Requested unsupported kojitag: %s' % kojitag + current_tag = repotags.index(kojitag)
hi_tags = repotags[(current_tag + 1):] # tags higher than current low_eq_tags = repotags[:(current_tag + 1)] # tags lower or equal @@ -150,6 +142,3 @@ class upgradepath(AutoQATest):
self.summary = packages_fail
- if self.test_result == 'FAIL': - raise error.TestFail -
--- lib/python/test.py | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/python/test.py b/lib/python/test.py index 20b35cd..28359f5 100644 --- a/lib/python/test.py +++ b/lib/python/test.py @@ -57,9 +57,9 @@ class AutoQATest(test.test, object): def run_once_failed(self, exc = None): self.result = "CRASHED" if exc is not None: - self.summary = "Run_once failed: %s: %s" % (exc.__class__.__name__, exc) + self.summary = "%s: %s" % (exc.__class__.__name__, exc) else: - self.summary = "Run_once failed: Exception: Unknown exception" + self.summary = "Exception: Unknown exception" try: self.postprocess_iteration() except:
This patch improves text output - it now mentions to which repository we want to push our package, and also mentions which condition failed after a FAIL status line. It also improves self.summary. And uses tabs instead of space, ahem. --- tests/upgradepath/upgradepath.py | 39 ++++++++++++++++++------------------- 1 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/tests/upgradepath/upgradepath.py b/tests/upgradepath/upgradepath.py index dfbbb81..1811aba 100755 --- a/tests/upgradepath/upgradepath.py +++ b/tests/upgradepath/upgradepath.py @@ -47,7 +47,7 @@ class upgradepath(AutoQATest): def setup(self): pass
- def compare(self, matching_build, tags, op): + def compare(self, matching_build, tags, op, opstr): koji = autoqa.koji_utils.SimpleKojiClientSession() for tag in tags: latest_build = koji.latest_by_tag(tag, matching_build['name']) @@ -56,28 +56,36 @@ class upgradepath(AutoQATest): self.log.append(msg) print msg
- msg = " Package %s in %s doesn't exist" % (matching_build['name'], tag) + msg = "\tNo such package: %s" % matching_build['name'] self.log.append(msg) print msg else: result = autoqa.koji_utils.compareEVR(matching_build, latest_build)
if op(result, 0): + ok = True msg ='{0:<7}{1}'.format('[ OK ]', tag) print msg self.log.append(msg) else: + ok = False self.test_result = 'FAIL' msg ='{0:<7}{1}'.format('[FAIL]', tag) print msg self.log.append(msg) self.envr_list.add(matching_build['envr']) - msg = " Pushing package: %s\n Latest package: %s" % (matching_build['envr'], latest_build['nvr']) + msg = "\tLatest package: %s" % latest_build['nvr'] self.log.append(msg) print msg + if not ok: + msg = '\tFailed condition: Requested package %s Latest package' % opstr + self.log.append(msg) + print msg
@ExceptionCatcher("self.run_once_failed") - def run_once(self, envrs, name, kojitag, **kwargs): + def run_once(self, envrs, kojitag, **kwargs): + update_id = kwargs['name'] or kwargs['id'] + reponames = [reponame for reponame in autoqa.koji_utils.repoinfo.repos() if not reponame.endswith('-testing')] repotags = [autoqa.koji_utils.repoinfo.getrepo(reponame)['tag'] for reponame in reponames] repotags.sort() @@ -88,11 +96,9 @@ class upgradepath(AutoQATest): low_eq_tags = repotags[:(current_tag + 1)] # tags lower or equal
for envr in envrs: - msg = 40*'=' + '\n' + envr + '\n' + 40*'=' + msg = '%s\n%s into %s\n%s' % (40*'=', envr, kojitag, 40*'=') + print msg self.log.append(msg) - print 40*'=' - print envr - print 40*'='
# our testing package (name, version, release, epoch, arch) = rpmUtils.miscutils.splitFilename(envr) @@ -113,15 +119,15 @@ class upgradepath(AutoQATest): latest_build = koji.latest_by_tag(kojitag, matching_build['name']) if latest_build is not None: result = autoqa.koji_utils.compareEVR(matching_build, latest_build) - if result != 1: - msg = "Warning: Pushing older or current version of package" + if result <= 0: + msg = "Warning: Same or newer build already exists: %s" % latest_build['nvr'] self.log.append(msg) print msg
# compare with lower or equal tags, so version has to be greater or equal - self.compare(matching_build, low_eq_tags, operator.ge) + self.compare(matching_build, low_eq_tags, operator.ge, '>=') # compare with higher tags, so version has to be lower or equal - self.compare(matching_build, hi_tags, operator.le) + self.compare(matching_build, hi_tags, operator.le, '<=')
summary = str(len(envrs) - len(self.envr_list)) + ' OK, ' + str(len(self.envr_list)) + ' FAILED' msg = 'SUMMARY: ' + summary @@ -132,13 +138,6 @@ class upgradepath(AutoQATest): self.outputs = "" for i in self.log: self.outputs += i + '\n' - self.outputs += '\n \n' - - packages_fail = "" - for i in self.envr_list: - packages_fail += i + ', ' - packages_fail = packages_fail[:-2] - - self.summary = packages_fail + self.summary = '%s for %s' % (summary, update_id)
On Tue, 2010-09-14 at 13:04 +0200, Kamil Páral wrote:
This is the remaining part of the multihook patchset. Now all raised exceptions should be reported as CRASHED, together with a short exception description in the summary. All tests were modified not to use exceptions for ending correctly.
All this code is also available in multitests branch, so it's enough to just merge the whole branch.
All the patches look good to me. Merged into master. Thanks a lot!
I think James may be working on rolling this into a new (0.4.5?) autoqa release shortly.
-w
autoqa-devel@lists.fedorahosted.org
-
Kamil Paral -
Will Woods