Merge lp:~dobey/tarmac/merge-specific into lp:tarmac

[dobey]

Add an option to merge a single specific proposal to a branch.

This adds the --proposal option to the tarmac merge command, so that a specific proposal can be merged. The argument is the web link to the MP to be merged. Passing this argument to the merge command will result in tarmac attempting to merge only that MP, and no others, not even branches for other targets which are set up in tarmac.conf.

[lp:~dobey/tarmac/merge-specific]

430. By dobey on 2014-02-24

Add a couple more tests.
Fix a typo.
Revert the changes to addProposal.
Clean up the branch created by addProposal.

431. By dobey on 2014-02-24

Always use the branch_url from the MP when --proposal is used.

432. By dobey on 2014-02-24

Revert one more line.

[Francis Ginther (fginther)]

This implements the requested feature. Tests and code changes look good.

review: Approve

=== modified file 'tarmac/bin/commands.py'
--- tarmac/bin/commands.py	2014-02-20 03:16:01 +0000
+++ tarmac/bin/commands.py	2014-02-24 23:41:56 +0000
@@ -174,6 +174,7 @@
         options.imply_commit_message_option,
         options.one_option,
         options.list_approved_option,
+        options.proposal_option,
     ]
 
     def _handle_merge_error(self, proposal, failure):
@@ -201,14 +202,17 @@
             proposal.setStatus(status=u'Needs review')
         proposal.lp_save()
 
-    def _do_merges(self, branch_url):
+    def _do_merges(self, branch_url, source_mp=None):
         """Merge the approved proposals for %branch_url."""
         lp_branch = self.launchpad.branches.getByUrl(url=branch_url)
         if lp_branch is None:
             self.logger.info('Not a valid branch: {0}'.format(branch_url))
             return
 
-        proposals = self._get_mergable_proposals_for_branch(lp_branch)
+        if source_mp is not None:
+            proposals = [source_mp]
+        else:
+            proposals = self._get_mergable_proposals_for_branch(lp_branch)
 
         if not proposals:
             self.logger.info(
@@ -409,6 +413,12 @@
 
         return reviews
 
+    def _get_proposal_from_mp_url(self, mp_url):
+        """Return a branch_merge_proposal object from its web URL."""
+        urlp = re.compile('http[s]?://code.launchpad\.net/')
+        api_url = urlp.sub('https://api.launchpad.net/1.0/', mp_url)
+        return self.launchpad.load(api_url)
+
     def run(self, branch_url=None, launchpad=None, **kwargs):
         for key, value in kwargs.iteritems():
             self.config.set('Tarmac', key, value)
@@ -429,12 +439,18 @@
             self.launchpad = self.get_launchpad_object()
             self.logger.debug('launchpad object loaded')
 
+        proposal = None
+        if self.config.proposal:
+            proposal = self._get_proposal_from_mp_url(self.config.proposal)
+            # Always override branch_url with the correct one.
+            branch_url = proposal.target_branch.bzr_identity
+
         if branch_url:
             self.logger.debug('%(branch_url)s specified as branch_url' % {
                 'branch_url': branch_url})
             if not branch_url.startswith('lp:'):
                 raise TarmacCommandError('Branch urls must start with lp:')
-            self._do_merges(branch_url)
+            self._do_merges(branch_url, source_mp=proposal)
 
         else:
             for branch in self.config.branches:

=== modified file 'tarmac/bin/options.py'
--- tarmac/bin/options.py	2014-02-17 20:12:51 +0000
+++ tarmac/bin/options.py	2014-02-24 23:41:56 +0000
@@ -36,3 +36,7 @@
 list_approved_option = Option(
     'list-approved', short_name='l',
     help='List Approved merge proposals for the target branch.')
+proposal_option = Option(
+    'proposal', short_name='p',
+    type=str, argname='url',
+    help='Pass the URL for a specific proposal to merge to the target branch.')

=== modified file 'tarmac/tests/test_commands.py'
--- tarmac/tests/test_commands.py	2014-02-20 03:16:01 +0000
+++ tarmac/tests/test_commands.py	2014-02-24 23:41:56 +0000
@@ -19,7 +19,7 @@
 import shutil
 import sys
 
-from mock import patch
+from mock import patch, MagicMock
 from tarmac.bin import commands
 from tarmac.bin.registry import CommandRegistry
 from tarmac.branch import Branch
@@ -157,7 +157,7 @@
                         reviewer=Thing(display_name=u'Reviewer'))]),
                           Thing(
                 self_link=u'https://api.launchpad.net/1.0/proposal1',
-                web_link=u'https://codelaunchpad.net/proposal1',
+                web_link=u'https://code.launchpad.net/proposal1',
                 queue_status=u'Approved',
                 commit_message=u'Commit this.',
                 source_branch=self.branches[0],
@@ -201,8 +201,8 @@
         branch3.lp_branch.unique_name = '~user/branch/' + name
         branch3.lp_branch.landing_candidates = []
         b3_proposal = Thing(
-            self_link=u'http://api.edge.launchpad.net/devel/proposal3',
-            web_link=u'http://edge.launchpad.net/proposal3',
+            self_link=u'https://api.launchpad.net/1.0/proposal3',
+            web_link=u'https://code.launchpad.net/proposal3',
             queue_status=u'Approved',
             commit_message=u'Commitable.',
             source_branch=branch3.lp_branch,
@@ -219,7 +219,7 @@
         branch3.lp_branch.landing_targets = [b3_proposal]
         self.proposals.append(b3_proposal)
         self.branches.append(branch3.lp_branch)
-
+        self.addCleanup(shutil.rmtree, branch3_dir)
 
     def lp_save(self, *args, **kwargs):
         """Do nothing here."""
@@ -491,3 +491,48 @@
         self.addProposal("one_prerequisite", self.branches[0])
         proposals = self.command._get_prerequisite_proposals(self.proposals[2])
         self.assertEqual(len(proposals), 2)
+
+    @patch('tarmac.bin.commands.Launchpad.load')
+    def test__get_proposal_from_mp_url(self, mocked):
+        """Test that the URL is substituted correctly."""
+        self.command.launchpad = MagicMock()
+        self.command.launchpad.load = mocked
+        self.command._get_proposal_from_mp_url(
+            'https://code.launchpad.net/~foo/bar/baz/+merge/10')
+        mocked.assert_called_once_with(
+            'https://api.launchpad.net/1.0/~foo/bar/baz/+merge/10')
+
+    @patch('tarmac.bin.commands.Launchpad.load')
+    def test__get_proposal_from_mp_url_with_api_url(self, mocked):
+        """Test that the URL is ignored correctly."""
+        self.command.launchpad = MagicMock()
+        self.command.launchpad.load = mocked
+        self.command._get_proposal_from_mp_url(
+            'https://api.launchpad.net/1.0/~foo/bar/baz/+merge/10')
+        mocked.assert_called_once_with(
+            'https://api.launchpad.net/1.0/~foo/bar/baz/+merge/10')
+
+    def test_run_merge_with_specific_proposal_without_branch_url(self):
+        """Test that a specific proposal is merged, with the others ignored."""
+        self.proposals[1].reviewed_revid = \
+            self.branch2.bzr_branch.last_revision()
+        self.addProposal('specific_merge_without_branch_url')
+        self.launchpad.load = MagicMock(return_value=self.proposals[1])
+        self.command._get_reviews = MagicMock()
+        self.config.proposal = self.proposals[1].web_link
+        self.command.run(launchpad=self.launchpad)
+        self.launchpad.load.assert_called_once_with(self.proposals[1].self_link)
+        self.command._get_reviews.assert_called_once_with(self.proposals[1])
+
+    def test_run_merge_with_specific_proposal_with_branch_url(self):
+        """Test that a specific proposal is merged, with the others ignored."""
+        self.proposals[1].reviewed_revid = \
+            self.branch2.bzr_branch.last_revision()
+        self.addProposal('specific_merge_with_branch_url')
+        self.launchpad.load = MagicMock(return_value=self.proposals[1])
+        self.command._get_reviews = MagicMock()
+        self.config.proposal = self.proposals[1].web_link
+        self.command.run(launchpad=self.launchpad,
+                         branch_url=self.branches[1].bzr_identity)
+        self.launchpad.load.assert_called_once_with(self.proposals[1].self_link)
+        self.command._get_reviews.assert_called_once_with(self.proposals[1])