Merge lp:~abentley/launchpad/unnecessary-upgrade into lp:launchpad
- unnecessary-upgrade
- Merge into devel
Status: | Merged |
---|---|
Merged at revision: | 13765 |
Proposed branch: | lp:~abentley/launchpad/unnecessary-upgrade |
Merge into: | lp:launchpad |
Prerequisite: | lp:~abentley/launchpad/upgrade-not-branch-error |
Diff against target: |
416 lines (+151/-23) 8 files modified
lib/lp/code/browser/branch.py (+5/-1) lib/lp/code/browser/tests/test_branch.py (+26/-4) lib/lp/code/errors.py (+34/-0) lib/lp/code/interfaces/branch.py (+6/-0) lib/lp/code/model/branch.py (+20/-7) lib/lp/code/model/branchjob.py (+1/-4) lib/lp/code/model/tests/test_branch.py (+52/-4) lib/lp/code/model/tests/test_branchjob.py (+7/-3) |
To merge this branch: | bzr merge lp:~abentley/launchpad/unnecessary-upgrade |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Deryck Hodge (community) | code | Approve | |
Review via email: mp+72480@code.launchpad.net |
Commit message
Nice error if branch cannot be upgraded.
Description of the change
= Summary =
Fix bug #823850: AssertionError raised upgrading a branch that doesn't need upgrade
== Proposed fix ==
Produce a notification explaining why the branch could not be upgraded.
== Pre-implementation notes ==
None
== Implementation details ==
Extracted the Branch.
Fixed lint.
== Tests ==
bin/test -t checkUpgrade -t test_upgrade_
== Demo and Q/A ==
Upgrade a branch, and then immediately request another upgrade. You should get a nice error notification.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
Preview Diff
1 | === modified file 'lib/lp/code/browser/branch.py' | |||
2 | --- lib/lp/code/browser/branch.py 2011-08-22 18:45:27 +0000 | |||
3 | +++ lib/lp/code/browser/branch.py 2011-08-22 18:45:28 +0000 | |||
4 | @@ -131,6 +131,7 @@ | |||
5 | 131 | from lp.code.errors import ( | 131 | from lp.code.errors import ( |
6 | 132 | BranchCreationForbidden, | 132 | BranchCreationForbidden, |
7 | 133 | BranchExists, | 133 | BranchExists, |
8 | 134 | CannotUpgradeBranch, | ||
9 | 134 | CodeImportAlreadyRequested, | 135 | CodeImportAlreadyRequested, |
10 | 135 | CodeImportAlreadyRunning, | 136 | CodeImportAlreadyRunning, |
11 | 136 | CodeImportNotInReviewedState, | 137 | CodeImportNotInReviewedState, |
12 | @@ -993,7 +994,10 @@ | |||
13 | 993 | 994 | ||
14 | 994 | @action('Upgrade', name='upgrade_branch') | 995 | @action('Upgrade', name='upgrade_branch') |
15 | 995 | def upgrade_branch_action(self, action, data): | 996 | def upgrade_branch_action(self, action, data): |
17 | 996 | self.context.requestUpgrade(self.user) | 997 | try: |
18 | 998 | self.context.requestUpgrade(self.user) | ||
19 | 999 | except CannotUpgradeBranch as e: | ||
20 | 1000 | self.request.response.addErrorNotification(e) | ||
21 | 997 | 1001 | ||
22 | 998 | 1002 | ||
23 | 999 | class BranchEditView(BranchEditFormView, BranchNameValidationMixin): | 1003 | class BranchEditView(BranchEditFormView, BranchNameValidationMixin): |
24 | 1000 | 1004 | ||
25 | === modified file 'lib/lp/code/browser/tests/test_branch.py' | |||
26 | --- lib/lp/code/browser/tests/test_branch.py 2011-07-21 02:18:54 +0000 | |||
27 | +++ lib/lp/code/browser/tests/test_branch.py 2011-08-22 18:45:28 +0000 | |||
28 | @@ -7,12 +7,10 @@ | |||
29 | 7 | 7 | ||
30 | 8 | from datetime import ( | 8 | from datetime import ( |
31 | 9 | datetime, | 9 | datetime, |
32 | 10 | timedelta, | ||
33 | 11 | ) | 10 | ) |
34 | 12 | from textwrap import dedent | 11 | from textwrap import dedent |
35 | 13 | 12 | ||
36 | 14 | import pytz | 13 | import pytz |
37 | 15 | import simplejson | ||
38 | 16 | from zope.security.proxy import removeSecurityProxy | 14 | from zope.security.proxy import removeSecurityProxy |
39 | 17 | 15 | ||
40 | 18 | from canonical.config import config | 16 | from canonical.config import config |
41 | @@ -41,7 +39,11 @@ | |||
42 | 41 | BranchView, | 39 | BranchView, |
43 | 42 | ) | 40 | ) |
44 | 43 | from lp.code.browser.branchlisting import PersonOwnedBranchesView | 41 | from lp.code.browser.branchlisting import PersonOwnedBranchesView |
46 | 44 | from lp.code.bzr import ControlFormat | 42 | from lp.code.bzr import ( |
47 | 43 | BranchFormat, | ||
48 | 44 | ControlFormat, | ||
49 | 45 | RepositoryFormat, | ||
50 | 46 | ) | ||
51 | 45 | from lp.code.enums import ( | 47 | from lp.code.enums import ( |
52 | 46 | BranchLifecycleStatus, | 48 | BranchLifecycleStatus, |
53 | 47 | BranchType, | 49 | BranchType, |
54 | @@ -400,7 +402,7 @@ | |||
55 | 400 | 402 | ||
56 | 401 | def _add_revisions(self, branch, nr_revisions=1): | 403 | def _add_revisions(self, branch, nr_revisions=1): |
57 | 402 | revisions = [] | 404 | revisions = [] |
59 | 403 | for seq in range(1, nr_revisions+1): | 405 | for seq in range(1, nr_revisions + 1): |
60 | 404 | revision = self.factory.makeRevision( | 406 | revision = self.factory.makeRevision( |
61 | 405 | author="Eric the Viking <eric@vikings-r-us.example.com>", | 407 | author="Eric the Viking <eric@vikings-r-us.example.com>", |
62 | 406 | log_body=( | 408 | log_body=( |
63 | @@ -806,3 +808,23 @@ | |||
64 | 806 | 'Some Product.')) | 808 | 'Some Product.')) |
65 | 807 | with person_logged_in(person): | 809 | with person_logged_in(person): |
66 | 808 | self.assertEquals(person, branch.owner) | 810 | self.assertEquals(person, branch.owner) |
67 | 811 | |||
68 | 812 | |||
69 | 813 | class TestBranchUpgradeView(TestCaseWithFactory): | ||
70 | 814 | |||
71 | 815 | layer = LaunchpadFunctionalLayer | ||
72 | 816 | |||
73 | 817 | def test_upgrade_branch_action_cannot_upgrade(self): | ||
74 | 818 | # A nice error is displayed if a branch cannot be upgraded. | ||
75 | 819 | branch = self.factory.makePersonalBranch( | ||
76 | 820 | branch_format=BranchFormat.BZR_BRANCH_6, | ||
77 | 821 | repository_format=RepositoryFormat.BZR_CHK_2A) | ||
78 | 822 | login_person(branch.owner) | ||
79 | 823 | self.addCleanup(logout) | ||
80 | 824 | branch.requestUpgrade(branch.owner) | ||
81 | 825 | view = create_initialized_view(branch, '+upgrade') | ||
82 | 826 | view.upgrade_branch_action.success({}) | ||
83 | 827 | self.assertEqual(1, len(view.request.notifications)) | ||
84 | 828 | self.assertEqual( | ||
85 | 829 | 'An upgrade is already in progress for branch %s.' % | ||
86 | 830 | branch.bzr_identity, view.request.notifications[0].message) | ||
87 | 809 | 831 | ||
88 | === modified file 'lib/lp/code/errors.py' | |||
89 | --- lib/lp/code/errors.py 2011-06-23 21:09:20 +0000 | |||
90 | +++ lib/lp/code/errors.py 2011-08-22 18:45:28 +0000 | |||
91 | @@ -5,6 +5,7 @@ | |||
92 | 5 | 5 | ||
93 | 6 | __metaclass__ = type | 6 | __metaclass__ = type |
94 | 7 | __all__ = [ | 7 | __all__ = [ |
95 | 8 | 'AlreadyLatestFormat', | ||
96 | 8 | 'BadBranchMergeProposalSearchContext', | 9 | 'BadBranchMergeProposalSearchContext', |
97 | 9 | 'BadStateTransition', | 10 | 'BadStateTransition', |
98 | 10 | 'BranchCannotBePrivate', | 11 | 'BranchCannotBePrivate', |
99 | @@ -20,6 +21,8 @@ | |||
100 | 20 | 'BuildNotAllowedForDistro', | 21 | 'BuildNotAllowedForDistro', |
101 | 21 | 'BranchMergeProposalExists', | 22 | 'BranchMergeProposalExists', |
102 | 22 | 'CannotDeleteBranch', | 23 | 'CannotDeleteBranch', |
103 | 24 | 'CannotUpgradeBranch', | ||
104 | 25 | 'CannotUpgradeNonHosted', | ||
105 | 23 | 'CannotHaveLinkedBranch', | 26 | 'CannotHaveLinkedBranch', |
106 | 24 | 'CodeImportAlreadyRequested', | 27 | 'CodeImportAlreadyRequested', |
107 | 25 | 'CodeImportAlreadyRunning', | 28 | 'CodeImportAlreadyRunning', |
108 | @@ -36,6 +39,7 @@ | |||
109 | 36 | 'TooManyBuilds', | 39 | 'TooManyBuilds', |
110 | 37 | 'TooNewRecipeFormat', | 40 | 'TooNewRecipeFormat', |
111 | 38 | 'UnknownBranchTypeError', | 41 | 'UnknownBranchTypeError', |
112 | 42 | 'UpgradePending', | ||
113 | 39 | 'UserHasExistingReview', | 43 | 'UserHasExistingReview', |
114 | 40 | 'UserNotBranchReviewer', | 44 | 'UserNotBranchReviewer', |
115 | 41 | 'WrongBranchMergeProposal', | 45 | 'WrongBranchMergeProposal', |
116 | @@ -167,6 +171,36 @@ | |||
117 | 167 | _msg_template = "%s cannot have linked branches." | 171 | _msg_template = "%s cannot have linked branches." |
118 | 168 | 172 | ||
119 | 169 | 173 | ||
120 | 174 | class CannotUpgradeBranch(Exception): | ||
121 | 175 | """"Made for subclassing.""" | ||
122 | 176 | |||
123 | 177 | def __init__(self, branch): | ||
124 | 178 | super(CannotUpgradeBranch, self).__init__( | ||
125 | 179 | self._msg_template % branch.bzr_identity) | ||
126 | 180 | self.branch = branch | ||
127 | 181 | |||
128 | 182 | |||
129 | 183 | class AlreadyLatestFormat(CannotUpgradeBranch): | ||
130 | 184 | """Raised on attempt to upgrade a branch already in the latest format.""" | ||
131 | 185 | |||
132 | 186 | _msg_template = ( | ||
133 | 187 | 'Branch %s is in the latest format, so it cannot be upgraded.') | ||
134 | 188 | |||
135 | 189 | |||
136 | 190 | class CannotUpgradeNonHosted(CannotUpgradeBranch): | ||
137 | 191 | |||
138 | 192 | """Raised on attempt to upgrade a non-Hosted branch.""" | ||
139 | 193 | |||
140 | 194 | _msg_template = 'Cannot upgrade non-hosted branch %s' | ||
141 | 195 | |||
142 | 196 | |||
143 | 197 | class UpgradePending(CannotUpgradeBranch): | ||
144 | 198 | |||
145 | 199 | """Raised on attempt to upgrade a branch already in the latest format.""" | ||
146 | 200 | |||
147 | 201 | _msg_template = 'An upgrade is already in progress for branch %s.' | ||
148 | 202 | |||
149 | 203 | |||
150 | 170 | class ClaimReviewFailed(Exception): | 204 | class ClaimReviewFailed(Exception): |
151 | 171 | """The user cannot claim the pending review.""" | 205 | """The user cannot claim the pending review.""" |
152 | 172 | 206 | ||
153 | 173 | 207 | ||
154 | === modified file 'lib/lp/code/interfaces/branch.py' | |||
155 | --- lib/lp/code/interfaces/branch.py 2011-08-22 18:45:27 +0000 | |||
156 | +++ lib/lp/code/interfaces/branch.py 2011-08-22 18:45:28 +0000 | |||
157 | @@ -944,6 +944,12 @@ | |||
158 | 944 | :return: A list of tuples like (date, count). | 944 | :return: A list of tuples like (date, count). |
159 | 945 | """ | 945 | """ |
160 | 946 | 946 | ||
161 | 947 | def checkUpgrade(): | ||
162 | 948 | """Check whether an upgrade should be performed, and raise if not. | ||
163 | 949 | |||
164 | 950 | :raises: a `CannotUpgradeBranch`, or a subclass. | ||
165 | 951 | """ | ||
166 | 952 | |||
167 | 947 | needs_upgrading = Attribute("Whether the branch needs to be upgraded.") | 953 | needs_upgrading = Attribute("Whether the branch needs to be upgraded.") |
168 | 948 | upgrade_pending = Attribute( | 954 | upgrade_pending = Attribute( |
169 | 949 | "Whether a branch has had an upgrade requested.") | 955 | "Whether a branch has had an upgrade requested.") |
170 | 950 | 956 | ||
171 | === modified file 'lib/lp/code/model/branch.py' | |||
172 | --- lib/lp/code/model/branch.py 2011-08-22 18:45:27 +0000 | |||
173 | +++ lib/lp/code/model/branch.py 2011-08-22 18:45:28 +0000 | |||
174 | @@ -87,14 +87,18 @@ | |||
175 | 87 | BranchType, | 87 | BranchType, |
176 | 88 | ) | 88 | ) |
177 | 89 | from lp.code.errors import ( | 89 | from lp.code.errors import ( |
178 | 90 | AlreadyLatestFormat, | ||
179 | 90 | BranchCannotBePrivate, | 91 | BranchCannotBePrivate, |
180 | 91 | BranchCannotBePublic, | 92 | BranchCannotBePublic, |
181 | 92 | BranchMergeProposalExists, | 93 | BranchMergeProposalExists, |
182 | 93 | BranchTargetError, | 94 | BranchTargetError, |
183 | 94 | BranchTypeError, | 95 | BranchTypeError, |
184 | 95 | CannotDeleteBranch, | 96 | CannotDeleteBranch, |
185 | 97 | CannotUpgradeBranch, | ||
186 | 98 | CannotUpgradeNonHosted, | ||
187 | 96 | InvalidBranchMergeProposal, | 99 | InvalidBranchMergeProposal, |
188 | 97 | InvalidMergeQueueConfig, | 100 | InvalidMergeQueueConfig, |
189 | 101 | UpgradePending, | ||
190 | 98 | ) | 102 | ) |
191 | 99 | from lp.code.event.branchmergeproposal import ( | 103 | from lp.code.event.branchmergeproposal import ( |
192 | 100 | BranchMergeProposalNeedsReviewEvent, | 104 | BranchMergeProposalNeedsReviewEvent, |
193 | @@ -1127,16 +1131,25 @@ | |||
194 | 1127 | DateTrunc(u'day', Revision.revision_date)) | 1131 | DateTrunc(u'day', Revision.revision_date)) |
195 | 1128 | return sorted(results) | 1132 | return sorted(results) |
196 | 1129 | 1133 | ||
200 | 1130 | @property | 1134 | def checkUpgrade(self): |
198 | 1131 | def needs_upgrading(self): | ||
199 | 1132 | """See `IBranch`.""" | ||
201 | 1133 | if self.branch_type is not BranchType.HOSTED: | 1135 | if self.branch_type is not BranchType.HOSTED: |
203 | 1134 | return False | 1136 | raise CannotUpgradeNonHosted(self) |
204 | 1135 | if self.upgrade_pending: | 1137 | if self.upgrade_pending: |
207 | 1136 | return False | 1138 | raise UpgradePending(self) |
208 | 1137 | return not ( | 1139 | if ( |
209 | 1138 | self.branch_format in CURRENT_BRANCH_FORMATS and | 1140 | self.branch_format in CURRENT_BRANCH_FORMATS and |
211 | 1139 | self.repository_format in CURRENT_REPOSITORY_FORMATS) | 1141 | self.repository_format in CURRENT_REPOSITORY_FORMATS): |
212 | 1142 | raise AlreadyLatestFormat(self) | ||
213 | 1143 | |||
214 | 1144 | @property | ||
215 | 1145 | def needs_upgrading(self): | ||
216 | 1146 | """See `IBranch`.""" | ||
217 | 1147 | try: | ||
218 | 1148 | self.checkUpgrade() | ||
219 | 1149 | except CannotUpgradeBranch: | ||
220 | 1150 | return False | ||
221 | 1151 | else: | ||
222 | 1152 | return True | ||
223 | 1140 | 1153 | ||
224 | 1141 | @property | 1154 | @property |
225 | 1142 | def upgrade_pending(self): | 1155 | def upgrade_pending(self): |
226 | 1143 | 1156 | ||
227 | === modified file 'lib/lp/code/model/branchjob.py' | |||
228 | --- lib/lp/code/model/branchjob.py 2011-08-22 18:45:27 +0000 | |||
229 | +++ lib/lp/code/model/branchjob.py 2011-08-22 18:45:28 +0000 | |||
230 | @@ -378,10 +378,7 @@ | |||
231 | 378 | @classmethod | 378 | @classmethod |
232 | 379 | def create(cls, branch, requester): | 379 | def create(cls, branch, requester): |
233 | 380 | """See `IBranchUpgradeJobSource`.""" | 380 | """See `IBranchUpgradeJobSource`.""" |
238 | 381 | if not branch.needs_upgrading: | 381 | branch.checkUpgrade() |
235 | 382 | raise AssertionError('Branch does not need upgrading.') | ||
236 | 383 | if branch.upgrade_pending: | ||
237 | 384 | raise AssertionError('Branch already has upgrade pending.') | ||
239 | 385 | branch_job = BranchJob( | 382 | branch_job = BranchJob( |
240 | 386 | branch, BranchJobType.UPGRADE_BRANCH, {}, requester=requester) | 383 | branch, BranchJobType.UPGRADE_BRANCH, {}, requester=requester) |
241 | 387 | return cls(branch_job) | 384 | return cls(branch_job) |
242 | 388 | 385 | ||
243 | === modified file 'lib/lp/code/model/tests/test_branch.py' | |||
244 | --- lib/lp/code/model/tests/test_branch.py 2011-08-22 18:45:27 +0000 | |||
245 | +++ lib/lp/code/model/tests/test_branch.py 2011-08-22 18:45:28 +0000 | |||
246 | @@ -19,6 +19,7 @@ | |||
247 | 19 | import simplejson | 19 | import simplejson |
248 | 20 | from sqlobject import SQLObjectNotFound | 20 | from sqlobject import SQLObjectNotFound |
249 | 21 | from storm.locals import Store | 21 | from storm.locals import Store |
250 | 22 | from testtools import ExpectedException | ||
251 | 22 | import transaction | 23 | import transaction |
252 | 23 | from zope.component import getUtility | 24 | from zope.component import getUtility |
253 | 24 | from zope.security.proxy import removeSecurityProxy | 25 | from zope.security.proxy import removeSecurityProxy |
254 | @@ -56,14 +57,17 @@ | |||
255 | 56 | CodeReviewNotificationLevel, | 57 | CodeReviewNotificationLevel, |
256 | 57 | ) | 58 | ) |
257 | 58 | from lp.code.errors import ( | 59 | from lp.code.errors import ( |
258 | 60 | AlreadyLatestFormat, | ||
259 | 59 | BranchCannotBePrivate, | 61 | BranchCannotBePrivate, |
260 | 60 | BranchCannotBePublic, | 62 | BranchCannotBePublic, |
261 | 61 | BranchCreatorNotMemberOfOwnerTeam, | 63 | BranchCreatorNotMemberOfOwnerTeam, |
262 | 62 | BranchCreatorNotOwner, | 64 | BranchCreatorNotOwner, |
263 | 63 | BranchTargetError, | 65 | BranchTargetError, |
264 | 64 | CannotDeleteBranch, | 66 | CannotDeleteBranch, |
265 | 67 | CannotUpgradeNonHosted, | ||
266 | 65 | InvalidBranchMergeProposal, | 68 | InvalidBranchMergeProposal, |
267 | 66 | InvalidMergeQueueConfig, | 69 | InvalidMergeQueueConfig, |
268 | 70 | UpgradePending, | ||
269 | 67 | ) | 71 | ) |
270 | 68 | from lp.code.interfaces.branch import ( | 72 | from lp.code.interfaces.branch import ( |
271 | 69 | DEFAULT_BRANCH_STATUS_IN_LISTING, | 73 | DEFAULT_BRANCH_STATUS_IN_LISTING, |
272 | @@ -586,6 +590,14 @@ | |||
273 | 586 | branch = self.factory.makePersonalBranch() | 590 | branch = self.factory.makePersonalBranch() |
274 | 587 | self.assertFalse(branch.needs_upgrading) | 591 | self.assertFalse(branch.needs_upgrading) |
275 | 588 | 592 | ||
276 | 593 | def test_checkUpgrade_empty_formats(self): | ||
277 | 594 | branch = self.factory.makePersonalBranch() | ||
278 | 595 | with ExpectedException( | ||
279 | 596 | AlreadyLatestFormat, | ||
280 | 597 | 'Branch lp://dev/~person-name.*junk/branch.* is in the latest' | ||
281 | 598 | ' format, so it cannot be upgraded.'): | ||
282 | 599 | branch.checkUpgrade() | ||
283 | 600 | |||
284 | 589 | def test_needsUpgrade_mirrored_branch(self): | 601 | def test_needsUpgrade_mirrored_branch(self): |
285 | 590 | branch = self.factory.makeBranch( | 602 | branch = self.factory.makeBranch( |
286 | 591 | branch_type=BranchType.MIRRORED, | 603 | branch_type=BranchType.MIRRORED, |
287 | @@ -593,6 +605,16 @@ | |||
288 | 593 | repository_format=RepositoryFormat.BZR_REPOSITORY_4) | 605 | repository_format=RepositoryFormat.BZR_REPOSITORY_4) |
289 | 594 | self.assertFalse(branch.needs_upgrading) | 606 | self.assertFalse(branch.needs_upgrading) |
290 | 595 | 607 | ||
291 | 608 | def test_checkUpgrade_mirrored_branch(self): | ||
292 | 609 | branch = self.factory.makeBranch( | ||
293 | 610 | branch_type=BranchType.MIRRORED, | ||
294 | 611 | branch_format=BranchFormat.BZR_BRANCH_6, | ||
295 | 612 | repository_format=RepositoryFormat.BZR_REPOSITORY_4) | ||
296 | 613 | with ExpectedException( | ||
297 | 614 | CannotUpgradeNonHosted, | ||
298 | 615 | 'Cannot upgrade non-hosted branch %s' % branch.bzr_identity): | ||
299 | 616 | branch.checkUpgrade() | ||
300 | 617 | |||
301 | 596 | def test_needsUpgrade_remote_branch(self): | 618 | def test_needsUpgrade_remote_branch(self): |
302 | 597 | branch = self.factory.makeBranch( | 619 | branch = self.factory.makeBranch( |
303 | 598 | branch_type=BranchType.REMOTE, | 620 | branch_type=BranchType.REMOTE, |
304 | @@ -621,6 +643,20 @@ | |||
305 | 621 | 643 | ||
306 | 622 | self.assertFalse(branch.needs_upgrading) | 644 | self.assertFalse(branch.needs_upgrading) |
307 | 623 | 645 | ||
308 | 646 | def test_checkUpgrade_already_requested(self): | ||
309 | 647 | branch = self.factory.makePersonalBranch( | ||
310 | 648 | branch_format=BranchFormat.BZR_BRANCH_6, | ||
311 | 649 | repository_format=RepositoryFormat.BZR_CHK_2A) | ||
312 | 650 | owner = removeSecurityProxy(branch).owner | ||
313 | 651 | login_person(owner) | ||
314 | 652 | self.addCleanup(logout) | ||
315 | 653 | branch.requestUpgrade(branch.owner) | ||
316 | 654 | with ExpectedException( | ||
317 | 655 | UpgradePending, | ||
318 | 656 | 'An upgrade is already in progress for branch' | ||
319 | 657 | ' lp://dev/~person-name.*junk/branch.*.'): | ||
320 | 658 | branch.checkUpgrade() | ||
321 | 659 | |||
322 | 624 | def test_needsUpgrading_branch_format_unrecognized(self): | 660 | def test_needsUpgrading_branch_format_unrecognized(self): |
323 | 625 | # A branch has a needs_upgrading attribute that returns whether or not | 661 | # A branch has a needs_upgrading attribute that returns whether or not |
324 | 626 | # a branch needs to be upgraded or not. If the format is | 662 | # a branch needs to be upgraded or not. If the format is |
325 | @@ -639,6 +675,17 @@ | |||
326 | 639 | repository_format=RepositoryFormat.BZR_CHK_2A) | 675 | repository_format=RepositoryFormat.BZR_CHK_2A) |
327 | 640 | self.assertFalse(branch.needs_upgrading) | 676 | self.assertFalse(branch.needs_upgrading) |
328 | 641 | 677 | ||
329 | 678 | def test_checkUpgrade_branch_format_upgrade_not_needed(self): | ||
330 | 679 | # If a branch is up-to-date, checkUpgrade raises AlreadyLatestFormat | ||
331 | 680 | branch = self.factory.makePersonalBranch( | ||
332 | 681 | branch_format=BranchFormat.BZR_BRANCH_8, | ||
333 | 682 | repository_format=RepositoryFormat.BZR_CHK_2A) | ||
334 | 683 | with ExpectedException( | ||
335 | 684 | AlreadyLatestFormat, | ||
336 | 685 | 'Branch lp://dev/~person-name.*junk/branch.* is in the latest' | ||
337 | 686 | ' format, so it cannot be upgraded.'): | ||
338 | 687 | branch.checkUpgrade() | ||
339 | 688 | |||
340 | 642 | def test_needsUpgrading_branch_format_upgrade_needed(self): | 689 | def test_needsUpgrading_branch_format_upgrade_needed(self): |
341 | 643 | # A branch has a needs_upgrading attribute that returns whether or not | 690 | # A branch has a needs_upgrading attribute that returns whether or not |
342 | 644 | # a branch needs to be upgraded or not. If a branch doesn't support | 691 | # a branch needs to be upgraded or not. If a branch doesn't support |
343 | @@ -691,18 +738,19 @@ | |||
344 | 691 | 738 | ||
345 | 692 | def test_requestUpgrade_no_upgrade_needed(self): | 739 | def test_requestUpgrade_no_upgrade_needed(self): |
346 | 693 | # If a branch doesn't need to be upgraded, requestUpgrade raises an | 740 | # If a branch doesn't need to be upgraded, requestUpgrade raises an |
348 | 694 | # AssertionError. | 741 | # AlreadyLatestFormat. |
349 | 695 | branch = self.factory.makeAnyBranch( | 742 | branch = self.factory.makeAnyBranch( |
350 | 696 | branch_format=BranchFormat.BZR_BRANCH_8, | 743 | branch_format=BranchFormat.BZR_BRANCH_8, |
351 | 697 | repository_format=RepositoryFormat.BZR_CHK_2A) | 744 | repository_format=RepositoryFormat.BZR_CHK_2A) |
352 | 698 | owner = removeSecurityProxy(branch).owner | 745 | owner = removeSecurityProxy(branch).owner |
353 | 699 | login_person(owner) | 746 | login_person(owner) |
354 | 700 | self.addCleanup(logout) | 747 | self.addCleanup(logout) |
356 | 701 | self.assertRaises(AssertionError, branch.requestUpgrade, branch.owner) | 748 | self.assertRaises( |
357 | 749 | AlreadyLatestFormat, branch.requestUpgrade, branch.owner) | ||
358 | 702 | 750 | ||
359 | 703 | def test_requestUpgrade_upgrade_pending(self): | 751 | def test_requestUpgrade_upgrade_pending(self): |
360 | 704 | # If there is a pending upgrade already requested, requestUpgrade | 752 | # If there is a pending upgrade already requested, requestUpgrade |
362 | 705 | # raises an AssertionError. | 753 | # raises an UpgradePending. |
363 | 706 | branch = self.factory.makeAnyBranch( | 754 | branch = self.factory.makeAnyBranch( |
364 | 707 | branch_format=BranchFormat.BZR_BRANCH_6) | 755 | branch_format=BranchFormat.BZR_BRANCH_6) |
365 | 708 | owner = removeSecurityProxy(branch).owner | 756 | owner = removeSecurityProxy(branch).owner |
366 | @@ -710,7 +758,7 @@ | |||
367 | 710 | self.addCleanup(logout) | 758 | self.addCleanup(logout) |
368 | 711 | branch.requestUpgrade(branch.owner) | 759 | branch.requestUpgrade(branch.owner) |
369 | 712 | 760 | ||
371 | 713 | self.assertRaises(AssertionError, branch.requestUpgrade, branch.owner) | 761 | self.assertRaises(UpgradePending, branch.requestUpgrade, branch.owner) |
372 | 714 | 762 | ||
373 | 715 | def test_upgradePending(self): | 763 | def test_upgradePending(self): |
374 | 716 | # If there is a BranchUpgradeJob pending for the branch, return True. | 764 | # If there is a BranchUpgradeJob pending for the branch, return True. |
375 | 717 | 765 | ||
376 | === modified file 'lib/lp/code/model/tests/test_branchjob.py' | |||
377 | --- lib/lp/code/model/tests/test_branchjob.py 2011-08-22 18:45:27 +0000 | |||
378 | +++ lib/lp/code/model/tests/test_branchjob.py 2011-08-22 18:45:28 +0000 | |||
379 | @@ -52,6 +52,7 @@ | |||
380 | 52 | BranchSubscriptionNotificationLevel, | 52 | BranchSubscriptionNotificationLevel, |
381 | 53 | CodeReviewNotificationLevel, | 53 | CodeReviewNotificationLevel, |
382 | 54 | ) | 54 | ) |
383 | 55 | from lp.code.errors import AlreadyLatestFormat | ||
384 | 55 | from lp.code.interfaces.branchjob import ( | 56 | from lp.code.interfaces.branchjob import ( |
385 | 56 | IBranchDiffJob, | 57 | IBranchDiffJob, |
386 | 57 | IBranchJob, | 58 | IBranchJob, |
387 | @@ -307,6 +308,8 @@ | |||
388 | 307 | 'Bazaar-NG Knit Repository Format 1') | 308 | 'Bazaar-NG Knit Repository Format 1') |
389 | 308 | 309 | ||
390 | 309 | job = BranchUpgradeJob.create(db_branch, self.factory.makePerson()) | 310 | job = BranchUpgradeJob.create(db_branch, self.factory.makePerson()) |
391 | 311 | |||
392 | 312 | dbuser = config.launchpad.dbuser | ||
393 | 310 | self.becomeDbUser(config.upgrade_branches.dbuser) | 313 | self.becomeDbUser(config.upgrade_branches.dbuser) |
394 | 311 | with TransactionFreeOperation.require(): | 314 | with TransactionFreeOperation.require(): |
395 | 312 | job.run() | 315 | job.run() |
396 | @@ -315,16 +318,17 @@ | |||
397 | 315 | new_branch.repository._format.get_format_string(), | 318 | new_branch.repository._format.get_format_string(), |
398 | 316 | 'Bazaar repository format 2a (needs bzr 1.16 or later)\n') | 319 | 'Bazaar repository format 2a (needs bzr 1.16 or later)\n') |
399 | 317 | 320 | ||
400 | 321 | self.becomeDbUser(dbuser) | ||
401 | 318 | self.assertFalse(db_branch.needs_upgrading) | 322 | self.assertFalse(db_branch.needs_upgrading) |
402 | 319 | 323 | ||
403 | 320 | def test_needs_no_upgrading(self): | 324 | def test_needs_no_upgrading(self): |
406 | 321 | # Branch upgrade job creation should raise an AssertionError if the | 325 | # Branch upgrade job creation should raise an AlreadyLatestFormat if |
407 | 322 | # branch does not need to be upgraded. | 326 | # the branch does not need to be upgraded. |
408 | 323 | branch = self.factory.makeAnyBranch( | 327 | branch = self.factory.makeAnyBranch( |
409 | 324 | branch_format=BranchFormat.BZR_BRANCH_7, | 328 | branch_format=BranchFormat.BZR_BRANCH_7, |
410 | 325 | repository_format=RepositoryFormat.BZR_CHK_2A) | 329 | repository_format=RepositoryFormat.BZR_CHK_2A) |
411 | 326 | self.assertRaises( | 330 | self.assertRaises( |
413 | 327 | AssertionError, BranchUpgradeJob.create, branch, | 331 | AlreadyLatestFormat, BranchUpgradeJob.create, branch, |
414 | 328 | self.factory.makePerson()) | 332 | self.factory.makePerson()) |
415 | 329 | 333 | ||
416 | 330 | def create_knit(self): | 334 | def create_knit(self): |
Looks good. Thanks for fixing this!