Code review comment for lp:~eeejay/mago/tweaks

Revision history for this message
Eitan Isaacson (eeejay) wrote :

> Same thing here, I think that the code is OK, but apart of the comment
> Javier's suggested, I would also appreciate a comment on why it was decided to
> remove application as an init method parameter.
> (commenting here in launchpad, I mean)

It is hard (not clean) to subclass an Application class, and include it as the factory for the TestSuite class.

The specific use-case is the pidgin_messaging.py script overrides Application.open() and Application.close().

Before the app/suite split, it was an easy override, but now we need to:
1. Override Pidgin as PidginUseApp
2. Override PidginUseTest.__init__(), re-implement it without calling the superclass init, because it will provide (and use) the wrong factory. Here is what it looks like:

class PidginUseApp(Pidgin):
    def open(self):
        Pidgin.open(self)
        self.buddy_login()
        self.wait_for_account_connect(self.get_account_name('XMPP'), 'XMPP')
        self.wait_for_buddy(self.get_account_alias('OtherXMPP'))

    def close(self):
        self.buddy.disconnect()
        Pidgin.exit(self)

class PidginUseTest(PidginTestSuite):
    def __init__(self, clean_profile=True,
                 profile_template='',
                 credentials=''):
        self.clean_profile = clean_profile
        self.profile_template = profile_template
        self.credentials = credentials
        SingleApplicationTestSuite.__init__(self, PidginUseApp)

With my changes, besides overriding Pidgin, you just need to override a static class attribute.

Phew!!

« Back to merge proposal