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

[Curtis Hovey]

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.

[Edwin Grubbs (edwin-grubbs)]

This looks good.

review: Approve (code)

=== 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 @@
 informing them of their new list.  We can ignore these.
 
     >>> from canonical.launchpad.ftests import login, logout
+    >>> from Mailman import MailList
+
+    >>> def makeMailmanList(lp_mailing_list):
+    ...     mlist = MailList.MailList()
+    ...     team = lp_mailing_list.team
+    ...     owner_email = team.teamowner.preferredemail.email
+    ...     mlist.Create(team.name, owner_email, 'password')
+    ...     mlist.host_name = 'lists.prod.launchpad.dev'
+    ...     mlist.web_page_url = 'http://lists.prod.launchpad.dev/mailman/'
+    ...     mlist.Save()
+    ...     mlist.addNewMember(owner_email)
+    ...     mlist.Unlock()
+    ...     return mlist
 
     >>> login('[e-mail]')
-
-    >>> from zope.security.proxy import removeSecurityProxy
-    >>> owner = removeSecurityProxy(factory.makePerson())
-    >>> team_one = factory.makeTeam(owner=owner, name='staging-one')
-    >>> team_two = factory.makeTeam(owner=owner, name='staging-two')
-    >>> transaction.commit()
-    >>> logout()
-
-    >>> from canonical.launchpad.testing.pages import setupBrowser
-    >>> browser = setupBrowser('Basic %s:%s' %
-    ...     (owner.preferredemail.email, owner._password_cleartext_cached))
-
-    >>> from lp.services.mailman.testing import helpers
-    >>> helpers.apply_for_list(browser, 'staging-one')
-    >>> list_one = helpers.review_list('staging-one')
-
-    >>> helpers.apply_for_list(browser, 'staging-two')
-    >>> list_two = helpers.review_list('staging-two')
-
-Anne subscribes to two mailing lists.  Bart and Cris both subscribe to only
-one of the mailing lists.
-
-    >>> helpers.subscribe('Anne', 'staging-one')
-    >>> helpers.subscribe('Anne', 'staging-two')
-    >>> helpers.subscribe('Bart', 'staging-one')
-    >>> helpers.subscribe('Cris', 'staging-two')
-
-A bunch of messages are sent to the mailing lists.
-
-    >>> smtpd.reset()
-    >>> from Mailman.Post import inject
-    >>> inject('staging-one', """\
-    ... From: [e-mail]
-    ... To: [e-mail]
-    ... Subject: hello
-    ... Message-ID: <first-injection>
-    ...
-    ... Hello team!
-    ... """)
-
-    # Wait for all recipients to receive the message.
-    >>> for index in range(3):
-    ...     smtpd_watcher.wait_for_mbox_delivery('first-injection')
-
-    >>> inject('staging-two', """\
-    ... From: [e-mail]
-    ... To: [e-mail]
-    ... Subject: hello
-    ... Message-ID: <second-injection>
-    ...
-    ... Hello team!
-    ... """)
-
-    # Wait for all recipients to receive the message.
-    >>> for index in range(3):
-    ...     smtpd_watcher.wait_for_mbox_delivery('second-injection')
-
-    >>> inject('staging-one', """\
-    ... From: [e-mail]
-    ... To: [e-mail]
-    ... Subject: hello
-    ... Message-ID: <third-injection>
-    ...
-    ... Hello team!
-    ... """)
-
-    # Wait for all recipients to receive the message.
-    >>> for index in range(3):
-    ...     smtpd_watcher.wait_for_mbox_delivery('third-injection')
-
-    >>> inject('staging-two', """\
-    ... From: [e-mail]
-    ... To: [e-mail]
-    ... Subject: hello
-    ... Message-ID: <fourth-injection>
-    ...
-    ... Hello team!
-    ... """)
-
-    # Wait for all recipients to receive the message.
-    >>> for index in range(3):
-    ...     smtpd_watcher.wait_for_mbox_delivery('fourth-injection')
-
-    >>> from operator import itemgetter
-    >>> for message in sorted(smtpd, key=itemgetter('sender')):
-    ...     print message['sender']
-    staging-one-bounces+anne.person=[e-mail]
-    staging-one-bounces+anne.person=[e-mail]
-    staging-one-bounces+archive=[e-mail]
-    staging-one-bounces+archive=[e-mail]
-    staging-one-bounces+bart.person=[e-mail]
-    staging-one-bounces+bart.person=[e-mail]
-    staging-two-bounces+anne.person=[e-mail]
-    staging-two-bounces+anne.person=[e-mail]
-    staging-two-bounces+archive=[e-mail]
-    staging-two-bounces+archive=[e-mail]
-    staging-two-bounces+cris.person=[e-mail]
-    staging-two-bounces+cris.person=[e-mail]
+    >>> team_one, list_one = factory.makeTeamAndMailingList(
+    ...     'staging-one', 'owner-one')
+    >>> team_two, list_two = factory.makeTeamAndMailingList(
+    ...     'staging-two', 'owner-two')
+    >>> mlist_one = makeMailmanList(list_one)
+    >>> mlist_two = makeMailmanList(list_two)
 
 Give one of the mailing lists a contact address, to further simulate what the
 sync script has to deal with.
 
-    >>> from canonical.launchpad.ftests import login, logout
-    >>> from canonical.launchpad.interfaces import (
-    ...     EmailAddressStatus, IEmailAddressSet, IPersonSet)
+    >>> from canonical.launchpad.interfaces.emailaddress import (
+    ...     EmailAddressStatus, IEmailAddressSet)
     >>> from zope.component import getUtility
-    >>> login('[e-mail]')
-    >>> team = getUtility(IPersonSet).getByName('staging-two')
     >>> email = getUtility(IEmailAddressSet).new(
-    ...     '[e-mail]', team, EmailAddressStatus.VALIDATED)
-    >>> team.setContactAddress(email)
+    ...     '[e-mail]', team_two, EmailAddressStatus.VALIDATED)
+    >>> team_two.setContactAddress(email)
     >>> logout()
     >>> transaction.commit()
 
+XXX sinzui 2010-10-07 bug=656417: The archive/mbox rsync rules are not tested
+and we need a way to build test versions.
+
 Now we need to simulate the difference between the staging and production
 databases.  To do this, we'll use a helper, but first we need to stop Mailman.
 

=== 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 @@
 from canonical.launchpad.interfaces import (
     IEmailAddressSet, IMailingListSet, IPersonSet)
 from lp.services.mailman.config import configure_prefix
-from lp.services.scripts.base import LaunchpadCronScript
+from lp.services.scripts.base import LaunchpadScript
 
 
 RSYNC_OPTIONS = ('-avz', '--delete')
@@ -49,7 +49,7 @@
 SPACE = ' '
 
 
-class MailingListSyncScript(LaunchpadCronScript):
+class MailingListSyncScript(LaunchpadScript):
     """
     %prog [options] source_url
 
@@ -205,7 +205,7 @@
             # Keep going.
 
     def main(self):
-        """See `LaunchpadCronScript`."""
+        """See `LaunchpadScript`."""
         source_url = None
         if len(self.args) == 0:
             self.parser.error('Missing source_url')