Merge lp:~thumper/launchpad/xmlrpc-tests into lp:launchpad/db-devel
- xmlrpc-tests
- Merge into db-devel
Proposed by
Tim Penhey
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 9758 | ||||
Proposed branch: | lp:~thumper/launchpad/xmlrpc-tests | ||||
Merge into: | lp:launchpad/db-devel | ||||
Diff against target: |
366 lines (+79/-145) 3 files modified
Makefile (+2/-2) lib/lp/code/xmlrpc/branch.py (+45/-15) lib/lp/code/xmlrpc/tests/test_branch.py (+32/-128) |
||||
To merge this branch: | bzr merge lp:~thumper/launchpad/xmlrpc-tests | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Graham Binns (community) | release-critical | Approve | |
Launchpad code reviewers | Pending | ||
Review via email: mp+34719@code.launchpad.net |
Commit message
Correctly resolve lp path names to use the +branch alias.
Description of the change
This branch addresses issues found when QA testing the new lp:short-name for private branches.
When going through the XMLRPC resolve_lp_path tests, I seem to have forgotten the complete point of the change, which was to not look up the branch in most cases.
In order to return the http urls for branches accessed using lp:foo a branch lookup is still needed until the branch rewrite map can understand the aliases.
Many tests were removed as they are now pointless as the checking for the linked branch has been passed on to the code path that resolves the aliased name (an earlier branch).
tests:
TestExpandURL
To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote : | # |
Revision history for this message
Graham Binns (gmb) : | # |
review:
Approve
(release-critical)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'Makefile' | |||
2 | --- Makefile 2010-09-05 20:29:40 +0000 | |||
3 | +++ Makefile 2010-09-07 05:51:45 +0000 | |||
4 | @@ -234,7 +234,7 @@ | |||
5 | 234 | -r librarian,google-webservice | 234 | -r librarian,google-webservice |
6 | 235 | > ${LPCONFIG}-nohup.out 2>&1 & | 235 | > ${LPCONFIG}-nohup.out 2>&1 & |
7 | 236 | 236 | ||
9 | 237 | run_all: check_schema inplace stop hosted_branches | 237 | run_all: check_schema inplace stop |
10 | 238 | $(RM) thread*.request | 238 | $(RM) thread*.request |
11 | 239 | bin/run -r librarian,sftp,mailman,codebrowse,google-webservice,memcached \ | 239 | bin/run -r librarian,sftp,mailman,codebrowse,google-webservice,memcached \ |
12 | 240 | -i $(LPCONFIG) | 240 | -i $(LPCONFIG) |
13 | @@ -248,7 +248,7 @@ | |||
14 | 248 | stop_codebrowse: | 248 | stop_codebrowse: |
15 | 249 | $(PY) scripts/stop-loggerhead.py | 249 | $(PY) scripts/stop-loggerhead.py |
16 | 250 | 250 | ||
18 | 251 | run_codehosting: check_schema inplace stop hosted_branches | 251 | run_codehosting: check_schema inplace stop |
19 | 252 | $(RM) thread*.request | 252 | $(RM) thread*.request |
20 | 253 | bin/run -r librarian,sftp,codebrowse -i $(LPCONFIG) | 253 | bin/run -r librarian,sftp,codebrowse -i $(LPCONFIG) |
21 | 254 | 254 | ||
22 | 255 | 255 | ||
23 | === modified file 'lib/lp/code/xmlrpc/branch.py' | |||
24 | --- lib/lp/code/xmlrpc/branch.py 2010-09-02 21:56:09 +0000 | |||
25 | +++ lib/lp/code/xmlrpc/branch.py 2010-09-07 05:51:45 +0000 | |||
26 | @@ -15,6 +15,8 @@ | |||
27 | 15 | ] | 15 | ] |
28 | 16 | 16 | ||
29 | 17 | 17 | ||
30 | 18 | from xmlrpclib import Fault | ||
31 | 19 | |||
32 | 18 | from bzrlib import urlutils | 20 | from bzrlib import urlutils |
33 | 19 | from zope.component import getUtility | 21 | from zope.component import getUtility |
34 | 20 | from zope.interface import ( | 22 | from zope.interface import ( |
35 | @@ -214,9 +216,9 @@ | |||
36 | 214 | 'http': _compose_http_url, | 216 | 'http': _compose_http_url, |
37 | 215 | } | 217 | } |
38 | 216 | 218 | ||
42 | 217 | def _getResultDict(self, branch, lp_path, suffix=None, | 219 | def _getUrlsForBranch(self, branch, lp_path, suffix=None, |
43 | 218 | supported_schemes=None): | 220 | supported_schemes=None): |
44 | 219 | """Return a result dict with a list of URLs for the given branch. | 221 | """Return a list of URLs for the given branch. |
45 | 220 | 222 | ||
46 | 221 | :param branch: A Branch object. | 223 | :param branch: A Branch object. |
47 | 222 | :param lp_path: The path that was used to traverse to the branch. | 224 | :param lp_path: The path that was used to traverse to the branch. |
48 | @@ -226,8 +228,8 @@ | |||
49 | 226 | """ | 228 | """ |
50 | 227 | if branch.branch_type == BranchType.REMOTE: | 229 | if branch.branch_type == BranchType.REMOTE: |
51 | 228 | if branch.url is None: | 230 | if branch.url is None: |
54 | 229 | return faults.NoUrlForBranch(branch.unique_name) | 231 | raise faults.NoUrlForBranch(branch.unique_name) |
55 | 230 | return dict(urls=[branch.url]) | 232 | return [branch.url] |
56 | 231 | else: | 233 | else: |
57 | 232 | return self._getUniqueNameResultDict( | 234 | return self._getUniqueNameResultDict( |
58 | 233 | branch.unique_name, suffix, supported_schemes, lp_path) | 235 | branch.unique_name, suffix, supported_schemes, lp_path) |
59 | @@ -238,11 +240,8 @@ | |||
60 | 238 | supported_schemes = SUPPORTED_SCHEMES | 240 | supported_schemes = SUPPORTED_SCHEMES |
61 | 239 | if path is None: | 241 | if path is None: |
62 | 240 | path = unique_name | 242 | path = unique_name |
68 | 241 | result = dict(urls=[]) | 243 | return [self.scheme_funcs[scheme](unique_name, path, suffix) |
69 | 242 | for scheme in supported_schemes: | 244 | for scheme in supported_schemes] |
65 | 243 | result['urls'].append( | ||
66 | 244 | self.scheme_funcs[scheme](unique_name, path, suffix)) | ||
67 | 245 | return result | ||
70 | 246 | 245 | ||
71 | 247 | @return_fault | 246 | @return_fault |
72 | 248 | def _resolve_lp_path(self, path): | 247 | def _resolve_lp_path(self, path): |
73 | @@ -254,11 +253,39 @@ | |||
74 | 254 | strip_path = path.strip('/') | 253 | strip_path = path.strip('/') |
75 | 255 | if strip_path == '': | 254 | if strip_path == '': |
76 | 256 | raise faults.InvalidBranchIdentifier(path) | 255 | raise faults.InvalidBranchIdentifier(path) |
78 | 257 | supported_schemes = SUPPORTED_SCHEMES | 256 | supported_schemes = list(SUPPORTED_SCHEMES) |
79 | 258 | hot_products = [product.strip() for product | 257 | hot_products = [product.strip() for product |
80 | 259 | in config.codehosting.hot_products.split(',')] | 258 | in config.codehosting.hot_products.split(',')] |
83 | 260 | if strip_path in hot_products: | 259 | # If we have been given something that looks like a branch name, just |
84 | 261 | supported_schemes = ['http'] | 260 | # look that up. |
85 | 261 | if strip_path.startswith('~'): | ||
86 | 262 | urls = self._getBranchPaths(strip_path, supported_schemes) | ||
87 | 263 | else: | ||
88 | 264 | # We only check the hot product code when accessed through the | ||
89 | 265 | # short name, so we can check it here. | ||
90 | 266 | if strip_path in hot_products: | ||
91 | 267 | supported_schemes = ['http'] | ||
92 | 268 | urls = [] | ||
93 | 269 | else: | ||
94 | 270 | urls = [self.scheme_funcs['bzr+ssh'](None, strip_path, None)] | ||
95 | 271 | supported_schemes.remove('bzr+ssh') | ||
96 | 272 | # Try to look up the branch at that url and add alternative URLs. | ||
97 | 273 | # This may well fail, and if it does, we just return the aliased | ||
98 | 274 | # url. | ||
99 | 275 | try: | ||
100 | 276 | urls.extend( | ||
101 | 277 | self._getBranchPaths(strip_path, supported_schemes)) | ||
102 | 278 | except Fault: | ||
103 | 279 | pass | ||
104 | 280 | return dict(urls=urls) | ||
105 | 281 | |||
106 | 282 | def _getBranchPaths(self, strip_path, supported_schemes): | ||
107 | 283 | """Get the specific paths for a branch. | ||
108 | 284 | |||
109 | 285 | If the branch is not found, but it looks like a branch name, then we | ||
110 | 286 | return a writable URL for it. If it doesn't look like a branch name a | ||
111 | 287 | fault is raised. | ||
112 | 288 | """ | ||
113 | 262 | branch_set = getUtility(IBranchLookup) | 289 | branch_set = getUtility(IBranchLookup) |
114 | 263 | try: | 290 | try: |
115 | 264 | branch, suffix = branch_set.getByLPPath(strip_path) | 291 | branch, suffix = branch_set.getByLPPath(strip_path) |
116 | @@ -267,7 +294,9 @@ | |||
117 | 267 | # resolve it anyway, treating the path like a branch's unique | 294 | # resolve it anyway, treating the path like a branch's unique |
118 | 268 | # name. This lets people push new branches up to Launchpad using | 295 | # name. This lets people push new branches up to Launchpad using |
119 | 269 | # lp: URL syntax. | 296 | # lp: URL syntax. |
121 | 270 | return self._getUniqueNameResultDict(strip_path) | 297 | supported_schemes = ['bzr+ssh'] |
122 | 298 | return self._getUniqueNameResultDict( | ||
123 | 299 | strip_path, supported_schemes=supported_schemes) | ||
124 | 271 | # XXX: JonathanLange 2009-03-21 bug=347728: All of this is repetitive | 300 | # XXX: JonathanLange 2009-03-21 bug=347728: All of this is repetitive |
125 | 272 | # and thus error prone. Alternatives are directly raising faults from | 301 | # and thus error prone. Alternatives are directly raising faults from |
126 | 273 | # the model code(blech) or some automated way of reraising as faults | 302 | # the model code(blech) or some automated way of reraising as faults |
127 | @@ -298,7 +327,8 @@ | |||
128 | 298 | # E.g. 'project/trunk/filename.txt' the suffix is 'filename.txt' | 327 | # E.g. 'project/trunk/filename.txt' the suffix is 'filename.txt' |
129 | 299 | # we want lp_path to be 'project/trunk'. | 328 | # we want lp_path to be 'project/trunk'. |
130 | 300 | lp_path = lp_path[:-(len(suffix)+1)] | 329 | lp_path = lp_path[:-(len(suffix)+1)] |
132 | 301 | return self._getResultDict(branch, lp_path, suffix, supported_schemes) | 330 | return self._getUrlsForBranch( |
133 | 331 | branch, lp_path, suffix, supported_schemes) | ||
134 | 302 | 332 | ||
135 | 303 | def resolve_lp_path(self, path): | 333 | def resolve_lp_path(self, path): |
136 | 304 | """See `IPublicCodehostingAPI`.""" | 334 | """See `IPublicCodehostingAPI`.""" |
137 | 305 | 335 | ||
138 | === modified file 'lib/lp/code/xmlrpc/tests/test_branch.py' | |||
139 | --- lib/lp/code/xmlrpc/tests/test_branch.py 2010-09-03 04:22:16 +0000 | |||
140 | +++ lib/lp/code/xmlrpc/tests/test_branch.py 2010-09-07 05:51:45 +0000 | |||
141 | @@ -59,7 +59,10 @@ | |||
142 | 59 | if lp_path is None: | 59 | if lp_path is None: |
143 | 60 | ssh_branch_path = public_branch_path | 60 | ssh_branch_path = public_branch_path |
144 | 61 | else: | 61 | else: |
146 | 62 | ssh_branch_path = '%s/%s' % (BRANCH_ALIAS_PREFIX, lp_path) | 62 | if lp_path.startswith('~'): |
147 | 63 | ssh_branch_path = lp_path | ||
148 | 64 | else: | ||
149 | 65 | ssh_branch_path = '%s/%s' % (BRANCH_ALIAS_PREFIX, lp_path) | ||
150 | 63 | # This improves the error message if results happens to be a fault. | 66 | # This improves the error message if results happens to be a fault. |
151 | 64 | if isinstance(results, LaunchpadFault): | 67 | if isinstance(results, LaunchpadFault): |
152 | 65 | raise results | 68 | raise results |
153 | @@ -70,6 +73,10 @@ | |||
154 | 70 | else: | 73 | else: |
155 | 71 | self.assertEqual('/' + ssh_branch_path, uri.path) | 74 | self.assertEqual('/' + ssh_branch_path, uri.path) |
156 | 72 | 75 | ||
157 | 76 | def assertOnlyWritableResolves(self, lp_url_path): | ||
158 | 77 | """Only the bzr+ssh url is returned.""" | ||
159 | 78 | self.assertResolves(lp_url_path, None, lp_url_path) | ||
160 | 79 | |||
161 | 73 | def assertFault(self, lp_url_path, expected_fault): | 80 | def assertFault(self, lp_url_path, expected_fault): |
162 | 74 | """Trying to resolve lp_url_path raises the expected fault.""" | 81 | """Trying to resolve lp_url_path raises the expected fault.""" |
163 | 75 | api = PublicCodehostingAPI(None, None) | 82 | api = PublicCodehostingAPI(None, None) |
164 | @@ -122,54 +129,15 @@ | |||
165 | 122 | lp_path='%s/%s' % (product.name, product.development_focus.name) | 129 | lp_path='%s/%s' % (product.name, product.development_focus.name) |
166 | 123 | self.assertResolves(lp_path, trunk.unique_name, lp_path) | 130 | self.assertResolves(lp_path, trunk.unique_name, lp_path) |
167 | 124 | 131 | ||
212 | 125 | def test_product_doesnt_exist(self): | 132 | def test_target_doesnt_exist(self): |
213 | 126 | # Return a NoSuchProduct fault if the product doesn't exist. | 133 | # The resolver doesn't care if the product exists or not. |
214 | 127 | self.assertFault( | 134 | self.assertOnlyWritableResolves('doesntexist') |
215 | 128 | 'doesntexist', faults.NoSuchProduct('doesntexist')) | 135 | self.assertOnlyWritableResolves('doesntexist/trunk') |
172 | 129 | self.assertFault( | ||
173 | 130 | 'doesntexist/trunk', faults.NoSuchProduct('doesntexist')) | ||
174 | 131 | |||
175 | 132 | def test_project_group(self): | ||
176 | 133 | # Resolving lp:///project_group_name' should explain that project | ||
177 | 134 | # groups don't have default branches. | ||
178 | 135 | project_group = self.factory.makeProject() | ||
179 | 136 | fault = self.assertFault( | ||
180 | 137 | project_group.name, faults.CannotHaveLinkedBranch(project_group)) | ||
181 | 138 | self.assertIn('project group', fault.faultString) | ||
182 | 139 | |||
183 | 140 | def test_distro_name(self): | ||
184 | 141 | # Resolving lp:///distro_name' should explain that distributions don't | ||
185 | 142 | # have default branches. | ||
186 | 143 | distro = self.factory.makeDistribution() | ||
187 | 144 | self.assertFault(distro.name, faults.CannotHaveLinkedBranch(distro)) | ||
188 | 145 | |||
189 | 146 | def test_distroseries_name(self): | ||
190 | 147 | # Resolving lp:///distro/series' should explain that distribution | ||
191 | 148 | # series don't have default branches. | ||
192 | 149 | series = self.factory.makeDistroSeries() | ||
193 | 150 | self.assertFault( | ||
194 | 151 | '%s/%s' % (series.distribution.name, series.name), | ||
195 | 152 | faults.CannotHaveLinkedBranch(series)) | ||
196 | 153 | |||
197 | 154 | def test_invalid_product_name(self): | ||
198 | 155 | # If we get a string that cannot be a name for a product where we | ||
199 | 156 | # expect the name of a product, we should error appropriately. | ||
200 | 157 | invalid_name = '_' + self.factory.getUniqueString() | ||
201 | 158 | self.assertFault( | ||
202 | 159 | invalid_name, | ||
203 | 160 | faults.InvalidProductIdentifier(invalid_name)) | ||
204 | 161 | |||
205 | 162 | def test_invalid_product_name_non_ascii(self): | ||
206 | 163 | # lp:<non-ascii-string> returns InvalidProductIdentifier with the name | ||
207 | 164 | # escaped. | ||
208 | 165 | invalid_name = '_' + NON_ASCII_NAME | ||
209 | 166 | self.assertFault( | ||
210 | 167 | invalid_name, | ||
211 | 168 | faults.InvalidProductIdentifier(urlutils.escape(invalid_name))) | ||
216 | 169 | 136 | ||
217 | 170 | def test_product_and_series(self): | 137 | def test_product_and_series(self): |
220 | 171 | # lp:product/series expands to the branch associated with the product | 138 | # lp:product/series expands to the writable alias for product/series |
221 | 172 | # series 'series' on 'product'. | 139 | # and to the branch associated with the product series 'series' on |
222 | 140 | # 'product'. | ||
223 | 173 | product = self.factory.makeProduct() | 141 | product = self.factory.makeProduct() |
224 | 174 | branch = branch=self.factory.makeProductBranch(product=product) | 142 | branch = branch=self.factory.makeProductBranch(product=product) |
225 | 175 | series = self.factory.makeSeries(product=product, branch=branch) | 143 | series = self.factory.makeSeries(product=product, branch=branch) |
226 | @@ -177,79 +145,23 @@ | |||
227 | 177 | self.assertResolves(lp_path, branch.unique_name, lp_path) | 145 | self.assertResolves(lp_path, branch.unique_name, lp_path) |
228 | 178 | 146 | ||
229 | 179 | def test_development_focus_has_no_branch(self): | 147 | def test_development_focus_has_no_branch(self): |
232 | 180 | # Return a NoLinkedBranch fault if the development focus has no branch | 148 | # A product with no trunk resolves to the writable alias. |
231 | 181 | # associated with it. | ||
233 | 182 | product = self.factory.makeProduct() | 149 | product = self.factory.makeProduct() |
236 | 183 | self.assertEqual(None, product.development_focus.branch) | 150 | self.assertOnlyWritableResolves(product.name) |
235 | 184 | self.assertFault(product.name, faults.NoLinkedBranch(product)) | ||
237 | 185 | 151 | ||
238 | 186 | def test_series_has_no_branch(self): | 152 | def test_series_has_no_branch(self): |
241 | 187 | # Return a NoLinkedBranch fault if the series has no branch | 153 | # A series with no branch resolves to the writable alias. |
240 | 188 | # associated with it. | ||
242 | 189 | series = self.factory.makeSeries(branch=None) | 154 | series = self.factory.makeSeries(branch=None) |
254 | 190 | self.assertFault( | 155 | self.assertOnlyWritableResolves( |
255 | 191 | '%s/%s' % (series.product.name, series.name), | 156 | '%s/%s' % (series.product.name, series.name)) |
245 | 192 | faults.NoLinkedBranch(series)) | ||
246 | 193 | |||
247 | 194 | def test_no_such_product_series(self): | ||
248 | 195 | # Return a NoSuchProductSeries fault if there is no series of the | ||
249 | 196 | # given name associated with the product. | ||
250 | 197 | product = self.factory.makeProduct() | ||
251 | 198 | self.assertFault( | ||
252 | 199 | '%s/%s' % (product.name, "doesntexist"), | ||
253 | 200 | faults.NoSuchProductSeries("doesntexist", product)) | ||
256 | 201 | 157 | ||
257 | 202 | def test_no_such_product_series_non_ascii(self): | 158 | def test_no_such_product_series_non_ascii(self): |
259 | 203 | # lp:product/<non-ascii-string> returns NoSuchProductSeries with the | 159 | # lp:product/<non-ascii-string> resolves to the branch alias with the |
260 | 204 | # name escaped. | 160 | # name escaped. |
261 | 205 | product = self.factory.makeProduct() | 161 | product = self.factory.makeProduct() |
309 | 206 | self.assertFault( | 162 | lp_path = '%s/%s' % (product.name, NON_ASCII_NAME) |
310 | 207 | '%s/%s' % (product.name, NON_ASCII_NAME), | 163 | escaped_name = urlutils.escape(lp_path) |
311 | 208 | faults.NoSuchProductSeries( | 164 | self.assertResolves(lp_path, None, escaped_name) |
265 | 209 | urlutils.escape(NON_ASCII_NAME), product)) | ||
266 | 210 | |||
267 | 211 | def test_no_such_distro_series(self): | ||
268 | 212 | # Return a NoSuchDistroSeries fault if there is no series of the given | ||
269 | 213 | # name on that distribution. | ||
270 | 214 | distro = self.factory.makeDistribution() | ||
271 | 215 | self.assertFault( | ||
272 | 216 | '%s/doesntexist/whocares' % distro.name, | ||
273 | 217 | faults.NoSuchDistroSeries("doesntexist")) | ||
274 | 218 | |||
275 | 219 | def test_no_such_distro_series_non_ascii(self): | ||
276 | 220 | # lp:distro/<non-ascii-string>/whatever returns NoSuchDistroSeries | ||
277 | 221 | # with the name escaped. | ||
278 | 222 | distro = self.factory.makeDistribution() | ||
279 | 223 | self.assertFault( | ||
280 | 224 | '%s/%s/whocares' % (distro.name, NON_ASCII_NAME), | ||
281 | 225 | faults.NoSuchDistroSeries(urlutils.escape(NON_ASCII_NAME))) | ||
282 | 226 | |||
283 | 227 | def test_no_such_source_package(self): | ||
284 | 228 | # Return a NoSuchSourcePackageName fault if there is no source package | ||
285 | 229 | # of the given name. | ||
286 | 230 | distroseries = self.factory.makeDistroRelease() | ||
287 | 231 | distribution = distroseries.distribution | ||
288 | 232 | self.assertFault( | ||
289 | 233 | '%s/%s/doesntexist' % (distribution.name, distroseries.name), | ||
290 | 234 | faults.NoSuchSourcePackageName('doesntexist')) | ||
291 | 235 | |||
292 | 236 | def test_no_such_source_package_non_ascii(self): | ||
293 | 237 | # lp:distro/series/<non-ascii-name> returns NoSuchSourcePackageName | ||
294 | 238 | # with the name escaped. | ||
295 | 239 | distroseries = self.factory.makeDistroRelease() | ||
296 | 240 | distribution = distroseries.distribution | ||
297 | 241 | self.assertFault( | ||
298 | 242 | '%s/%s/%s' % ( | ||
299 | 243 | distribution.name, distroseries.name, NON_ASCII_NAME), | ||
300 | 244 | faults.NoSuchSourcePackageName(urlutils.escape(NON_ASCII_NAME))) | ||
301 | 245 | |||
302 | 246 | def test_no_linked_branch_for_source_package(self): | ||
303 | 247 | # Return a NoLinkedBranch fault if there's no linked branch for the | ||
304 | 248 | # sourcepackage. | ||
305 | 249 | suite_sourcepackage = self.factory.makeSuiteSourcePackage() | ||
306 | 250 | self.assertFault( | ||
307 | 251 | suite_sourcepackage.path, | ||
308 | 252 | faults.NoLinkedBranch(suite_sourcepackage)) | ||
312 | 253 | 165 | ||
313 | 254 | def test_branch(self): | 166 | def test_branch(self): |
314 | 255 | # The unique name of a branch resolves to the unique name of the | 167 | # The unique name of a branch resolves to the unique name of the |
315 | @@ -396,35 +308,27 @@ | |||
316 | 396 | # private branch, and tests are currently running as an anonymous | 308 | # private branch, and tests are currently running as an anonymous |
317 | 397 | # user. | 309 | # user. |
318 | 398 | unique_name = removeSecurityProxy(branch).unique_name | 310 | unique_name = removeSecurityProxy(branch).unique_name |
320 | 399 | self.assertResolves(unique_name, unique_name) | 311 | self.assertOnlyWritableResolves(unique_name) |
321 | 400 | 312 | ||
322 | 401 | def test_private_branch_on_series(self): | 313 | def test_private_branch_on_series(self): |
326 | 402 | # We resolve invisible branches as if they don't exist. For | 314 | # We resolve private linked branches using the writable alias. |
324 | 403 | # references to product series, this means returning a | ||
325 | 404 | # NoLinkedBranch fault. | ||
327 | 405 | # | 315 | # |
328 | 406 | # Removing security proxy because we need to be able to get at | 316 | # Removing security proxy because we need to be able to get at |
329 | 407 | # attributes of a private branch and these tests are running as an | 317 | # attributes of a private branch and these tests are running as an |
330 | 408 | # anonymous user. | 318 | # anonymous user. |
331 | 409 | branch = self.factory.makeAnyBranch(private=True) | 319 | branch = self.factory.makeAnyBranch(private=True) |
332 | 410 | series = self.factory.makeSeries(branch=branch) | 320 | series = self.factory.makeSeries(branch=branch) |
336 | 411 | self.assertFault( | 321 | lp_path='%s/%s' % (series.product.name, series.name) |
337 | 412 | '%s/%s' % (series.product.name, series.name), | 322 | self.assertOnlyWritableResolves(lp_path) |
335 | 413 | faults.NoLinkedBranch(series)) | ||
338 | 414 | 323 | ||
339 | 415 | def test_private_branch_as_development_focus(self): | 324 | def test_private_branch_as_development_focus(self): |
346 | 416 | # We resolve invisible branches as if they don't exist. | 325 | # We resolve private linked branches using the writable alias. |
341 | 417 | # | ||
342 | 418 | # References to a product resolve to the branch associated with the | ||
343 | 419 | # development focus. If that branch is private, other views will | ||
344 | 420 | # indicate that there is no branch on the development focus. We do the | ||
345 | 421 | # same. | ||
347 | 422 | product, trunk = self.makeProdutWithTrunk() | 326 | product, trunk = self.makeProdutWithTrunk() |
348 | 423 | removeSecurityProxy(trunk).private = True | 327 | removeSecurityProxy(trunk).private = True |
350 | 424 | self.assertFault(product.name, faults.NoLinkedBranch(product)) | 328 | self.assertOnlyWritableResolves(product.name) |
351 | 425 | 329 | ||
352 | 426 | def test_private_branch_as_user(self): | 330 | def test_private_branch_as_user(self): |
354 | 427 | # We resolve invisible branches as if they don't exist. | 331 | # We resolve private branches as if they don't exist. |
355 | 428 | # | 332 | # |
356 | 429 | # References to a product resolve to the branch associated with the | 333 | # References to a product resolve to the branch associated with the |
357 | 430 | # development focus. If that branch is private, other views will | 334 | # development focus. If that branch is private, other views will |
358 | @@ -436,7 +340,7 @@ | |||
359 | 436 | owner = self.factory.makePerson() | 340 | owner = self.factory.makePerson() |
360 | 437 | branch = self.factory.makeAnyBranch(owner=owner, private=True) | 341 | branch = self.factory.makeAnyBranch(owner=owner, private=True) |
361 | 438 | path = removeSecurityProxy(branch).unique_name | 342 | path = removeSecurityProxy(branch).unique_name |
363 | 439 | self.assertResolves(path, path) | 343 | self.assertOnlyWritableResolves(path) |
364 | 440 | 344 | ||
365 | 441 | def test_remote_branch(self): | 345 | def test_remote_branch(self): |
366 | 442 | # For remote branches, return results that link to the actual remote | 346 | # For remote branches, return results that link to the actual remote |
The Makefile change was a drive-by that I needed in order to run some interactive testing. The hosted_branches make target calls a broken script right now, and that was out of scope for this change.