Merge lp:~eeejay/mago/tweaks into lp:~mago-contributors/mago/mago-1.0

Proposed by Eitan Isaacson
Status: Merged
Merged at revision: not available
Proposed branch: lp:~eeejay/mago/tweaks
Merge into: lp:~mago-contributors/mago/mago-1.0
Diff against target: None lines
To merge this branch: bzr merge lp:~eeejay/mago/tweaks
Reviewer Review Type Date Requested Status
Javier Collado (community) Approve
Ara Pulido Approve
Review via email: mp+6820@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Eitan Isaacson (eeejay) wrote :

Makes the application class factory a class attribute of the test suite.

Revision history for this message
Javier Collado (javier.collado) wrote :

The code change it's OK to me. I just think that the in the TestSuite class documentation string a comment should be added to state clearly what's the aim of the new APPLICATION_FACTORY class attribute.

review: Needs Information
Revision history for this message
Ara Pulido (ara) 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)

review: Needs Information
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!!

lp:~eeejay/mago/tweaks updated
54. By Eitan Isaacson

add helpful docstring for SingleApplicationTestSuite.APPLICATION_FACTORY

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

> The code change it's OK to me. I just think that the in the TestSuite class
> documentation string a comment should be added to state clearly what's the aim
> of the new APPLICATION_FACTORY class attribute.

Added a docstring.

Revision history for this message
Ara Pulido (ara) wrote :

Thanks for the comments :-)

review: Approve
Revision history for this message
Javier Collado (javier.collado) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'desktoptesting/test_suite/deskex.py'
--- desktoptesting/test_suite/deskex.py 2009-05-06 09:37:46 +0000
+++ desktoptesting/test_suite/deskex.py 2009-05-27 10:45:38 +0000
@@ -10,9 +10,7 @@
10 """10 """
11 Default test suite for Seahorse11 Default test suite for Seahorse
12 """12 """
13 def __init__(self):13 APPLICATION_FACTORY = IndicatorApplet
14 SingleApplicationTestSuite.__init__(self, IndicatorApplet)
15
16 def setup(self):14 def setup(self):
17 self.application.open()15 self.application.open()
1816
@@ -26,9 +24,7 @@
26 """24 """
27 Default test suite for Seahorse25 Default test suite for Seahorse
28 """26 """
29 def __init__(self):27 APPLICATION_FACTORY = NotifyOSD
30 SingleApplicationTestSuite.__init__(self, NotifyOSD)
31
32 def setup(self):28 def setup(self):
33 self.application.open()29 self.application.open()
3430
3531
=== modified file 'desktoptesting/test_suite/gnome.py'
--- desktoptesting/test_suite/gnome.py 2009-04-21 10:15:09 +0000
+++ desktoptesting/test_suite/gnome.py 2009-05-27 10:45:38 +0000
@@ -10,9 +10,7 @@
10 """10 """
11 Default test suite for Seahorse11 Default test suite for Seahorse
12 """12 """
13 def __init__(self):13 APPLICATION_FACTORY = Seahorse
14 SingleApplicationTestSuite.__init__(self, Seahorse)
15
16 def setup(self):14 def setup(self):
17 self.application.open()15 self.application.open()
1816
@@ -28,9 +26,7 @@
28 """26 """
29 Default test suite for GEdit27 Default test suite for GEdit
30 """28 """
31 def __init__(self):29 APPLICATION_FACTORY = GEdit
32 SingleApplicationTestSuite.__init__(self, GEdit)
33
34 def setup(self):30 def setup(self):
35 self.application.open()31 self.application.open()
3632
3733
=== modified file 'desktoptesting/test_suite/main.py'
--- desktoptesting/test_suite/main.py 2009-04-21 10:15:09 +0000
+++ desktoptesting/test_suite/main.py 2009-05-27 10:45:38 +0000
@@ -2,6 +2,8 @@
2test_suite module contains the definition of the TestSuite class that2test_suite module contains the definition of the TestSuite class that
3must be used by all test suites written for the desktoptesting package3must be used by all test suites written for the desktoptesting package
4"""4"""
5from ..application.main import Application
6
5class TestSuite:7class TestSuite:
6 """8 """
7 TestSuite that implements all the test suite methods desired in a9 TestSuite that implements all the test suite methods desired in a
@@ -22,8 +24,9 @@
22 Test suite intended to make sure that a single application is24 Test suite intended to make sure that a single application is
23 running25 running
24 """26 """
25 def __init__(self, application_factory):27 APPLICATION_FACTORY = Application
26 self.application = application_factory()28 def __init__(self):
29 self.application = self.APPLICATION_FACTORY()
2730
28 def cleanup(self):31 def cleanup(self):
29 self.application.set_name(self.application.WINDOW)32 self.application.set_name(self.application.WINDOW)
3033
=== modified file 'desktoptesting/test_suite/ubuntu.py'
--- desktoptesting/test_suite/ubuntu.py 2009-04-21 09:58:40 +0000
+++ desktoptesting/test_suite/ubuntu.py 2009-05-27 10:45:38 +0000
@@ -6,9 +6,7 @@
6from ..application.ubuntu import UbuntuMenu, UpdateManager6from ..application.ubuntu import UbuntuMenu, UpdateManager
77
8class UbuntuMenuTestSuite(SingleApplicationTestSuite):8class UbuntuMenuTestSuite(SingleApplicationTestSuite):
9 def __init__(self):9 APPLICATION_FACTORY = UbuntuMenu
10 SingleApplicationTestSuite.__init__(self, UbuntuMenu)
11
12 def teardown(self):10 def teardown(self):
13 self.cleanup() 11 self.cleanup()
1412
@@ -18,5 +16,4 @@
1816
1917
20class UpdateManagerTestSuite(SingleApplicationTestSuite):18class UpdateManagerTestSuite(SingleApplicationTestSuite):
21 def __init__(self):19 APPLICATION_FACTORY = UpdateManager
22 SingleApplicationTestSuite.__init__(self, UpdateManager)

Subscribers

People subscribed via source and target branches

to status/vote changes: