[PATCH] Removed individual XYZ_failed methods in test.py

[Josef Skladanka]

According to the
<https://fedorahosted.org/pipermail/autoqa-devel/2010-October/001214.html>
I've removed the individual {setup,initialize,run_once}_failed()
methods and replaced these with one generic process_exception().

I also set the self.process_exception as default method for
the ExceptionCatcher decorator, so we can use it without arguments.

The possibility to provide the name of some particular method
is, of course, still there.

Will change the wiki documentation, once this patch is in master.
---
 lib/python/decorators.py           |    2 +-
 lib/python/test.py                 |   30 ++++--------------------------
 tests/conflicts/conflicts.py       |    2 +-
 tests/helloworld/helloworld.py     |    2 +-
 tests/initscripts/initscripts.py   |    6 +++---
 tests/rats_install/rats_install.py |    6 +++---
 tests/rats_sanity/rats_sanity.py   |    6 +++---
 tests/repoclosure/repoclosure.py   |    4 ++--
 tests/rpmguard/rpmguard.py         |    6 +++---
 tests/rpmlint/rpmlint.py           |    6 +++---
 tests/upgradepath/upgradepath.py   |    4 ++--
 11 files changed, 26 insertions(+), 48 deletions(-)

diff --git a/lib/python/decorators.py b/lib/python/decorators.py
index 0b920b4..7db78ad 100644
--- a/lib/python/decorators.py
+++ b/lib/python/decorators.py
@@ -46,7 +46,7 @@ class ExceptionCatcher(object):
         Exception
     """
 
-    def __init__(self, on_exception, *args, **kwargs):
+    def __init__(self, on_exception = "self.process_exception", *args,
**kwargs):
         """
         on_exception - string with class function name, or pointer to function
         *args, **kwargs - args for the on_exception function
diff --git a/lib/python/test.py b/lib/python/test.py
index 61e44e7..f04ad10 100644
--- a/lib/python/test.py
+++ b/lib/python/test.py
@@ -35,42 +35,20 @@ class AutoQATest(test.test, object):
         self.outputs = "" #second part of mail_body
         self.mail_to = [] #list of emails to send results to. Mail is always sent to
results list
 
-    @ExceptionCatcher("self.setup_failed")
+    @ExceptionCatcher()
     def setup(self, **kwargs):
         pass
 
-    @ExceptionCatcher("self.initialize_failed")
+    @ExceptionCatcher()
     def initialize(self, config, **kwargs):
         self.config = config_loader(config, self.tmpdir)
         self.autotest_url = make_autotest_url(self.config)
 
-    @ExceptionCatcher("self.run_once_failed")
+    @ExceptionCatcher()
     def run_once(self, **kwargs):
         pass
 
-    def setup_failed(self, exc = None):
-        self.result = "CRASHED"
-        if exc is not None:
-            self.summary = "Setup failed: %s: %s" % (exc.__class__.__name__,
exc)
-        else:
-            self.summary = "Setup failed: Exception: Unknown exception"
-        try:
-            self.postprocess_iteration()
-        except Exception, e:
-            traceback.print_exc(file=sys.stderr)
-
-    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 Exception, e:
-            traceback.print_exc(file=sys.stderr)
-
-    def run_once_failed(self, exc = None):
+    def process_exception(self, exc = None):
         self.result = "CRASHED"
         if exc is not None:
             self.summary = "%s: %s" % (exc.__class__.__name__, exc)
diff --git a/tests/conflicts/conflicts.py b/tests/conflicts/conflicts.py
index 4328728..5a93a2b 100644
--- a/tests/conflicts/conflicts.py
+++ b/tests/conflicts/conflicts.py
@@ -32,7 +32,7 @@ class conflicts(AutoQATest):
         f.write(logdata)
         f.close()
 
-    @ExceptionCatcher("self.run_once_failed")
+    @ExceptionCatcher()
     def run_once(self, baseurl, parents, name, **kwargs):
         if name:
             name = "%s-%s" % (name, autoqa.util.get_basearch())
diff --git a/tests/helloworld/helloworld.py b/tests/helloworld/helloworld.py
index 9f6ac50..0af1c52 100644
--- a/tests/helloworld/helloworld.py
+++ b/tests/helloworld/helloworld.py
@@ -25,7 +25,7 @@ from autoqa.decorators import ExceptionCatcher
 class helloworld(AutoQATest):
     version = 1 # increment if setup() changes
 
-    @ExceptionCatcher("self.run_once_failed")
+    @ExceptionCatcher()
     def run_once(self, *args, **kwargs):
         self.summary = 'Hello, World!'
         self.outputs = "===Printing passed params===\n"
diff --git a/tests/initscripts/initscripts.py b/tests/initscripts/initscripts.py
index 133d058..e0f95bb 100644
--- a/tests/initscripts/initscripts.py
+++ b/tests/initscripts/initscripts.py
@@ -33,7 +33,7 @@ from autoqa.decorators import ExceptionCatcher
 class initscripts(AutoQATest):
     version = 1 # increment if setup() changes
 
-    @ExceptionCatcher("self.setup_failed")
+    @ExceptionCatcher()
     def setup(self, **kwargs):
         """
         adds beakerlib repository and installs beakerlib
@@ -45,7 +45,7 @@ class initscripts(AutoQATest):
             f.close()
         utils.system('yum -y install beakerlib') 
 
-    @ExceptionCatcher("self.initialize_failed")
+    @ExceptionCatcher()
     def initialize(self, config, **kwargs):
         super(initscripts, self).initialize(config)
         self.rpmdir = os.path.join(self.tmpdir, 'rpms')
@@ -99,7 +99,7 @@ class initscripts(AutoQATest):
 
         return (result, outputs)
 
-    @ExceptionCatcher("self.run_once_failed")
+    @ExceptionCatcher()
     def run_once(self, kojitag, **kwargs):
         if kwargs['hook'] == 'post-koji-build':
             envrs = [kwargs['envr']]
diff --git a/tests/rats_install/rats_install.py b/tests/rats_install/rats_install.py
index 360be3f..df23451 100644
--- a/tests/rats_install/rats_install.py
+++ b/tests/rats_install/rats_install.py
@@ -30,12 +30,12 @@ from autoqa.decorators import ExceptionCatcher
 class rats_install(AutoQATest):
     version = 1
 
-    @ExceptionCatcher("self.initialize_failed")
+    @ExceptionCatcher()
     def initialize(self, config, **kwargs):
         super(rats_install, self).initialize(config)
         self.irb_conf = os.path.join(self.bindir, 'irb.cfg')
 
-    @ExceptionCatcher("self.setup_failed")
+    @ExceptionCatcher()
     def setup(self, **kwargs):
         if os.path.exists(self.irb_conf):
             utils.system('yum -y install python-fedora')
@@ -51,7 +51,7 @@ class rats_install(AutoQATest):
         #       use the big hammer until we figure out the right thing to do
         utils.system('lokkit --disabled')
 
-    @ExceptionCatcher("self.run_once_failed")
+    @ExceptionCatcher()
     def run_once(self, baseurl, name, image_url="", boot_args="",
**kwargs):
         if name:
             name = "%s-%s" % (name, util.get_basearch())
diff --git a/tests/rats_sanity/rats_sanity.py b/tests/rats_sanity/rats_sanity.py
index 5a2a4f1..d6a6357 100644
--- a/tests/rats_sanity/rats_sanity.py
+++ b/tests/rats_sanity/rats_sanity.py
@@ -28,17 +28,17 @@ from autoqa.decorators import ExceptionCatcher
 class rats_sanity(AutoQATest):
     version = 1
 
-    @ExceptionCatcher("self.initialize_failed")
+    @ExceptionCatcher()
     def initialize(self, config, **kwargs):
         super(rats_sanity, self).initialize(config)
         self.irb_conf = os.path.join(self.bindir, 'irb.cfg')
 
-    @ExceptionCatcher("self.setup_failed")
+    @ExceptionCatcher()
     def setup(self, **kwargs):
         if os.path.exists(self.irb_conf):
             utils.system('yum -y install python-fedora')
 
-    @ExceptionCatcher("self.run_once_failed")
+    @ExceptionCatcher()
     def run_once(self, baseurl, parents, name, **kwargs):
         if name:
             name = "%s-%s" % (name, util.get_basearch())
diff --git a/tests/repoclosure/repoclosure.py b/tests/repoclosure/repoclosure.py
index 5c8752e..b27b684 100644
--- a/tests/repoclosure/repoclosure.py
+++ b/tests/repoclosure/repoclosure.py
@@ -25,11 +25,11 @@ from autoqa.decorators import ExceptionCatcher
 class repoclosure(AutoQATest):
     version = 1
 
-    @ExceptionCatcher("self.setup_failed")
+    @ExceptionCatcher()
     def setup(self, **kwargs):
         utils.system('yum -y install yum-utils')
 
-    @ExceptionCatcher("self.run_once_failed")
+    @ExceptionCatcher()
     def run_once(self, baseurl, parents='', name='', **kwargs):
         if name:
             name = "%s-%s" % (name, autoqa.util.get_basearch())
diff --git a/tests/rpmguard/rpmguard.py b/tests/rpmguard/rpmguard.py
index 4e4ee04..588a58f 100644
--- a/tests/rpmguard/rpmguard.py
+++ b/tests/rpmguard/rpmguard.py
@@ -30,18 +30,18 @@ from autoqa.decorators import ExceptionCatcher
 class rpmguard(AutoQATest):
     version = 1 # increment if setup() changes
 
-    @ExceptionCatcher("self.setup_failed")
+    @ExceptionCatcher()
     def setup(self, **kwargs):
         utils.system('yum -y install rpmlint')
 
-    @ExceptionCatcher("self.initialize_failed")
+    @ExceptionCatcher()
     def initialize(self, config, **kwargs):
         super(rpmguard, self).initialize(config)
         self.rpmguard = os.path.join(self.bindir, 'rpmguard')
         self.rpmdir = os.path.join(self.tmpdir, 'rpms')
         os.makedirs(self.rpmdir)
 
-    @ExceptionCatcher("self.run_once_failed")
+    @ExceptionCatcher()
     def run_once(self, kojitag, **kwargs):
         if kwargs['hook'] == 'post-koji-build':
             envrs = [kwargs['envr']]
diff --git a/tests/rpmlint/rpmlint.py b/tests/rpmlint/rpmlint.py
index ce92a1d..bb25dbc 100644
--- a/tests/rpmlint/rpmlint.py
+++ b/tests/rpmlint/rpmlint.py
@@ -30,17 +30,17 @@ from autoqa.decorators import ExceptionCatcher
 class rpmlint(AutoQATest):
     version = 1 # increment if setup() changes
 
-    @ExceptionCatcher("self.setup_failed")
+    @ExceptionCatcher()
     def setup(self, **kwargs):
         utils.system('yum -y install rpmlint')
 
-    @ExceptionCatcher("self.initialize_failed")
+    @ExceptionCatcher()
     def initialize(self, config, **kwargs):
         super(rpmlint, self).initialize(config)
         self.rpmdir = os.path.join(self.tmpdir, 'rpms')
         os.makedirs(self.rpmdir)
 
-    @ExceptionCatcher("self.run_once_failed")
+    @ExceptionCatcher()
     def run_once(self, kojitag, **kwargs):
         if kwargs['hook'] == 'post-koji-build':
             envrs = [kwargs['envr']]
diff --git a/tests/upgradepath/upgradepath.py b/tests/upgradepath/upgradepath.py
index 4f298b1..edbe4ea 100755
--- a/tests/upgradepath/upgradepath.py
+++ b/tests/upgradepath/upgradepath.py
@@ -37,7 +37,7 @@ from autotest_lib.client.bin.test_config import config_loader
 class upgradepath(AutoQATest):
     version = 1 # increment if setup() changes
 
-    @ExceptionCatcher("self.initialize_failed")
+    @ExceptionCatcher()
     def initialize(self, config, **kwargs):
         self.config = config_loader(config, self.tmpdir)
         #URL of logs/results stored on autotest-server
@@ -86,7 +86,7 @@ class upgradepath(AutoQATest):
                     result = 'FAILED'
         return result
 
-    @ExceptionCatcher("self.run_once_failed")
+    @ExceptionCatcher()
     def run_once(self, envrs, kojitag, **kwargs):
         update_id = kwargs['name'] or kwargs['id']
 
-- 
1.7.2.3

[Kamil Paral]

ACK from me.

----- "Josef Skladanka" <jskladan(a)redhat.com&gt; wrote:

>  According to the
>  <https://fedorahosted.org/pipermail/autoqa-devel/2010-October/001214.html>
>  I've removed the individual {setup,initialize,run_once}_failed()
>  methods and replaced these with one generic process_exception().
>  
>  I also set the self.process_exception as default method for
>  the ExceptionCatcher decorator, so we can use it without arguments.
>  
>  The possibility to provide the name of some particular method
>  is, of course, still there.
>  
>  Will change the wiki documentation, once this patch is in master.
>  ---
>   lib/python/decorators.py           |    2 +-
>   lib/python/test.py                 |   30
>  ++++--------------------------
>   tests/conflicts/conflicts.py       |    2 +-
>   tests/helloworld/helloworld.py     |    2 +-
>   tests/initscripts/initscripts.py   |    6 +++---
>   tests/rats_install/rats_install.py |    6 +++---
>   tests/rats_sanity/rats_sanity.py   |    6 +++---
>   tests/repoclosure/repoclosure.py   |    4 ++--
>   tests/rpmguard/rpmguard.py         |    6 +++---
>   tests/rpmlint/rpmlint.py           |    6 +++---
>   tests/upgradepath/upgradepath.py   |    4 ++--
>   11 files changed, 26 insertions(+), 48 deletions(-)
>  
>  diff --git a/lib/python/decorators.py b/lib/python/decorators.py
>  index 0b920b4..7db78ad 100644
>  --- a/lib/python/decorators.py
>  +++ b/lib/python/decorators.py
>  @@ -46,7 +46,7 @@ class ExceptionCatcher(object):
>           Exception
>       """
>   
>  -    def __init__(self, on_exception, *args, **kwargs):
>  +    def __init__(self, on_exception = "self.process_exception",
>  *args, **kwargs):
>           """
>           on_exception - string with class function name, or pointer to
>  function
>           *args, **kwargs - args for the on_exception function
>  diff --git a/lib/python/test.py b/lib/python/test.py
>  index 61e44e7..f04ad10 100644
>  --- a/lib/python/test.py
>  +++ b/lib/python/test.py
>  @@ -35,42 +35,20 @@ class AutoQATest(test.test, object):
>           self.outputs = "" #second part of mail_body
>           self.mail_to = [] #list of emails to send results to. Mail is
>  always sent to results list
>   
>  -    @ExceptionCatcher("self.setup_failed")
>  +    @ExceptionCatcher()
>       def setup(self, **kwargs):
>           pass
>   
>  -    @ExceptionCatcher("self.initialize_failed")
>  +    @ExceptionCatcher()
>       def initialize(self, config, **kwargs):
>           self.config = config_loader(config, self.tmpdir)
>           self.autotest_url = make_autotest_url(self.config)
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, **kwargs):
>           pass
>   
>  -    def setup_failed(self, exc = None):
>  -        self.result = "CRASHED"
>  -        if exc is not None:
>  -            self.summary = "Setup failed: %s: %s" %
>  (exc.__class__.__name__, exc)
>  -        else:
>  -            self.summary = "Setup failed: Exception: Unknown
>  exception"
>  -        try:
>  -            self.postprocess_iteration()
>  -        except Exception, e:
>  -            traceback.print_exc(file=sys.stderr)
>  -
>  -    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 Exception, e:
>  -            traceback.print_exc(file=sys.stderr)
>  -
>  -    def run_once_failed(self, exc = None):
>  +    def process_exception(self, exc = None):
>           self.result = "CRASHED"
>           if exc is not None:
>               self.summary = "%s: %s" % (exc.__class__.__name__, exc)
>  diff --git a/tests/conflicts/conflicts.py
>  b/tests/conflicts/conflicts.py
>  index 4328728..5a93a2b 100644
>  --- a/tests/conflicts/conflicts.py
>  +++ b/tests/conflicts/conflicts.py
>  @@ -32,7 +32,7 @@ class conflicts(AutoQATest):
>           f.write(logdata)
>           f.close()
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, baseurl, parents, name, **kwargs):
>           if name:
>               name = "%s-%s" % (name, autoqa.util.get_basearch())
>  diff --git a/tests/helloworld/helloworld.py
>  b/tests/helloworld/helloworld.py
>  index 9f6ac50..0af1c52 100644
>  --- a/tests/helloworld/helloworld.py
>  +++ b/tests/helloworld/helloworld.py
>  @@ -25,7 +25,7 @@ from autoqa.decorators import ExceptionCatcher
>   class helloworld(AutoQATest):
>       version = 1 # increment if setup() changes
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, *args, **kwargs):
>           self.summary = 'Hello, World!'
>           self.outputs = "===Printing passed params===\n"
>  diff --git a/tests/initscripts/initscripts.py
>  b/tests/initscripts/initscripts.py
>  index 133d058..e0f95bb 100644
>  --- a/tests/initscripts/initscripts.py
>  +++ b/tests/initscripts/initscripts.py
>  @@ -33,7 +33,7 @@ from autoqa.decorators import ExceptionCatcher
>   class initscripts(AutoQATest):
>       version = 1 # increment if setup() changes
>   
>  -    @ExceptionCatcher("self.setup_failed")
>  +    @ExceptionCatcher()
>       def setup(self, **kwargs):
>           """
>           adds beakerlib repository and installs beakerlib
>  @@ -45,7 +45,7 @@ class initscripts(AutoQATest):
>               f.close()
>           utils.system('yum -y install beakerlib') 
>   
>  -    @ExceptionCatcher("self.initialize_failed")
>  +    @ExceptionCatcher()
>       def initialize(self, config, **kwargs):
>           super(initscripts, self).initialize(config)
>           self.rpmdir = os.path.join(self.tmpdir, 'rpms')
>  @@ -99,7 +99,7 @@ class initscripts(AutoQATest):
>   
>           return (result, outputs)
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, kojitag, **kwargs):
>           if kwargs['hook'] == 'post-koji-build':
>               envrs = [kwargs['envr']]
>  diff --git a/tests/rats_install/rats_install.py
>  b/tests/rats_install/rats_install.py
>  index 360be3f..df23451 100644
>  --- a/tests/rats_install/rats_install.py
>  +++ b/tests/rats_install/rats_install.py
>  @@ -30,12 +30,12 @@ from autoqa.decorators import ExceptionCatcher
>   class rats_install(AutoQATest):
>       version = 1
>   
>  -    @ExceptionCatcher("self.initialize_failed")
>  +    @ExceptionCatcher()
>       def initialize(self, config, **kwargs):
>           super(rats_install, self).initialize(config)
>           self.irb_conf = os.path.join(self.bindir, 'irb.cfg')
>   
>  -    @ExceptionCatcher("self.setup_failed")
>  +    @ExceptionCatcher()
>       def setup(self, **kwargs):
>           if os.path.exists(self.irb_conf):
>               utils.system('yum -y install python-fedora')
>  @@ -51,7 +51,7 @@ class rats_install(AutoQATest):
>           #       use the big hammer until we figure out the right
>  thing to do
>           utils.system('lokkit --disabled')
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, baseurl, name, image_url="", boot_args="",
>  **kwargs):
>           if name:
>               name = "%s-%s" % (name, util.get_basearch())
>  diff --git a/tests/rats_sanity/rats_sanity.py
>  b/tests/rats_sanity/rats_sanity.py
>  index 5a2a4f1..d6a6357 100644
>  --- a/tests/rats_sanity/rats_sanity.py
>  +++ b/tests/rats_sanity/rats_sanity.py
>  @@ -28,17 +28,17 @@ from autoqa.decorators import ExceptionCatcher
>   class rats_sanity(AutoQATest):
>       version = 1
>   
>  -    @ExceptionCatcher("self.initialize_failed")
>  +    @ExceptionCatcher()
>       def initialize(self, config, **kwargs):
>           super(rats_sanity, self).initialize(config)
>           self.irb_conf = os.path.join(self.bindir, 'irb.cfg')
>   
>  -    @ExceptionCatcher("self.setup_failed")
>  +    @ExceptionCatcher()
>       def setup(self, **kwargs):
>           if os.path.exists(self.irb_conf):
>               utils.system('yum -y install python-fedora')
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, baseurl, parents, name, **kwargs):
>           if name:
>               name = "%s-%s" % (name, util.get_basearch())
>  diff --git a/tests/repoclosure/repoclosure.py
>  b/tests/repoclosure/repoclosure.py
>  index 5c8752e..b27b684 100644
>  --- a/tests/repoclosure/repoclosure.py
>  +++ b/tests/repoclosure/repoclosure.py
>  @@ -25,11 +25,11 @@ from autoqa.decorators import ExceptionCatcher
>   class repoclosure(AutoQATest):
>       version = 1
>   
>  -    @ExceptionCatcher("self.setup_failed")
>  +    @ExceptionCatcher()
>       def setup(self, **kwargs):
>           utils.system('yum -y install yum-utils')
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, baseurl, parents='', name='', **kwargs):
>           if name:
>               name = "%s-%s" % (name, autoqa.util.get_basearch())
>  diff --git a/tests/rpmguard/rpmguard.py b/tests/rpmguard/rpmguard.py
>  index 4e4ee04..588a58f 100644
>  --- a/tests/rpmguard/rpmguard.py
>  +++ b/tests/rpmguard/rpmguard.py
>  @@ -30,18 +30,18 @@ from autoqa.decorators import ExceptionCatcher
>   class rpmguard(AutoQATest):
>       version = 1 # increment if setup() changes
>   
>  -    @ExceptionCatcher("self.setup_failed")
>  +    @ExceptionCatcher()
>       def setup(self, **kwargs):
>           utils.system('yum -y install rpmlint')
>   
>  -    @ExceptionCatcher("self.initialize_failed")
>  +    @ExceptionCatcher()
>       def initialize(self, config, **kwargs):
>           super(rpmguard, self).initialize(config)
>           self.rpmguard = os.path.join(self.bindir, 'rpmguard')
>           self.rpmdir = os.path.join(self.tmpdir, 'rpms')
>           os.makedirs(self.rpmdir)
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, kojitag, **kwargs):
>           if kwargs['hook'] == 'post-koji-build':
>               envrs = [kwargs['envr']]
>  diff --git a/tests/rpmlint/rpmlint.py b/tests/rpmlint/rpmlint.py
>  index ce92a1d..bb25dbc 100644
>  --- a/tests/rpmlint/rpmlint.py
>  +++ b/tests/rpmlint/rpmlint.py
>  @@ -30,17 +30,17 @@ from autoqa.decorators import ExceptionCatcher
>   class rpmlint(AutoQATest):
>       version = 1 # increment if setup() changes
>   
>  -    @ExceptionCatcher("self.setup_failed")
>  +    @ExceptionCatcher()
>       def setup(self, **kwargs):
>           utils.system('yum -y install rpmlint')
>   
>  -    @ExceptionCatcher("self.initialize_failed")
>  +    @ExceptionCatcher()
>       def initialize(self, config, **kwargs):
>           super(rpmlint, self).initialize(config)
>           self.rpmdir = os.path.join(self.tmpdir, 'rpms')
>           os.makedirs(self.rpmdir)
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, kojitag, **kwargs):
>           if kwargs['hook'] == 'post-koji-build':
>               envrs = [kwargs['envr']]
>  diff --git a/tests/upgradepath/upgradepath.py
>  b/tests/upgradepath/upgradepath.py
>  index 4f298b1..edbe4ea 100755
>  --- a/tests/upgradepath/upgradepath.py
>  +++ b/tests/upgradepath/upgradepath.py
>  @@ -37,7 +37,7 @@ from autotest_lib.client.bin.test_config import
>  config_loader
>   class upgradepath(AutoQATest):
>       version = 1 # increment if setup() changes
>   
>  -    @ExceptionCatcher("self.initialize_failed")
>  +    @ExceptionCatcher()
>       def initialize(self, config, **kwargs):
>           self.config = config_loader(config, self.tmpdir)
>           #URL of logs/results stored on autotest-server
>  @@ -86,7 +86,7 @@ class upgradepath(AutoQATest):
>                       result = 'FAILED'
>           return result
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, envrs, kojitag, **kwargs):
>           update_id = kwargs['name'] or kwargs['id']
>   
>  -- 
>  1.7.2.3
>  
>  _______________________________________________
>  autoqa-devel mailing list
>  autoqa-devel(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/autoqa-devel

[James Laska]

On Tue, 2010-10-05 at 13:40 +0200, Josef Skladanka wrote:
>  According to the
> <https://fedorahosted.org/pipermail/autoqa-devel/2010-October/001214.html>
>  I've removed the individual {setup,initialize,run_once}_failed()
>  methods and replaced these with one generic process_exception().
>  
>  I also set the self.process_exception as default method for
>  the ExceptionCatcher decorator, so we can use it without arguments.
>  
>  The possibility to provide the name of some particular method
>  is, of course, still there. 
Geez, I wasn't expecting this so fast.  The deletions noted below sure
are appealing! :)

>  Will change the wiki documentation, once this patch is in master.
>  ---
>   lib/python/decorators.py           |    2 +-
>   lib/python/test.py                 |   30 ++++--------------------------
>   tests/conflicts/conflicts.py       |    2 +-
>   tests/helloworld/helloworld.py     |    2 +-
>   tests/initscripts/initscripts.py   |    6 +++---
>   tests/rats_install/rats_install.py |    6 +++---
>   tests/rats_sanity/rats_sanity.py   |    6 +++---
>   tests/repoclosure/repoclosure.py   |    4 ++--
>   tests/rpmguard/rpmguard.py         |    6 +++---
>   tests/rpmlint/rpmlint.py           |    6 +++---
>   tests/upgradepath/upgradepath.py   |    4 ++--
>   11 files changed, 26 insertions(+), 48 deletions(-)
>  
>  diff --git a/lib/python/decorators.py b/lib/python/decorators.py
>  index 0b920b4..7db78ad 100644
>  --- a/lib/python/decorators.py
>  +++ b/lib/python/decorators.py
>  @@ -46,7 +46,7 @@ class ExceptionCatcher(object):
>           Exception
>       """
>   
>  -    def __init__(self, on_exception, *args, **kwargs):
>  +    def __init__(self, on_exception = "self.process_exception", *args,
> **kwargs): 
Since we define the exception handler in decorators.py to use a specific
on_exception method (in this case, "self.process_exception"), should we
move the definition of that method into decorators.py also?  Note, I
don't have a really strong understanding of decorators, so this might
not make sense.

At the least, perhaps having some conditional logic to make sure that
the method defined in on_exception exists?  I don't think this is a big
problem since we (test.py) are the only ones using ExceptionHandler(),
but just in case?

>           """
>           on_exception - string with class function name, or pointer to function
>           *args, **kwargs - args for the on_exception function
>  diff --git a/lib/python/test.py b/lib/python/test.py
>  index 61e44e7..f04ad10 100644
>  --- a/lib/python/test.py
>  +++ b/lib/python/test.py
>  @@ -35,42 +35,20 @@ class AutoQATest(test.test, object):
>           self.outputs = "" #second part of mail_body
>           self.mail_to = [] #list of emails to send results to. Mail is always sent to
> results list
>   
>  -    @ExceptionCatcher("self.setup_failed")
>  +    @ExceptionCatcher()
>       def setup(self, **kwargs):
>           pass
>   
>  -    @ExceptionCatcher("self.initialize_failed")
>  +    @ExceptionCatcher()
>       def initialize(self, config, **kwargs):
>           self.config = config_loader(config, self.tmpdir)
>           self.autotest_url = make_autotest_url(self.config)
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, **kwargs):
>           pass
>   
>  -    def setup_failed(self, exc = None):
>  -        self.result = "CRASHED"
>  -        if exc is not None:
>  -            self.summary = "Setup failed: %s: %s" % (exc.__class__.__name__,
> exc)
>  -        else:
>  -            self.summary = "Setup failed: Exception: Unknown exception"
>  -        try:
>  -            self.postprocess_iteration()
>  -        except Exception, e:
>  -            traceback.print_exc(file=sys.stderr)
>  -
>  -    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"
This was one thing I wasn't sure about.  With the previous mechanism, we
had a self.summary that clearly identified where the failure occurred
(e.g. "Initialize failed").  When using a single ExceptionHandler
method, I wasn't sure if having exc.__class__.__name__ alone was enough
for us to recognize where the failure was triggered.  I can't find any
crash examples to tell if this would make problem determination hardware
right now other than crashing tests (such as
https://fedorahosted.org/pipermail/autoqa-results/2010-October/041964.html).

>  -        try:
>  -            self.postprocess_iteration()
>  -        except Exception, e:
>  -            traceback.print_exc(file=sys.stderr)
>  -
>  -    def run_once_failed(self, exc = None):
>  +    def process_exception(self, exc = None):
>           self.result = "CRASHED"
>           if exc is not None:
>               self.summary = "%s: %s" % (exc.__class__.__name__, exc)
>  diff --git a/tests/conflicts/conflicts.py b/tests/conflicts/conflicts.py
>  index 4328728..5a93a2b 100644
>  --- a/tests/conflicts/conflicts.py
>  +++ b/tests/conflicts/conflicts.py
>  @@ -32,7 +32,7 @@ class conflicts(AutoQATest):
>           f.write(logdata)
>           f.close()
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, baseurl, parents, name, **kwargs):
>           if name:
>               name = "%s-%s" % (name, autoqa.util.get_basearch())
>  diff --git a/tests/helloworld/helloworld.py b/tests/helloworld/helloworld.py
>  index 9f6ac50..0af1c52 100644
>  --- a/tests/helloworld/helloworld.py
>  +++ b/tests/helloworld/helloworld.py
>  @@ -25,7 +25,7 @@ from autoqa.decorators import ExceptionCatcher
>   class helloworld(AutoQATest):
>       version = 1 # increment if setup() changes
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, *args, **kwargs):
>           self.summary = 'Hello, World!'
>           self.outputs = "===Printing passed params===\n"
>  diff --git a/tests/initscripts/initscripts.py b/tests/initscripts/initscripts.py
>  index 133d058..e0f95bb 100644
>  --- a/tests/initscripts/initscripts.py
>  +++ b/tests/initscripts/initscripts.py
>  @@ -33,7 +33,7 @@ from autoqa.decorators import ExceptionCatcher
>   class initscripts(AutoQATest):
>       version = 1 # increment if setup() changes
>   
>  -    @ExceptionCatcher("self.setup_failed")
>  +    @ExceptionCatcher()
>       def setup(self, **kwargs):
>           """
>           adds beakerlib repository and installs beakerlib
>  @@ -45,7 +45,7 @@ class initscripts(AutoQATest):
>               f.close()
>           utils.system('yum -y install beakerlib') 
>   
>  -    @ExceptionCatcher("self.initialize_failed")
>  +    @ExceptionCatcher()
>       def initialize(self, config, **kwargs):
>           super(initscripts, self).initialize(config)
>           self.rpmdir = os.path.join(self.tmpdir, 'rpms')
>  @@ -99,7 +99,7 @@ class initscripts(AutoQATest):
>   
>           return (result, outputs)
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, kojitag, **kwargs):
>           if kwargs['hook'] == 'post-koji-build':
>               envrs = [kwargs['envr']]
>  diff --git a/tests/rats_install/rats_install.py b/tests/rats_install/rats_install.py
>  index 360be3f..df23451 100644
>  --- a/tests/rats_install/rats_install.py
>  +++ b/tests/rats_install/rats_install.py
>  @@ -30,12 +30,12 @@ from autoqa.decorators import ExceptionCatcher
>   class rats_install(AutoQATest):
>       version = 1
>   
>  -    @ExceptionCatcher("self.initialize_failed")
>  +    @ExceptionCatcher()
>       def initialize(self, config, **kwargs):
>           super(rats_install, self).initialize(config)
>           self.irb_conf = os.path.join(self.bindir, 'irb.cfg')
>   
>  -    @ExceptionCatcher("self.setup_failed")
>  +    @ExceptionCatcher()
>       def setup(self, **kwargs):
>           if os.path.exists(self.irb_conf):
>               utils.system('yum -y install python-fedora')
>  @@ -51,7 +51,7 @@ class rats_install(AutoQATest):
>           #       use the big hammer until we figure out the right thing to do
>           utils.system('lokkit --disabled')
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, baseurl, name, image_url="", boot_args="",
> **kwargs):
>           if name:
>               name = "%s-%s" % (name, util.get_basearch())
>  diff --git a/tests/rats_sanity/rats_sanity.py b/tests/rats_sanity/rats_sanity.py
>  index 5a2a4f1..d6a6357 100644
>  --- a/tests/rats_sanity/rats_sanity.py
>  +++ b/tests/rats_sanity/rats_sanity.py
>  @@ -28,17 +28,17 @@ from autoqa.decorators import ExceptionCatcher
>   class rats_sanity(AutoQATest):
>       version = 1
>   
>  -    @ExceptionCatcher("self.initialize_failed")
>  +    @ExceptionCatcher()
>       def initialize(self, config, **kwargs):
>           super(rats_sanity, self).initialize(config)
>           self.irb_conf = os.path.join(self.bindir, 'irb.cfg')
>   
>  -    @ExceptionCatcher("self.setup_failed")
>  +    @ExceptionCatcher()
>       def setup(self, **kwargs):
>           if os.path.exists(self.irb_conf):
>               utils.system('yum -y install python-fedora')
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, baseurl, parents, name, **kwargs):
>           if name:
>               name = "%s-%s" % (name, util.get_basearch())
>  diff --git a/tests/repoclosure/repoclosure.py b/tests/repoclosure/repoclosure.py
>  index 5c8752e..b27b684 100644
>  --- a/tests/repoclosure/repoclosure.py
>  +++ b/tests/repoclosure/repoclosure.py
>  @@ -25,11 +25,11 @@ from autoqa.decorators import ExceptionCatcher
>   class repoclosure(AutoQATest):
>       version = 1
>   
>  -    @ExceptionCatcher("self.setup_failed")
>  +    @ExceptionCatcher()
>       def setup(self, **kwargs):
>           utils.system('yum -y install yum-utils')
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, baseurl, parents='', name='', **kwargs):
>           if name:
>               name = "%s-%s" % (name, autoqa.util.get_basearch())
>  diff --git a/tests/rpmguard/rpmguard.py b/tests/rpmguard/rpmguard.py
>  index 4e4ee04..588a58f 100644
>  --- a/tests/rpmguard/rpmguard.py
>  +++ b/tests/rpmguard/rpmguard.py
>  @@ -30,18 +30,18 @@ from autoqa.decorators import ExceptionCatcher
>   class rpmguard(AutoQATest):
>       version = 1 # increment if setup() changes
>   
>  -    @ExceptionCatcher("self.setup_failed")
>  +    @ExceptionCatcher()
>       def setup(self, **kwargs):
>           utils.system('yum -y install rpmlint')
>   
>  -    @ExceptionCatcher("self.initialize_failed")
>  +    @ExceptionCatcher()
>       def initialize(self, config, **kwargs):
>           super(rpmguard, self).initialize(config)
>           self.rpmguard = os.path.join(self.bindir, 'rpmguard')
>           self.rpmdir = os.path.join(self.tmpdir, 'rpms')
>           os.makedirs(self.rpmdir)
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, kojitag, **kwargs):
>           if kwargs['hook'] == 'post-koji-build':
>               envrs = [kwargs['envr']]
>  diff --git a/tests/rpmlint/rpmlint.py b/tests/rpmlint/rpmlint.py
>  index ce92a1d..bb25dbc 100644
>  --- a/tests/rpmlint/rpmlint.py
>  +++ b/tests/rpmlint/rpmlint.py
>  @@ -30,17 +30,17 @@ from autoqa.decorators import ExceptionCatcher
>   class rpmlint(AutoQATest):
>       version = 1 # increment if setup() changes
>   
>  -    @ExceptionCatcher("self.setup_failed")
>  +    @ExceptionCatcher()
>       def setup(self, **kwargs):
>           utils.system('yum -y install rpmlint')
>   
>  -    @ExceptionCatcher("self.initialize_failed")
>  +    @ExceptionCatcher()
>       def initialize(self, config, **kwargs):
>           super(rpmlint, self).initialize(config)
>           self.rpmdir = os.path.join(self.tmpdir, 'rpms')
>           os.makedirs(self.rpmdir)
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, kojitag, **kwargs):
>           if kwargs['hook'] == 'post-koji-build':
>               envrs = [kwargs['envr']]
>  diff --git a/tests/upgradepath/upgradepath.py b/tests/upgradepath/upgradepath.py
>  index 4f298b1..edbe4ea 100755
>  --- a/tests/upgradepath/upgradepath.py
>  +++ b/tests/upgradepath/upgradepath.py
>  @@ -37,7 +37,7 @@ from autotest_lib.client.bin.test_config import config_loader
>   class upgradepath(AutoQATest):
>       version = 1 # increment if setup() changes
>   
>  -    @ExceptionCatcher("self.initialize_failed")
>  +    @ExceptionCatcher()
>       def initialize(self, config, **kwargs):
>           self.config = config_loader(config, self.tmpdir)
>           #URL of logs/results stored on autotest-server
>  @@ -86,7 +86,7 @@ class upgradepath(AutoQATest):
>                       result = 'FAILED'
>           return result
>   
>  -    @ExceptionCatcher("self.run_once_failed")
>  +    @ExceptionCatcher()
>       def run_once(self, envrs, kojitag, **kwargs):
>           update_id = kwargs['name'] or kwargs['id'] 
I don't see any issues with the proposed patch.  Just the clarifying
comments noted above.

Thanks,
James

[Josef Skladanka]

On 10/05/2010 02:53 PM, James Laska wrote:
>  On Tue, 2010-10-05 at 13:40 +0200, Josef Skladanka wrote:
>
>  Geez, I wasn't expecting this so fast.  The deletions noted below sure
>  are appealing! :)
>
>  Since we define the exception handler in decorators.py to use a specific
>  on_exception method (in this case, "self.process_exception"), should we
>  move the definition of that method into decorators.py also?  Note, I
>  don't have a really strong understanding of decorators, so this might
>  not make sense.
>  
Well, I would not move that method there - it should most certainly stay 
in the class, in which the decorator is then used. But yes, this 
behaviour should be documented, so we can avoid errors from 'non clear 
code'.

>  At the least, perhaps having some conditional logic to make sure
> that
>  the method defined in on_exception exists?  I don't think this is a big
>  problem since we (test.py) are the only ones using ExceptionHandler(),
>  but just in case?
>  
Well, that's an interesting idea, I'll have to give it some thought, 
because, at the moment, I'm not sure how to implement that functionality.


>  This was one thing I wasn't sure about.  With the previous
> mechanism, we
>  had a self.summary that clearly identified where the failure occurred
>  (e.g. "Initialize failed").  When using a single ExceptionHandler
>  method, I wasn't sure if having exc.__class__.__name__ alone was enough
>  for us to recognize where the failure was triggered.  I can't find any
>  crash examples to tell if this would make problem determination hardware
>  right now other than crashing tests (such as
>  https://fedorahosted.org/pipermail/autoqa-results/2010-October/041964.html). 
Yes, these were also my concerns, and no, we can not really tell from 
the exc class name itself. But the autotest logs for the case you sent 
it is 
<http://autoqa.fedoraproject.org/results/9896-autotest/qa02.c.fedoraprojec...

store the whole traceback (which we would still need to see, to be able 
to determine exactly which line of the code raised the exception, even 
if there was the 'initialize/run once failed' string), so I don't think, 
that we're actually loosing any _really important_ information.

>  I don't see any issues with the proposed patch.  Just the
> clarifying
>  comments noted above.
>
>  Thanks,
>  James
>  
Hopefully, it is more clear now, but, as always, feel free to ask as 
many questions as you need.

Joza

[James Laska]

On Tue, 2010-10-05 at 15:13 +0200, Josef Skladanka wrote:
>  On 10/05/2010 02:53 PM, James Laska wrote:
>  > On Tue, 2010-10-05 at 13:40 +0200, Josef Skladanka wrote:
>  > At the least, perhaps having some conditional logic to make sure that
>  > the method defined in on_exception exists?  I don't think this is a big
>  > problem since we (test.py) are the only ones using ExceptionHandler(),
>  > but just in case?
>  >
>  
>  Well, that's an interesting idea, I'll have to give it some thought, 
>  because, at the moment, I'm not sure how to implement that functionality. 
Me neither.  I'm not sure if this is just a call to hasattr() (which
accepts a string) or using the inspect module (inspect.ismethod() -- may
require some conversion to convert the string into an method instance)?

>  > This was one thing I wasn't sure about.  With the previous
> mechanism, we
>  > had a self.summary that clearly identified where the failure occurred
>  > (e.g. "Initialize failed").  When using a single ExceptionHandler
>  > method, I wasn't sure if having exc.__class__.__name__ alone was enough
>  > for us to recognize where the failure was triggered.  I can't find any
>  > crash examples to tell if this would make problem determination hardware
>  > right now other than crashing tests (such as
>  > https://fedorahosted.org/pipermail/autoqa-results/2010-October/041964.html).
>  
>  Yes, these were also my concerns, and no, we can not really tell from 
>  the exc class name itself. But the autotest logs for the case you sent 
>  it is 
>
> <http://autoqa.fedoraproject.org/results/9896-autotest/qa02.c.fedoraprojec...
>
>  store the whole traceback (which we would still need to see, to be able 
>  to determine exactly which line of the code raised the exception, even 
>  if there was the 'initialize/run once failed' string), so I don't think, 
>  that we're actually loosing any _really important_ information. 
Yeah, that traceback is the juicy bit! 

>  > I don't see any issues with the proposed patch.  Just the
> clarifying
>  > comments noted above. 
>  Hopefully, it is more clear now, but, as always, feel free to ask as
>
>  many questions as you need. 
Thanks,
James