Merge lp:~paelzer/cloud-init/bug-1589174-fix-tests-in-adt-env into lp:~cloud-init-dev/cloud-init/trunk
- bug-1589174-fix-tests-in-adt-env
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 1230 |
Proposed branch: | lp:~paelzer/cloud-init/bug-1589174-fix-tests-in-adt-env |
Merge into: | lp:~cloud-init-dev/cloud-init/trunk |
Diff against target: |
727 lines (+205/-179) 5 files modified
cloudinit/config/cc_apt_configure.py (+16/-40) cloudinit/gpg.py (+76/-0) packages/bddeb (+6/-0) tests/unittests/test_handler/test_handler_apt_configure_sources_list.py (+37/-24) tests/unittests/test_handler/test_handler_apt_source.py (+70/-115) |
To merge this branch: | bzr merge lp:~paelzer/cloud-init/bug-1589174-fix-tests-in-adt-env |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
cloud-init Commiters | Pending | ||
Review via email: mp+296643@code.launchpad.net |
Commit message
Fixes to the unittests to run in more environments;
As well as some improvements that were found along testing them and due to the fact that we review some of that code again in the scope of curtin currently.
Tests:
- add a test for an alternate keyserver
- harden mirrorfail tests to detect and skip if no network is available
- improve apt_source related tests to work on CentOS7
Changes:
- gpg key handling is now in python instead of a shell blob
- packages/bddeb has an option to sign as someone else than smoser
- make exception handling of apt_source features more specific (i.e. no catch Exception)
- rename some functions to relfect better what they actually do
- capture some helper subp calls output to avoid spilling into stdout when not intended
Description of the change
Fixes to the unittests to run in more environments;
As well as some improvements that were found along testing them and due to the fact that we review some of that code again in the scope of curtin currently.
Tests:
- add a test for an alternate keyserver
- harden mirrorfail tests to detect and skip if no network is available
- improve apt_source related tests to work on CentOS7
Changes:
- gpg key handling is now in python instead of a shell blob
- packages/bddeb has an option to sign as someone else than smoser
- make exception handling of apt_source features more specific (i.e. no catch Exception)
- rename some functions to relfect better what they actually do
- capture some helper subp calls output to avoid spilling into stdout when not intended
- 1246. By Christian Ehrhardt
-
merge with upstream to avoid merge conflicts on the merge proposal
Christian Ehrhardt (paelzer) wrote : | # |
Scott Moser (smoser) wrote : | # |
mostly looks good. a couple comments. thank you.
- 1247. By Christian Ehrhardt
-
move gpg functions into gpg.py
This helps for cleaner code structuring.
ALong that makeing sure all these functions have a gpg_prefix. - 1248. By Christian Ehrhardt
-
improve error handling and reporting in gpg functions
- 1249. By Christian Ehrhardt
-
move SkipTest definition to tests/unittests
/helpers. py to be reusable - 1250. By Christian Ehrhardt
-
remove unused BIN_APT constant
- 1251. By Christian Ehrhardt
-
fix docstring for check connectivity
- 1252. By Christian Ehrhardt
-
mock is_resolvable in mirrorfail tests to remove dependency to external net
Christian Ehrhardt (paelzer) wrote : | # |
Thanks, addressed all you mentioned - will throw it through more testing now.
Christian Ehrhardt (paelzer) wrote : | # |
Ok, fine in sbuild, a partially shutdown network env and buildd (https:/
Preview Diff
1 | === modified file 'cloudinit/config/cc_apt_configure.py' |
2 | --- cloudinit/config/cc_apt_configure.py 2016-06-03 19:27:32 +0000 |
3 | +++ cloudinit/config/cc_apt_configure.py 2016-06-09 08:37:01 +0000 |
4 | @@ -24,6 +24,7 @@ |
5 | |
6 | from cloudinit import templater |
7 | from cloudinit import util |
8 | +from cloudinit import gpg |
9 | |
10 | distros = ['ubuntu', 'debian'] |
11 | |
12 | @@ -34,21 +35,6 @@ |
13 | # this will match 'XXX:YYY' (ie, 'cloud-archive:foo' or 'ppa:bar') |
14 | ADD_APT_REPO_MATCH = r"^[\w-]+:\w" |
15 | |
16 | -# A temporary shell program to get a given gpg key |
17 | -# from a given keyserver |
18 | -EXPORT_GPG_KEYID = """ |
19 | - k=${1} ks=${2}; |
20 | - exec 2>/dev/null |
21 | - [ -n "$k" ] || exit 1; |
22 | - armour=$(gpg --export --armour "${k}") |
23 | - if [ -z "${armour}" ]; then |
24 | - gpg --keyserver ${ks} --recv "${k}" >/dev/null && |
25 | - armour=$(gpg --export --armour "${k}") && |
26 | - gpg --batch --yes --delete-keys "${k}" |
27 | - fi |
28 | - [ -n "${armour}" ] && echo "${armour}" |
29 | -""" |
30 | - |
31 | |
32 | def handle(name, cfg, cloud, log, _args): |
33 | if util.is_false(cfg.get('apt_configure_enabled', True)): |
34 | @@ -94,8 +80,8 @@ |
35 | def matcher(x): |
36 | return False |
37 | |
38 | - errors = add_sources(cfg['apt_sources'], params, |
39 | - aa_repo_match=matcher) |
40 | + errors = add_apt_sources(cfg['apt_sources'], params, |
41 | + aa_repo_match=matcher) |
42 | for e in errors: |
43 | log.warn("Add source error: %s", ':'.join(e)) |
44 | |
45 | @@ -108,17 +94,7 @@ |
46 | util.logexc(log, "Failed to run debconf-set-selections") |
47 | |
48 | |
49 | -# get gpg keyid from keyserver |
50 | -def getkeybyid(keyid, keyserver): |
51 | - with util.ExtendedTemporaryFile(suffix='.sh', mode="w+", ) as fh: |
52 | - fh.write(EXPORT_GPG_KEYID) |
53 | - fh.flush() |
54 | - cmd = ['/bin/sh', fh.name, keyid, keyserver] |
55 | - (stdout, _stderr) = util.subp(cmd) |
56 | - return stdout.strip() |
57 | - |
58 | - |
59 | -def mirror2lists_fileprefix(mirror): |
60 | +def mirrorurl_to_apt_fileprefix(mirror): |
61 | string = mirror |
62 | # take off http:// or ftp:// |
63 | if string.endswith("/"): |
64 | @@ -135,8 +111,8 @@ |
65 | nmirror = new_mirrors.get(name) |
66 | if not nmirror: |
67 | continue |
68 | - oprefix = os.path.join(lists_d, mirror2lists_fileprefix(omirror)) |
69 | - nprefix = os.path.join(lists_d, mirror2lists_fileprefix(nmirror)) |
70 | + oprefix = os.path.join(lists_d, mirrorurl_to_apt_fileprefix(omirror)) |
71 | + nprefix = os.path.join(lists_d, mirrorurl_to_apt_fileprefix(nmirror)) |
72 | if oprefix == nprefix: |
73 | continue |
74 | olen = len(oprefix) |
75 | @@ -171,7 +147,7 @@ |
76 | templater.render_to_file(template_fn, '/etc/apt/sources.list', params) |
77 | |
78 | |
79 | -def add_key_raw(key): |
80 | +def add_apt_key_raw(key): |
81 | """ |
82 | actual adding of a key as defined in key argument |
83 | to the system |
84 | @@ -179,10 +155,10 @@ |
85 | try: |
86 | util.subp(('apt-key', 'add', '-'), key) |
87 | except util.ProcessExecutionError: |
88 | - raise Exception('failed add key') |
89 | - |
90 | - |
91 | -def add_key(ent): |
92 | + raise ValueError('failed to add apt GPG Key to apt keyring') |
93 | + |
94 | + |
95 | +def add_apt_key(ent): |
96 | """ |
97 | add key to the system as defined in ent (if any) |
98 | supports raw keys or keyid's |
99 | @@ -192,10 +168,10 @@ |
100 | keyserver = "keyserver.ubuntu.com" |
101 | if 'keyserver' in ent: |
102 | keyserver = ent['keyserver'] |
103 | - ent['key'] = getkeybyid(ent['keyid'], keyserver) |
104 | + ent['key'] = gpg.gpg_getkeybyid(ent['keyid'], keyserver) |
105 | |
106 | if 'key' in ent: |
107 | - add_key_raw(ent['key']) |
108 | + add_apt_key_raw(ent['key']) |
109 | |
110 | |
111 | def convert_to_new_format(srclist): |
112 | @@ -222,7 +198,7 @@ |
113 | return srcdict |
114 | |
115 | |
116 | -def add_sources(srclist, template_params=None, aa_repo_match=None): |
117 | +def add_apt_sources(srclist, template_params=None, aa_repo_match=None): |
118 | """ |
119 | add entries in /etc/apt/sources.list.d for each abbreviated |
120 | sources.list entry in 'srclist'. When rendering template, also |
121 | @@ -245,8 +221,8 @@ |
122 | |
123 | # keys can be added without specifying a source |
124 | try: |
125 | - add_key(ent) |
126 | - except Exception as detail: |
127 | + add_apt_key(ent) |
128 | + except ValueError as detail: |
129 | errorlist.append([ent, detail]) |
130 | |
131 | if 'source' not in ent: |
132 | |
133 | === added file 'cloudinit/gpg.py' |
134 | --- cloudinit/gpg.py 1970-01-01 00:00:00 +0000 |
135 | +++ cloudinit/gpg.py 2016-06-09 08:37:01 +0000 |
136 | @@ -0,0 +1,76 @@ |
137 | +"""gpg.py - Collection of gpg key related functions""" |
138 | +# vi: ts=4 expandtab |
139 | +# |
140 | +# Copyright (C) 2016 Canonical Ltd. |
141 | +# |
142 | +# Author: Scott Moser <scott.moser@canonical.com> |
143 | +# Author: Juerg Haefliger <juerg.haefliger@hp.com> |
144 | +# Author: Joshua Harlow <harlowja@yahoo-inc.com> |
145 | +# Author: Christian Ehrhardt <christian.ehrhardt@canonical.com> |
146 | +# |
147 | +# This program is free software: you can redistribute it and/or modify |
148 | +# it under the terms of the GNU General Public License version 3, as |
149 | +# published by the Free Software Foundation. |
150 | +# |
151 | +# This program is distributed in the hope that it will be useful, |
152 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of |
153 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
154 | +# GNU General Public License for more details. |
155 | +# |
156 | +# You should have received a copy of the GNU General Public License |
157 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
158 | + |
159 | +from cloudinit import util |
160 | +from cloudinit import log as logging |
161 | + |
162 | +LOG = logging.getLogger(__name__) |
163 | + |
164 | + |
165 | +def gpg_export_armour(key): |
166 | + """Export gpg key, armoured key gets returned""" |
167 | + try: |
168 | + (armour, _) = util.subp(["gpg", "--export", "--armour", key], |
169 | + capture=True) |
170 | + except util.ProcessExecutionError as error: |
171 | + # debug, since it happens for any key not on the system initially |
172 | + LOG.debug('Failed to export armoured key "%s": %s', key, error) |
173 | + armour = None |
174 | + return armour |
175 | + |
176 | + |
177 | +def gpg_recv_key(key, keyserver): |
178 | + """Receive gpg key from the specified keyserver""" |
179 | + LOG.debug('Receive gpg key "%s"', key) |
180 | + try: |
181 | + util.subp(["gpg", "--keyserver", keyserver, "--recv", key], |
182 | + capture=True) |
183 | + except util.ProcessExecutionError as error: |
184 | + raise ValueError(('Failed to import key "%s" ' |
185 | + 'from server "%s" - error %s') % |
186 | + (key, keyserver, error)) |
187 | + |
188 | + |
189 | +def gpg_delete_key(key): |
190 | + """Delete the specified key from the local gpg ring""" |
191 | + try: |
192 | + util.subp(["gpg", "--batch", "--yes", "--delete-keys", key], |
193 | + capture=True) |
194 | + except util.ProcessExecutionError as error: |
195 | + LOG.warn('Failed delete key "%s": %s', key, error) |
196 | + |
197 | + |
198 | +def gpg_getkeybyid(keyid, keyserver): |
199 | + """get gpg keyid from keyserver""" |
200 | + armour = gpg_export_armour(keyid) |
201 | + if not armour: |
202 | + try: |
203 | + gpg_recv_key(keyid, keyserver=keyserver) |
204 | + armour = gpg_export_armour(keyid) |
205 | + except ValueError: |
206 | + LOG.exception('Failed to obtain gpg key %s', keyid) |
207 | + raise |
208 | + finally: |
209 | + # delete just imported key to leave environment as it was before |
210 | + gpg_delete_key(keyid) |
211 | + |
212 | + return armour.rstrip('\n') |
213 | |
214 | === modified file 'packages/bddeb' |
215 | --- packages/bddeb 2016-05-26 15:51:38 +0000 |
216 | +++ packages/bddeb 2016-06-09 08:37:01 +0000 |
217 | @@ -148,11 +148,17 @@ |
218 | parser.add_argument("--sign", default=False, action='store_true', |
219 | help="sign result. do not pass -us -uc to debuild") |
220 | |
221 | + parser.add_argument("--signuser", default=False, action='store', |
222 | + help="user to sign, see man dpkg-genchanges") |
223 | + |
224 | args = parser.parse_args() |
225 | |
226 | if not args.sign: |
227 | args.debuild_args.extend(['-us', '-uc']) |
228 | |
229 | + if args.signuser: |
230 | + args.debuild_args.extend(['-e%s' % args.signuser]) |
231 | + |
232 | os.environ['INIT_SYSTEM'] = args.init_system |
233 | |
234 | capture = True |
235 | |
236 | === modified file 'tests/unittests/test_handler/test_handler_apt_configure_sources_list.py' |
237 | --- tests/unittests/test_handler/test_handler_apt_configure_sources_list.py 2016-06-05 00:50:37 +0000 |
238 | +++ tests/unittests/test_handler/test_handler_apt_configure_sources_list.py 2016-06-09 08:37:01 +0000 |
239 | @@ -20,6 +20,8 @@ |
240 | from cloudinit.config import cc_apt_configure |
241 | from cloudinit.sources import DataSourceNone |
242 | |
243 | +from cloudinit.distros.debian import Distro |
244 | + |
245 | from .. import helpers as t_help |
246 | |
247 | LOG = logging.getLogger(__name__) |
248 | @@ -115,38 +117,47 @@ |
249 | {'codename': '', 'primary': mirrorcheck, 'mirror': mirrorcheck}) |
250 | |
251 | def test_apt_source_list_debian(self): |
252 | - """test_apt_source_list_debian |
253 | - Test rendering of a source.list from template for debian |
254 | - """ |
255 | + """Test rendering of a source.list from template for debian""" |
256 | self.apt_source_list('debian', 'http://httpredir.debian.org/debian') |
257 | |
258 | def test_apt_source_list_ubuntu(self): |
259 | - """test_apt_source_list_ubuntu |
260 | - Test rendering of a source.list from template for ubuntu |
261 | - """ |
262 | + """Test rendering of a source.list from template for ubuntu""" |
263 | self.apt_source_list('ubuntu', 'http://archive.ubuntu.com/ubuntu/') |
264 | |
265 | - @t_help.skipIf(True, "LP: #1589174") |
266 | + @staticmethod |
267 | + def myresolve(name): |
268 | + """Fake util.is_resolvable for mirrorfail tests""" |
269 | + if name == "does.not.exist": |
270 | + print("Faking FAIL for '%s'" % name) |
271 | + return False |
272 | + else: |
273 | + print("Faking SUCCESS for '%s'" % name) |
274 | + return True |
275 | + |
276 | def test_apt_srcl_debian_mirrorfail(self): |
277 | - """test_apt_source_list_debian_mirrorfail |
278 | - Test rendering of a source.list from template for debian |
279 | - """ |
280 | - self.apt_source_list('debian', ['http://does.not.exist', |
281 | - 'http://httpredir.debian.org/debian'], |
282 | - 'http://httpredir.debian.org/debian') |
283 | + """Test rendering of a source.list from template for debian""" |
284 | + with mock.patch.object(util, 'is_resolvable', |
285 | + side_effect=self.myresolve) as mockresolve: |
286 | + self.apt_source_list('debian', |
287 | + ['http://does.not.exist', |
288 | + 'http://httpredir.debian.org/debian'], |
289 | + 'http://httpredir.debian.org/debian') |
290 | + mockresolve.assert_any_call("does.not.exist") |
291 | + mockresolve.assert_any_call("httpredir.debian.org") |
292 | |
293 | def test_apt_srcl_ubuntu_mirrorfail(self): |
294 | - """test_apt_source_list_ubuntu_mirrorfail |
295 | - Test rendering of a source.list from template for ubuntu |
296 | - """ |
297 | - self.apt_source_list('ubuntu', ['http://does.not.exist', |
298 | - 'http://archive.ubuntu.com/ubuntu/'], |
299 | - 'http://archive.ubuntu.com/ubuntu/') |
300 | + """Test rendering of a source.list from template for ubuntu""" |
301 | + with mock.patch.object(util, 'is_resolvable', |
302 | + side_effect=self.myresolve) as mockresolve: |
303 | + self.apt_source_list('ubuntu', |
304 | + ['http://does.not.exist', |
305 | + 'http://archive.ubuntu.com/ubuntu/'], |
306 | + 'http://archive.ubuntu.com/ubuntu/') |
307 | + mockresolve.assert_any_call("does.not.exist") |
308 | + mockresolve.assert_any_call("archive.ubuntu.com") |
309 | |
310 | def test_apt_srcl_custom(self): |
311 | - """test_apt_srcl_custom |
312 | - Test rendering from a custom source.list template |
313 | - """ |
314 | + """Test rendering from a custom source.list template""" |
315 | cfg = util.load_yaml(YAML_TEXT_CUSTOM_SL) |
316 | mycloud = self._get_cloud('ubuntu') |
317 | |
318 | @@ -155,8 +166,10 @@ |
319 | with mock.patch.object(util, 'subp', self.subp): |
320 | with mock.patch.object(cc_apt_configure, 'get_release', |
321 | return_value='fakerelease'): |
322 | - cc_apt_configure.handle("notimportant", cfg, mycloud, |
323 | - LOG, None) |
324 | + with mock.patch.object(Distro, 'get_primary_arch', |
325 | + return_value='amd64'): |
326 | + cc_apt_configure.handle("notimportant", cfg, mycloud, |
327 | + LOG, None) |
328 | |
329 | mockwrite.assert_called_once_with( |
330 | '/etc/apt/sources.list', |
331 | |
332 | === modified file 'tests/unittests/test_handler/test_handler_apt_source.py' |
333 | --- tests/unittests/test_handler/test_handler_apt_source.py 2016-06-04 02:56:45 +0000 |
334 | +++ tests/unittests/test_handler/test_handler_apt_source.py 2016-06-09 08:37:01 +0000 |
335 | @@ -14,6 +14,7 @@ |
336 | |
337 | from cloudinit.config import cc_apt_configure |
338 | from cloudinit import util |
339 | +from cloudinit import gpg |
340 | |
341 | from ..helpers import TestCase |
342 | |
343 | @@ -62,7 +63,8 @@ |
344 | get_rel.return_value = self.release |
345 | self.addCleanup(patcher.stop) |
346 | |
347 | - def _get_default_params(self): |
348 | + @staticmethod |
349 | + def _get_default_params(): |
350 | """get_default_params |
351 | Get the most basic default mrror and release info to be used in tests |
352 | """ |
353 | @@ -86,7 +88,7 @@ |
354 | """ |
355 | params = self._get_default_params() |
356 | |
357 | - cc_apt_configure.add_sources(cfg, params) |
358 | + cc_apt_configure.add_apt_sources(cfg, params) |
359 | |
360 | self.assertTrue(os.path.isfile(filename)) |
361 | |
362 | @@ -98,10 +100,7 @@ |
363 | contents, flags=re.IGNORECASE)) |
364 | |
365 | def test_apt_src_basic(self): |
366 | - """test_apt_src_basic |
367 | - Test Fix deb source string, has to overwrite mirror conf in params. |
368 | - Test with a filename provided in config. |
369 | - """ |
370 | + """Test deb source string, overwrite mirror and filename""" |
371 | cfg = {'source': ('deb http://archive.ubuntu.com/ubuntu' |
372 | ' karmic-backports' |
373 | ' main universe multiverse restricted'), |
374 | @@ -109,11 +108,7 @@ |
375 | self.apt_src_basic(self.aptlistfile, [cfg]) |
376 | |
377 | def test_apt_src_basic_dict(self): |
378 | - """test_apt_src_basic_dict |
379 | - Test Fix deb source string, has to overwrite mirror conf in params. |
380 | - Test with a filename provided in config. |
381 | - Provided in a dictionary with filename being the key (new format) |
382 | - """ |
383 | + """Test deb source string, overwrite mirror and filename (dict)""" |
384 | cfg = {self.aptlistfile: {'source': |
385 | ('deb http://archive.ubuntu.com/ubuntu' |
386 | ' karmic-backports' |
387 | @@ -143,10 +138,7 @@ |
388 | contents, flags=re.IGNORECASE)) |
389 | |
390 | def test_apt_src_basic_tri(self): |
391 | - """test_apt_src_basic_tri |
392 | - Test Fix three deb source string, has to overwrite mirror conf in |
393 | - params. Test with filenames provided in config. |
394 | - """ |
395 | + """Test Fix three deb source string with filenames""" |
396 | cfg1 = {'source': ('deb http://archive.ubuntu.com/ubuntu' |
397 | ' karmic-backports' |
398 | ' main universe multiverse restricted'), |
399 | @@ -162,11 +154,7 @@ |
400 | self.apt_src_basic_tri([cfg1, cfg2, cfg3]) |
401 | |
402 | def test_apt_src_basic_dict_tri(self): |
403 | - """test_apt_src_basic_dict_tri |
404 | - Test Fix three deb source string, has to overwrite mirror conf in |
405 | - params. Test with filenames provided in config. |
406 | - Provided in a dictionary with filename being the key (new format) |
407 | - """ |
408 | + """Test Fix three deb source string with filenames (dict)""" |
409 | cfg = {self.aptlistfile: {'source': |
410 | ('deb http://archive.ubuntu.com/ubuntu' |
411 | ' karmic-backports' |
412 | @@ -182,10 +170,7 @@ |
413 | self.apt_src_basic_tri(cfg) |
414 | |
415 | def test_apt_src_basic_nofn(self): |
416 | - """test_apt_src_basic_nofn |
417 | - Test Fix deb source string, has to overwrite mirror conf in params. |
418 | - Test without a filename provided in config and test for known fallback. |
419 | - """ |
420 | + """Test Fix three deb source string without filenames (dict)""" |
421 | cfg = {'source': ('deb http://archive.ubuntu.com/ubuntu' |
422 | ' karmic-backports' |
423 | ' main universe multiverse restricted')} |
424 | @@ -197,7 +182,7 @@ |
425 | Test Autoreplacement of MIRROR and RELEASE in source specs |
426 | """ |
427 | params = self._get_default_params() |
428 | - cc_apt_configure.add_sources(cfg, params) |
429 | + cc_apt_configure.add_apt_sources(cfg, params) |
430 | |
431 | self.assertTrue(os.path.isfile(filename)) |
432 | |
433 | @@ -208,10 +193,7 @@ |
434 | contents, flags=re.IGNORECASE)) |
435 | |
436 | def test_apt_src_replace(self): |
437 | - """test_apt_src_replace |
438 | - Test Autoreplacement of MIRROR and RELEASE in source specs with |
439 | - Filename being set |
440 | - """ |
441 | + """Test Autoreplacement of MIRROR and RELEASE in source specs""" |
442 | cfg = {'source': 'deb $MIRROR $RELEASE multiverse', |
443 | 'filename': self.aptlistfile} |
444 | self.apt_src_replacement(self.aptlistfile, [cfg]) |
445 | @@ -237,10 +219,7 @@ |
446 | contents, flags=re.IGNORECASE)) |
447 | |
448 | def test_apt_src_replace_tri(self): |
449 | - """test_apt_src_replace_tri |
450 | - Test three autoreplacements of MIRROR and RELEASE in source specs with |
451 | - Filename being set |
452 | - """ |
453 | + """Test triple Autoreplacement of MIRROR and RELEASE in source specs""" |
454 | cfg1 = {'source': 'deb $MIRROR $RELEASE multiverse', |
455 | 'filename': self.aptlistfile} |
456 | cfg2 = {'source': 'deb $MIRROR $RELEASE main', |
457 | @@ -250,13 +229,7 @@ |
458 | self.apt_src_replace_tri([cfg1, cfg2, cfg3]) |
459 | |
460 | def test_apt_src_replace_dict_tri(self): |
461 | - """test_apt_src_replace_dict_tri |
462 | - Test three autoreplacements of MIRROR and RELEASE in source specs with |
463 | - Filename being set |
464 | - Provided in a dictionary with filename being the key (new format) |
465 | - We also test a new special conditions of the new format that allows |
466 | - filenames to be overwritten inside the directory entry. |
467 | - """ |
468 | + """Test triple Autoreplacement in source specs (dict)""" |
469 | cfg = {self.aptlistfile: {'source': 'deb $MIRROR $RELEASE multiverse'}, |
470 | 'notused': {'source': 'deb $MIRROR $RELEASE main', |
471 | 'filename': self.aptlistfile2}, |
472 | @@ -264,10 +237,7 @@ |
473 | self.apt_src_replace_tri(cfg) |
474 | |
475 | def test_apt_src_replace_nofn(self): |
476 | - """test_apt_src_replace_nofn |
477 | - Test Autoreplacement of MIRROR and RELEASE in source specs with |
478 | - No filename being set |
479 | - """ |
480 | + """Test Autoreplacement of MIRROR and RELEASE in source specs nofile""" |
481 | cfg = {'source': 'deb $MIRROR $RELEASE multiverse'} |
482 | with mock.patch.object(os.path, 'join', side_effect=self.myjoin): |
483 | self.apt_src_replacement(self.fallbackfn, [cfg]) |
484 | @@ -280,7 +250,7 @@ |
485 | |
486 | with mock.patch.object(util, 'subp', |
487 | return_value=('fakekey 1234', '')) as mockobj: |
488 | - cc_apt_configure.add_sources(cfg, params) |
489 | + cc_apt_configure.add_apt_sources(cfg, params) |
490 | |
491 | # check if it added the right ammount of keys |
492 | calls = [] |
493 | @@ -299,9 +269,7 @@ |
494 | contents, flags=re.IGNORECASE)) |
495 | |
496 | def test_apt_src_keyid(self): |
497 | - """test_apt_src_keyid |
498 | - Test specification of a source + keyid with filename being set |
499 | - """ |
500 | + """Test specification of a source + keyid with filename being set""" |
501 | cfg = {'source': ('deb ' |
502 | 'http://ppa.launchpad.net/' |
503 | 'smoser/cloud-init-test/ubuntu' |
504 | @@ -311,10 +279,7 @@ |
505 | self.apt_src_keyid(self.aptlistfile, [cfg], 1) |
506 | |
507 | def test_apt_src_keyid_tri(self): |
508 | - """test_apt_src_keyid_tri |
509 | - Test specification of a source + keyid with filename being set |
510 | - Setting three of such, check for content and keys |
511 | - """ |
512 | + """Test 3x specification of a source + keyid with filename being set""" |
513 | cfg1 = {'source': ('deb ' |
514 | 'http://ppa.launchpad.net/' |
515 | 'smoser/cloud-init-test/ubuntu' |
516 | @@ -351,9 +316,7 @@ |
517 | contents, flags=re.IGNORECASE)) |
518 | |
519 | def test_apt_src_keyid_nofn(self): |
520 | - """test_apt_src_keyid_nofn |
521 | - Test specification of a source + keyid without filename being set |
522 | - """ |
523 | + """Test specification of a source + keyid without filename being set""" |
524 | cfg = {'source': ('deb ' |
525 | 'http://ppa.launchpad.net/' |
526 | 'smoser/cloud-init-test/ubuntu' |
527 | @@ -369,7 +332,7 @@ |
528 | params = self._get_default_params() |
529 | |
530 | with mock.patch.object(util, 'subp') as mockobj: |
531 | - cc_apt_configure.add_sources([cfg], params) |
532 | + cc_apt_configure.add_apt_sources([cfg], params) |
533 | |
534 | mockobj.assert_called_with(('apt-key', 'add', '-'), 'fakekey 4321') |
535 | |
536 | @@ -384,9 +347,7 @@ |
537 | contents, flags=re.IGNORECASE)) |
538 | |
539 | def test_apt_src_key(self): |
540 | - """test_apt_src_key |
541 | - Test specification of a source + key with filename being set |
542 | - """ |
543 | + """Test specification of a source + key with filename being set""" |
544 | cfg = {'source': ('deb ' |
545 | 'http://ppa.launchpad.net/' |
546 | 'smoser/cloud-init-test/ubuntu' |
547 | @@ -396,9 +357,7 @@ |
548 | self.apt_src_key(self.aptlistfile, cfg) |
549 | |
550 | def test_apt_src_key_nofn(self): |
551 | - """test_apt_src_key_nofn |
552 | - Test specification of a source + key without filename being set |
553 | - """ |
554 | + """Test specification of a source + key without filename being set""" |
555 | cfg = {'source': ('deb ' |
556 | 'http://ppa.launchpad.net/' |
557 | 'smoser/cloud-init-test/ubuntu' |
558 | @@ -408,15 +367,13 @@ |
559 | self.apt_src_key(self.fallbackfn, cfg) |
560 | |
561 | def test_apt_src_keyonly(self): |
562 | - """test_apt_src_keyonly |
563 | - Test specification key without source |
564 | - """ |
565 | + """Test specifying key without source""" |
566 | params = self._get_default_params() |
567 | cfg = {'key': "fakekey 4242", |
568 | 'filename': self.aptlistfile} |
569 | |
570 | with mock.patch.object(util, 'subp') as mockobj: |
571 | - cc_apt_configure.add_sources([cfg], params) |
572 | + cc_apt_configure.add_apt_sources([cfg], params) |
573 | |
574 | mockobj.assert_called_once_with(('apt-key', 'add', '-'), |
575 | 'fakekey 4242') |
576 | @@ -425,66 +382,68 @@ |
577 | self.assertFalse(os.path.isfile(self.aptlistfile)) |
578 | |
579 | def test_apt_src_keyidonly(self): |
580 | - """test_apt_src_keyidonly |
581 | - Test specification of a keyid without source |
582 | - """ |
583 | + """Test specification of a keyid without source""" |
584 | params = self._get_default_params() |
585 | cfg = {'keyid': "03683F77", |
586 | 'filename': self.aptlistfile} |
587 | |
588 | with mock.patch.object(util, 'subp', |
589 | return_value=('fakekey 1212', '')) as mockobj: |
590 | - cc_apt_configure.add_sources([cfg], params) |
591 | + cc_apt_configure.add_apt_sources([cfg], params) |
592 | |
593 | mockobj.assert_called_with(('apt-key', 'add', '-'), 'fakekey 1212') |
594 | |
595 | # filename should be ignored on key only |
596 | self.assertFalse(os.path.isfile(self.aptlistfile)) |
597 | |
598 | + def apt_src_keyid_real(self, cfg, expectedkey): |
599 | + """apt_src_keyid_real |
600 | + Test specification of a keyid without source including |
601 | + up to addition of the key (add_apt_key_raw mocked to keep the |
602 | + environment as is) |
603 | + """ |
604 | + params = self._get_default_params() |
605 | + |
606 | + with mock.patch.object(cc_apt_configure, 'add_apt_key_raw') as mockkey: |
607 | + with mock.patch.object(gpg, 'gpg_getkeybyid', |
608 | + return_value=expectedkey) as mockgetkey: |
609 | + cc_apt_configure.add_apt_sources([cfg], params) |
610 | + |
611 | + mockgetkey.assert_called_with(cfg['keyid'], |
612 | + cfg.get('keyserver', |
613 | + 'keyserver.ubuntu.com')) |
614 | + mockkey.assert_called_with(expectedkey) |
615 | + |
616 | + # filename should be ignored on key only |
617 | + self.assertFalse(os.path.isfile(self.aptlistfile)) |
618 | + |
619 | def test_apt_src_keyid_real(self): |
620 | - """test_apt_src_keyid_real |
621 | - Test specification of a keyid without source incl |
622 | - up to addition of the key (add_key_raw, getkeybyid mocked) |
623 | - """ |
624 | + """test_apt_src_keyid_real - Test keyid including key add""" |
625 | keyid = "03683F77" |
626 | - params = self._get_default_params() |
627 | cfg = {'keyid': keyid, |
628 | 'filename': self.aptlistfile} |
629 | |
630 | - with mock.patch.object(cc_apt_configure, 'add_key_raw') as mockobj: |
631 | - with mock.patch.object(cc_apt_configure, 'getkeybyid') as gkbi: |
632 | - gkbi.return_value = EXPECTEDKEY |
633 | - cc_apt_configure.add_sources([cfg], params) |
634 | - |
635 | - mockobj.assert_called_with(EXPECTEDKEY) |
636 | - |
637 | - # filename should be ignored on key only |
638 | - self.assertFalse(os.path.isfile(self.aptlistfile)) |
639 | + self.apt_src_keyid_real(cfg, EXPECTEDKEY) |
640 | |
641 | def test_apt_src_longkeyid_real(self): |
642 | - """test_apt_src_longkeyid_real |
643 | - Test specification of a long key fingerprint without source incl |
644 | - up to addition of the key (nothing but add_key_raw mocked) |
645 | - """ |
646 | - keyid = "B59D 5F15 97A5 04B7 E230 6DCA 0620 BBCF 0368 3F77" |
647 | - params = self._get_default_params() |
648 | - cfg = {'keyid': keyid, |
649 | - 'filename': self.aptlistfile} |
650 | - |
651 | - with mock.patch.object(cc_apt_configure, 'add_key_raw') as mockobj: |
652 | - with mock.patch.object(cc_apt_configure, 'getkeybyid') as gkbi: |
653 | - gkbi.return_value = EXPECTEDKEY |
654 | - cc_apt_configure.add_sources([cfg], params) |
655 | - |
656 | - mockobj.assert_called_with(EXPECTEDKEY) |
657 | - |
658 | - # filename should be ignored on key only |
659 | - self.assertFalse(os.path.isfile(self.aptlistfile)) |
660 | + """test_apt_src_longkeyid_real - Test long keyid including key add""" |
661 | + keyid = "B59D 5F15 97A5 04B7 E230 6DCA 0620 BBCF 0368 3F77" |
662 | + cfg = {'keyid': keyid, |
663 | + 'filename': self.aptlistfile} |
664 | + |
665 | + self.apt_src_keyid_real(cfg, EXPECTEDKEY) |
666 | + |
667 | + def test_apt_src_longkeyid_ks_real(self): |
668 | + """test_apt_src_longkeyid_ks_real - Test long keyid from other ks""" |
669 | + keyid = "B59D 5F15 97A5 04B7 E230 6DCA 0620 BBCF 0368 3F77" |
670 | + cfg = {'keyid': keyid, |
671 | + 'keyserver': 'keys.gnupg.net', |
672 | + 'filename': self.aptlistfile} |
673 | + |
674 | + self.apt_src_keyid_real(cfg, EXPECTEDKEY) |
675 | |
676 | def test_apt_src_ppa(self): |
677 | - """test_apt_src_ppa |
678 | - Test specification of a ppa |
679 | - """ |
680 | + """Test adding a ppa""" |
681 | params = self._get_default_params() |
682 | cfg = {'source': 'ppa:smoser/cloud-init-test', |
683 | 'filename': self.aptlistfile} |
684 | @@ -493,7 +452,8 @@ |
685 | matcher = re.compile(r'^[\w-]+:\w').search |
686 | |
687 | with mock.patch.object(util, 'subp') as mockobj: |
688 | - cc_apt_configure.add_sources([cfg], params, aa_repo_match=matcher) |
689 | + cc_apt_configure.add_apt_sources([cfg], params, |
690 | + aa_repo_match=matcher) |
691 | mockobj.assert_called_once_with(['add-apt-repository', |
692 | 'ppa:smoser/cloud-init-test']) |
693 | |
694 | @@ -501,9 +461,7 @@ |
695 | self.assertFalse(os.path.isfile(self.aptlistfile)) |
696 | |
697 | def test_apt_src_ppa_tri(self): |
698 | - """test_apt_src_ppa_tri |
699 | - Test specification of a ppa |
700 | - """ |
701 | + """Test adding three ppa's""" |
702 | params = self._get_default_params() |
703 | cfg1 = {'source': 'ppa:smoser/cloud-init-test', |
704 | 'filename': self.aptlistfile} |
705 | @@ -516,8 +474,8 @@ |
706 | matcher = re.compile(r'^[\w-]+:\w').search |
707 | |
708 | with mock.patch.object(util, 'subp') as mockobj: |
709 | - cc_apt_configure.add_sources([cfg1, cfg2, cfg3], params, |
710 | - aa_repo_match=matcher) |
711 | + cc_apt_configure.add_apt_sources([cfg1, cfg2, cfg3], params, |
712 | + aa_repo_match=matcher) |
713 | calls = [call(['add-apt-repository', 'ppa:smoser/cloud-init-test']), |
714 | call(['add-apt-repository', 'ppa:smoser/cloud-init-test2']), |
715 | call(['add-apt-repository', 'ppa:smoser/cloud-init-test3'])] |
716 | @@ -529,10 +487,7 @@ |
717 | self.assertFalse(os.path.isfile(self.aptlistfile3)) |
718 | |
719 | def test_convert_to_new_format(self): |
720 | - """test_convert_to_new_format |
721 | - Test the conversion of old to new format |
722 | - And the noop conversion of new to new format as well |
723 | - """ |
724 | + """Test the conversion of old to new format""" |
725 | cfg1 = {'source': 'deb $MIRROR $RELEASE multiverse', |
726 | 'filename': self.aptlistfile} |
727 | cfg2 = {'source': 'deb $MIRROR $RELEASE main', |
I hope it merges without conflict now