We should rework the handling of exceptions in all of the automation scripts.

Move of Mozmill related project bugs to newly created components. You can filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.

For the first step let us fix the BaseException deprecation messages, so we can see the real exception messages if something fails.

Comment on attachment 562764 [details] [diff] [review] Fix for BaseException deprecation messages ># HG changeset patch ># Parent 1f4177ac557bd4a95c8a4e87140c1240eaf4fe5b ># User Henrik Skupin <hskupin@mozilla.com> >Bug 568726 - Fix BaseException deprecation messages. r=dhunt > >diff --git a/libs/install.py b/libs/install.py >--- a/libs/install.py >+++ b/libs/install.py >@@ -123,17 +123,17 @@ class Installer(object): > targetFolder = destination + os.path.basename(bundle) > > # If there is an existing installation remove it now > self.uninstall(targetFolder) > print "*** Copying %s to %s" % (bundle, targetFolder) > shutil.copytree(bundle, targetFolder) > > destination = targetFolder >- except Exception, e: >+ except Exception: > if not os.path.exists(targetFolder): > print "*** Failure in copying the application files" > raise e Wouldn't this fail as 'e' is not defined?

Thanks for spotting this. Here the updated version.

Comment on attachment 562912 [details] [diff] [review] Patch v2 >- except IOError, e: >+ except IOError as e: > raise e We can simplify this as just raise if we can to bubble it up

it might be worth raising another bug to change print statements to log calls then we can get error calls when something fails, e.g. > print "*** Failure in copying the application files" could be changed to logger.critical('Failure in copying the application files') just a thought

(In reply to David Burns :automatedtester from comment #5) > Comment on attachment 562912 [details] [diff] [review] [diff] [details] [review] > Patch v2 > > >- except IOError, e: > >+ except IOError as e: > > raise e > > > We can simplify this as just raise if we can to bubble it up Can you be specific please? Can I simply drop the except block before the 'else'? (In reply to David Burns :automatedtester from comment #6) > it might be worth raising another bug to change print statements to log > calls then we can get error calls when something fails, e.g. > > > print "*** Failure in copying the application files" > > could be changed to > > logger.critical('Failure in copying the application files') > > just a thought Yeah. Sounds fine. Please do so.

Comment on attachment 562912 [details] [diff] [review] Patch v2 I noticed in scraper.py that we've used the following: try: # Try to download the signed candidate build MozillaScraper.download(self) except NotFoundException as ex: print str(ex) Is it worth bringing this inline with the other try...except blocks, and change 'ex' to 'e'? r+ me with or without that change, as it's a minor nit/question.

(In reply to Henrik Skupin (:whimboo) from comment #7) > (In reply to David Burns :automatedtester from comment #5) > > Comment on attachment 562912 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review] > > Patch v2 > > > > >- except IOError, e: > > >+ except IOError as e: > > > raise e > > > > > > We can simplify this as just raise if we can to bubble it up > What I was meaning was we can do except IOError: raise The error variable e is not needed since we are just raising the exception up the stack.

Updated patch with all comments addressed. Please have a quick look over it.

Comment on attachment 563165 [details] [diff] [review] Fix for BaseException messages v3 [checked-in] I will work on the remaining code next quarter.

Landed as: http://hg.mozilla.org/qa/mozmill-automation/rev/cdddd5b6913a

We can't make use of 'as' because it's not supported in Python 2.5. As long as we support platforms with Python 2.5 installed, we also have to support it in our scripts.

Comment on attachment 568351 [details] [diff] [review] Don't use 'as' because it's not supported in Python 2.5 [checked-in] Review of attachment 568351 [details] [diff] [review]: ----------------------------------------------------------------- lgtm

Comment on attachment 568351 [details] [diff] [review] Don't use 'as' because it's not supported in Python 2.5 [checked-in] Landed as: http://hg.mozilla.org/qa/mozmill-automation/rev/68b25ecb16c6

I think that we are good enough here. Lets call it fixed and get a new bug opened if necessary.