Merge lp:~sinzui/launchpad/mlist-sync-0 into lp:launchpad

Proposed by Curtis Hovey
Status: Merged
Merged at revision: 11691
Proposed branch: lp:~sinzui/launchpad/mlist-sync-0
Merge into: lp:launchpad
Diff against target: 183 lines (+29/-108)
2 files modified
lib/lp/services/mailman/doc/staging.txt (+26/-105)
scripts/mlist-sync.py (+3/-3)
To merge this branch: bzr merge lp:~sinzui/launchpad/mlist-sync-0
Reviewer Review Type Date Requested Status
Edwin Grubbs (community) code Approve
Review via email: mp+37885@code.launchpad.net

Description of the change

This is my branch to switch mlist-sync to a LaunchpadScript.

    lp:~sinzui/launchpad/mlist-sync-0
    Diff size: 184
    Launchpad bug:
          https://bugs.launchpad.net/bugs/652007
    Test command: ./bin/test -vv --layer=Mailman -t staging
    Pre-implementation: no one
    Target release: 10.10

Switch mlist-sync to a LaunchpadScript
---------------------------------------

mlist-sync generated a oops, but it should not. scripts/mlist-sync.py gets
this behavior as it inherits from LaunchpadCronScript, despite being in the
scripts directory. This would be a bug - it should inherit from
LaunchpadScript.

Rules
-----

    * Change the script base to LaunchpadScript.
    * Update staging.txt to reliably run. It times out often because
      it indirectly setups of the test using integration tools. Directly
      create the objects under test.

QA
--

    * Verify mlist-sync does not appear in the oopses

Lint
----

Linting changed files:
  lib/lp/services/mailman/doc/staging.txt
  scripts/mlist-sync.py

Test
----

    * lib/lp/services/mailman/doc/staging.txt
      * We would not normally want to change this test because LaunchpadScript
        is interchangeable with LaunchpadCronScript in this case, but the
        test is very brittle. It often times out because it takes too long
        for messages to move through the smtp and xmlrpc servers.
      * I was able to replace the browser-based helpers with factory methods
        that make the objects under test. This made the test stable.
      * I could not discover a way to build the mboxes and archives, but I
        did discover they are not required for the test! I reported bug
        656417 because I think the rsync portion of the script should be
        tested.

Implementation
--------------

    * scripts/mlist-sync.py
      * Changed the base class.

To post a comment you must log in.
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

This looks good.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/services/mailman/doc/staging.txt'
--- lib/lp/services/mailman/doc/staging.txt 2010-10-03 15:30:06 +0000
+++ lib/lp/services/mailman/doc/staging.txt 2010-10-07 18:59:42 +0000
@@ -19,122 +19,43 @@
19informing them of their new list. We can ignore these.19informing them of their new list. We can ignore these.
2020
21 >>> from canonical.launchpad.ftests import login, logout21 >>> from canonical.launchpad.ftests import login, logout
22 >>> from Mailman import MailList
23
24 >>> def makeMailmanList(lp_mailing_list):
25 ... mlist = MailList.MailList()
26 ... team = lp_mailing_list.team
27 ... owner_email = team.teamowner.preferredemail.email
28 ... mlist.Create(team.name, owner_email, 'password')
29 ... mlist.host_name = 'lists.prod.launchpad.dev'
30 ... mlist.web_page_url = 'http://lists.prod.launchpad.dev/mailman/'
31 ... mlist.Save()
32 ... mlist.addNewMember(owner_email)
33 ... mlist.Unlock()
34 ... return mlist
2235
23 >>> login('admin@canonical.com')36 >>> login('admin@canonical.com')
2437 >>> team_one, list_one = factory.makeTeamAndMailingList(
25 >>> from zope.security.proxy import removeSecurityProxy38 ... 'staging-one', 'owner-one')
26 >>> owner = removeSecurityProxy(factory.makePerson())39 >>> team_two, list_two = factory.makeTeamAndMailingList(
27 >>> team_one = factory.makeTeam(owner=owner, name='staging-one')40 ... 'staging-two', 'owner-two')
28 >>> team_two = factory.makeTeam(owner=owner, name='staging-two')41 >>> mlist_one = makeMailmanList(list_one)
29 >>> transaction.commit()42 >>> mlist_two = makeMailmanList(list_two)
30 >>> logout()
31
32 >>> from canonical.launchpad.testing.pages import setupBrowser
33 >>> browser = setupBrowser('Basic %s:%s' %
34 ... (owner.preferredemail.email, owner._password_cleartext_cached))
35
36 >>> from lp.services.mailman.testing import helpers
37 >>> helpers.apply_for_list(browser, 'staging-one')
38 >>> list_one = helpers.review_list('staging-one')
39
40 >>> helpers.apply_for_list(browser, 'staging-two')
41 >>> list_two = helpers.review_list('staging-two')
42
43Anne subscribes to two mailing lists. Bart and Cris both subscribe to only
44one of the mailing lists.
45
46 >>> helpers.subscribe('Anne', 'staging-one')
47 >>> helpers.subscribe('Anne', 'staging-two')
48 >>> helpers.subscribe('Bart', 'staging-one')
49 >>> helpers.subscribe('Cris', 'staging-two')
50
51A bunch of messages are sent to the mailing lists.
52
53 >>> smtpd.reset()
54 >>> from Mailman.Post import inject
55 >>> inject('staging-one', """\
56 ... From: anne.person@example.com
57 ... To: staging-one@lists.launchpad.dev
58 ... Subject: hello
59 ... Message-ID: <first-injection>
60 ...
61 ... Hello team!
62 ... """)
63
64 # Wait for all recipients to receive the message.
65 >>> for index in range(3):
66 ... smtpd_watcher.wait_for_mbox_delivery('first-injection')
67
68 >>> inject('staging-two', """\
69 ... From: anne.person@example.com
70 ... To: staging-two@lists.launchpad.dev
71 ... Subject: hello
72 ... Message-ID: <second-injection>
73 ...
74 ... Hello team!
75 ... """)
76
77 # Wait for all recipients to receive the message.
78 >>> for index in range(3):
79 ... smtpd_watcher.wait_for_mbox_delivery('second-injection')
80
81 >>> inject('staging-one', """\
82 ... From: bart.person@example.com
83 ... To: staging-one@lists.launchpad.dev
84 ... Subject: hello
85 ... Message-ID: <third-injection>
86 ...
87 ... Hello team!
88 ... """)
89
90 # Wait for all recipients to receive the message.
91 >>> for index in range(3):
92 ... smtpd_watcher.wait_for_mbox_delivery('third-injection')
93
94 >>> inject('staging-two', """\
95 ... From: cris.person@example.com
96 ... To: staging-two@lists.launchpad.dev
97 ... Subject: hello
98 ... Message-ID: <fourth-injection>
99 ...
100 ... Hello team!
101 ... """)
102
103 # Wait for all recipients to receive the message.
104 >>> for index in range(3):
105 ... smtpd_watcher.wait_for_mbox_delivery('fourth-injection')
106
107 >>> from operator import itemgetter
108 >>> for message in sorted(smtpd, key=itemgetter('sender')):
109 ... print message['sender']
110 staging-one-bounces+anne.person=example.com@lists.launchpad.dev
111 staging-one-bounces+anne.person=example.com@lists.launchpad.dev
112 staging-one-bounces+archive=mail-archive.dev@lists.launchpad.dev
113 staging-one-bounces+archive=mail-archive.dev@lists.launchpad.dev
114 staging-one-bounces+bart.person=example.com@lists.launchpad.dev
115 staging-one-bounces+bart.person=example.com@lists.launchpad.dev
116 staging-two-bounces+anne.person=example.com@lists.launchpad.dev
117 staging-two-bounces+anne.person=example.com@lists.launchpad.dev
118 staging-two-bounces+archive=mail-archive.dev@lists.launchpad.dev
119 staging-two-bounces+archive=mail-archive.dev@lists.launchpad.dev
120 staging-two-bounces+cris.person=example.com@lists.launchpad.dev
121 staging-two-bounces+cris.person=example.com@lists.launchpad.dev
12243
123Give one of the mailing lists a contact address, to further simulate what the44Give one of the mailing lists a contact address, to further simulate what the
124sync script has to deal with.45sync script has to deal with.
12546
126 >>> from canonical.launchpad.ftests import login, logout47 >>> from canonical.launchpad.interfaces.emailaddress import (
127 >>> from canonical.launchpad.interfaces import (48 ... EmailAddressStatus, IEmailAddressSet)
128 ... EmailAddressStatus, IEmailAddressSet, IPersonSet)
129 >>> from zope.component import getUtility49 >>> from zope.component import getUtility
130 >>> login('admin@canonical.com')
131 >>> team = getUtility(IPersonSet).getByName('staging-two')
132 >>> email = getUtility(IEmailAddressSet).new(50 >>> email = getUtility(IEmailAddressSet).new(
133 ... 'contact@example.com', team, EmailAddressStatus.VALIDATED)51 ... 'contact@example.com', team_two, EmailAddressStatus.VALIDATED)
134 >>> team.setContactAddress(email)52 >>> team_two.setContactAddress(email)
135 >>> logout()53 >>> logout()
136 >>> transaction.commit()54 >>> transaction.commit()
13755
56XXX sinzui 2010-10-07 bug=656417: The archive/mbox rsync rules are not tested
57and we need a way to build test versions.
58
138Now we need to simulate the difference between the staging and production59Now we need to simulate the difference between the staging and production
139databases. To do this, we'll use a helper, but first we need to stop Mailman.60databases. To do this, we'll use a helper, but first we need to stop Mailman.
14061
14162
=== modified file 'scripts/mlist-sync.py'
--- scripts/mlist-sync.py 2010-10-03 15:30:06 +0000
+++ scripts/mlist-sync.py 2010-10-07 18:59:42 +0000
@@ -40,7 +40,7 @@
40from canonical.launchpad.interfaces import (40from canonical.launchpad.interfaces import (
41 IEmailAddressSet, IMailingListSet, IPersonSet)41 IEmailAddressSet, IMailingListSet, IPersonSet)
42from lp.services.mailman.config import configure_prefix42from lp.services.mailman.config import configure_prefix
43from lp.services.scripts.base import LaunchpadCronScript43from lp.services.scripts.base import LaunchpadScript
4444
4545
46RSYNC_OPTIONS = ('-avz', '--delete')46RSYNC_OPTIONS = ('-avz', '--delete')
@@ -49,7 +49,7 @@
49SPACE = ' '49SPACE = ' '
5050
5151
52class MailingListSyncScript(LaunchpadCronScript):52class MailingListSyncScript(LaunchpadScript):
53 """53 """
54 %prog [options] source_url54 %prog [options] source_url
5555
@@ -205,7 +205,7 @@
205 # Keep going.205 # Keep going.
206206
207 def main(self):207 def main(self):
208 """See `LaunchpadCronScript`."""208 """See `LaunchpadScript`."""
209 source_url = None209 source_url = None
210 if len(self.args) == 0:210 if len(self.args) == 0:
211 self.parser.error('Missing source_url')211 self.parser.error('Missing source_url')