Merge ppa-dev-tools:fix-lp1990244-create-for-owner into ppa-dev-tools:main
- Git
- lp:ppa-dev-tools
- fix-lp1990244-create-for-owner
- Merge into main
Status: | Merged |
---|---|
Merged at revision: | 9dbaa6f713ae958ad415607c771549434fa1db93 |
Proposed branch: | ppa-dev-tools:fix-lp1990244-create-for-owner |
Merge into: | ppa-dev-tools:main |
Diff against target: |
732 lines (+207/-111) 8 files modified
NEWS.md (+9/-0) ppa/ppa.py (+35/-29) ppa/ppa_group.py (+9/-9) scripts/ppa (+38/-18) tests/helpers.py (+3/-2) tests/test_ppa.py (+32/-32) tests/test_ppa_group.py (+5/-5) tests/test_scripts_ppa.py (+76/-16) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Lena Voytek (community) | Approve | ||
Canonical Server packageset reviewers | Pending | ||
PpaDevTools Developers | Pending | ||
Canonical Server Reporter | Pending | ||
Review via email: mp+443452@code.launchpad.net |
Commit message
Description of the change
This adds the ability to create PPAs against other teams that your LP user account has access to, fixing LP: #1990244. You can specify the team by a command line argument, config file variable, or the ppa address.
For example:
$ ./scripts/ppa create --team ppa-dev-tools-devs foobar
PPA 'foobar' created for the following architectures:
i386, amd64, armhf, ppc64el, s390x, arm64, powerpc
The PPA can be viewed at:
https:/
...
$ ./scripts/ppa create ppa-dev-
PPA 'example' created for the following architectures:
i386, amd64, armhf, ppc64el, s390x, arm64, powerpc
The PPA can be viewed at:
https:/
...
Christian Ehrhardt (paelzer) wrote : | # |
Lena Voytek (lvoytek) wrote : | # |
LGTM, added a few comments below for formatting/
Bryce Harrington (bryce) wrote : | # |
Thanks, I've updated with the changes as suggested and landed:
Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
To git+ssh:
2a8e2ef..9dbaa6f main -> main
Preview Diff
1 | diff --git a/NEWS.md b/NEWS.md |
2 | index 61c117e..e6bdaeb 100644 |
3 | --- a/NEWS.md |
4 | +++ b/NEWS.md |
5 | @@ -9,6 +9,15 @@ jobs that it is monitoring are failed builds. These options are aimed |
6 | to facilitate CI/CD integration, but can also improve performance of the |
7 | waiting operation on larger PPAs. |
8 | |
9 | +It is now possible to create PPAs under a different team's ownership via |
10 | +the --owner option: |
11 | + |
12 | + $ ppa create --owner foobar my-ppa |
13 | + |
14 | +As a convenience, this can also be specified in ppa address form, i.e.: |
15 | + |
16 | + $ ppa create ppa:foobar/my-ppa |
17 | + |
18 | |
19 | # 0.4.0 # |
20 | |
21 | diff --git a/ppa/ppa.py b/ppa/ppa.py |
22 | index 6e3a071..0cccb10 100755 |
23 | --- a/ppa/ppa.py |
24 | +++ b/ppa/ppa.py |
25 | @@ -36,14 +36,15 @@ class PendingReason(enum.Enum): |
26 | class PpaDoesNotExist(BaseException): |
27 | """Exception indicating a requested PPA could not be found.""" |
28 | |
29 | - def __init__(self, ppa_name, team_name, message=None): |
30 | + def __init__(self, ppa_name, owner_name, message=None): |
31 | """Initializes the exception object. |
32 | |
33 | :param str ppa_name: The name of the missing PPA. |
34 | + :param str owner_name: The person or team the PPA belongs to. |
35 | :param str message: An error message. |
36 | """ |
37 | self.ppa_name = ppa_name |
38 | - self.team_name = team_name |
39 | + self.owner_name = owner_name |
40 | self.message = message |
41 | |
42 | def __str__(self): |
43 | @@ -54,7 +55,7 @@ class PpaDoesNotExist(BaseException): |
44 | """ |
45 | if self.message: |
46 | return self.message |
47 | - return f"The PPA '{self.ppa_name}' does not exist for team or user '{self.team_name}'" |
48 | + return f"The PPA '{self.ppa_name}' does not exist for person or team '{self.owner_name}'" |
49 | |
50 | |
51 | class Ppa: |
52 | @@ -70,25 +71,27 @@ class Ppa: |
53 | " + Launchpad Build Page: {build_page}\n" |
54 | ) |
55 | |
56 | - def __init__(self, ppa_name, team_name, ppa_description=None, service=None): |
57 | + def __init__(self, ppa_name, owner_name, ppa_description=None, service=None): |
58 | """Initializes a new Ppa object for a given PPA. |
59 | |
60 | This creates only the local representation of the PPA, it does |
61 | not cause a new PPA to be created in Launchpad. For that, see |
62 | PpaGroup.create() |
63 | |
64 | - :param str ppa_name: The name of the PPA within the team's namespace. |
65 | - :param str team_name: The name of the team or user that owns the PPA. |
66 | + :param str ppa_name: The name of the PPA within the owning |
67 | + person or team's namespace. |
68 | + :param str owner_name: The name of the person or team the PPA |
69 | + belongs to. |
70 | :param str ppa_description: Optional description text for the PPA. |
71 | :param launchpadlib.service service: The Launchpad service object. |
72 | """ |
73 | if not ppa_name: |
74 | raise ValueError("undefined ppa_name.") |
75 | - if not team_name: |
76 | - raise ValueError("undefined team_name.") |
77 | + if not owner_name: |
78 | + raise ValueError("undefined owner_name.") |
79 | |
80 | self.ppa_name = ppa_name |
81 | - self.team_name = team_name |
82 | + self.owner_name = owner_name |
83 | if ppa_description is None: |
84 | self.ppa_description = '' |
85 | else: |
86 | @@ -102,7 +105,7 @@ class Ppa: |
87 | :returns: Official string representation of the object. |
88 | """ |
89 | return (f'{self.__class__.__name__}(' |
90 | - f'ppa_name={self.ppa_name!r}, team_name={self.team_name!r})') |
91 | + f'ppa_name={self.ppa_name!r}, owner_name={self.owner_name!r})') |
92 | |
93 | def __str__(self) -> str: |
94 | """Returns a displayable string identifying the PPA. |
95 | @@ -110,7 +113,7 @@ class Ppa: |
96 | :rtype: str |
97 | :returns: Displayable representation of the PPA. |
98 | """ |
99 | - return f"{self.team_name}/{self.name}" |
100 | + return f"{self.owner_name}/{self.name}" |
101 | |
102 | @property |
103 | @lru_cache |
104 | @@ -124,10 +127,10 @@ class Ppa: |
105 | if not self._service: |
106 | raise AttributeError("Ppa object not connected to the Launchpad service") |
107 | try: |
108 | - owner = self._service.people[self.team_name] |
109 | + owner = self._service.people[self.owner_name] |
110 | return owner.getPPAByName(name=self.ppa_name) |
111 | except NotFound: |
112 | - raise PpaDoesNotExist(self.ppa_name, self.team_name) |
113 | + raise PpaDoesNotExist(self.ppa_name, self.owner_name) |
114 | |
115 | @lru_cache |
116 | def exists(self) -> bool: |
117 | @@ -146,7 +149,7 @@ class Ppa: |
118 | :rtype: str |
119 | :returns: The full identification string for the PPA. |
120 | """ |
121 | - return "ppa:{}/{}".format(self.team_name, self.ppa_name) |
122 | + return "ppa:{}/{}".format(self.owner_name, self.ppa_name) |
123 | |
124 | @property |
125 | def name(self): |
126 | @@ -269,8 +272,8 @@ class Ppa: |
127 | base = self._service.API_ROOT_URL.rstrip('/') |
128 | new_ppa_deps = [] |
129 | for ppa_address in ppa_addresses: |
130 | - team_name, ppa_name = ppa_address_split(ppa_address) |
131 | - new_ppa_dep = f'{base}/~{team_name}/+archive/ubuntu/{ppa_name}' |
132 | + owner_name, ppa_name = ppa_address_split(ppa_address) |
133 | + new_ppa_dep = f'{base}/~{owner_name}/+archive/ubuntu/{ppa_name}' |
134 | new_ppa_deps.append(new_ppa_dep) |
135 | |
136 | # TODO: Remove all existing dependencies |
137 | @@ -529,44 +532,47 @@ class Ppa: |
138 | return [] |
139 | |
140 | |
141 | -def ppa_address_split(ppa_address, default_team=None): |
142 | - """Parse an address for a PPA into its team and name components. |
143 | +def ppa_address_split(ppa_address): |
144 | + """Parse an address for a PPA into its owner and name components. |
145 | |
146 | :param str ppa_address: A ppa name or address. |
147 | - :param str default_team: (Optional) name of team to use if missing. |
148 | :rtype: tuple(str, str) |
149 | - :returns: The team name and ppa name as a tuple, or (None, None) on error. |
150 | + :returns: The owner name and ppa name as a tuple, or (None, None) on error. |
151 | """ |
152 | + owner_name = None |
153 | + |
154 | if not ppa_address or len(ppa_address) < 2: |
155 | return (None, None) |
156 | if ppa_address.startswith('ppa:'): |
157 | if '/' not in ppa_address: |
158 | return (None, None) |
159 | rem = ppa_address.split('ppa:', 1)[1] |
160 | - team_name = rem.split('/', 1)[0] |
161 | + owner_name = rem.split('/', 1)[0] |
162 | ppa_name = rem.split('/', 1)[1] |
163 | elif ppa_address.startswith('http'): |
164 | # Only launchpad PPA urls are supported |
165 | m = re.search(r'https:\/\/launchpad\.net\/~([^/]+)\/\+archive\/ubuntu\/(.+)$', ppa_address) |
166 | if not m: |
167 | return (None, None) |
168 | - team_name = m.group(1) |
169 | + owner_name = m.group(1) |
170 | ppa_name = m.group(2) |
171 | elif '/' in ppa_address: |
172 | - team_name = ppa_address.split('/', 1)[0] |
173 | + owner_name = ppa_address.split('/', 1)[0] |
174 | ppa_name = ppa_address.split('/', 1)[1] |
175 | else: |
176 | - team_name = default_team |
177 | ppa_name = ppa_address |
178 | |
179 | - if (team_name |
180 | - and ppa_name |
181 | - and not (any(x.isupper() for x in team_name)) |
182 | + if owner_name is not None: |
183 | + if len(owner_name) < 1: |
184 | + return (None, None) |
185 | + owner_name = owner_name.lower() |
186 | + |
187 | + if (ppa_name |
188 | and not (any(x.isupper() for x in ppa_name)) |
189 | and ppa_name.isascii() |
190 | and '/' not in ppa_name |
191 | and len(ppa_name) > 1): |
192 | - return (team_name, ppa_name) |
193 | + return (owner_name, ppa_name) |
194 | |
195 | return (None, None) |
196 | |
197 | @@ -600,7 +606,7 @@ def get_ppa(lp, config): |
198 | """ |
199 | return Ppa( |
200 | ppa_name=config.get('ppa_name', None), |
201 | - team_name=config.get('team_name', None), |
202 | + owner_name=config.get('owner_name', None), |
203 | service=lp) |
204 | |
205 | |
206 | diff --git a/ppa/ppa_group.py b/ppa/ppa_group.py |
207 | index 59011ea..c9747a5 100755 |
208 | --- a/ppa/ppa_group.py |
209 | +++ b/ppa/ppa_group.py |
210 | @@ -8,7 +8,7 @@ |
211 | # Released under GNU GPLv2 or later, read the file 'LICENSE.GPLv2+' for |
212 | # more information. |
213 | |
214 | -"""A team or person that owns one or more PPAs in Launchpad.""" |
215 | +"""A person or team that owns one or more PPAs in Launchpad.""" |
216 | |
217 | from functools import lru_cache |
218 | from lazr.restfulclient.errors import BadRequest |
219 | @@ -42,7 +42,7 @@ class PpaAlreadyExists(BaseException): |
220 | |
221 | |
222 | class PpaGroup: |
223 | - """Represents a team or person that owns one or more PPAs. |
224 | + """Represents a person or team that owns one or more PPAs. |
225 | |
226 | This class provides a proxy object for interacting with collections |
227 | of PPA. |
228 | @@ -51,7 +51,7 @@ class PpaGroup: |
229 | """Initializes a new PpaGroup object for a named person or team. |
230 | |
231 | :param launchpadlib.service service: The Launchpad service object. |
232 | - :param str name: Launchpad username or team name. |
233 | + :param str name: Launchpad username for a person or team. |
234 | """ |
235 | if not service: |
236 | raise ValueError("undefined service.") |
237 | @@ -80,8 +80,8 @@ class PpaGroup: |
238 | |
239 | @property |
240 | @lru_cache |
241 | - def team(self): |
242 | - """The team that owns this collection of PPAs. |
243 | + def owner(self): |
244 | + """The person or team that owns this collection of PPAs. |
245 | |
246 | :rtype: launchpadlib.person |
247 | :returns: Launchpad person object that owns this PPA. |
248 | @@ -106,11 +106,11 @@ class PpaGroup: |
249 | } |
250 | |
251 | try: |
252 | - self.team.createPPA( |
253 | + self.owner.createPPA( |
254 | name=ppa_name, |
255 | **ppa_settings |
256 | ) |
257 | - self.team.lp_save() |
258 | + self.owner.lp_save() |
259 | except BadRequest as e: |
260 | if "You already have a PPA" in o2str(e.content): |
261 | raise PpaAlreadyExists(ppa_name, e.content) |
262 | @@ -127,7 +127,7 @@ class PpaGroup: |
263 | :rtype: Iterator[ppa.Ppa] |
264 | :returns: Each PPA in the group as a ppa.Ppa object. |
265 | """ |
266 | - for lp_ppa in self.team.ppas: |
267 | + for lp_ppa in self.owner.ppas: |
268 | if '-deletedppa' in lp_ppa.name: |
269 | continue |
270 | yield Ppa(lp_ppa.name, self.name, |
271 | @@ -140,7 +140,7 @@ class PpaGroup: |
272 | :rtype: ppa.Ppa |
273 | :returns: A Ppa object describing the named ppa. |
274 | """ |
275 | - lp_ppa = self.team.getPPAByName(name=ppa_name) |
276 | + lp_ppa = self.owner.getPPAByName(name=ppa_name) |
277 | if not lp_ppa: |
278 | return None |
279 | return Ppa(lp_ppa.name, self.name, |
280 | diff --git a/scripts/ppa b/scripts/ppa |
281 | index b6aebbb..ee97f4d 100755 |
282 | --- a/scripts/ppa |
283 | +++ b/scripts/ppa |
284 | @@ -254,6 +254,10 @@ def create_arg_parser() -> argparse.ArgumentParser: |
285 | create_parser.add_argument('ppa_name', metavar='ppa-name', |
286 | action='store', |
287 | help="Name of the PPA to be created") |
288 | + create_parser.add_argument('--owner-name', '--owner', '--team-name', '--team', metavar='NAME', |
289 | + action='store', |
290 | + default=None, |
291 | + help="Person or team to create PPA under, if not specified via the ppa address (defaults to current LP user)") |
292 | add_basic_config_options(create_parser) |
293 | |
294 | # Desc Command |
295 | @@ -395,7 +399,7 @@ def create_arg_parser() -> argparse.ArgumentParser: |
296 | DEFAULT_CONFIG = { |
297 | 'debug': False, |
298 | 'ppa_name': None, |
299 | - 'team_name': None, |
300 | + 'owner_name': None, |
301 | 'wait_seconds': 10.0 |
302 | } |
303 | |
304 | @@ -430,16 +434,32 @@ def create_config(lp: Lp, args: argparse.Namespace) -> dict[str, Any]: |
305 | for k, v in DEFAULT_CONFIG.items(): |
306 | config.setdefault(k, v) |
307 | |
308 | - lp_username = None |
309 | - if lp.me: |
310 | - lp_username = lp.me.name |
311 | if not hasattr(args, 'ppa_name'): |
312 | warn("No ppa name given") |
313 | return None |
314 | |
315 | - config['team_name'], config['ppa_name'] = ppa_address_split(args.ppa_name, lp_username) |
316 | - if not config['team_name'] or not config['ppa_name']: |
317 | + owner_name, ppa_name = ppa_address_split(args.ppa_name) |
318 | + if owner_name: |
319 | + # First use the owner if present in the PPA address itself, |
320 | + # overriding any configured defaults or specified arguments. |
321 | + config['owner_name'] = owner_name |
322 | + elif config.get('owner_name'): |
323 | + # Next use any owner name from config file or cli args. |
324 | + pass |
325 | + elif config.get('team_name'): |
326 | + # Support legacy config term 'team_name' as alias for 'owner_name' |
327 | + config['owner_name'] = config['team_name'] |
328 | + del config['team_name'] |
329 | + elif lp.me: |
330 | + # Lastly, fallback to the current Launchpad username, if available. |
331 | + config['owner_name'] = lp.me.name |
332 | + else: |
333 | + warn("No owning person or team identified for the PPA") |
334 | + return None |
335 | + |
336 | + if not ppa_name: |
337 | raise ValueError("Invalid ppa name '{}'".format(args.ppa_name)) |
338 | + config['ppa_name'] = ppa_name |
339 | |
340 | if args.dry_run: |
341 | config['dry_run'] = True |
342 | @@ -472,9 +492,9 @@ def command_create(lp: Lp, config: dict[str, str]) -> int: |
343 | warn("Could not determine PPA name") |
344 | return os.EX_USAGE |
345 | |
346 | - team_name = config.get('team_name') |
347 | - if not team_name: |
348 | - warn("Could not determine team name") |
349 | + owner_name = config.get('owner_name') |
350 | + if not owner_name: |
351 | + warn("Could not determine owning person or team LP username") |
352 | return os.EX_USAGE |
353 | |
354 | publish = config.get('publish', None) |
355 | @@ -485,7 +505,7 @@ def command_create(lp: Lp, config: dict[str, str]) -> int: |
356 | |
357 | try: |
358 | if not config.get('dry_run', False): |
359 | - ppa_group = PpaGroup(service=lp, name=team_name) |
360 | + ppa_group = PpaGroup(service=lp, name=owner_name) |
361 | the_ppa = ppa_group.create(ppa_name, ppa_description=description) |
362 | the_ppa.set_publish(publish) |
363 | if architectures: |
364 | @@ -498,7 +518,7 @@ def command_create(lp: Lp, config: dict[str, str]) -> int: |
365 | the_ppa.set_dependencies(ppa_addresses) |
366 | |
367 | else: |
368 | - the_ppa = Ppa(ppa_name, team_name, description) |
369 | + the_ppa = Ppa(ppa_name, owner_name, description) |
370 | arch_str = ', '.join(architectures) |
371 | if not config.get('quiet', False): |
372 | print("PPA '{}' created for the following architectures:\n".format(the_ppa.ppa_name)) |
373 | @@ -590,16 +610,16 @@ def command_list(lp: Lp, config: dict[str, str], filter_func=None) -> int: |
374 | if not lp: |
375 | return 1 |
376 | |
377 | - team_name = config.get('team_name') |
378 | - if not team_name: |
379 | + owner_name = config.get('owner_name') |
380 | + if not owner_name: |
381 | if lp.me: |
382 | - team_name = lp.me.name |
383 | + owner_name = lp.me.name |
384 | else: |
385 | - warn("Could not determine team name") |
386 | + warn("Could not determine owning person or team name") |
387 | return os.EX_USAGE |
388 | |
389 | try: |
390 | - ppa_group = PpaGroup(service=lp, name=team_name) |
391 | + ppa_group = PpaGroup(service=lp, name=owner_name) |
392 | for p in ppa_group.ppas: |
393 | print(p.address) |
394 | return os.EX_OK |
395 | @@ -826,7 +846,7 @@ def command_tests(lp: Lp, config: dict[str, str]) -> int: |
396 | |
397 | the_ppa = get_ppa(lp, config) |
398 | if not the_ppa.exists(): |
399 | - error(f"PPA {the_ppa.name} does not exist for user {the_ppa.team_name}") |
400 | + error(f"PPA {the_ppa.name} does not exist for user {the_ppa.owner_name}") |
401 | return 1 |
402 | |
403 | architectures = config.get('architectures', ARCHES_AUTOPKGTEST) |
404 | @@ -909,7 +929,7 @@ def command_tests(lp: Lp, config: dict[str, str]) -> int: |
405 | results = [] |
406 | for release in releases: |
407 | base_results_fmt = f"{URL_AUTOPKGTEST}/results/autopkgtest-%s-%s-%s/" |
408 | - base_results_url = base_results_fmt % (release, the_ppa.team_name, the_ppa.name) |
409 | + base_results_url = base_results_fmt % (release, the_ppa.owner_name, the_ppa.name) |
410 | url = f"{base_results_url}?format=plain" |
411 | response = open_url(url) |
412 | if response: |
413 | diff --git a/tests/helpers.py b/tests/helpers.py |
414 | index 72a965e..c607d3c 100644 |
415 | --- a/tests/helpers.py |
416 | +++ b/tests/helpers.py |
417 | @@ -40,9 +40,10 @@ class ProcessorMock: |
418 | |
419 | class ArchiveMock: |
420 | """A stand-in for a Launchpad Archive object.""" |
421 | - def __init__(self, name, description): |
422 | + def __init__(self, name, description, owner): |
423 | self.displayname = name |
424 | self.description = description |
425 | + self.owner = owner |
426 | self.publish = True |
427 | self.processors = [ProcessorMock(proc_name) for proc_name in ARCHES_PPA_DEFAULT] |
428 | self.published_sources = [] |
429 | @@ -68,7 +69,7 @@ class PersonMock: |
430 | if ppa.name == name: |
431 | raise PpaAlreadyExists(name) |
432 | ppa = Ppa(name, self.name, description) |
433 | - Ppa.archive = ArchiveMock(ppa.name, ppa.description) |
434 | + Ppa.archive = ArchiveMock(ppa.name, ppa.description, self) |
435 | self._ppas.append(ppa) |
436 | return True |
437 | |
438 | diff --git a/tests/test_ppa.py b/tests/test_ppa.py |
439 | index 2410f83..9bb76e9 100644 |
440 | --- a/tests/test_ppa.py |
441 | +++ b/tests/test_ppa.py |
442 | @@ -21,59 +21,59 @@ from ppa.ppa import Ppa, ppa_address_split, get_ppa |
443 | |
444 | def test_object(): |
445 | """Check that PPA objects can be instantiated.""" |
446 | - ppa = Ppa('test-ppa-name', 'test-team-name') |
447 | + ppa = Ppa('test-ppa-name', 'test-owner-name') |
448 | assert ppa |
449 | |
450 | |
451 | def test_description(): |
452 | """Check specifying a description when creating a PPA.""" |
453 | - ppa = Ppa('test-ppa-name', 'test-team-name', 'test-description') |
454 | + ppa = Ppa('test-ppa-name', 'test-owner-name', 'test-description') |
455 | |
456 | assert 'test-description' in ppa.ppa_description |
457 | |
458 | |
459 | def test_address(): |
460 | """Check getting the PPA address.""" |
461 | - ppa = Ppa('test', 'team') |
462 | - assert ppa.address == "ppa:team/test" |
463 | + ppa = Ppa('test', 'owner') |
464 | + assert ppa.address == "ppa:owner/test" |
465 | |
466 | |
467 | -@pytest.mark.parametrize('address, default_team, expected', [ |
468 | +@pytest.mark.parametrize('address, expected', [ |
469 | # Successful cases |
470 | - ('bb', 'me', ('me', 'bb')), |
471 | - ('123', 'me', ('me', '123')), |
472 | - ('a/123', 'me', ('a', '123')), |
473 | - ('ppa:a/bb', None, ('a', 'bb')), |
474 | - ('ppa:ç/bb', None, ('ç', 'bb')), |
475 | - ('https://launchpad.net/~a/+archive/ubuntu/bb', None, ('a', 'bb')), |
476 | + ('bb', (None, 'bb')), |
477 | + ('123', (None, '123')), |
478 | + ('a/123', ('a', '123')), |
479 | + ('ppa:A/bb', ('a', 'bb')), |
480 | + ('ppa:a/bb', ('a', 'bb')), |
481 | + ('ppa:ç/bb', ('ç', 'bb')), |
482 | + ('https://launchpad.net/~a/+archive/ubuntu/bb', ('a', 'bb')), |
483 | |
484 | # Expected failure cases |
485 | - ('ppa:', None, (None, None)), |
486 | - (None, None, (None, None)), |
487 | - ('', None, (None, None)), |
488 | - ('/', None, (None, None)), |
489 | - (':/', None, (None, None)), |
490 | - ('////', None, (None, None)), |
491 | - ('ppa:/', None, (None, None)), |
492 | - ('ppa:a/', None, (None, None)), |
493 | - ('ppa:/bb', None, (None, None)), |
494 | - ('ppa:a/bç', None, (None, None)), |
495 | - ('ppa:A/bb', None, (None, None)), |
496 | - ('ppa/a/bb', None, (None, None)), |
497 | - ('ppa:a/bb/c', None, (None, None)), |
498 | - ('ppa:a/bB', None, (None, None)), |
499 | - ('http://launchpad.net/~a/+archive/ubuntu/bb', None, (None, None)), |
500 | - ('https://example.com/~a/+archive/ubuntu/bb', None, (None, None)), |
501 | - ('https://launchpad.net/~a/+archive/nobuntu/bb', None, (None, None)), |
502 | + ('ppa:', (None, None)), |
503 | + (None, (None, None)), |
504 | + ('', (None, None)), |
505 | + ('/', (None, None)), |
506 | + (':/', (None, None)), |
507 | + ('////', (None, None)), |
508 | + ('ppa:/', (None, None)), |
509 | + ('ppa:a/', (None, None)), |
510 | + ('ppa:/bb', (None, None)), |
511 | + ('ppa:a/bç', (None, None)), |
512 | + ('ppa/a/bb', (None, None)), |
513 | + ('ppa:a/bb/c', (None, None)), |
514 | + ('ppa:a/bB', (None, None)), |
515 | + ('http://launchpad.net/~a/+archive/ubuntu/bb', (None, None)), |
516 | + ('https://example.com/~a/+archive/ubuntu/bb', (None, None)), |
517 | + ('https://launchpad.net/~a/+archive/nobuntu/bb', (None, None)), |
518 | ]) |
519 | -def test_ppa_address_split(address, default_team, expected): |
520 | +def test_ppa_address_split(address, expected): |
521 | """Check ppa address input strings can be parsed properly.""" |
522 | - result = ppa_address_split(address, default_team=default_team) |
523 | + result = ppa_address_split(address) |
524 | assert result == expected |
525 | |
526 | |
527 | def test_get_ppa(): |
528 | - ppa = get_ppa(None, {'team_name': 'a', 'ppa_name': 'bb'}) |
529 | + ppa = get_ppa(None, {'owner_name': 'a', 'ppa_name': 'bb'}) |
530 | assert type(ppa) is Ppa |
531 | - assert ppa.team_name == 'a' |
532 | + assert ppa.owner_name == 'a' |
533 | assert ppa.ppa_name == 'bb' |
534 | diff --git a/tests/test_ppa_group.py b/tests/test_ppa_group.py |
535 | index d9d83de..f53799f 100644 |
536 | --- a/tests/test_ppa_group.py |
537 | +++ b/tests/test_ppa_group.py |
538 | @@ -59,14 +59,14 @@ def test_create_with_description(): |
539 | assert ppa.description == description |
540 | |
541 | |
542 | -def test_create_with_team(): |
543 | - """Check creating a PPA for a particular team.""" |
544 | +def test_create_with_owner(): |
545 | + """Check creating a PPA for a particular owner.""" |
546 | lp = LpServiceMock() |
547 | - lp.launchpad.add_person('test_team_name') |
548 | - ppa_group = PpaGroup(service=lp, name='test_team_name') |
549 | + lp.launchpad.add_person('test_owner_name') |
550 | + ppa_group = PpaGroup(service=lp, name='test_owner_name') |
551 | ppa = ppa_group.create('ppa_test_name') |
552 | assert ppa is not None |
553 | - assert ppa.address == 'ppa:test_team_name/ppa_test_name' |
554 | + assert ppa.address == 'ppa:test_owner_name/ppa_test_name' |
555 | |
556 | |
557 | def test_list_ppas(): |
558 | diff --git a/tests/test_scripts_ppa.py b/tests/test_scripts_ppa.py |
559 | index 80347ba..f82a9e4 100644 |
560 | --- a/tests/test_scripts_ppa.py |
561 | +++ b/tests/test_scripts_ppa.py |
562 | @@ -53,7 +53,7 @@ loader.exec_module(script) |
563 | def fake_config(): |
564 | return { |
565 | 'ppa_name': 'testing', |
566 | - 'team_name': 'me', |
567 | + 'owner_name': 'me', |
568 | 'wait_seconds': 0.1, |
569 | 'quiet': True |
570 | } |
571 | @@ -257,6 +257,39 @@ def test_create_arg_parser_basic_config(command): |
572 | args.publish = None |
573 | |
574 | |
575 | +def test_create_arg_parser_create(): |
576 | + """Checks argument parsing for the 'create' command. |
577 | + |
578 | + Most of the create command's args are covered by |
579 | + test_create_arg_parser_basic_config(), this just verifies |
580 | + the few that aren't. |
581 | + """ |
582 | + parser = script.create_arg_parser() |
583 | + command = 'create' |
584 | + |
585 | + # Check ppa_name |
586 | + args = parser.parse_args([command, 'test-ppa']) |
587 | + assert args.ppa_name == 'test-ppa' |
588 | + args = parser.parse_args([command, 'my-team/test-ppa']) |
589 | + assert args.ppa_name == 'my-team/test-ppa' |
590 | + args = parser.parse_args([command, 'ppa:my-team/test-ppa']) |
591 | + assert args.ppa_name == 'ppa:my-team/test-ppa' |
592 | + |
593 | + # Check --owner, --owner-name, --team, --team-name |
594 | + args = parser.parse_args([command, 'test-ppa', '--owner', 'x']) |
595 | + assert args.owner_name == 'x' |
596 | + args.owner_name = None |
597 | + args = parser.parse_args([command, 'test-ppa', '--owner-name', 'x']) |
598 | + assert args.owner_name == 'x' |
599 | + args.owner_name = None |
600 | + args = parser.parse_args([command, 'test-ppa', '--team', 'x']) |
601 | + assert args.owner_name == 'x' |
602 | + args.owner_name = None |
603 | + args = parser.parse_args([command, 'test-ppa', '--team-name', 'x']) |
604 | + assert args.owner_name == 'x' |
605 | + args.owner_name = None |
606 | + |
607 | + |
608 | def test_create_arg_parser_show(): |
609 | """Checks argument parsing for the 'show' command.""" |
610 | parser = script.create_arg_parser() |
611 | @@ -387,13 +420,19 @@ def test_create_arg_parser_tests(): |
612 | |
613 | @pytest.mark.parametrize('command_line_options, expected_config', [ |
614 | pytest.param([], {}), |
615 | - (['status', 'ppa:aa/bb'], {'command': 'status', 'team_name': 'aa', 'ppa_name': 'bb'}), |
616 | - (['status', 'aa/bb'], {'team_name': 'aa', 'ppa_name': 'bb'}), |
617 | - (['status', 'bb'], {'team_name': 'me', 'ppa_name': 'bb'}), |
618 | + (['status', 'ppa:aa/bb'], {'command': 'status', 'owner_name': 'aa', 'ppa_name': 'bb'}), |
619 | + (['status', 'aa/bb'], {'owner_name': 'aa', 'ppa_name': 'bb'}), |
620 | + (['status', 'bb'], {'owner_name': 'me', 'ppa_name': 'bb'}), |
621 | (['--debug', 'status', 'ppa:aa/bb'], {'debug': True}), |
622 | (['--dry-run', 'status', 'ppa:aa/bb'], {'dry_run': True}), |
623 | (['--verbose', 'status', 'ppa:aa/bb'], {'verbose': True}), |
624 | (['--quiet', 'status', 'ppa:aa/bb'], {'quiet': True}), |
625 | + (['create', 'ppa:aa/bb'], {'command': 'create', 'owner_name': 'aa', 'ppa_name': 'bb'}), |
626 | + (['create', 'aa/bb'], {'command': 'create', 'owner_name': 'aa', 'ppa_name': 'bb'}), |
627 | + (['create', 'bb', '--owner', 'aa'], {'command': 'create', 'owner_name': 'aa', 'ppa_name': 'bb'}), |
628 | + (['create', 'bb', '--owner-name', 'aa'], {'command': 'create', 'owner_name': 'aa', 'ppa_name': 'bb'}), |
629 | + (['create', 'bb', '--team', 'aa'], {'command': 'create', 'owner_name': 'aa', 'ppa_name': 'bb'}), |
630 | + (['create', 'bb', '--team-name', 'aa'], {'command': 'create', 'owner_name': 'aa', 'ppa_name': 'bb'}), |
631 | ]) |
632 | def test_create_config_from_args(command_line_options, expected_config): |
633 | '''Checks creation of a config object from an argparser object. |
634 | @@ -462,8 +501,8 @@ def test_command_create(fake_config, monkeypatch, stdin, params, expected_ppa_co |
635 | assert script.command_create(lp, config) == 0 |
636 | |
637 | # Retrieve the newly created PPA |
638 | - team = lp.people[config['team_name']] |
639 | - lp_ppa = team.getPPAByName(config['ppa_name']) |
640 | + owner = lp.people[config['owner_name']] |
641 | + lp_ppa = owner.getPPAByName(config['ppa_name']) |
642 | assert lp_ppa |
643 | |
644 | # Verify the expected items are present in the new PPA |
645 | @@ -471,6 +510,27 @@ def test_command_create(fake_config, monkeypatch, stdin, params, expected_ppa_co |
646 | assert getattr(lp_ppa, key) == value |
647 | |
648 | |
649 | +@pytest.mark.parametrize('params, owner_name', [ |
650 | + # Defaults |
651 | + ({'owner_name': 'a', 'ppa_name': 'x'}, 'a'), |
652 | +]) |
653 | +def test_command_create_with_owner(fake_config, monkeypatch, params, owner_name): |
654 | + '''Checks create command produces a PPA for a specified owner.''' |
655 | + lp = LpServiceMock() |
656 | + lp.launchpad.add_person(owner_name) |
657 | + monkeypatch.setattr("sys.stdin", io.StringIO('x')) |
658 | + |
659 | + # Check success of the create command |
660 | + config = {**fake_config, **params} |
661 | + print(config) |
662 | + assert script.command_create(lp, config) == 0 |
663 | + |
664 | + # Retrieve the newly created PPA |
665 | + owner = lp.people[owner_name] |
666 | + lp_ppa = owner.getPPAByName(config['ppa_name']) |
667 | + assert lp_ppa |
668 | + |
669 | + |
670 | @pytest.mark.parametrize('architectures, expected_processors', [ |
671 | (None, ARCHES_PPA_DEFAULT), |
672 | ('a', ['a']), |
673 | @@ -487,8 +547,8 @@ def test_command_create_with_architectures(monkeypatch, fake_config, architectur |
674 | assert script.command_create(lp, config) == 0 |
675 | |
676 | # Retrieve the newly created PPA |
677 | - team = lp.people[config['team_name']] |
678 | - lp_ppa = team.getPPAByName(config['ppa_name']) |
679 | + owner = lp.people[config['owner_name']] |
680 | + lp_ppa = owner.getPPAByName(config['ppa_name']) |
681 | |
682 | # Check processor architectures |
683 | assert lp_ppa.processors |
684 | @@ -549,15 +609,15 @@ def test_command_set(fake_config, params, expected_ppa_config): |
685 | lp = LpServiceMock() |
686 | |
687 | # Create a default PPA, for modification later |
688 | - team = lp.people[fake_config['team_name']] |
689 | - team.createPPA(fake_config['ppa_name'], 'x', 'y') |
690 | + owner = lp.people[fake_config['owner_name']] |
691 | + owner.createPPA(fake_config['ppa_name'], 'x', 'y') |
692 | |
693 | # Check success of the set command |
694 | config = {**fake_config, **params} |
695 | assert script.command_set(lp, config) |
696 | |
697 | # Retrieve the PPA we created earlier |
698 | - lp_ppa = team.getPPAByName(fake_config['ppa_name']) |
699 | + lp_ppa = owner.getPPAByName(fake_config['ppa_name']) |
700 | |
701 | # Verify the expected items are present in the updated PPA |
702 | for key, value in expected_ppa_config.items(): |
703 | @@ -577,15 +637,15 @@ def test_command_set_architectures(fake_config, architectures, expected_processo |
704 | lp = LpServiceMock() |
705 | |
706 | # Create a default PPA, for modification later |
707 | - team = lp.people[fake_config['team_name']] |
708 | - team.createPPA(fake_config['ppa_name'], 'x', 'y') |
709 | + owner = lp.people[fake_config['owner_name']] |
710 | + owner.createPPA(fake_config['ppa_name'], 'x', 'y') |
711 | |
712 | # Check success of the set command |
713 | config = {**fake_config, **{'architectures': architectures}} |
714 | assert script.command_set(lp, config) |
715 | |
716 | # Retrieve the PPA we created earlier |
717 | - lp_ppa = team.getPPAByName(fake_config['ppa_name']) |
718 | + lp_ppa = owner.getPPAByName(fake_config['ppa_name']) |
719 | |
720 | # Check processor architectures |
721 | assert lp_ppa.processors |
722 | @@ -629,8 +689,8 @@ def test_command_tests(urlopen_mock, |
723 | Ppa.get_autopkgtest_waiting = lambda x, y: [] |
724 | |
725 | # Create a default PPA, for modification later |
726 | - team = lp.people[fake_config['team_name']] |
727 | - team.createPPA(fake_config['ppa_name'], 'x', 'y') |
728 | + owner = lp.people[fake_config['owner_name']] |
729 | + owner.createPPA(fake_config['ppa_name'], 'x', 'y') |
730 | the_ppa = lp.me.getPPAByName(fake_config['ppa_name']) |
731 | |
732 | # Add some fake publications |
I like this, I do not feel I was able to do a full review - but for whatever it is worth it LGTM.
Furthermore I have left a few small comments inline for you to think about.