Merge lp:~jtatum/mago/pidgin-exception into lp:~mago-contributors/mago/mago-1.0

[James Tatum]

[James Tatum (jtatum)]

If the pidgin suite is invoked and python-pyxmpp or python-msn are not installed, mago crashes. This handles the imports so import errors throw exceptions at test execution time rather than at import time.

[Nagappan Alagappan (nagappan)]

I think, you can use

if ClientXMPP:
    self.client = ClientXMPP(userid, passwd)
else:
    raise Exception, "pidgin testing requires python-pyxmpp package."

The same can be applied to ClientMSN

review: Approve

[Javier Collado (javier.collado)]

Hello,

I'm not sure I agree on this change. I would say that the correct way to solve this issue is at the package level, that is, adding those packages as dependencies to mago.

In my opinion, catching ImportError exception should be done if there's an alternative library that could do the same job (for example some xml libraries implement the same ElementTree interface) so the problem can be solved at runtime. If the problem isn't really solved, then the exception should be raised as soon as possible to be able to locate the problem and fix it.

Please let me know what do you think about this.

Best regards,
    Javier

review: Needs Information

[James Tatum (jtatum)]

Hi Javier,

I agree in principle. Many mago tests already require some configuration of some kind or another and so users should be examining the various README files to see what the prerequisites are. I think the case here is for users that want to use the mago framework but who aren't running all the test suites that are shipping with it. Should the mago package pull in a bunch of libraries that only get used for specific tests? This is a philosophical question.

If the answer is yes, we should probably add the debian directory in to the bzr tree so dependencies can get added as they are added to the source. Mago is not just an ubuntu project, so they need to be documented elsewhere as well.

For what it's worth, without this patch, mago actually crashes when running the suite and does not output a log for the pidgin tests or continue to more tests if you do not have one of the libraries.

[Rick McBride (rmcbride)]

Hi,

I think for these particular imports that this conditional handling is appropriate. I'd like to be able to run these tests without adding packages that I may not otherwise have installed. The success/failure of a given pidgin installations should not be dependent on me having the MSN or XMPP modules available.

> Hello,
>
> I'm not sure I agree on this change. I would say that the correct way to solve
> this issue is at the package level, that is, adding those packages as
> dependencies to mago.
>
> In my opinion, catching ImportError exception should be done if there's an
> alternative library that could do the same job (for example some xml libraries
> implement the same ElementTree interface) so the problem can be solved at
> runtime. If the problem isn't really solved, then the exception should be
> raised as soon as possible to be able to locate the problem and fix it.
>
> Please let me know what do you think about this.
>
> Best regards,
> Javier

[Ara Pulido (ara)]

I agree with the change. If included in the packaging it should only be included as a recommends.

If the user does not have it installed, then putting a correct error is better

review: Approve

=== modified file 'mago/application/pidgin/buddy.py'
--- mago/application/pidgin/buddy.py	2009-06-03 14:22:04 +0000
+++ mago/application/pidgin/buddy.py	2009-08-17 16:11:02 +0000
@@ -2,8 +2,15 @@
 
 import gobject, gtk
 
-from xmpp_utils import ClientXMPP
-from msn_utils import ClientMSN
+try:
+    from xmpp_utils import ClientXMPP
+except ImportError:
+    ClientXMPP = None
+
+try:
+    from msn_utils import ClientMSN
+except ImportError:
+    ClientMSN = None
 
 class _Buddy(object):
     """
@@ -72,7 +79,10 @@
     def __init__(self, userid, passwd):
                 
         self.account = (userid, passwd)
-        self.client = ClientXMPP(userid, passwd)
+        try:
+            self.client = ClientXMPP(userid, passwd)
+        except AttributeError:
+            raise Exception, "pidgin testing requires python-pyxmpp package."
         
     def connect(self, register=False, name='', email=''):
         def _idle_cb(client):
@@ -138,7 +148,10 @@
 class _BuddyMSN(_Buddy):
     def __init__(self, userid, passwd):
         self.account = (userid, passwd)
-        self.client  = ClientMSN(self.account)
+        try:
+            self.client  = ClientMSN(self.account)
+        except AttributeError:
+            raise Exception, "pidgin testing requires python-msn package."
 
     def connect(self, register=False, name='', email=''):
         def _idle_cb(client):