Merge ppa-dev-tools:set-command-private-ppa into ppa-dev-tools:main
- Git
- lp:ppa-dev-tools
- set-command-private-ppa
- Merge into main
Status: | Merged |
---|---|
Merge reported by: | Bryce Harrington |
Merged at revision: | a116288136ae712e726d5e8f7830b64ca77830ba |
Proposed branch: | ppa-dev-tools:set-command-private-ppa |
Merge into: | ppa-dev-tools:main |
Diff against target: |
395 lines (+173/-31) 7 files modified
ppa/ppa.py (+24/-0) ppa/ppa_group.py (+3/-1) scripts/ppa (+43/-21) tests/helpers.py (+5/-2) tests/smoketest_ppa_set (+70/-0) tests/test_ppa_group.py (+10/-0) tests/test_scripts_ppa.py (+18/-7) |
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+447032@code.launchpad.net |
Commit message
Description of the change
Implements support for creating private PPAs and toggling PPAs public vs. private.
Creating private PPAs requires special authorization that I don't seem to have, with the result that I've not been able to verify that this actually works correctly "live". However, I've included checks for some of the authorization errors I've hit, and implemented a slew of unit tests. I'll try to find someone with access to do the live verification, but apart from that I think this code is ready for review and landing.
As usual, to run the testsuite, do:
$ make check
or
$ pytest-3
And the smoketest has been updated to include toggling --private:
$ ./tests/
However as mentioned, unless you have authorization to modify privacy on PPAs that will merely generate errors.
Bryce Harrington (bryce) wrote : | # |
Thanks Lena, I've squashed that change in, updated this branch, and landed to main.
stirling: ~/src/PpaDevToo
Updating 4fa781a..0f7a075
Fast-forward
ppa/ppa.py | 24 +++++++
ppa/ppa_group.py | 4 +++-
scripts/ppa | 64 +++++++
tests/helpers.py | 7 +++++--
tests/
tests/
tests/
7 files changed, 173 insertions(+), 31 deletions(-)
create mode 100755 tests/smoketest
stirling: ~/src/PpaDevToo
Enumerating objects: 57, done.
Counting objects: 100% (57/57), done.
Delta compression using up to 12 threads
Compressing objects: 100% (40/40), done.
Writing objects: 100% (47/47), 6.94 KiB | 6.94 MiB/s, done.
Total 47 (delta 29), reused 0 (delta 0), pack-reused 0
To git+ssh:
4fa781a..0f7a075 main -> main
Preview Diff
1 | diff --git a/ppa/ppa.py b/ppa/ppa.py | |||
2 | index 2dd46eb..c4ce1fe 100755 | |||
3 | --- a/ppa/ppa.py | |||
4 | +++ b/ppa/ppa.py | |||
5 | @@ -198,6 +198,30 @@ class Ppa: | |||
6 | 198 | 198 | ||
7 | 199 | @property | 199 | @property |
8 | 200 | @lru_cache | 200 | @lru_cache |
9 | 201 | def is_private(self) -> bool: | ||
10 | 202 | """Indicates if the PPA is private or public. | ||
11 | 203 | |||
12 | 204 | :rtype: bool | ||
13 | 205 | :returns: True if the archive is private, False if public. | ||
14 | 206 | """ | ||
15 | 207 | return self.archive.private | ||
16 | 208 | |||
17 | 209 | def set_private(self, private: bool): | ||
18 | 210 | """Attempts to configure the PPA as private. | ||
19 | 211 | |||
20 | 212 | Note that PPAs can't be changed to private if they ever had any | ||
21 | 213 | sources published, or if the owning person or team is not | ||
22 | 214 | permitted to hold private PPAs. | ||
23 | 215 | |||
24 | 216 | :param bool private: Whether the PPA should be private or public. | ||
25 | 217 | """ | ||
26 | 218 | if private is None: | ||
27 | 219 | return | ||
28 | 220 | self.archive.private = private | ||
29 | 221 | self.archive.lp_save() | ||
30 | 222 | |||
31 | 223 | @property | ||
32 | 224 | @lru_cache | ||
33 | 201 | def publish(self): | 225 | def publish(self): |
34 | 202 | return self.archive.publish | 226 | return self.archive.publish |
35 | 203 | 227 | ||
36 | diff --git a/ppa/ppa_group.py b/ppa/ppa_group.py | |||
37 | index c9747a5..3061100 100755 | |||
38 | --- a/ppa/ppa_group.py | |||
39 | +++ b/ppa/ppa_group.py | |||
40 | @@ -88,13 +88,14 @@ class PpaGroup: | |||
41 | 88 | """ | 88 | """ |
42 | 89 | return self.service.people[self.name] | 89 | return self.service.people[self.name] |
43 | 90 | 90 | ||
45 | 91 | def create(self, ppa_name='ppa', ppa_description=None): | 91 | def create(self, ppa_name='ppa', ppa_description=None, private=False): |
46 | 92 | """Registers a new PPA with Launchpad. | 92 | """Registers a new PPA with Launchpad. |
47 | 93 | 93 | ||
48 | 94 | If a description is not provided a default one will be generated. | 94 | If a description is not provided a default one will be generated. |
49 | 95 | 95 | ||
50 | 96 | :param str ppa_name: Name for the PPA to create. | 96 | :param str ppa_name: Name for the PPA to create. |
51 | 97 | :param str ppa_description: Text description of the PPA. | 97 | :param str ppa_description: Text description of the PPA. |
52 | 98 | :param bool private: Limit access to PPA to only subscribed users and owner. | ||
53 | 98 | :rtype: Ppa | 99 | :rtype: Ppa |
54 | 99 | :returns: A Ppa object that describes the created PPA. | 100 | :returns: A Ppa object that describes the created PPA. |
55 | 100 | 101 | ||
56 | @@ -103,6 +104,7 @@ class PpaGroup: | |||
57 | 103 | ppa_settings = { | 104 | ppa_settings = { |
58 | 104 | 'description': ppa_description, | 105 | 'description': ppa_description, |
59 | 105 | 'displayname': ppa_name, | 106 | 'displayname': ppa_name, |
60 | 107 | 'private': private | ||
61 | 106 | } | 108 | } |
62 | 107 | 109 | ||
63 | 108 | try: | 110 | try: |
64 | diff --git a/scripts/ppa b/scripts/ppa | |||
65 | index 67dec11..ea624e0 100755 | |||
66 | --- a/scripts/ppa | |||
67 | +++ b/scripts/ppa | |||
68 | @@ -55,7 +55,7 @@ from inspect import currentframe | |||
69 | 55 | from textwrap import indent | 55 | from textwrap import indent |
70 | 56 | from typing import Any | 56 | from typing import Any |
71 | 57 | from distro_info import UbuntuDistroInfo | 57 | from distro_info import UbuntuDistroInfo |
73 | 58 | from lazr.restfulclient.errors import BadRequest | 58 | from lazr.restfulclient.errors import BadRequest, Unauthorized |
74 | 59 | 59 | ||
75 | 60 | try: | 60 | try: |
76 | 61 | from ruamel import yaml | 61 | from ruamel import yaml |
77 | @@ -193,33 +193,37 @@ def add_basic_config_options(parser: argparse.ArgumentParser) -> None: | |||
78 | 193 | help="A short description of the archive. URLs will be rendered as links. (See also 'ppa desc'.)" | 193 | help="A short description of the archive. URLs will be rendered as links. (See also 'ppa desc'.)" |
79 | 194 | ) | 194 | ) |
80 | 195 | 195 | ||
82 | 196 | # Enable/Disable uploading | 196 | # Dependencies |
83 | 197 | parser.add_argument( | 197 | parser.add_argument( |
88 | 198 | '--enable', | 198 | '--ppa-dependencies', '--ppa-depends', |
89 | 199 | dest="set_enabled", | 199 | dest="ppa_dependencies", action='store', |
90 | 200 | action='store_true', | 200 | help="The set of other PPAs this PPA should use for satisfying build dependencies." |
87 | 201 | help="Accept and build packages uploaded to the PPA." | ||
91 | 202 | ) | 201 | ) |
92 | 202 | |||
93 | 203 | # Public/Private access | ||
94 | 203 | parser.add_argument( | 204 | parser.add_argument( |
97 | 204 | '--disable', | 205 | '--private', '-P', |
98 | 205 | dest="set_disabled", | 206 | dest="private", |
99 | 206 | action='store_true', | 207 | action='store_true', |
101 | 207 | help="Do not accept or build packages uploaded to the PPA." | 208 | help=( |
102 | 209 | "Restrict access to the PPA to its owner and subscribers. " + | ||
103 | 210 | "This can only be changed if the archive has never had any " + | ||
104 | 211 | "sources published and the owner/group has permission to do so." | ||
105 | 212 | ) | ||
106 | 208 | ) | 213 | ) |
107 | 209 | |||
108 | 210 | # Dependencies | ||
109 | 211 | parser.add_argument( | 214 | parser.add_argument( |
113 | 212 | '--ppa-dependencies', '--ppa-depends', | 215 | '--public', |
114 | 213 | dest="ppa_dependencies", action='store', | 216 | dest="private", |
115 | 214 | help="The set of other PPAs this PPA should use for satisfying build dependencies." | 217 | action='store_false', |
116 | 218 | help="Allow access to the PPA to anyone." | ||
117 | 215 | ) | 219 | ) |
118 | 216 | 220 | ||
119 | 221 | # Publishing | ||
120 | 217 | parser.add_argument( | 222 | parser.add_argument( |
121 | 218 | '--publish', | 223 | '--publish', |
122 | 219 | dest="publish", action='store_true', | 224 | dest="publish", action='store_true', |
123 | 220 | help=("Allow built packages in the PPA to be made available for download.") | 225 | help=("Allow built packages in the PPA to be made available for download.") |
124 | 221 | ) | 226 | ) |
125 | 222 | |||
126 | 223 | parser.add_argument( | 227 | parser.add_argument( |
127 | 224 | '--no-publish', | 228 | '--no-publish', |
128 | 225 | dest="publish", action='store_false', | 229 | dest="publish", action='store_false', |
129 | @@ -304,7 +308,7 @@ def create_arg_parser() -> argparse.ArgumentParser: | |||
130 | 304 | set_parser = subparser.add_parser( | 308 | set_parser = subparser.add_parser( |
131 | 305 | 'set', | 309 | 'set', |
132 | 306 | argument_default=argparse.SUPPRESS, | 310 | argument_default=argparse.SUPPRESS, |
134 | 307 | help='set help', | 311 | help='Applies one or more configuration changes to the PPA.', |
135 | 308 | prog=progname, | 312 | prog=progname, |
136 | 309 | ) | 313 | ) |
137 | 310 | add_global_options(set_parser) | 314 | add_global_options(set_parser) |
138 | @@ -429,7 +433,7 @@ def create_config(lp: Lp, args: argparse.Namespace) -> dict[str, Any]: | |||
139 | 429 | config = dict(DEFAULT_CONFIG) | 433 | config = dict(DEFAULT_CONFIG) |
140 | 430 | 434 | ||
141 | 431 | # Map all non-empty elements from argparse Namespace into config dict | 435 | # Map all non-empty elements from argparse Namespace into config dict |
143 | 432 | config.update({k: v for k, v in vars(args).items() if v}) | 436 | config.update({k: v for k, v in vars(args).items() if v is not None}) |
144 | 433 | 437 | ||
145 | 434 | # Use defaults for any remaining parameters not yet configured | 438 | # Use defaults for any remaining parameters not yet configured |
146 | 435 | for k, v in DEFAULT_CONFIG.items(): | 439 | for k, v in DEFAULT_CONFIG.items(): |
147 | @@ -498,8 +502,6 @@ def command_create(lp: Lp, config: dict[str, str]) -> int: | |||
148 | 498 | warn("Could not determine owning person or team LP username") | 502 | warn("Could not determine owning person or team LP username") |
149 | 499 | return os.EX_USAGE | 503 | return os.EX_USAGE |
150 | 500 | 504 | ||
151 | 501 | publish = config.get('publish', None) | ||
152 | 502 | |||
153 | 503 | architectures = config.get('architectures', ARCHES_PPA_ALL) | 505 | architectures = config.get('architectures', ARCHES_PPA_ALL) |
154 | 504 | if type(architectures) is str: | 506 | if type(architectures) is str: |
155 | 505 | architectures = unpack_to_dict(architectures).keys() | 507 | architectures = unpack_to_dict(architectures).keys() |
156 | @@ -507,8 +509,12 @@ def command_create(lp: Lp, config: dict[str, str]) -> int: | |||
157 | 507 | try: | 509 | try: |
158 | 508 | if not config.get('dry_run', False): | 510 | if not config.get('dry_run', False): |
159 | 509 | ppa_group = PpaGroup(service=lp, name=owner_name) | 511 | ppa_group = PpaGroup(service=lp, name=owner_name) |
162 | 510 | the_ppa = ppa_group.create(ppa_name, ppa_description=description) | 512 | the_ppa = ppa_group.create( |
163 | 511 | the_ppa.set_publish(publish) | 513 | ppa_name, |
164 | 514 | ppa_description=description, | ||
165 | 515 | private=config.get('private', False) | ||
166 | 516 | ) | ||
167 | 517 | the_ppa.set_publish(config.get('publish', None)) | ||
168 | 512 | if architectures: | 518 | if architectures: |
169 | 513 | the_ppa.set_architectures(architectures) | 519 | the_ppa.set_architectures(architectures) |
170 | 514 | arch_str = ', '.join(the_ppa.architectures) | 520 | arch_str = ', '.join(the_ppa.architectures) |
171 | @@ -534,6 +540,12 @@ def command_create(lp: Lp, config: dict[str, str]) -> int: | |||
172 | 534 | print(" sudo add-apt-repository -yus {}".format(the_ppa.address)) | 540 | print(" sudo add-apt-repository -yus {}".format(the_ppa.address)) |
173 | 535 | print(" sudo apt-get install <package(s)>") | 541 | print(" sudo apt-get install <package(s)>") |
174 | 536 | return os.EX_OK | 542 | return os.EX_OK |
175 | 543 | except Unauthorized as e: | ||
176 | 544 | error(f"Insufficient authorization to create '{ppa_name}' under ownership of '{owner_name}'.") | ||
177 | 545 | return os.EX_NOPERM | ||
178 | 546 | except KeyError as e: | ||
179 | 547 | error(f"No such person or team '{owner_name}'") | ||
180 | 548 | return os.EX_NOUSER | ||
181 | 537 | except PpaAlreadyExists as e: | 549 | except PpaAlreadyExists as e: |
182 | 538 | warn(o2str(e.message)) | 550 | warn(o2str(e.message)) |
183 | 539 | return 3 | 551 | return 3 |
184 | @@ -678,9 +690,18 @@ def command_set(lp: Lp, config: dict[str, str]) -> int: | |||
185 | 678 | if 'publish' in config: | 690 | if 'publish' in config: |
186 | 679 | the_ppa.archive.publish = config.get('publish') | 691 | the_ppa.archive.publish = config.get('publish') |
187 | 680 | 692 | ||
188 | 693 | the_ppa.set_private(config.get('private', None)) | ||
189 | 694 | |||
190 | 681 | return the_ppa.archive.lp_save() | 695 | return the_ppa.archive.lp_save() |
191 | 696 | except Unauthorized as e: | ||
192 | 697 | if b'private' in e.content: | ||
193 | 698 | error(f"Insufficient authorization to change privacy for PPA '{the_ppa.name}'.") | ||
194 | 699 | else: | ||
195 | 700 | error(f"Insufficient authorization to modify PPA '{the_ppa.name}'.") | ||
196 | 701 | return os.EX_NOPERM | ||
197 | 682 | except PpaDoesNotExist as e: | 702 | except PpaDoesNotExist as e: |
198 | 683 | print(e) | 703 | print(e) |
199 | 704 | return 1 | ||
200 | 684 | except ValueError as e: | 705 | except ValueError as e: |
201 | 685 | print(f"Error: {e}") | 706 | print(f"Error: {e}") |
202 | 686 | return os.EX_USAGE | 707 | return os.EX_USAGE |
203 | @@ -819,6 +840,7 @@ def command_wait(lp: Lp, config: dict[str, str]) -> int: | |||
204 | 819 | return os.EX_OK | 840 | return os.EX_OK |
205 | 820 | except PpaDoesNotExist as e: | 841 | except PpaDoesNotExist as e: |
206 | 821 | print(e) | 842 | print(e) |
207 | 843 | return 1 | ||
208 | 822 | except ValueError as e: | 844 | except ValueError as e: |
209 | 823 | print(f"Error: {e}") | 845 | print(f"Error: {e}") |
210 | 824 | return os.EX_USAGE | 846 | return os.EX_USAGE |
211 | diff --git a/tests/helpers.py b/tests/helpers.py | |||
212 | index c607d3c..99d5e38 100644 | |||
213 | --- a/tests/helpers.py | |||
214 | +++ b/tests/helpers.py | |||
215 | @@ -44,8 +44,9 @@ class ArchiveMock: | |||
216 | 44 | self.displayname = name | 44 | self.displayname = name |
217 | 45 | self.description = description | 45 | self.description = description |
218 | 46 | self.owner = owner | 46 | self.owner = owner |
220 | 47 | self.publish = True | 47 | self.private = False |
221 | 48 | self.processors = [ProcessorMock(proc_name) for proc_name in ARCHES_PPA_DEFAULT] | 48 | self.processors = [ProcessorMock(proc_name) for proc_name in ARCHES_PPA_DEFAULT] |
222 | 49 | self.publish = True | ||
223 | 49 | self.published_sources = [] | 50 | self.published_sources = [] |
224 | 50 | 51 | ||
225 | 51 | def setProcessors(self, processors): | 52 | def setProcessors(self, processors): |
226 | @@ -64,12 +65,14 @@ class PersonMock: | |||
227 | 64 | self.name = name | 65 | self.name = name |
228 | 65 | self._ppas = [] | 66 | self._ppas = [] |
229 | 66 | 67 | ||
231 | 67 | def createPPA(self, name, description, displayname): | 68 | def createPPA(self, name, description, displayname, private=None): |
232 | 68 | for ppa in self._ppas: | 69 | for ppa in self._ppas: |
233 | 69 | if ppa.name == name: | 70 | if ppa.name == name: |
234 | 70 | raise PpaAlreadyExists(name) | 71 | raise PpaAlreadyExists(name) |
235 | 71 | ppa = Ppa(name, self.name, description) | 72 | ppa = Ppa(name, self.name, description) |
236 | 72 | Ppa.archive = ArchiveMock(ppa.name, ppa.description, self) | 73 | Ppa.archive = ArchiveMock(ppa.name, ppa.description, self) |
237 | 74 | if isinstance(private, bool): | ||
238 | 75 | Ppa.archive.private = private | ||
239 | 73 | self._ppas.append(ppa) | 76 | self._ppas.append(ppa) |
240 | 74 | return True | 77 | return True |
241 | 75 | 78 | ||
242 | diff --git a/tests/smoketest_ppa_set b/tests/smoketest_ppa_set | |||
243 | 76 | new file mode 100755 | 79 | new file mode 100755 |
244 | index 0000000..7724d27 | |||
245 | --- /dev/null | |||
246 | +++ b/tests/smoketest_ppa_set | |||
247 | @@ -0,0 +1,70 @@ | |||
248 | 1 | #!/bin/bash | ||
249 | 2 | |||
250 | 3 | testsdir=$(dirname "$(readlink -f "${0}")") | ||
251 | 4 | progdir="${testsdir}/../scripts" | ||
252 | 5 | script="${progdir}/ppa" | ||
253 | 6 | ppa_name="ppa-test-set-${RANDOM}" | ||
254 | 7 | |||
255 | 8 | [ -e "${script}" ] || { | ||
256 | 9 | echo "Could not locate ppa command '${script}'." | ||
257 | 10 | exit 1 | ||
258 | 11 | } | ||
259 | 12 | |||
260 | 13 | ppa_create() { | ||
261 | 14 | echo -n "* Creating ${ppa_name} .." | ||
262 | 15 | if "${script}" create "${ppa_name}"; then | ||
263 | 16 | echo ". OK" | ||
264 | 17 | else | ||
265 | 18 | echo "! ERROR" | ||
266 | 19 | exit 1 | ||
267 | 20 | fi | ||
268 | 21 | } | ||
269 | 22 | |||
270 | 23 | ppa_destroy() { | ||
271 | 24 | echo -n "* Destroying ${ppa_name} .." | ||
272 | 25 | if "${script}" destroy "${ppa_name}"; then | ||
273 | 26 | echo ". OK" | ||
274 | 27 | else | ||
275 | 28 | echo "! ERROR" | ||
276 | 29 | exit 1 | ||
277 | 30 | fi | ||
278 | 31 | } | ||
279 | 32 | |||
280 | 33 | ppa_status() { | ||
281 | 34 | "${script}" status "${ppa_name}" | ||
282 | 35 | } | ||
283 | 36 | |||
284 | 37 | ppa_set() { | ||
285 | 38 | echo -n "* Setting ${1} .." | ||
286 | 39 | if "${script}" set "${ppa_name}" "${@}"; then | ||
287 | 40 | echo ". OK" | ||
288 | 41 | else | ||
289 | 42 | echo "! ERROR" | ||
290 | 43 | fi | ||
291 | 44 | } | ||
292 | 45 | |||
293 | 46 | echo "### Default configuration ###" | ||
294 | 47 | ppa_create && \ | ||
295 | 48 | trap "ppa_destroy" EXIT | ||
296 | 49 | ppa_status | ||
297 | 50 | |||
298 | 51 | echo | ||
299 | 52 | echo "### Set parameters ###" | ||
300 | 53 | ppa_set --displayname "parameters-set" | ||
301 | 54 | ppa_set --description "Updated PPA with various settings changed" | ||
302 | 55 | ppa_set --all-architectures | ||
303 | 56 | ppa_set --no-publish | ||
304 | 57 | ppa_status | ||
305 | 58 | |||
306 | 59 | echo | ||
307 | 60 | echo "### Restore to default parameters ###" | ||
308 | 61 | ppa_set --displayname "${ppa_name}" | ||
309 | 62 | ppa_set --description "" | ||
310 | 63 | ppa_set --default-architectures | ||
311 | 64 | ppa_set --publish | ||
312 | 65 | ppa_status | ||
313 | 66 | |||
314 | 67 | echo | ||
315 | 68 | echo "### Private PPA ###" | ||
316 | 69 | ppa_set --private | ||
317 | 70 | ppa_set --public | ||
318 | diff --git a/tests/test_ppa_group.py b/tests/test_ppa_group.py | |||
319 | index f53799f..6539c3a 100644 | |||
320 | --- a/tests/test_ppa_group.py | |||
321 | +++ b/tests/test_ppa_group.py | |||
322 | @@ -69,6 +69,16 @@ def test_create_with_owner(): | |||
323 | 69 | assert ppa.address == 'ppa:test_owner_name/ppa_test_name' | 69 | assert ppa.address == 'ppa:test_owner_name/ppa_test_name' |
324 | 70 | 70 | ||
325 | 71 | 71 | ||
326 | 72 | def test_create_private(): | ||
327 | 73 | """Check creating a private PPA.""" | ||
328 | 74 | lp = LpServiceMock() | ||
329 | 75 | ppa_group = PpaGroup(service=lp, name='me') | ||
330 | 76 | ppa = ppa_group.create('private_ppa', private=True) | ||
331 | 77 | assert ppa is not None | ||
332 | 78 | assert ppa.address == 'ppa:me/private_ppa' | ||
333 | 79 | assert ppa.is_private is True | ||
334 | 80 | |||
335 | 81 | |||
336 | 72 | def test_list_ppas(): | 82 | def test_list_ppas(): |
337 | 73 | """Check listing the PPAs for a PPA group.""" | 83 | """Check listing the PPAs for a PPA group.""" |
338 | 74 | test_ppa_list = ['a', 'b', 'c', 'd'] | 84 | test_ppa_list = ['a', 'b', 'c', 'd'] |
339 | diff --git a/tests/test_scripts_ppa.py b/tests/test_scripts_ppa.py | |||
340 | index f82a9e4..93988d0 100644 | |||
341 | --- a/tests/test_scripts_ppa.py | |||
342 | +++ b/tests/test_scripts_ppa.py | |||
343 | @@ -232,13 +232,16 @@ def test_create_arg_parser_basic_config(command): | |||
344 | 232 | assert args.description == 'x' | 232 | assert args.description == 'x' |
345 | 233 | args.description = None | 233 | args.description = None |
346 | 234 | 234 | ||
354 | 235 | # Check --enable / --disable | 235 | # Check --public / -P|--private |
355 | 236 | args = parser.parse_args([command, 'test-ppa', '--enable']) | 236 | args = parser.parse_args([command, 'test-ppa', '--public']) |
356 | 237 | assert args.set_enabled is True | 237 | assert args.private is False |
357 | 238 | args.set_enabled = False | 238 | args.public = None |
358 | 239 | args = parser.parse_args([command, 'test-ppa', '--disable']) | 239 | args = parser.parse_args([command, 'test-ppa', '--private']) |
359 | 240 | assert args.set_disabled is True | 240 | assert args.private is True |
360 | 241 | args.set_disabled = False | 241 | args.private = None |
361 | 242 | args = parser.parse_args([command, 'test-ppa', '-P']) | ||
362 | 243 | assert args.private is True | ||
363 | 244 | args.private = None | ||
364 | 242 | 245 | ||
365 | 243 | # Check --ppa-dependencies <PPA[,...]> | 246 | # Check --ppa-dependencies <PPA[,...]> |
366 | 244 | args = parser.parse_args([command, 'test-ppa', '--ppa-dependencies', 'a,b,c']) | 247 | args = parser.parse_args([command, 'test-ppa', '--ppa-dependencies', 'a,b,c']) |
367 | @@ -431,6 +434,8 @@ def test_create_arg_parser_tests(): | |||
368 | 431 | (['create', 'aa/bb'], {'command': 'create', 'owner_name': 'aa', 'ppa_name': 'bb'}), | 434 | (['create', 'aa/bb'], {'command': 'create', 'owner_name': 'aa', 'ppa_name': 'bb'}), |
369 | 432 | (['create', 'bb', '--owner', 'aa'], {'command': 'create', 'owner_name': 'aa', 'ppa_name': 'bb'}), | 435 | (['create', 'bb', '--owner', 'aa'], {'command': 'create', 'owner_name': 'aa', 'ppa_name': 'bb'}), |
370 | 433 | (['create', 'bb', '--owner-name', 'aa'], {'command': 'create', 'owner_name': 'aa', 'ppa_name': 'bb'}), | 436 | (['create', 'bb', '--owner-name', 'aa'], {'command': 'create', 'owner_name': 'aa', 'ppa_name': 'bb'}), |
371 | 437 | (['create', 'bb', '--private'], {'command': 'create', 'private': True}), | ||
372 | 438 | (['create', 'bb', '--public'], {'command': 'create', 'private': False}), | ||
373 | 434 | (['create', 'bb', '--team', 'aa'], {'command': 'create', 'owner_name': 'aa', 'ppa_name': 'bb'}), | 439 | (['create', 'bb', '--team', 'aa'], {'command': 'create', 'owner_name': 'aa', 'ppa_name': 'bb'}), |
374 | 435 | (['create', 'bb', '--team-name', 'aa'], {'command': 'create', 'owner_name': 'aa', 'ppa_name': 'bb'}), | 440 | (['create', 'bb', '--team-name', 'aa'], {'command': 'create', 'owner_name': 'aa', 'ppa_name': 'bb'}), |
375 | 436 | ]) | 441 | ]) |
376 | @@ -489,6 +494,8 @@ def test_create_config_from_args_error(args, expected_exception): | |||
377 | 489 | (None, {'ppa_name': 'a'}, {'displayname': 'a'}), | 494 | (None, {'ppa_name': 'a'}, {'displayname': 'a'}), |
378 | 490 | (None, {'publish': True}, {'publish': True}), | 495 | (None, {'publish': True}, {'publish': True}), |
379 | 491 | (None, {'publish': False}, {'publish': False}), | 496 | (None, {'publish': False}, {'publish': False}), |
380 | 497 | (None, {'private': True}, {'private': True}), | ||
381 | 498 | (None, {'private': False}, {'private': False}), | ||
382 | 492 | ]) | 499 | ]) |
383 | 493 | def test_command_create(fake_config, monkeypatch, stdin, params, expected_ppa_config): | 500 | def test_command_create(fake_config, monkeypatch, stdin, params, expected_ppa_config): |
384 | 494 | '''Checks create command produces a PPA with expected configuration.''' | 501 | '''Checks create command produces a PPA with expected configuration.''' |
385 | @@ -603,6 +610,10 @@ def test_command_not_exists(fake_config): | |||
386 | 603 | ({}, {'publish': True}), | 610 | ({}, {'publish': True}), |
387 | 604 | ({'publish': False}, {'publish': False}), | 611 | ({'publish': False}, {'publish': False}), |
388 | 605 | ({'publish': True}, {'publish': True}), | 612 | ({'publish': True}, {'publish': True}), |
389 | 613 | |||
390 | 614 | ({}, {'private': False}), | ||
391 | 615 | ({'private': False}, {'private': False}), | ||
392 | 616 | ({'private': True}, {'private': True}), | ||
393 | 606 | ]) | 617 | ]) |
394 | 607 | def test_command_set(fake_config, params, expected_ppa_config): | 618 | def test_command_set(fake_config, params, expected_ppa_config): |
395 | 608 | '''Checks that the set command properly requests PPA configuration changes.''' | 619 | '''Checks that the set command properly requests PPA configuration changes.''' |
LGTM! just one note for one of the help strings