Merge ppa-dev-tools:make-check-fixes into ppa-dev-tools:main
- Git
- lp:ppa-dev-tools
- make-check-fixes
- Merge into main
Proposed by
Bryce Harrington
Status: | Merged |
---|---|
Merged at revision: | c1451af3a7d727b1499275858e4c24f49e82321e |
Proposed branch: | ppa-dev-tools:make-check-fixes |
Merge into: | ppa-dev-tools:main |
Diff against target: |
817 lines (+132/-82) 14 files modified
ppa/debug.py (+3/-0) ppa/job.py (+9/-6) ppa/lp.py (+4/-4) ppa/ppa.py (+11/-7) ppa/ppa_group.py (+5/-3) ppa/processes.py (+8/-6) ppa/result.py (+1/-2) ppa/subtest.py (+2/-3) ppa/text.py (+18/-13) tests/test_lp.py (+11/-3) tests/test_ppa.py (+3/-3) tests/test_ppa_group.py (+15/-7) tests/test_scripts_ppa.py (+40/-22) tests/test_version.py (+2/-3) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Christian Ehrhardt (community) | Approve | ||
Canonical Server | Pending | ||
Canonical Server Reporter | Pending | ||
Review via email: mp+430005@code.launchpad.net |
Commit message
Description of the change
Cleanup to prep for 0.2 release.
To post a comment you must log in.
Revision history for this message
Bryce Harrington (bryce) wrote : | # |
Excellent, thanks
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/ppa/debug.py b/ppa/debug.py |
2 | index 8339e73..009ead4 100644 |
3 | --- a/ppa/debug.py |
4 | +++ b/ppa/debug.py |
5 | @@ -13,6 +13,7 @@ import pprint |
6 | |
7 | DEBUGGING = False |
8 | |
9 | + |
10 | def dbg(msg): |
11 | """Prints information if debugging is enabled""" |
12 | if DEBUGGING: |
13 | @@ -21,10 +22,12 @@ def dbg(msg): |
14 | else: |
15 | pprint.pprint(msg) |
16 | |
17 | + |
18 | def warn(msg): |
19 | """Prints message to stderr""" |
20 | sys.stderr.write("Warning: {}\n".format(msg)) |
21 | |
22 | + |
23 | def die(msg, code=1): |
24 | """Prints message to stderr and exits with given code""" |
25 | sys.stderr.write("Error: {}\n".format(msg)) |
26 | diff --git a/ppa/job.py b/ppa/job.py |
27 | index 2ae06ba..91f015c 100755 |
28 | --- a/ppa/job.py |
29 | +++ b/ppa/job.py |
30 | @@ -148,9 +148,9 @@ def show_running(jobs): |
31 | if n == 1: |
32 | print("Running:") |
33 | ppa_str = ','.join(e.ppas) |
34 | - trigger_str = ','.join(e.triggers) |
35 | + trig_str = ','.join(e.triggers) |
36 | print(rformat % ("time", "pkg", "release", "arch", "ppa", "trigger")) |
37 | - print(rformat % (str(e.submit_time), e.source_package, e.series, e.arch, ppa_str, trigger_str)) |
38 | + print(rformat % (str(e.submit_time), e.source_package, e.series, e.arch, ppa_str, trig_str)) |
39 | if n == 0: |
40 | print("Running: (none)") |
41 | |
42 | @@ -164,7 +164,10 @@ def show_waiting(jobs): |
43 | if n == 1: |
44 | print("Waiting:") |
45 | print(rformat % ("Q-num", "pkg", "release", "arch", "ppa", "trigger")) |
46 | - print(rformat % (e.number, e.source_package, e.series, e.arch, ','.join(e.ppas), ','.join(e.triggers))) |
47 | + |
48 | + ppa_str = ','.join(e.ppas) |
49 | + trig_str = ','.join(e.triggers) |
50 | + print(rformat % (e.number, e.source_package, e.series, e.arch, ppa_str, trig_str)) |
51 | if n == 0: |
52 | print("Waiting: (none)") |
53 | |
54 | @@ -178,8 +181,8 @@ if __name__ == "__main__": |
55 | root_dir = os.path.realpath(os.path.join(os.path.dirname(os.path.realpath(__file__)), '..')) |
56 | jobinfo = { |
57 | 'triggers': ['a/1', 'b/2.1', 'c/3.2.1'], |
58 | - 'ppas': ['ppa:me/myppa'], |
59 | - } |
60 | + 'ppas': ['ppa:me/myppa'] |
61 | + } |
62 | job_1 = Job( |
63 | number=0, |
64 | submit_time='time', |
65 | @@ -188,7 +191,7 @@ if __name__ == "__main__": |
66 | arch='amd64', |
67 | triggers=jobinfo.get('triggers', None), |
68 | ppas=jobinfo.get('ppas', None) |
69 | - ) |
70 | + ) |
71 | print(job_1) |
72 | print(f"triggers: {job_1.triggers}") |
73 | print(f"ppas: {job_1.ppas}") |
74 | diff --git a/ppa/lp.py b/ppa/lp.py |
75 | index c3ebaac..1762345 100644 |
76 | --- a/ppa/lp.py |
77 | +++ b/ppa/lp.py |
78 | @@ -14,12 +14,12 @@ |
79 | High level wrapper object for Launchpad's API |
80 | """ |
81 | |
82 | -from os.path import join |
83 | from contextlib import suppress |
84 | from functools import lru_cache |
85 | |
86 | from launchpadlib.launchpad import Launchpad |
87 | |
88 | + |
89 | class Lp: |
90 | """ |
91 | This class wrappers the Launchpadlib service to cache object queries |
92 | @@ -32,8 +32,8 @@ class Lp: |
93 | itself is passed directly to the Launchpadlib object, so the entire |
94 | API is available in exactly the same way. |
95 | """ |
96 | - ROOT_URL = 'https://launchpad.net/' |
97 | - API_ROOT_URL = 'https://api.launchpad.net/devel/' |
98 | + ROOT_URL = 'https://launchpad.net/' |
99 | + API_ROOT_URL = 'https://api.launchpad.net/devel/' |
100 | BUGS_ROOT_URL = 'https://bugs.launchpad.net/' |
101 | CODE_ROOT_URL = 'https://code.launchpad.net/' |
102 | |
103 | @@ -68,7 +68,7 @@ class Lp: |
104 | |
105 | def __getattr__(self, attr): |
106 | """Wrap launchpadlib so tightly you can't tell the difference.""" |
107 | - assert not attr.startswith('_'), "Can't getattr for %s" %(attr) |
108 | + assert not attr.startswith('_'), f"Can't getattr for {attr}" |
109 | instance = super(Lp, self).__getattribute__('_instance') |
110 | return getattr(instance, attr) |
111 | |
112 | diff --git a/ppa/ppa.py b/ppa/ppa.py |
113 | index 6f744a4..763ed7f 100755 |
114 | --- a/ppa/ppa.py |
115 | +++ b/ppa/ppa.py |
116 | @@ -44,7 +44,7 @@ def ppa_address_split(ppa_address, default_team='me'): |
117 | if ppa_address.startswith('ppa:'): |
118 | if '/' not in ppa_address: |
119 | return (None, None) |
120 | - rem = ppa_address.split('ppa:',1)[1] |
121 | + rem = ppa_address.split('ppa:', 1)[1] |
122 | team_name = rem.split('/', 1)[0] |
123 | ppa_name = rem.split('/', 1)[1] |
124 | else: |
125 | @@ -193,7 +193,7 @@ class Ppa: |
126 | :returns: List of architecture names, or None on error. |
127 | """ |
128 | try: |
129 | - return [ proc.name for proc in self.archive.processors ] |
130 | + return [proc.name for proc in self.archive.processors] |
131 | except PpaDoesNotExist as e: |
132 | print(e) |
133 | return None |
134 | @@ -208,7 +208,7 @@ class Ppa: |
135 | :returns: List of architecture names, or None on error. |
136 | """ |
137 | uri_base = "https://api.launchpad.net/devel/+processors/{}" |
138 | - procs = [ uri_base.format(arch) for arch in architectures ] |
139 | + procs = [uri_base.format(arch) for arch in architectures] |
140 | try: |
141 | self.archive.setProcessors(processors=procs) |
142 | return True |
143 | @@ -225,7 +225,7 @@ class Ppa: |
144 | :rtype: list[binary_package_publishing_history] |
145 | :returns: List of binaries, or None on error |
146 | """ |
147 | - if distro == None and series == None and arch == None: |
148 | + if distro is None and series is None and arch is None: |
149 | try: |
150 | return self.archive.getPublishedBinaries() |
151 | except PpaDoesNotExist as e: |
152 | @@ -323,7 +323,9 @@ class Ppa: |
153 | published_builds[binary_publication.build_link] = binary_publication |
154 | |
155 | retval = False |
156 | - num_builds_waiting = len(required_builds) - len(pending_publication_builds) - len(published_builds) |
157 | + num_builds_waiting = ( |
158 | + len(required_builds) - len(pending_publication_builds) - len(published_builds) |
159 | + ) |
160 | if num_builds_waiting != 0: |
161 | num_build_failures = 0 |
162 | builds_waiting_output = '' |
163 | @@ -353,12 +355,14 @@ class Ppa: |
164 | retval = True |
165 | |
166 | if len(pending_publication_builds) != 0: |
167 | - print("* Still waiting on {} build publications:".format(len(pending_publication_builds))) |
168 | + num = len(pending_publication_builds) |
169 | + print(f"* Still waiting on {num} build publications:") |
170 | for pub in pending_publication_builds.values(): |
171 | print(" - {}".format(pub.display_name)) |
172 | retval = True |
173 | if len(pending_publication_sources) != 0: |
174 | - print("* Still waiting on {} source publications:".format(len(pending_publication_sources))) |
175 | + num = len(pending_publication_sources) |
176 | + print(f"* Still waiting on {num} source publications:") |
177 | for pub in pending_publication_sources.values(): |
178 | print(" - {}".format(pub.display_name)) |
179 | retval = True |
180 | diff --git a/ppa/ppa_group.py b/ppa/ppa_group.py |
181 | index cb769ad..1a9c8b7 100755 |
182 | --- a/ppa/ppa_group.py |
183 | +++ b/ppa/ppa_group.py |
184 | @@ -14,6 +14,7 @@ from .text import o2str |
185 | from functools import lru_cache |
186 | from lazr.restfulclient.errors import BadRequest |
187 | |
188 | + |
189 | class PpaAlreadyExists(BaseException): |
190 | '''Exception indicating a PPA operation could not be performed''' |
191 | |
192 | @@ -34,7 +35,8 @@ class PpaAlreadyExists(BaseException): |
193 | if self.message: |
194 | return self.message |
195 | elif self.ppa_name: |
196 | - return "The PPA %s already exists" %(self.ppa_name) |
197 | + return f"The PPA {self.ppa_name} already exists" |
198 | + |
199 | |
200 | class PpaGroup: |
201 | """Represents a team or person that owns one or more PPAs. |
202 | @@ -58,7 +60,7 @@ class PpaGroup: |
203 | |
204 | def __repr__(self): |
205 | return (f'{self.__class__.__name__}(' |
206 | - f'service={self.service!r}, name={self.name!r})') |
207 | + f'service={self.service!r}, name={self.name!r})') |
208 | |
209 | def __str__(self): |
210 | return 'tbd' |
211 | @@ -91,7 +93,7 @@ class PpaGroup: |
212 | ppa_displayname = ppa_name |
213 | |
214 | try: |
215 | - results = self.team.createPPA( |
216 | + self.team.createPPA( |
217 | name=ppa_name, |
218 | description=ppa_description, |
219 | displayname=ppa_displayname) |
220 | diff --git a/ppa/processes.py b/ppa/processes.py |
221 | index e008c0c..5f54e39 100644 |
222 | --- a/ppa/processes.py |
223 | +++ b/ppa/processes.py |
224 | @@ -28,7 +28,6 @@ from subprocess import (Popen, PIPE) |
225 | from .debug import dbg |
226 | from .text import o2str |
227 | |
228 | -# TODO: Integrate use of dbg and ERR from utils |
229 | |
230 | class ReturnCode(BaseException): |
231 | def __init__(self, code, errors=None): |
232 | @@ -40,11 +39,12 @@ class ReturnCode(BaseException): |
233 | |
234 | def __str__(self): |
235 | text = '\n'.join(self.errors) |
236 | - return "%sReturned error code %d" %(text, self.code) |
237 | + return f"{text}Returned error code {self.code}" |
238 | + |
239 | |
240 | def shell(command, in_text=None): |
241 | """Executes command in a shell, returns stdout, raises exception on error""" |
242 | - dbg("shell: %s" %(command)) |
243 | + dbg(f"shell: {command}") |
244 | if in_text: |
245 | p = Popen(command, shell=True, stdout=PIPE, stderr=PIPE, stdin=PIPE) |
246 | output, errors = p.communicate(input=in_text) |
247 | @@ -55,14 +55,15 @@ def shell(command, in_text=None): |
248 | raise ReturnCode(p.returncode, o2str(errors)) |
249 | return o2str(output) |
250 | |
251 | + |
252 | def execute(command, in_text=None): |
253 | """Executes command, returns stdout; prints errors to stderr""" |
254 | - dbg("execute: `%s`" %(command)) |
255 | + dbg(f"execute: `{command}`") |
256 | if in_text is None: |
257 | p = Popen(command, shell=False, stdout=PIPE, stderr=PIPE) |
258 | else: |
259 | p = Popen(command, shell=False, stdout=PIPE, stderr=PIPE, stdin=PIPE) |
260 | - dbg("execute: polling (%s)..." %(in_text)) |
261 | + dbg(f"execute: polling ({in_text})...") |
262 | while p.poll() is None and p.stdin is not None: |
263 | dbg("execute: Sending to process stdin") |
264 | p.stdin.write(in_text) |
265 | @@ -70,10 +71,11 @@ def execute(command, in_text=None): |
266 | sleep(0.01) |
267 | output = p.stdout.read() |
268 | if p.returncode: |
269 | - dbg("Received return code %d" %(p.returncode)) |
270 | + dbg(f"Received return code {p.returncode}") |
271 | raise ReturnCode(p.returncode, p.stderr.readlines()) |
272 | return o2str(output) |
273 | |
274 | + |
275 | def execute_with_input(command, in_text): |
276 | """Executes command, passing in_text to stdin if provided""" |
277 | execute(command, in_text) |
278 | diff --git a/ppa/result.py b/ppa/result.py |
279 | index 7a2ca5a..4f4aadd 100755 |
280 | --- a/ppa/result.py |
281 | +++ b/ppa/result.py |
282 | @@ -30,7 +30,7 @@ class Result: |
283 | VALUES = { |
284 | 'PASS': "✅", |
285 | 'FAIL': "❌", |
286 | - 'BAD': "⛔", |
287 | + 'BAD': "⛔" |
288 | } |
289 | |
290 | def __init__(self, url, time, series, arch, source): |
291 | @@ -145,7 +145,6 @@ class Result: |
292 | package, version = t.split('/', 1) |
293 | yield Trigger(package, version, arch=self.arch, series=self.series) |
294 | |
295 | - |
296 | @lru_cache |
297 | def get_subtests(self, name=None) -> Iterator[Subtest]: |
298 | """Provides list of Subtests that were run for this Result. |
299 | diff --git a/ppa/subtest.py b/ppa/subtest.py |
300 | index c63dda6..90134b8 100755 |
301 | --- a/ppa/subtest.py |
302 | +++ b/ppa/subtest.py |
303 | @@ -12,7 +12,6 @@ |
304 | """An individual DEP8 test run""" |
305 | |
306 | from functools import cached_property |
307 | -from typing import List, Iterator |
308 | |
309 | |
310 | class Subtest: |
311 | @@ -25,9 +24,9 @@ class Subtest: |
312 | 'PASS': "🟩", |
313 | 'SKIP': "🟧", |
314 | 'FAIL': "🟥", |
315 | - 'BAD': "⛔", |
316 | + 'BAD': "⛔", |
317 | 'UNKNOWN': "⚪" |
318 | - } |
319 | + } |
320 | |
321 | def __init__(self, line): |
322 | """Initializes a new Subtext object. |
323 | diff --git a/ppa/text.py b/ppa/text.py |
324 | index fb8c558..b1e1453 100644 |
325 | --- a/ppa/text.py |
326 | +++ b/ppa/text.py |
327 | @@ -24,9 +24,11 @@ |
328 | |
329 | '''Routines to encode or convert to and from text''' |
330 | |
331 | -from decimal import Decimal |
332 | +import sys |
333 | +from decimal import Decimal |
334 | from functools import lru_cache |
335 | |
336 | + |
337 | @lru_cache |
338 | def quote(msg): |
339 | """ |
340 | @@ -39,13 +41,14 @@ def quote(msg): |
341 | msg = msg.replace('>', '>') |
342 | return msg |
343 | |
344 | + |
345 | @lru_cache |
346 | def o2str(obj): |
347 | """ |
348 | Convert a unicode, decimal.Decimal, datetime object, etc. to a str. |
349 | Converts lists and tuples of objects into lists of strings. |
350 | """ |
351 | - retval = None |
352 | + |
353 | if type(obj) == str: |
354 | return obj |
355 | elif type(obj) == bytes: |
356 | @@ -60,9 +63,10 @@ def o2str(obj): |
357 | elif str(type(obj)) == "<type 'datetime.datetime'>": |
358 | return obj.ctime() |
359 | else: |
360 | - #print str(type(obj)) |
361 | + # print str(type(obj)) |
362 | return obj |
363 | |
364 | + |
365 | @lru_cache |
366 | def to_bool(value): |
367 | """ |
368 | @@ -70,14 +74,15 @@ def to_bool(value): |
369 | Possible True values: 1, True, '1', 'TRue', 'yes', 'y', 't' |
370 | Possible False values: 0, False, None, [], {}, '', '0', 'faLse', 'no', 'n', 'f', 0.0 |
371 | """ |
372 | - if type(value) == type(''): |
373 | - if value.lower() in ("yes", "y", "true", "t", "1"): |
374 | + if isinstance(value, str): |
375 | + if value.lower() in ("yes", "y", "true", "t", "1"): |
376 | return True |
377 | - if value.lower() in ("no", "n", "false", "f", "0", "none", ""): |
378 | + if value.lower() in ("no", "n", "false", "f", "0", "none", ""): |
379 | return False |
380 | raise Exception('Invalid value for boolean conversion: ' + value) |
381 | return bool(value) |
382 | |
383 | + |
384 | @lru_cache |
385 | def o2float(value): |
386 | '''Converts strings like 42%, 123M, 1.2B into floating point numbers |
387 | @@ -93,21 +98,21 @@ def o2float(value): |
388 | elif value == '--': |
389 | return 0.0 |
390 | |
391 | - value = value.replace(',','') |
392 | - last = value[len(value)-1] |
393 | + value = value.replace(',', '') |
394 | + last = value[len(value) - 1] |
395 | if last == 'M': |
396 | return float(value[:-1]) |
397 | elif last == 'B': |
398 | return float(value[:-1]) * 1000 |
399 | elif last == '%': |
400 | - return float(value[:-1])/100.0 |
401 | + return float(value[:-1]) / 100.0 |
402 | elif last == ')' and value[0] == '(': |
403 | return -1 * o2float(value[1:-1]) |
404 | |
405 | try: |
406 | return float(value) |
407 | except ValueError: |
408 | - sys.stderr.write("ofloat: Could not convert '%s' to float\n" %(value)) |
409 | + sys.stderr.write(f"ofloat: Could not convert '{value}' to float\n") |
410 | raise |
411 | |
412 | |
413 | @@ -132,10 +137,10 @@ if __name__ == "__main__": |
414 | ({}, False), |
415 | ((), False), |
416 | ([1], True), |
417 | - ({1:2}, True), |
418 | + ({1: 2}, True), |
419 | ((1,), True), |
420 | (None, False), |
421 | (object(), True), |
422 | - ] |
423 | + ] |
424 | for test, expected in test_cases: |
425 | - assert to_bool(test) == expected, "to_bool("+test+") failed to return "+expected |
426 | + assert to_bool(test) == expected, f"to_bool({test}) failed to return {expected}" |
427 | diff --git a/tests/test_lp.py b/tests/test_lp.py |
428 | index a8fcdcb..d5978ce 100644 |
429 | --- a/tests/test_lp.py |
430 | +++ b/tests/test_lp.py |
431 | @@ -30,7 +30,7 @@ Tests for our launchpadlib wrapper & helper methods. |
432 | import sys |
433 | import os.path |
434 | |
435 | -from mock import Mock, patch |
436 | +from mock import Mock |
437 | import pytest |
438 | import logging |
439 | |
440 | @@ -41,6 +41,7 @@ from launchpadlib.launchpad import Launchpad |
441 | |
442 | APPNAME = 'test-lp' |
443 | |
444 | + |
445 | @pytest.fixture |
446 | def fakelp(tmp_path): |
447 | """Connect to Launchpad.""" |
448 | @@ -71,16 +72,19 @@ def test_new_connection(tmp_path): |
449 | version='devel', |
450 | ) |
451 | |
452 | + |
453 | def test_api_root(fakelp): |
454 | """Ensure we can get LP's API root.""" |
455 | fakelp.load(Lp.API_ROOT_URL + 'hi') |
456 | fakelp._instance.load.assert_called_once_with( |
457 | 'https://api.launchpad.net/devel/hi') |
458 | |
459 | + |
460 | def test_ubuntu(fakelp): |
461 | - fakelp._instance.distributions={'ubuntu': 'UBUNTU'} |
462 | + fakelp._instance.distributions = {'ubuntu': 'UBUNTU'} |
463 | assert fakelp.ubuntu == 'UBUNTU' |
464 | |
465 | + |
466 | def test_ubuntu_active_series(fakelp): |
467 | mock_hirsute = Mock(active=False) |
468 | mock_hirsute.name = 'hirsute' |
469 | @@ -93,10 +97,12 @@ def test_ubuntu_active_series(fakelp): |
470 | assert fakelp.ubuntu_active_series()[0] == mock_jammy |
471 | assert fakelp.ubuntu_active_series()[0].name == 'jammy' |
472 | |
473 | + |
474 | def test_debian(fakelp): |
475 | - fakelp._instance.distributions={'debian': 'DEBIAN'} |
476 | + fakelp._instance.distributions = {'debian': 'DEBIAN'} |
477 | assert fakelp.debian == 'DEBIAN' |
478 | |
479 | + |
480 | def test_debian_active_series(fakelp): |
481 | mock_woody = Mock(active=False) |
482 | mock_woody.name = 'woody' |
483 | @@ -111,6 +117,7 @@ def test_debian_active_series(fakelp): |
484 | fakelp._instance.distributions = {'debian': mock_debian} |
485 | assert fakelp.debian_active_series()[0].name == 'buster' |
486 | |
487 | + |
488 | def test_debian_experimental_series(fakelp): |
489 | mock_woody = Mock(active=False) |
490 | mock_woody.name = 'woody' |
491 | @@ -125,6 +132,7 @@ def test_debian_experimental_series(fakelp): |
492 | fakelp._instance.distributions = {'debian': mock_debian} |
493 | assert fakelp.debian_experimental_series() == mock_sid |
494 | |
495 | + |
496 | def test_ppa_team(fakelp): |
497 | """Return the PPA team object.""" |
498 | mock_me = Mock(memberships_details=[ |
499 | diff --git a/tests/test_ppa.py b/tests/test_ppa.py |
500 | index a632228..a71336f 100644 |
501 | --- a/tests/test_ppa.py |
502 | +++ b/tests/test_ppa.py |
503 | @@ -11,10 +11,8 @@ |
504 | import os |
505 | import sys |
506 | |
507 | -import pytest |
508 | - |
509 | sys.path.insert(0, os.path.realpath( |
510 | - os.path.join(os.path.dirname(__file__), ".."))) |
511 | + os.path.join(os.path.dirname(__file__), ".."))) |
512 | |
513 | from ppa.ppa import Ppa |
514 | |
515 | @@ -24,12 +22,14 @@ def test_object(): |
516 | ppa = Ppa('test-ppa-name', 'test-team-name') |
517 | assert ppa |
518 | |
519 | + |
520 | def test_description(): |
521 | """Check specifying a description when creating a PPA""" |
522 | ppa = Ppa('test-ppa-name', 'test-team-name', 'test-description') |
523 | |
524 | assert 'test-description' in ppa.ppa_description |
525 | |
526 | + |
527 | def test_address(): |
528 | """Check getting the PPA address""" |
529 | ppa = Ppa('test', 'team') |
530 | diff --git a/tests/test_ppa_group.py b/tests/test_ppa_group.py |
531 | index dd45e80..e03a841 100644 |
532 | --- a/tests/test_ppa_group.py |
533 | +++ b/tests/test_ppa_group.py |
534 | @@ -14,11 +14,12 @@ import sys |
535 | import pytest |
536 | |
537 | sys.path.insert(0, os.path.realpath( |
538 | - os.path.join(os.path.dirname(__file__), ".."))) |
539 | + os.path.join(os.path.dirname(__file__), ".."))) |
540 | |
541 | from ppa.ppa import Ppa |
542 | from ppa.ppa_group import PpaGroup, PpaAlreadyExists |
543 | |
544 | + |
545 | class PersonMock: |
546 | def __init__(self, name): |
547 | self.name = name |
548 | @@ -42,10 +43,10 @@ class PersonMock: |
549 | |
550 | class LaunchpadMock: |
551 | def __init__(self): |
552 | - self.people = { 'me': PersonMock('me') } |
553 | + self.people = {'me': PersonMock('me')} |
554 | |
555 | def add_person(self, name): |
556 | - print("Adding person %s" %(name)) |
557 | + print(f"Adding person {name}") |
558 | self.people[name] = PersonMock(name) |
559 | |
560 | @property |
561 | @@ -83,6 +84,7 @@ def test_object(): |
562 | ppa_group = PpaGroup(service=LpServiceMock()) |
563 | assert(ppa_group) |
564 | |
565 | + |
566 | def test_create_ppa(): |
567 | """Checks that PpaGroups can create PPAs""" |
568 | name = 'test_ppa' |
569 | @@ -92,13 +94,15 @@ def test_create_ppa(): |
570 | assert(name in ppa.address) |
571 | assert(type(ppa.description) is str) |
572 | |
573 | + |
574 | def test_create_existing_ppa(): |
575 | """Check exception creating an already created PPA""" |
576 | name = 'test_ppa' |
577 | ppa_group = PpaGroup(service=LpServiceMock()) |
578 | - ppa = ppa_group.create(name) |
579 | + ppa_group.create(name) |
580 | with pytest.raises(PpaAlreadyExists): |
581 | - ppa = ppa_group.create(name) |
582 | + ppa_group.create(name) |
583 | + |
584 | |
585 | def test_create_with_description(): |
586 | """Check setting a description for a PPA""" |
587 | @@ -108,6 +112,7 @@ def test_create_with_description(): |
588 | assert(ppa is not None) |
589 | assert(ppa.description == description) |
590 | |
591 | + |
592 | def test_create_with_team(): |
593 | """Check creating a PPA for a particular team""" |
594 | lp = LpServiceMock() |
595 | @@ -117,22 +122,25 @@ def test_create_with_team(): |
596 | assert(ppa is not None) |
597 | assert(ppa.address == 'ppa:test_team_name/ppa_test_name') |
598 | |
599 | + |
600 | def test_create_for_lpbug(): |
601 | """Check associating a bug # when creating a PPA""" |
602 | ppa_group = PpaGroup(service=LpServiceMock()) |
603 | lpbug = '1234567' |
604 | - ppa = ppa_group.create('lp'+lpbug) |
605 | + ppa = ppa_group.create('lp' + lpbug) |
606 | assert(ppa is not None) |
607 | assert(lpbug in ppa.description) |
608 | |
609 | + |
610 | def test_create_for_merge_proposal(): |
611 | """Check associating a merge proposal when creating a PPA""" |
612 | ppa_group = PpaGroup(service=LpServiceMock()) |
613 | version = '1.2.3-4' |
614 | - ppa = ppa_group.create('merge.'+version) |
615 | + ppa = ppa_group.create('merge.' + version) |
616 | assert(ppa is not None) |
617 | assert(version in ppa.description) |
618 | |
619 | + |
620 | def test_list_ppas(): |
621 | """Check listing the PPAs for a PPA group""" |
622 | test_ppa_list = ['a', 'b', 'c', 'd'] |
623 | diff --git a/tests/test_scripts_ppa.py b/tests/test_scripts_ppa.py |
624 | index dc4e411..4c37e29 100644 |
625 | --- a/tests/test_scripts_ppa.py |
626 | +++ b/tests/test_scripts_ppa.py |
627 | @@ -25,14 +25,15 @@ loader = importlib.machinery.SourceFileLoader(SCRIPT_NAME, script_path) |
628 | script = types.ModuleType(loader.name) |
629 | loader.exec_module(script) |
630 | |
631 | + |
632 | @pytest.fixture |
633 | def fake_config(): |
634 | return { |
635 | 'ppa_name': 'testing', |
636 | 'team_name': 'tester', |
637 | 'wait_seconds': 0.1 |
638 | - } |
639 | - return script.create_config(args) |
640 | + } |
641 | + |
642 | |
643 | @pytest.fixture |
644 | def fake_source_package(): |
645 | @@ -41,10 +42,11 @@ def fake_source_package(): |
646 | 'name': 'test-source', |
647 | 'version': '1.0-2ubuntu3', |
648 | 'binaries': [ |
649 | - { 'name': 'foo', } |
650 | - { 'name': 'bar', } |
651 | - ] |
652 | - } |
653 | + {'name': 'foo'}, |
654 | + {'name': 'bar'} |
655 | + ] |
656 | + } |
657 | + |
658 | |
659 | @pytest.fixture |
660 | def fake_binary_package(): |
661 | @@ -52,7 +54,8 @@ def fake_binary_package(): |
662 | return { |
663 | 'name': 'test-binary', |
664 | 'source': 'test-source', |
665 | - } |
666 | + } |
667 | + |
668 | |
669 | @pytest.fixture |
670 | def fake_build(): |
671 | @@ -60,7 +63,8 @@ def fake_build(): |
672 | return { |
673 | 'binary': 'foo', |
674 | 'status': 'Failed', |
675 | - } |
676 | + } |
677 | + |
678 | |
679 | @pytest.fixture |
680 | def fake_bug_report(): |
681 | @@ -69,10 +73,11 @@ def fake_bug_report(): |
682 | 'id': 1234, |
683 | 'title': 'foobar', |
684 | 'desc': 'baz', |
685 | - } |
686 | + } |
687 | + |
688 | |
689 | def test_create_parser(): |
690 | - parser = script.create_parser() |
691 | + # parser = script.create_parser() |
692 | |
693 | # TODO: Don't I have an existing example arg-parse test somewhere? |
694 | # TODO: Check command is recognized |
695 | @@ -85,74 +90,87 @@ def test_create_parser(): |
696 | |
697 | pass |
698 | |
699 | + |
700 | @pytest.mark.xfail(reason="Unimplemented") |
701 | def test_create_config(): |
702 | - args = [] |
703 | + # args = [] |
704 | # TODO: Set config_filename, ppa_name, team_name in args |
705 | - config = script.create_config(args) |
706 | + # config = script.create_config(args) |
707 | |
708 | pass |
709 | |
710 | + |
711 | # TODO: Monkeypatch in Lp = MockLp |
712 | @pytest.mark.xfail(reason="Unimplemented") |
713 | def test_get_ppa(): |
714 | # TODO: In fake_config, set ppa_name, team_name, and lp |
715 | - ppa = script.get_ppa(fake_config) |
716 | + # ppa = script.get_ppa(fake_config) |
717 | # TODO: Verify type(ppa) is Ppa |
718 | # TODO: Verify that ppa_name is set properly in ppa |
719 | # TODO: Verify that team_name is set properly in ppa |
720 | # TODO: Verify that lp is set properly in ppa |
721 | + pass |
722 | + |
723 | |
724 | @pytest.mark.xfail(reason="Unimplemented") |
725 | def test_command_create(fake_config): |
726 | - assert command_create(fake_config) == 0 |
727 | + assert script.command_create(fake_config) == 0 |
728 | |
729 | # TODO: Specify a ppa_name and verify it gets used properly |
730 | # TODO: Specify a team name |
731 | # TODO: Verify if no team name specified, the lp_user is used |
732 | # instead |
733 | # TODO: Verify the ppa_address is set as expected |
734 | + pass |
735 | + |
736 | |
737 | @pytest.mark.xfail(reason="Unimplemented") |
738 | def test_command_desc(fake_config): |
739 | - assert command_desc(fake_config) == 0 |
740 | + assert script.command_desc(fake_config) == 0 |
741 | # TODO: Assert that if --dry-run specified, there are no actual |
742 | # changes requested of launchpad |
743 | # TODO: Verify the description gets set as expected |
744 | |
745 | + |
746 | @pytest.mark.xfail(reason="Unimplemented") |
747 | def test_command_destroy(fake_config): |
748 | # TODO: Create a fake ppa to be destroyed |
749 | - assert command_destroy(fake_config) == 0 |
750 | + assert script.command_destroy(fake_config) == 0 |
751 | # TODO: Verify the ppa is requested to be deleted |
752 | |
753 | + |
754 | @pytest.mark.xfail(reason="Unimplemented") |
755 | def test_command_list(fake_config): |
756 | # TODO: Create a fake ppa with contents to be listed |
757 | - assert command_list(fake_config) == 0 |
758 | + assert script.command_list(fake_config) == 0 |
759 | # TODO: Verify the ppa addresses get listed |
760 | |
761 | + |
762 | @pytest.mark.xfail(reason="Unimplemented") |
763 | def test_command_exists(fake_config): |
764 | # TODO: Create fake ppa that exists |
765 | - assert command_exists(fake_config) == 0 |
766 | + assert script.command_exists(fake_config) == 0 |
767 | # TODO: Verify this returns true when the ppa does exist |
768 | |
769 | + |
770 | @pytest.mark.xfail(reason="Unimplemented") |
771 | def test_command_not_exists(fake_config): |
772 | # TODO: Verify this returns true when the ppa does not exist |
773 | - assert command_exists(fake_config) == 1 |
774 | + assert script.command_exists(fake_config) == 1 |
775 | + |
776 | |
777 | @pytest.mark.xfail(reason="Unimplemented") |
778 | def test_command_show(fake_config): |
779 | - assert command_show(fake_config) == 0 |
780 | + assert script.command_show(fake_config) == 0 |
781 | + |
782 | |
783 | @pytest.mark.xfail(reason="Unimplemented") |
784 | def test_command_status(fake_config): |
785 | - assert command_status(fake_config) == 0 |
786 | + assert script.command_status(fake_config) == 0 |
787 | # TODO: Capture stdout and compare with expected |
788 | |
789 | + |
790 | @pytest.mark.xfail(reason="Unimplemented") |
791 | def test_command_wait(fake_config): |
792 | # TODO: Set wait period to 1 sec |
793 | - assert command_wait(fake_config) == 0 |
794 | + assert script.command_wait(fake_config) == 0 |
795 | diff --git a/tests/test_version.py b/tests/test_version.py |
796 | index 0a1c397..fa6066f 100644 |
797 | --- a/tests/test_version.py |
798 | +++ b/tests/test_version.py |
799 | @@ -11,9 +11,7 @@ |
800 | import os |
801 | import sys |
802 | sys.path.insert(0, os.path.realpath( |
803 | - os.path.join(os.path.dirname(__file__), ".."))) |
804 | - |
805 | -import pytest |
806 | + os.path.join(os.path.dirname(__file__), ".."))) |
807 | |
808 | from ppa._version import __version__, __version_info__ |
809 | |
810 | @@ -24,6 +22,7 @@ def test_version(): |
811 | assert(__version__[0].isdigit()) |
812 | assert(__version__[-1] != '.') |
813 | |
814 | + |
815 | def test_version_info(): |
816 | assert(type(__version_info__) is tuple) |
817 | assert(len(__version_info__) > 1) |
Really seems to be just cleanups, whitespace lines, line length, f-string, import order and useless imports ... the stuff you know from your friendly linters.
I didn't see anything odd, therefore +1 to this