Merge lp:~matsubara/bzr-pqm/NewMergeWorkflow into lp:bzr-pqm

[Diogo Matsubara]

Implements additional options to the lpland command: removes the restriction to use the --no-qa and --incremental option together and implements the --rollback option.

This branch implements additional options to the lpland command. More specifically it implements use cases 3 and 6 of https://dev.launchpad.net/QAProcessContinuousRollouts. It removes the restriction to use the --no-qa and --incremental option together and implements the --rollback option.

[Ursula Junque (ursinha)]

Hi Diogo, the logic here seems correct. I'm approving it from the merge workflow perspective.

Cheers!

[Ursula Junque (ursinha)]

review: Approve (qa-logic)

=== modified file '__init__.py'
--- __init__.py	2010-08-06 16:52:30 +0000
+++ __init__.py	2010-09-09 12:34:39 +0000
@@ -137,28 +137,30 @@
         Option(
             'incremental',
             help="Incremental to other bug fix (tags commit with [incr])."),
+        Option(
+            'rollback', type=int,
+            help=(
+                "Rollback given revision number. (tags commit with "
+                "[rollback=revno]).")),
         ]
 
-    def run(self, location=None, dry_run=False, testfix=False, 
-            no_qa=False, incremental=False):
+    def run(self, location=None, dry_run=False, testfix=False,
+            no_qa=False, incremental=False, rollback=None):
         from bzrlib.plugins.pqm.lpland import Submitter
         from bzrlib import branch as _mod_branch
         from bzrlib.plugins.pqm.lpland import (
             MissingReviewError, MissingBugsError, MissingBugsIncrementalError)
 
-
-        if no_qa and incremental:
-            raise BzrCommandError(
-                "--no-qa and --incremental cannot be given at the same time.")
-
         branch = _mod_branch.Branch.open_containing('.')[0]
         if dry_run:
             outf = self.outf
         else:
             outf = None
+        if rollback and (no_qa or incremental):
+            print "--rollback option used. Ignoring --no-qa and --incremental."
         try:
-            submitter = Submitter(branch, location, testfix, no_qa, incremental
-                ).run(outf)
+            submitter = Submitter(branch, location, testfix, no_qa,
+                incremental, rollback=rollback).run(outf)
         except MissingReviewError:
             raise BzrCommandError(
                 "Cannot land branches that haven't got approved code "

=== modified file 'lpland.py'
--- lpland.py	2010-08-13 11:46:04 +0000
+++ lpland.py	2010-09-09 12:34:39 +0000
@@ -180,7 +180,7 @@
         return branch.composePublicURL(scheme="bzr+ssh")
 
     def get_commit_message(self, commit_text, testfix=False, no_qa=False,
-                           incremental=False):
+                           incremental=False, rollback=None):
         """Get the Launchpad-style commit message for a merge proposal."""
         reviews = self.get_reviews()
         bugs = self.get_bugs()
@@ -190,7 +190,7 @@
             get_reviewer_clause(reviews),
             get_bugs_clause(bugs),
             get_qa_clause(bugs, no_qa,
-                incremental),
+                incremental, rollback=rollback),
             ])
 
         return '%s %s' % (tags, commit_text)
@@ -199,11 +199,12 @@
 class Submitter(object):
 
     def __init__(self, branch, location, testfix=False, no_qa=False,
-                 incremental=False):
+                 incremental=False, rollback=None):
         self.branch = branch
         self.testfix = testfix
         self.no_qa = no_qa
         self.incremental = incremental
+        self.rollback = rollback
         self.config = self.branch.get_config()
         self.mail_to = self.config.get_user_option('pqm_email')
         if not self.mail_to:
@@ -225,11 +226,12 @@
         submission.check_public_branch()
 
     @staticmethod
-    def set_message(submission, mp, testfix, no_qa, incremental):
+    def set_message(submission, mp, testfix, no_qa, incremental,
+            rollback=None):
         pqm_command = ''.join(submission.to_lines())
         commit_message = mp.commit_message or ''
         start_message = mp.get_commit_message(commit_message, testfix, no_qa,
-            incremental)
+            incremental, rollback=rollback)
         message = msgeditor.edit_commit_message(
             'pqm command:\n%s' % pqm_command,
             start_message=start_message).rstrip('\n')
@@ -243,7 +245,7 @@
         submission = self.submission(mp)
         self.check_submission(submission)
         self.set_message(submission, mp, self.testfix, self.no_qa,
-            self.incremental)
+            self.incremental, rollback=self.rollback)
         email = submission.to_email(self.mail_from, self.mail_to)
         if outf is not None:
             outf.write(email.as_string())
@@ -281,24 +283,31 @@
     return testfix_clause
 
 
-def get_qa_clause(bugs, no_qa=False, incremental=False):
+def get_qa_clause(bugs, no_qa=False, incremental=False, rollback=None):
     """Check the no-qa and incremental options, getting the qa clause.
 
-    The qa clause will always be or no-qa, or incremental or no tags, never
-    both at the same time.
+    The qa clause will always be or no-qa, or incremental, or no-qa and
+    incremental, or a revno for the rollback clause, or no tags.
+
+    See https://dev.launchpad.net/QAProcessContinuousRollouts for detailed
+    explanation of each clause.
     """
     qa_clause = ""
 
-    if not bugs and not no_qa and not incremental:
+    if not bugs and not no_qa and not incremental and not rollback:
         raise MissingBugsError
 
     if incremental and not bugs:
         raise MissingBugsIncrementalError
 
-    if incremental:
+    if no_qa and incremental:
+        qa_clause = '[no-qa][incr]'
+    elif incremental:
         qa_clause = '[incr]'
     elif no_qa:
         qa_clause = '[no-qa]'
+    elif rollback:
+        qa_clause = '[rollback=%d]' % rollback
     else:
         qa_clause = ''
 

=== modified file 'tests/test_lpland.py'
--- tests/test_lpland.py	2010-08-13 11:46:04 +0000
+++ tests/test_lpland.py	2010-09-09 12:34:39 +0000
@@ -141,6 +141,25 @@
         self.assertRaises(MissingBugsIncrementalError,
             get_qa_clause, bugs, no_qa, incr)
 
+    def test_bugs_incr_and_noqa_option_given(self):
+        bug1 = FakeBug(20)
+        no_qa = True
+        incr = True
+        self.assertEqual('[no-qa][incr]',
+            get_qa_clause([bug1], no_qa, incr))
+
+    def test_rollback_given(self):
+        bugs = None
+        self.assertEqual('[rollback=123]',
+            get_qa_clause(bugs, rollback=123))
+
+    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))
+
 
 class TestGetReviewerHandle(unittest.TestCase):
     """Tests for `get_reviewer_handle`."""
@@ -252,6 +271,28 @@
             self.mp.get_commit_message("Foobaring the sbrubble.",
                 testfix, no_qa, incr))
 
+    def test_commit_with_noqa_and_incr(self):
+        incr = True
+        no_qa = True
+        testfix = False
+
+        self.mp.get_bugs = FakeMethod([self.fake_bug])
+        self.mp.get_reviews = FakeMethod({None : [self.fake_person]})
+
+        self.assertEqual(
+            "[r=foo][ui=none][bug=20][no-qa][incr] Foobaring the sbrubble.",
+            self.mp.get_commit_message("Foobaring the sbrubble.", 
+                testfix, no_qa, incr))
+
+    def test_commit_with_rollback(self):
+        self.mp.get_bugs = FakeMethod([self.fake_bug])
+        self.mp.get_reviews = FakeMethod({None : [self.fake_person]})
+
+        self.assertEqual(
+            "[r=foo][ui=none][bug=20][rollback=123] Foobaring the sbrubble.",
+            self.mp.get_commit_message("Foobaring the sbrubble.", 
+                rollback=123))
+
 
 class TestGetReviewerClause(unittest.TestCase):
     """Tests for `get_reviewer_clause`."""