launchpad-reviewers team mailing list archive

[Jonathan Lange <jml@xxxxxxxxx>]

Jonathan Lange has proposed merging lp:~jml/launchpad/various-ec2-fixes into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #540016 in Launchpad itself: "ec2 needs to handle EINTR"
  https://bugs.launchpad.net/launchpad/+bug/540016
  Bug #638468 in Launchpad itself: "ec2 land: do not include already fix-committed or fix-released bugs in commit message"
  https://bugs.launchpad.net/launchpad/+bug/638468
  Bug #732029 in Launchpad itself: "ec2 land complains about multiple merge proposals when it need not"
  https://bugs.launchpad.net/launchpad/+bug/732029

For more details, see:
https://code.launchpad.net/~jml/launchpad/various-ec2-fixes/+merge/55763

A whole bunch of fixes for ec2 test.

Don't review this yet. Am testing.
-- 
https://code.launchpad.net/~jml/launchpad/various-ec2-fixes/+merge/55763
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jml/launchpad/various-ec2-fixes into lp:launchpad.

=== modified file 'lib/devscripts/autoland.py'
--- lib/devscripts/autoland.py	2011-02-01 00:43:23 +0000
+++ lib/devscripts/autoland.py	2011-03-31 15:04:21 +0000
@@ -67,16 +67,19 @@
         """Get the merge proposal from the branch."""
 
         lp_branch = self.get_lp_branch(branch)
-        proposals = lp_branch.landing_targets
+        proposals = [
+            mp for mp in lp_branch.landing_targets
+            if mp.queue_status in ('Needs review', 'Approved')]
         if len(proposals) == 0:
             raise BzrCommandError(
-                "The public branch has no source merge proposals.  "
+                "The public branch has no open source merge proposals.  "
                 "You must have a merge proposal before attempting to "
                 "land the branch.")
         elif len(proposals) > 1:
             raise BzrCommandError(
-                "The public branch has multiple source merge proposals.  "
-                "You must provide the URL to the one you wish to use.")
+                "The public branch has multiple open source merge "
+                "proposals.  You must provide the URL to the one you wish "
+                "to use.")
         return MergeProposal(proposals[0])
 
 
@@ -249,7 +252,16 @@
     """
     if not bugs:
         return ''
-    return '[bug=%s]' % ','.join(str(bug.id) for bug in bugs)
+    bug_ids = []
+    for bug in bugs:
+        for task in bug.bug_tasks:
+            if (task.bug_target_name == 'launchpad'
+                and task.status not in ['Fix Committed', 'Fix Released']):
+                bug_ids.append(str(bug.id))
+                break
+    if not bug_ids:
+        return ''
+    return '[bug=%s]' % ','.join(bug_ids)
 
 
 def get_reviewer_handle(reviewer):

=== modified file 'lib/devscripts/ec2test/instance.py'
--- lib/devscripts/ec2test/instance.py	2010-11-23 17:26:02 +0000
+++ lib/devscripts/ec2test/instance.py	2011-03-31 15:04:21 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Code to represent a single machine instance in EC2."""
@@ -9,6 +9,7 @@
     ]
 
 import code
+import errno
 import glob
 import os
 import select
@@ -622,7 +623,11 @@
         session.exec_command(cmd)
         session.shutdown_write()
         while 1:
-            select.select([session], [], [], 0.5)
+            try:
+                select.select([session], [], [], 0.5)
+            except (IOError, select.error), e:
+                if e.errno == errno.EINTR:
+                    continue
             if session.recv_ready():
                 data = session.recv(4096)
                 if data:

=== modified file 'lib/devscripts/ec2test/tests/test_remote.py'
--- lib/devscripts/ec2test/tests/test_remote.py	2011-02-28 00:54:12 +0000
+++ lib/devscripts/ec2test/tests/test_remote.py	2011-03-31 15:04:21 +0000
@@ -540,7 +540,9 @@
         [body, attachment] = email.get_payload()
         self.assertIsInstance(body, MIMEText)
         self.assertEqual('inline', body['Content-Disposition'])
-        self.assertEqual('text/plain; charset="utf-8"', body['Content-Type'])
+        self.assertIn(
+            body['Content-Type'],
+            ['text/plain; charset="utf-8"', 'text/plain; charset="utf8"'])
         self.assertEqual("foo", body.get_payload(decode=True))
 
     def test_report_email_attachment(self):
@@ -820,7 +822,9 @@
         [body, attachment] = email.get_payload()
         self.assertIsInstance(body, MIMEText)
         self.assertEqual('inline', body['Content-Disposition'])
-        self.assertEqual('text/plain; charset="utf-8"', body['Content-Type'])
+        self.assertIn(
+            body['Content-Type'],
+            ['text/plain; charset="utf-8"', 'text/plain; charset="utf8"'])
         self.assertEqual(
             logger.get_summary_contents(), body.get_payload(decode=True))
         self.assertIsInstance(attachment, MIMEApplication)

=== modified file 'lib/devscripts/tests/test_autoland.py'
--- lib/devscripts/tests/test_autoland.py	2011-02-01 00:43:23 +0000
+++ lib/devscripts/tests/test_autoland.py	2011-03-31 15:04:21 +0000
@@ -19,14 +19,24 @@
     MergeProposal)
 
 
+class FakeBugTask:
+
+    def __init__(self, target_name, status):
+        self.bug_target_name = target_name
+        self.status = status
+
+
 class FakeBug:
     """Fake launchpadlib Bug object.
 
     Only used for the purposes of testing.
     """
 
-    def __init__(self, id):
+    def __init__(self, id, bug_tasks=None):
         self.id = id
+        if bug_tasks is None:
+            bug_tasks = [FakeBugTask('launchpad', 'Triaged')]
+        self.bug_tasks = bug_tasks
 
 
 class FakePerson:
@@ -113,7 +123,7 @@
         self._test_commit_message_match(incr=True, no_qa=False, testfix=False)
 
 
-class TestBugsClaused(unittest.TestCase):
+class TestBugsClause(unittest.TestCase):
     """Tests for `get_bugs_clause`."""
 
     def test_no_bugs(self):
@@ -134,6 +144,36 @@
         bugs_clause = get_bugs_clause([bug1, bug2])
         self.assertEqual('[bug=20,45]', bugs_clause)
 
+    def test_fixed_bugs_are_excluded(self):
+        # If a bug is fixed then it is excluded from the bugs clause.
+        bug1 = FakeBug(20)
+        bug2 = FakeBug(45, bug_tasks=[
+            FakeBugTask('fake-project', 'Fix Released')])
+        bug3 = FakeBug(67, bug_tasks=[
+            FakeBugTask('fake-project', 'Fix Committed')])
+        bugs_clause = get_bugs_clause([bug1, bug2, bug3])
+        self.assertEqual('[bug=20]', bugs_clause)
+
+    def test_bugs_open_on_launchpad_are_included(self):
+        # If a bug has been fixed on one target but not in launchpad, then it
+        # is included in the bugs clause, because it's relevant to launchpad
+        # QA.
+        bug = FakeBug(20, bug_tasks=[
+            FakeBugTask('fake-project', 'Fix Released'),
+            FakeBugTask('launchpad', 'Triaged')])
+        bugs_clause = get_bugs_clause([bug])
+        self.assertEqual('[bug=20]', bugs_clause)
+
+    def test_bugs_fixed_on_launchpad_but_open_in_others_are_excluded(self):
+        # If a bug has been fixed in Launchpad but not fixed on a different
+        # target, then it is excluded from the bugs clause, since we don't
+        # want to QA it.
+        bug = FakeBug(20, bug_tasks=[
+            FakeBugTask('fake-project', 'Triaged'),
+            FakeBugTask('launchpad', 'Fix Released')])
+        bugs_clause = get_bugs_clause([bug])
+        self.assertEqual('', bugs_clause)
+
 
 class TestGetCommitMessage(unittest.TestCase):
 
@@ -229,7 +269,7 @@
 
         self.assertEqual(
             "[r=foo][bug=20][no-qa][incr] Foobaring the sbrubble.",
-            self.mp.build_commit_message("Foobaring the sbrubble.", 
+            self.mp.build_commit_message("Foobaring the sbrubble.",
                 testfix, no_qa, incr))
 
     def test_commit_with_rollback(self):
@@ -334,10 +374,7 @@
 
     def test_rollback_and_noqa_and_incr_given(self):
         bugs = None
-        no_qa = True
-        incr = True
-        self.assertEqual('[rollback=123]',
-            get_qa_clause(bugs, rollback=123))
+        self.assertEqual('[rollback=123]', get_qa_clause(bugs, rollback=123))
 
 
 class TestGetReviewerHandle(unittest.TestCase):