Merge lp:~abentley/bzr/find-proposal-branch into lp:bzr

[Aaron Bentley]

Add BRANCH option to the lp-find-proposal command.

This branch adds an optional BRANCH argument to the lp-find-proposal command.

lp-find-proposal must search the branch that any proposals were targetted against; LP does not provide a mechanism that uses solely the revision-id. This means that often, the current branch is not a suitable place to look up proposals, so it makes sense to permit an override.

At the same time, this branch introduces testing for launchpad plugin commands, starting with lp-find-proposal. Until now, these commands have gone untested, because they use Launchpad. This meant that they didn't have a way to set up test objects, and they incurred network traffic. This approach uses a fake, in-memory version of the Launchpadlib API. Only necessary functionality is implemented.

[Jelmer Vernooij (jelmer)]

Thanks, nice to have some extra testing.

There's an extra dot in """Specified branch is used.."""

[Jelmer Vernooij (jelmer)]

review: Approve (code)

[lp:~abentley/bzr/find-proposal-branch]

6528. By Aaron Bentley on 2012-06-20

Merged dev into find-proposal-branch.

6529. By Aaron Bentley on 2012-06-26

Update from review.

6530. By Aaron Bentley on 2012-06-26

Merged dev into find-proposal-branch.

[Martin Packman (gz)]

sent to pqm by email

[Aaron Bentley (abentley)]

This branch can't land because it requires launchpadlib on the build slave. But it doesn't need to land, because as of r6541 the branch is no longer considered when looking up merge proposals.

Martin Packman <email address hidden> wrote:

>The proposal to merge lp:~abentley/bzr/find-proposal-branch into lp:bzr has been updated.
>
> Status: Needs review => Approved
>
>For more details, see:
>https://code.launchpad.net/~abentley/bzr/find-proposal-branch/+merge/110615
>--
>https://code.launchpad.net/~abentley/bzr/find-proposal-branch/+merge/110615
>You are the owner of lp:~abentley/bzr/find-proposal-branch.
>

[Martin Packman (gz)]

Thanks Aaron, wondered why there was an approved branch from you sitting around unmerged.

Unmerged revisions

6530. By Aaron Bentley on 2012-06-26

Merged dev into find-proposal-branch.

6529. By Aaron Bentley on 2012-06-26

Update from review.

6528. By Aaron Bentley on 2012-06-20

Merged dev into find-proposal-branch.

6527. By Aaron Bentley on 2012-06-15

Cleanup.

6526. By Aaron Bentley on 2012-06-15

Support specifying branch for lp-find-proposal.

6525. By Aaron Bentley on 2012-06-15

Add tests of default behaviour.

=== modified file 'bzrlib/plugins/launchpad/__init__.py'
--- bzrlib/plugins/launchpad/__init__.py	2012-03-10 19:11:06 +0000
+++ bzrlib/plugins/launchpad/__init__.py	2012-06-26 15:04:27 +0000
@@ -141,6 +141,7 @@
 def load_tests(basic_tests, module, loader):
     testmod_names = [
         'test_account',
+        'test_cmds',
         'test_register',
         'test_lp_api',
         'test_lp_api_lite',

=== modified file 'bzrlib/plugins/launchpad/cmds.py'
--- bzrlib/plugins/launchpad/cmds.py	2012-03-12 14:35:26 +0000
+++ bzrlib/plugins/launchpad/cmds.py	2012-06-26 15:04:27 +0000
@@ -22,6 +22,7 @@
     branch as _mod_branch,
     controldir,
     trace,
+    ui,
     )
 from bzrlib.commands import (
     Command,
@@ -40,6 +41,11 @@
     )
 
 
+def launchpad_factory():
+    from bzrlib.plugins.launchpad import (lp_api, lp_registration)
+    return lp_api.login(lp_registration.LaunchpadService())
+
+
 class cmd_register_branch(Command):
     __doc__ = """Register a branch with launchpad.net.
 
@@ -345,15 +351,30 @@
     So, to find the merge proposal that reviewed line 1 of README::
 
       bzr lp-find-proposal -r annotate:README:1
+
+    If BRANCH is specified, it indicates the branch to use when looking up the
+    proposal.  :submit may be a useful value, especially for aliases.
     """
 
     takes_options = ['revision']
 
-    def run(self, revision=None):
-        from bzrlib import ui
+    takes_args = ['branch?']
+
+    def __init__(self, launchpad_factory=launchpad_factory,
+                 open_web_page=None):
+        super(cmd_lp_find_proposal, self).__init__()
+        self.launchpad_factory = launchpad_factory
+        if open_web_page is None:
+            import webbrowser
+            open_web_page = webbrowser.open
+        self.open_web_page = open_web_page
+
+    def run(self, branch=None, revision=None):
         from bzrlib.plugins.launchpad import lp_api
-        import webbrowser
-        b = _mod_branch.Branch.open_containing('.')[0]
+        if branch is not None:
+            b = _mod_branch.Branch.open(branch)
+        else:
+            b = _mod_branch.Branch.open_containing('.')[0]
         pb = ui.ui_factory.nested_progress_bar()
         b.lock_read()
         try:
@@ -363,7 +384,7 @@
                 raise BzrCommandError(gettext('No review found.'))
             trace.note(gettext('%d proposals(s) found.') % len(merged))
             for mp in merged:
-                webbrowser.open(lp_api.canonical_url(mp))
+                self.open_web_page(lp_api.canonical_url(mp))
         finally:
             b.unlock()
             pb.finished()
@@ -387,8 +408,8 @@
         return b.revision_id_to_revno(merging_revision)
 
     def _find_proposals(self, revno, b, pb):
-        from bzrlib.plugins.launchpad import (lp_api, lp_registration)
-        launchpad = lp_api.login(lp_registration.LaunchpadService())
+        from bzrlib.plugins.launchpad import lp_api
+        launchpad = self.launchpad_factory()
         pb.update(gettext('Finding Launchpad branch'))
         lpb = lp_api.LaunchpadBranch.from_bzr(launchpad, b,
                                               create_missing=False)
@@ -405,6 +426,3 @@
             return False
         else:
             return True
-
-
-

=== added file 'bzrlib/plugins/launchpad/test_cmds.py'
--- bzrlib/plugins/launchpad/test_cmds.py	1970-01-01 00:00:00 +0000
+++ bzrlib/plugins/launchpad/test_cmds.py	2012-06-26 15:04:27 +0000
@@ -0,0 +1,90 @@
+# Copyright (C) 2012 Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+
+from bzrlib.tests import TestCaseWithTransport
+from bzrlib.plugins.launchpad.cmds import cmd_lp_find_proposal
+
+
+class FakeBranchMergeProposal(object):
+
+    def __init__(self, status):
+        self.status = 'Merged'
+        self.self_link = 'http://api.example.org/1/2/3/4'
+
+
+class FakeBranch(object):
+
+    def __init__(self):
+        self._bmp = {}
+
+    def getMergeProposals(self, status, merged_revnos):
+        return [self._bmp[revno] for revno in merged_revnos if
+                self._bmp[revno].status in status]
+
+
+class FakeBranchCollection(object):
+
+    def __init__(self):
+        self._by_url = {}
+
+    def getByUrl(self, url):
+        return self._by_url[url]
+
+
+class FakeLaunchpad(object):
+
+    def __init__(self):
+        self._root_uri = None
+        self.branches = FakeBranchCollection()
+
+
+class TestLPFindProposal(TestCaseWithTransport):
+
+    def make_branch_and_lp(self, location, lp_location):
+        """Make a branch and FakeLaunchpad to work together."""
+        tree = self.make_branch_and_tree(location)
+        tree.commit('r1')
+        tree.branch.set_push_location(lp_location)
+
+        fake_launchpad = FakeLaunchpad()
+        fake_branch = FakeBranch()
+        fake_launchpad.branches._by_url[lp_location] = fake_branch
+        fake_bmp = FakeBranchMergeProposal('Merged')
+        fake_branch._bmp[1] = fake_bmp
+        return fake_launchpad
+
+    def get_lp_find_proposal(self, location):
+        """"Return a specially-configured cmd_lp_find_proposal.
+
+        web_urls is the list of urls that would be opened.
+        """
+        fake_launchpad = self.make_branch_and_lp(location, 'lp:asdf')
+        web_urls = []
+        cmd = cmd_lp_find_proposal(lambda: fake_launchpad, web_urls.append)
+        return cmd, web_urls
+
+    def test_run_default_branch(self):
+        """Current directory is used for branch if unspecified."""
+        cmd, web_urls = self.get_lp_find_proposal('.')
+        cmd.run_argv_aliases(['-r', '1'])
+        self.assertEqual(['http://code.example.org/2/3/4'], web_urls)
+
+    def test_run_specified_branch(self):
+        """Specified branch is used."""
+        cmd, web_urls = self.get_lp_find_proposal('specific-branch')
+        cmd.run_argv_aliases(['-r', '1', 'specific-branch'])
+        self.assertEqual(['http://code.example.org/2/3/4'], web_urls)