Status: | Merged |
---|---|
Merged at revision: | not available |
Proposed branch: | lp:~abentley/bzr-pqm/lpland |
Merge into: | lp:bzr-pqm |
Diff against target: |
602 lines (+575/-1) 3 files modified
__init__.py (+31/-1) lpland.py (+347/-0) test_lpland.py (+197/-0) |
To merge this branch: | bzr merge lp:~abentley/bzr-pqm/lpland |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Robert Collins | Approve | ||
Review via email: mp+17718@code.launchpad.net |
Commit message
Description of the change
Aaron Bentley (abentley) wrote : | # |
Robert Collins (lifeless) wrote : | # |
This may seem nitpicky:
+ """
+ Land the merge proposal for this branch via PQM.
...
That adds a leading line to the docstring and thus to the help, I think. So please do
"""Land ....
instead.
What happens if there are multiple outstanding proposals - does it error or take an arbitrary one?
The
+# XXX: JonathanLange 2009-09-24: Bo
todo looks like its probably trivially available? If not thats fine, but if we can action it lets do that now.
602 +
603 +
604 +def test_suite():
605 + return unittest.
at the bottom of the test script is not idiomatic for bzr.
Other than that,I think 'it works for you, land it please'.
Cheers,
Rob
Aaron Bentley (abentley) wrote : | # |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Robert Collins wrote:
> Review: Approve
> This may seem nitpicky:
> + """
> + Land the merge proposal for this branch via PQM.
> ...
>
> That adds a leading line to the docstring and thus to the help, I think. So please do
> """Land ....
> instead.
Done.
> What happens if there are multiple outstanding proposals - does it error or take an arbitrary one?
It errors, but in that case you can specify the specific proposal you
want to land. I've updated the docstring.
> The
> +# XXX: JonathanLange 2009-09-24: Bo
>
> todo looks like its probably trivially available?
Not on my laptop with launchpadlib 1.5.1.
> If not thats fine, but if we can action it lets do that now.
>
> 602 +
> 603 +
> 604 +def test_suite():
> 605 + return unittest.
>
> at the bottom of the test script is not idiomatic for bzr.
Removed.
Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://
iEYEARECAAYFAkt
p68Anip1gWcOcPA
=PJhQ
-----END PGP SIGNATURE-----
- 64. By Aaron Bentley
-
Add lp-submit command.
Preview Diff
1 | === modified file '__init__.py' |
2 | --- __init__.py 2009-10-14 06:47:52 +0000 |
3 | +++ __init__.py 2010-01-21 23:09:12 +0000 |
4 | @@ -112,13 +112,43 @@ |
5 | submit_location=submit_branch, |
6 | tree=tree, ignore_local=ignore_local) |
7 | |
8 | +class cmd_lp_land(Command): |
9 | + """Land the merge proposal for this branch via PQM. |
10 | + |
11 | + The branch will be submitted to PQM according to the merge proposal. If |
12 | + there is more than one one outstanding proposal for the branch, its |
13 | + location must be specified. |
14 | + """ |
15 | + |
16 | + takes_args = ['location?'] |
17 | + |
18 | + takes_options = [Option( |
19 | + 'dry-run', help='Display the PQM message instead of sending.')] |
20 | + |
21 | + def run(self, location=None, dry_run=False): |
22 | + from bzrlib.plugins.pqm.lpland import Submitter |
23 | + from bzrlib import branch as _mod_branch |
24 | + branch = _mod_branch.Branch.open_containing('.')[0] |
25 | + if dry_run: |
26 | + outf = self.outf |
27 | + else: |
28 | + outf = None |
29 | + submitter = Submitter(branch, location).run(outf) |
30 | + |
31 | |
32 | register_command(cmd_pqm_submit) |
33 | +register_command(cmd_lp_land) |
34 | |
35 | |
36 | def test_suite(): |
37 | from bzrlib.tests import TestLoader |
38 | + from unittest import TestSuite |
39 | + |
40 | + import test_lpland |
41 | import test_pqm_submit |
42 | |
43 | loader = TestLoader() |
44 | - return loader.loadTestsFromModule(test_pqm_submit) |
45 | + return TestSuite([ |
46 | + loader.loadTestsFromModule(test_pqm_submit), |
47 | + loader.loadTestsFromModule(test_lpland), |
48 | + ]) |
49 | |
50 | === added file 'lpland.py' |
51 | --- lpland.py 1970-01-01 00:00:00 +0000 |
52 | +++ lpland.py 2010-01-21 23:09:12 +0000 |
53 | @@ -0,0 +1,347 @@ |
54 | +# Copyright (C) 2010 by Canonical Ltd |
55 | +# |
56 | +# This program is free software; you can redistribute it and/or modify |
57 | +# it under the terms of the GNU General Public License as published by |
58 | +# the Free Software Foundation; either version 2 of the License, or |
59 | +# (at your option) any later version. |
60 | +# |
61 | +# This program is distributed in the hope that it will be useful, |
62 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of |
63 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
64 | +# GNU General Public License for more details. |
65 | +# |
66 | +# You should have received a copy of the GNU General Public License along |
67 | +# with this program; if not, write to the Free Software Foundation, Inc., |
68 | +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. |
69 | + |
70 | +"""Tools for landing branches with Launchpad.""" |
71 | + |
72 | +import os |
73 | + |
74 | +from launchpadlib.launchpad import ( |
75 | + Launchpad, EDGE_SERVICE_ROOT, STAGING_SERVICE_ROOT) |
76 | +from lazr.uri import URI |
77 | +from bzrlib import msgeditor |
78 | +from bzrlib.errors import BzrCommandError |
79 | +from bzrlib.plugins.pqm import pqm_submit |
80 | +from bzrlib import smtp_connection |
81 | + |
82 | + |
83 | +# XXX: JonathanLange 2009-09-24: Both of these are available in more recent |
84 | +# versions of launchpadlib. When we start using such versions, we should |
85 | +# instead import these from launchpadlib. |
86 | +DEV_SERVICE_ROOT = 'https://api.launchpad.dev/beta/' |
87 | +LPNET_SERVICE_ROOT = 'https://api.launchpad.net/beta/' |
88 | + |
89 | + |
90 | +class MissingReviewError(Exception): |
91 | + """Raised when we try to get a review message without enough reviewers.""" |
92 | + |
93 | + |
94 | +class LaunchpadBranchLander: |
95 | + |
96 | + name = 'launchpad-branch-lander' |
97 | + cache_dir = '~/.launchpadlib/cache' |
98 | + |
99 | + def __init__(self, launchpad): |
100 | + self._launchpad = launchpad |
101 | + |
102 | + @classmethod |
103 | + def load(cls, service_root=EDGE_SERVICE_ROOT): |
104 | + # XXX: JonathanLange 2009-09-24: No unit tests. |
105 | + cache_dir = os.path.expanduser(cls.cache_dir) |
106 | + # XXX: JonathanLange 2009-09-24 bug=435813: If cached data invalid, |
107 | + # there's no easy way to delete it and try again. |
108 | + launchpad = Launchpad.login_with(cls.name, service_root, cache_dir) |
109 | + return cls(launchpad) |
110 | + |
111 | + def load_merge_proposal(self, mp_url): |
112 | + """Get the merge proposal object for the 'mp_url'.""" |
113 | + # XXX: JonathanLange 2009-09-24: No unit tests. |
114 | + web_mp_uri = URI(mp_url) |
115 | + api_mp_uri = self._launchpad._root_uri.append( |
116 | + web_mp_uri.path.lstrip('/')) |
117 | + return MergeProposal(self._launchpad.load(str(api_mp_uri))) |
118 | + |
119 | + def get_lp_branch(self, branch): |
120 | + """Get the launchpadlib branch based on a bzr branch.""" |
121 | + # First try the public branch. |
122 | + branch_url = branch.get_public_branch() |
123 | + if branch_url: |
124 | + lp_branch = self._launchpad.branches.getByUrl( |
125 | + url=branch_url) |
126 | + if lp_branch is not None: |
127 | + return lp_branch |
128 | + # If that didn't work try the push location. |
129 | + branch_url = branch.get_push_location() |
130 | + if branch_url: |
131 | + lp_branch = self._launchpad.branches.getByUrl( |
132 | + url=branch_url) |
133 | + if lp_branch is not None: |
134 | + return lp_branch |
135 | + raise BzrCommandError( |
136 | + "No public branch could be found. Please re-run and specify " |
137 | + "the URL for the merge proposal.") |
138 | + |
139 | + def get_merge_proposal_from_branch(self, branch): |
140 | + """Get the merge proposal from the branch.""" |
141 | + |
142 | + lp_branch = self.get_lp_branch(branch) |
143 | + proposals = lp_branch.landing_targets |
144 | + if len(proposals) == 0: |
145 | + raise BzrCommandError( |
146 | + "The public branch has no source merge proposals. " |
147 | + "You must have a merge proposal before attempting to " |
148 | + "land the branch.") |
149 | + elif len(proposals) > 1: |
150 | + raise BzrCommandError( |
151 | + "The public branch has multiple source merge proposals. " |
152 | + "You must provide the URL to the one you wish to use.") |
153 | + return MergeProposal(proposals[0]) |
154 | + |
155 | + |
156 | +class MergeProposal: |
157 | + """Wrapper around launchpadlib `IBranchMergeProposal` for landing.""" |
158 | + |
159 | + def __init__(self, mp): |
160 | + """Construct a merge proposal. |
161 | + |
162 | + :param mp: A launchpadlib `IBranchMergeProposal`. |
163 | + """ |
164 | + self._mp = mp |
165 | + self._launchpad = mp._root |
166 | + |
167 | + @property |
168 | + def source_branch(self): |
169 | + """The push URL of the source branch.""" |
170 | + return str(self._get_push_url(self._mp.source_branch)).rstrip('/') |
171 | + |
172 | + @property |
173 | + def target_branch(self): |
174 | + """The push URL of the target branch.""" |
175 | + return str(self._get_push_url(self._mp.target_branch)).rstrip('/') |
176 | + |
177 | + @property |
178 | + def commit_message(self): |
179 | + """The commit message specified on the merge proposal.""" |
180 | + return self._mp.commit_message |
181 | + |
182 | + @property |
183 | + def is_approved(self): |
184 | + """Is this merge proposal approved for landing.""" |
185 | + return self._mp.queue_status == 'Approved' |
186 | + |
187 | + def get_stakeholder_emails(self): |
188 | + """Return a collection of people who should know about branch landing. |
189 | + |
190 | + Used to determine who to email with the ec2 test results. |
191 | + |
192 | + :return: A set of `IPerson`s. |
193 | + """ |
194 | + # XXX: JonathanLange 2009-09-24: No unit tests. |
195 | + return set( |
196 | + map(get_email, |
197 | + [self._mp.source_branch.owner, self._launchpad.me])) |
198 | + |
199 | + def get_reviews(self): |
200 | + """Return a dictionary of all Approved reviews. |
201 | + |
202 | + Used to determine who has actually approved a branch for landing. The |
203 | + key of the dictionary is the type of review, and the value is the list |
204 | + of people who have voted Approve with that type. |
205 | + |
206 | + Common types include 'code', 'db', 'ui' and of course `None`. |
207 | + """ |
208 | + reviews = {} |
209 | + for vote in self._mp.votes: |
210 | + comment = vote.comment |
211 | + if comment is None or comment.vote != "Approve": |
212 | + continue |
213 | + reviewers = reviews.setdefault(vote.review_type, []) |
214 | + reviewers.append(vote.reviewer) |
215 | + return reviews |
216 | + |
217 | + def get_bugs(self): |
218 | + """Return a collection of bugs linked to the source branch.""" |
219 | + return self._mp.source_branch.linked_bugs |
220 | + |
221 | + def _get_push_url(self, branch): |
222 | + """Return the push URL for 'branch'. |
223 | + |
224 | + This function is a work-around for Launchpad's lack of exposing the |
225 | + branch's push URL. |
226 | + |
227 | + :param branch: A launchpadlib `IBranch`. |
228 | + """ |
229 | + # XXX: JonathanLange 2009-09-24: No unit tests. |
230 | + host = get_bazaar_host(str(self._launchpad._root_uri)) |
231 | + # XXX: JonathanLange 2009-09-24 bug=435790: lazr.uri allows a path |
232 | + # without a leading '/' and then doesn't insert a '/' in the final |
233 | + # URL. Do it ourselves. |
234 | + return URI(scheme='bzr+ssh', host=host, path='/' + branch.unique_name) |
235 | + |
236 | + def get_commit_message(self, commit_text, testfix=False): |
237 | + """Get the Launchpad-style commit message for a merge proposal.""" |
238 | + reviews = self.get_reviews() |
239 | + bugs = self.get_bugs() |
240 | + if testfix: |
241 | + testfix = '[testfix]' |
242 | + else: |
243 | + testfix = '' |
244 | + return '%s%s%s %s' % ( |
245 | + testfix, |
246 | + get_reviewer_clause(reviews), |
247 | + get_bugs_clause(bugs), |
248 | + commit_text) |
249 | + |
250 | + |
251 | +class Submitter(object): |
252 | + |
253 | + def __init__(self, branch, location): |
254 | + self.branch = branch |
255 | + self.config = self.branch.get_config() |
256 | + self.mail_to = self.config.get_user_option('pqm_email') |
257 | + if not self.mail_to: |
258 | + raise pqm_submit.NoPQMSubmissionAddress(branch) |
259 | + self.mail_from = self.config.get_user_option('pqm_user_email') |
260 | + if not self.mail_from: |
261 | + self.mail_from = self.config.username() |
262 | + self.lander = LaunchpadBranchLander.load() |
263 | + self.location = location |
264 | + |
265 | + def submission(self, mp): |
266 | + submission = pqm_submit.PQMSubmission(self.branch, |
267 | + mp.source_branch, mp.target_branch, '') |
268 | + return submission |
269 | + |
270 | + @staticmethod |
271 | + def check_submission(submission): |
272 | + submission.check_tree() |
273 | + submission.check_public_branch() |
274 | + |
275 | + @staticmethod |
276 | + def set_message(submission, mp): |
277 | + pqm_command = ''.join(submission.to_lines()) |
278 | + commit_message = mp.commit_message or '' |
279 | + start_message = mp.get_commit_message(commit_message) |
280 | + message = msgeditor.edit_commit_message( |
281 | + 'pqm command:\n%s' % pqm_command, |
282 | + start_message=start_message).rstrip('\n') |
283 | + submission.message = message.encode('utf-8') |
284 | + |
285 | + def run(self, outf): |
286 | + if self.location is None: |
287 | + mp = self.lander.get_merge_proposal_from_branch(self.branch) |
288 | + else: |
289 | + mp = self.lander.load_merge_proposal(self.location) |
290 | + submission = self.submission(mp) |
291 | + self.check_submission(submission) |
292 | + self.set_message(submission, mp) |
293 | + email = submission.to_email(self.mail_from, self.mail_to) |
294 | + if outf is not None: |
295 | + outf.write(email.as_string()) |
296 | + else: |
297 | + smtp_connection.SMTPConnection(self.config).send_email(email) |
298 | + |
299 | + |
300 | +def get_email(person): |
301 | + """Get the preferred email address for 'person'.""" |
302 | + email_object = person.preferred_email_address |
303 | + # XXX: JonathanLange 2009-09-24 bug=319432: This raises a very obscure |
304 | + # error when the email address isn't set. e.g. with name12 in the sample |
305 | + # data. e.g. "httplib2.RelativeURIError: Only absolute URIs are allowed. |
306 | + # uri = tag:launchpad.net:2008:redacted". |
307 | + return email_object.email |
308 | + |
309 | + |
310 | +def get_bugs_clause(bugs): |
311 | + """Return the bugs clause of a commit message. |
312 | + |
313 | + :param bugs: A collection of `IBug` objects. |
314 | + :return: A string of the form "[bug=A,B,C]". |
315 | + """ |
316 | + if not bugs: |
317 | + return '' |
318 | + return '[bug=%s]' % ','.join(str(bug.id) for bug in bugs) |
319 | + |
320 | + |
321 | +def get_reviewer_handle(reviewer): |
322 | + """Get the handle for 'reviewer'. |
323 | + |
324 | + The handles of reviewers are included in the commit message for Launchpad |
325 | + changes. Historically, these handles have been the IRC nicks. Thus, if |
326 | + 'reviewer' has an IRC nickname for Freenode, we use that. Otherwise we use |
327 | + their Launchpad username. |
328 | + |
329 | + :param reviewer: A launchpadlib `IPerson` object. |
330 | + :return: unicode text. |
331 | + """ |
332 | + irc_handles = reviewer.irc_nicknames |
333 | + for handle in irc_handles: |
334 | + if handle.network == 'irc.freenode.net': |
335 | + return handle.nickname |
336 | + return reviewer.name |
337 | + |
338 | + |
339 | +def _comma_separated_names(things): |
340 | + """Return a string of comma-separated names of 'things'. |
341 | + |
342 | + The list is sorted before being joined. |
343 | + """ |
344 | + return ','.join(sorted(thing.name for thing in things)) |
345 | + |
346 | + |
347 | +def get_reviewer_clause(reviewers): |
348 | + """Get the reviewer section of a commit message, given the reviewers. |
349 | + |
350 | + :param reviewers: A dict mapping review types to lists of reviewers, as |
351 | + returned by 'get_reviews'. |
352 | + :return: A string like u'[r=foo,bar][ui=plop]'. |
353 | + """ |
354 | + # If no review type is specified it is assumed to be a code review. |
355 | + code_reviewers = reviewers.get(None, []) |
356 | + code_reviewers.extend(reviewers.get('', [])) |
357 | + ui_reviewers = [] |
358 | + rc_reviewers = [] |
359 | + for review_type, reviewer in reviewers.items(): |
360 | + if review_type is None: |
361 | + continue |
362 | + if review_type == '': |
363 | + code_reviewers.extend(reviewer) |
364 | + if 'code' in review_type or 'db' in review_type: |
365 | + code_reviewers.extend(reviewer) |
366 | + if 'ui' in review_type: |
367 | + ui_reviewers.extend(reviewer) |
368 | + if 'release-critical' in review_type: |
369 | + rc_reviewers.extend(reviewer) |
370 | + if not code_reviewers: |
371 | + raise MissingReviewError("Need approved votes in order to land.") |
372 | + if ui_reviewers: |
373 | + ui_clause = _comma_separated_names(ui_reviewers) |
374 | + else: |
375 | + ui_clause = 'none' |
376 | + if rc_reviewers: |
377 | + rc_clause = ( |
378 | + '[release-critical=%s]' % _comma_separated_names(rc_reviewers)) |
379 | + else: |
380 | + rc_clause = '' |
381 | + return '%s[r=%s][ui=%s]' % ( |
382 | + rc_clause, _comma_separated_names(code_reviewers), ui_clause) |
383 | + |
384 | + |
385 | +def get_bazaar_host(api_root): |
386 | + """Get the Bazaar service for the given API root.""" |
387 | + # XXX: JonathanLange 2009-09-24 bug=435803: This is only needed because |
388 | + # Launchpad doesn't expose the push URL for branches. |
389 | + if api_root == EDGE_SERVICE_ROOT: |
390 | + return 'bazaar.launchpad.net' |
391 | + elif api_root == DEV_SERVICE_ROOT: |
392 | + return 'bazaar.launchpad.dev' |
393 | + elif api_root == STAGING_SERVICE_ROOT: |
394 | + return 'bazaar.staging.launchpad.net' |
395 | + elif api_root == LPNET_SERVICE_ROOT: |
396 | + return 'bazaar.launchpad.net' |
397 | + else: |
398 | + raise ValueError( |
399 | + 'Cannot determine Bazaar host. "%s" not a recognized Launchpad ' |
400 | + 'API root.' % (api_root,)) |
401 | |
402 | === added file 'test_lpland.py' |
403 | --- test_lpland.py 1970-01-01 00:00:00 +0000 |
404 | +++ test_lpland.py 2010-01-21 23:09:12 +0000 |
405 | @@ -0,0 +1,197 @@ |
406 | +# Copyright 2009 Canonical Ltd. This software is licensed under the |
407 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
408 | + |
409 | +"""Tests for automatic landing thing.""" |
410 | + |
411 | +__metaclass__ = type |
412 | + |
413 | +import unittest |
414 | + |
415 | +from launchpadlib.launchpad import EDGE_SERVICE_ROOT, STAGING_SERVICE_ROOT |
416 | + |
417 | +from bzrlib.plugins.pqm.lpland import ( |
418 | + get_bazaar_host, get_bugs_clause, get_reviewer_clause, |
419 | + get_reviewer_handle, MissingReviewError) |
420 | + |
421 | + |
422 | +class FakeBug: |
423 | + """Fake launchpadlib Bug object. |
424 | + |
425 | + Only used for the purposes of testing. |
426 | + """ |
427 | + |
428 | + def __init__(self, id): |
429 | + self.id = id |
430 | + |
431 | + |
432 | +class FakePerson: |
433 | + """Fake launchpadlib Person object. |
434 | + |
435 | + Only used for the purposes of testing. |
436 | + """ |
437 | + |
438 | + def __init__(self, name, irc_handles): |
439 | + self.name = name |
440 | + self.irc_nicknames = list(irc_handles) |
441 | + |
442 | + |
443 | +class FakeIRC: |
444 | + """Fake IRC handle. |
445 | + |
446 | + Only used for the purposes of testing. |
447 | + """ |
448 | + |
449 | + def __init__(self, nickname, network): |
450 | + self.nickname = nickname |
451 | + self.network = network |
452 | + |
453 | + |
454 | +class TestBugsClaused(unittest.TestCase): |
455 | + """Tests for `get_bugs_clause`.""" |
456 | + |
457 | + def test_no_bugs(self): |
458 | + # If there are no bugs, then there is no bugs clause. |
459 | + bugs_clause = get_bugs_clause([]) |
460 | + self.assertEqual('', bugs_clause) |
461 | + |
462 | + def test_one_bug(self): |
463 | + # If there's a bug, then the bugs clause is [bug=$ID]. |
464 | + bug = FakeBug(45) |
465 | + bugs_clause = get_bugs_clause([bug]) |
466 | + self.assertEqual('[bug=45]', bugs_clause) |
467 | + |
468 | + def test_two_bugs(self): |
469 | + # If there are two bugs, then the bugs clause is [bug=$ID,$ID]. |
470 | + bug1 = FakeBug(20) |
471 | + bug2 = FakeBug(45) |
472 | + bugs_clause = get_bugs_clause([bug1, bug2]) |
473 | + self.assertEqual('[bug=20,45]', bugs_clause) |
474 | + |
475 | + |
476 | +class TestGetReviewerHandle(unittest.TestCase): |
477 | + """Tests for `get_reviewer_handle`.""" |
478 | + |
479 | + def makePerson(self, name, irc_handles): |
480 | + return FakePerson(name, irc_handles) |
481 | + |
482 | + def test_no_irc_nicknames(self): |
483 | + # If the person has no IRC nicknames, their reviewer handle is their |
484 | + # Launchpad user name. |
485 | + person = self.makePerson(name='foo', irc_handles=[]) |
486 | + self.assertEqual('foo', get_reviewer_handle(person)) |
487 | + |
488 | + def test_freenode_irc_nick_preferred(self): |
489 | + # If the person has a Freenode IRC nickname, then that is preferred as |
490 | + # their user handle. |
491 | + person = self.makePerson( |
492 | + name='foo', irc_handles=[FakeIRC('bar', 'irc.freenode.net')]) |
493 | + self.assertEqual('bar', get_reviewer_handle(person)) |
494 | + |
495 | + def test_non_freenode_nicks_ignored(self): |
496 | + # If the person has IRC nicks that aren't freenode, we ignore them. |
497 | + person = self.makePerson( |
498 | + name='foo', irc_handles=[FakeIRC('bar', 'irc.efnet.net')]) |
499 | + self.assertEqual('foo', get_reviewer_handle(person)) |
500 | + |
501 | + |
502 | +class TestGetReviewerClause(unittest.TestCase): |
503 | + """Tests for `get_reviewer_clause`.""" |
504 | + |
505 | + def makePerson(self, name): |
506 | + return FakePerson(name, []) |
507 | + |
508 | + def get_reviewer_clause(self, reviewers): |
509 | + return get_reviewer_clause(reviewers) |
510 | + |
511 | + def test_one_reviewer_no_type(self): |
512 | + # It's very common for a merge proposal to be reviewed by one person |
513 | + # with no specified type of review. It such cases the review clause is |
514 | + # '[r=<person>][ui=none]'. |
515 | + clause = self.get_reviewer_clause({None: [self.makePerson('foo')]}) |
516 | + self.assertEqual('[r=foo][ui=none]', clause) |
517 | + |
518 | + def test_two_reviewers_no_type(self): |
519 | + # Branches can have more than one reviewer. |
520 | + clause = self.get_reviewer_clause( |
521 | + {None: [self.makePerson('foo'), self.makePerson('bar')]}) |
522 | + self.assertEqual('[r=bar,foo][ui=none]', clause) |
523 | + |
524 | + def test_mentat_reviewers(self): |
525 | + # A mentat review sometimes is marked like 'ui*'. Due to the |
526 | + # unordered nature of dictionaries, the reviewers are sorted before |
527 | + # being put into the clause for predictability. |
528 | + clause = self.get_reviewer_clause( |
529 | + {None: [self.makePerson('foo')], |
530 | + 'code*': [self.makePerson('newguy')], |
531 | + 'ui': [self.makePerson('beuno')], |
532 | + 'ui*': [self.makePerson('bac')]}) |
533 | + self.assertEqual('[r=foo,newguy][ui=bac,beuno]', clause) |
534 | + |
535 | + def test_code_reviewer_counts(self): |
536 | + # Some people explicitly specify the 'code' type when they do code |
537 | + # reviews, these are treated in the same way as reviewers without any |
538 | + # given type. |
539 | + clause = self.get_reviewer_clause({'code': [self.makePerson('foo')]}) |
540 | + self.assertEqual('[r=foo][ui=none]', clause) |
541 | + |
542 | + def test_release_critical(self): |
543 | + # Reviews that are marked as release-critical are included in a |
544 | + # separate clause. |
545 | + clause = self.get_reviewer_clause( |
546 | + {'code': [self.makePerson('foo')], |
547 | + 'release-critical': [self.makePerson('bar')]}) |
548 | + self.assertEqual('[release-critical=bar][r=foo][ui=none]', clause) |
549 | + |
550 | + def test_db_reviewer_counts(self): |
551 | + # There's no special way of annotating database reviews in Launchpad |
552 | + # commit messages, so they are included with the code reviews. |
553 | + clause = self.get_reviewer_clause({'db': [self.makePerson('foo')]}) |
554 | + self.assertEqual('[r=foo][ui=none]', clause) |
555 | + |
556 | + def test_ui_reviewers(self): |
557 | + # If someone has done a UI review, then that appears in the clause |
558 | + # separately from the code reviews. |
559 | + clause = self.get_reviewer_clause( |
560 | + {'code': [self.makePerson('foo')], |
561 | + 'ui': [self.makePerson('bar')], |
562 | + }) |
563 | + self.assertEqual('[r=foo][ui=bar]', clause) |
564 | + |
565 | + def test_no_reviewers(self): |
566 | + # If the merge proposal hasn't been approved by anyone, we cannot |
567 | + # generate a valid clause. |
568 | + self.assertRaises(MissingReviewError, self.get_reviewer_clause, {}) |
569 | + |
570 | + |
571 | +class TestGetBazaarHost(unittest.TestCase): |
572 | + """Tests for `get_bazaar_host`.""" |
573 | + |
574 | + def test_dev_service(self): |
575 | + # The Bazaar host for the dev service is bazaar.launchpad.dev. |
576 | + self.assertEqual( |
577 | + 'bazaar.launchpad.dev', |
578 | + get_bazaar_host('https://api.launchpad.dev/beta/')) |
579 | + |
580 | + def test_edge_service(self): |
581 | + # The Bazaar host for the edge service is bazaar.launchpad.net, since |
582 | + # there's no edge codehosting service. |
583 | + self.assertEqual( |
584 | + 'bazaar.launchpad.net', get_bazaar_host(EDGE_SERVICE_ROOT)) |
585 | + |
586 | + def test_production_service(self): |
587 | + # The Bazaar host for the production service is bazaar.launchpad.net. |
588 | + self.assertEqual( |
589 | + 'bazaar.launchpad.net', |
590 | + get_bazaar_host('https://api.launchpad.net/beta/')) |
591 | + |
592 | + def test_staging_service(self): |
593 | + # The Bazaar host for the staging service is |
594 | + # bazaar.staging.launchpad.net. |
595 | + self.assertEqual( |
596 | + 'bazaar.staging.launchpad.net', |
597 | + get_bazaar_host(STAGING_SERVICE_ROOT)) |
598 | + |
599 | + def test_unrecognized_service(self): |
600 | + # Any unrecognized URL will raise a ValueError. |
601 | + self.assertRaises( |
602 | + ValueError, get_bazaar_host, 'https://api.lunchpad.net') |
As you suggested, we've moved the lp-land command int pqm-submit itself. I can
has review?