Merge ~raharper/curtin:fix/unittest-prevent-subp-by-default into curtin:master
- Git
- lp:~raharper/curtin
- fix/unittest-prevent-subp-by-default
- Merge into master
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Ryan Harper | ||||
Approved revision: | 56df4215c9d7b4671f435ca84284f1bd52f89705 | ||||
Merge reported by: | Server Team CI bot | ||||
Merged at revision: | not available | ||||
Proposed branch: | ~raharper/curtin:fix/unittest-prevent-subp-by-default | ||||
Merge into: | curtin:master | ||||
Diff against target: |
457 lines (+165/-77) 10 files modified
tests/unittests/helpers.py (+52/-3) tests/unittests/test_apt_custom_sources_list.py (+2/-0) tests/unittests/test_apt_source.py (+2/-0) tests/unittests/test_block.py (+11/-7) tests/unittests/test_block_zfs.py (+24/-20) tests/unittests/test_clear_holders.py (+5/-2) tests/unittests/test_commands_block_meta.py (+4/-0) tests/unittests/test_commands_collect_logs.py (+2/-0) tests/unittests/test_gpg.py (+50/-44) tests/unittests/test_util.py (+13/-1) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Server Team CI bot | continuous-integration | Approve | |
Dan Watkins (community) | Approve | ||
Review via email: mp+382604@code.launchpad.net |
Commit message
unittest: do not allow util.subp by default
Bring over our util.subp mock from cloud-init. Rework unittest to
enable subp as needed.
LP: #1873913
Description of the change
Server Team CI bot (server-team-bot) wrote : | # |
Dan Watkins (oddbloke) wrote : | # |
Good thinking to pull this over, I have some comments inline.
Ryan Harper (raharper) wrote : | # |
Let me drop the logging bits; I replied in line; I think the per-test enablement with a tearDown() is reasonable. Let me see if I can sub-class the main test class and put the subp_enabled ones within that; hoping to avoid duplicating code from one class to another when all we need is to enable/disable subp.
Dan Watkins (oddbloke) wrote : | # |
On Mon, Apr 20, 2020 at 08:27:59PM -0000, Ryan Harper wrote:
> Let me drop the logging bits
Thanks!
> > diff --git a/tests/
> > index 3508d4b..2be7c41 100644
> > --- a/tests/
> > +++ b/tests/
> > @@ -425,6 +425,9 @@ class TestAssertZfsSu
> > def setUp(self):
> > super(TestAsser
> >
> > + def tearDown(self):
> > + self.allowed_subp = False
>
> I think this is reasonable safe. It's off by default. The test-cases
> that know they need it set it at the start of their test_*; this just
> retains the default off unless you need it and I think help reduce the
> scope of when subp is enabled.
Yep, I think the case I was worried about isn't applicable here, so
let's stick with this.
Dan Watkins (oddbloke) wrote : | # |
Sorry, noticed one more thing! Other than that, this looks good now, thanks for the refactors!
Ryan Harper (raharper) wrote : | # |
Fixing
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:86771d11f41
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
Server Team CI bot (server-team-bot) wrote : | # |
Commit message lints:
- Line #2 has 15 too many characters. Line starts with: "Bring over our util.subp"...
Server Team CI bot (server-team-bot) wrote : | # |
Autolanding: FAILED
More details in the following jenkins job:
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Server Team CI bot (server-team-bot) wrote : | # |
Autolanding: FAILED
More details in the following jenkins job:
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Server Team CI bot (server-team-bot) wrote : | # |
Autolanding: FAILED
More details in the following jenkins job:
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Server Team CI bot (server-team-bot) wrote : | # |
Autolanding: FAILED
More details in the following jenkins job:
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Server Team CI bot (server-team-bot) wrote : | # |
Autolanding: FAILED
More details in the following jenkins job:
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
- 56df421... by Ryan Harper
-
Fix new calls of util.subp and multipath.
is_mpath_ member
Ryan Harper (raharper) wrote : | # |
wait for CI to say yes, then mark approve
Server Team CI bot (server-team-bot) wrote : | # |
Autolanding: FAILED
More details in the following jenkins job:
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:f1e57f03529
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:56df4215c9d
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
Preview Diff
1 | diff --git a/tests/unittests/helpers.py b/tests/unittests/helpers.py |
2 | index 9514745..2f5e51a 100644 |
3 | --- a/tests/unittests/helpers.py |
4 | +++ b/tests/unittests/helpers.py |
5 | @@ -1,6 +1,5 @@ |
6 | # This file is part of curtin. See LICENSE file for copyright and license info. |
7 | |
8 | -import contextlib |
9 | import imp |
10 | import importlib |
11 | import mock |
12 | @@ -10,9 +9,11 @@ import shutil |
13 | import string |
14 | import tempfile |
15 | from unittest import TestCase, skipIf |
16 | - |
17 | +from contextlib import contextmanager |
18 | from curtin import util |
19 | |
20 | +_real_subp = util.subp |
21 | + |
22 | |
23 | def builtin_module_name(): |
24 | options = ('builtins', '__builtin__') |
25 | @@ -27,7 +28,7 @@ def builtin_module_name(): |
26 | return name |
27 | |
28 | |
29 | -@contextlib.contextmanager |
30 | +@contextmanager |
31 | def simple_mocked_open(content=None): |
32 | if not content: |
33 | content = '' |
34 | @@ -54,6 +55,54 @@ def skipUnlessJsonSchema(): |
35 | class CiTestCase(TestCase): |
36 | """Common testing class which all curtin unit tests subclass.""" |
37 | |
38 | + allowed_subp = False |
39 | + SUBP_SHELL_TRUE = "shell=True" |
40 | + |
41 | + @contextmanager |
42 | + def allow_subp(self, allowed_subp): |
43 | + orig = self.allowed_subp |
44 | + try: |
45 | + self.allowed_subp = allowed_subp |
46 | + yield |
47 | + finally: |
48 | + self.allowed_subp = orig |
49 | + |
50 | + def setUp(self): |
51 | + super(CiTestCase, self).setUp() |
52 | + if self.allowed_subp is True: |
53 | + util.subp = _real_subp |
54 | + else: |
55 | + util.subp = self._fake_subp |
56 | + |
57 | + def _fake_subp(self, *args, **kwargs): |
58 | + if 'args' in kwargs: |
59 | + cmd = kwargs['args'] |
60 | + else: |
61 | + cmd = args[0] |
62 | + |
63 | + if not isinstance(cmd, str): |
64 | + cmd = cmd[0] |
65 | + pass_through = False |
66 | + if not isinstance(self.allowed_subp, (list, bool)): |
67 | + raise TypeError("self.allowed_subp supports list or bool.") |
68 | + if isinstance(self.allowed_subp, bool): |
69 | + pass_through = self.allowed_subp |
70 | + else: |
71 | + pass_through = ( |
72 | + (cmd in self.allowed_subp) or |
73 | + (self.SUBP_SHELL_TRUE in self.allowed_subp and |
74 | + kwargs.get('shell'))) |
75 | + if pass_through: |
76 | + return _real_subp(*args, **kwargs) |
77 | + raise Exception( |
78 | + "called subp. set self.allowed_subp=True to allow\n subp(%s)" % |
79 | + ', '.join([str(repr(a)) for a in args] + |
80 | + ["%s=%s" % (k, repr(v)) for k, v in kwargs.items()])) |
81 | + |
82 | + def tearDown(self): |
83 | + util.subp = _real_subp |
84 | + super(CiTestCase, self).tearDown() |
85 | + |
86 | def add_patch(self, target, attr, **kwargs): |
87 | """Patches specified target object and sets it as attr on test |
88 | instance also schedules cleanup""" |
89 | diff --git a/tests/unittests/test_apt_custom_sources_list.py b/tests/unittests/test_apt_custom_sources_list.py |
90 | index fb6eb0c..bf004b1 100644 |
91 | --- a/tests/unittests/test_apt_custom_sources_list.py |
92 | +++ b/tests/unittests/test_apt_custom_sources_list.py |
93 | @@ -93,7 +93,9 @@ class TestAptSourceConfigSourceList(CiTestCase): |
94 | def setUp(self): |
95 | super(TestAptSourceConfigSourceList, self).setUp() |
96 | self.new_root = self.tmp_dir() |
97 | + self.add_patch('curtin.util.subp', 'm_subp') |
98 | # self.patchUtils(self.new_root) |
99 | + self.m_subp.return_value = ("amd64", "") |
100 | |
101 | def _apt_source_list(self, cfg, expected): |
102 | "_apt_source_list - Test rendering from template (generic)" |
103 | diff --git a/tests/unittests/test_apt_source.py b/tests/unittests/test_apt_source.py |
104 | index 353cdf8..6ae5579 100644 |
105 | --- a/tests/unittests/test_apt_source.py |
106 | +++ b/tests/unittests/test_apt_source.py |
107 | @@ -75,6 +75,8 @@ class TestAptSourceConfig(CiTestCase): |
108 | self.aptlistfile3 = os.path.join(self.tmp, "single-deb3.list") |
109 | self.join = os.path.join |
110 | self.matcher = re.compile(ADD_APT_REPO_MATCH).search |
111 | + self.add_patch('curtin.util.subp', 'm_subp') |
112 | + self.m_subp.return_value = ('s390x', '') |
113 | |
114 | @staticmethod |
115 | def _add_apt_sources(*args, **kwargs): |
116 | diff --git a/tests/unittests/test_block.py b/tests/unittests/test_block.py |
117 | index 707386f..dcdcf51 100644 |
118 | --- a/tests/unittests/test_block.py |
119 | +++ b/tests/unittests/test_block.py |
120 | @@ -522,6 +522,12 @@ class TestPartTableSignature(CiTestCase): |
121 | gpt_content_4k = b'\x00' * 0x800 + b'EFI PART' + b'\x00' * (0x800 - 8) |
122 | null_content = b'\x00' * 0xf00 |
123 | |
124 | + def setUp(self): |
125 | + super(TestPartTableSignature, self).setUp() |
126 | + self.add_patch('curtin.util.subp', 'm_subp') |
127 | + self.m_subp.side_effect = iter([ |
128 | + util.ProcessExecutionError(stdout="", stderr="", exit_code=1)]) |
129 | + |
130 | def _test_util_load_file(self, content, device, read_len, offset, decode): |
131 | return (bytes if not decode else str)(content[offset:offset+read_len]) |
132 | |
133 | @@ -577,15 +583,13 @@ class TestPartTableSignature(CiTestCase): |
134 | (self.assertTrue if expected else self.assertFalse)( |
135 | block.check_efi_signature(self.blockdev)) |
136 | |
137 | - @mock.patch('curtin.block.util.subp') |
138 | - def test_check_vtoc_signature_finds_vtoc_returns_true(self, mock_subp): |
139 | - mock_subp.return_value = ("vtoc.....ok", "") |
140 | + def test_check_vtoc_signature_finds_vtoc_returns_true(self): |
141 | + self.m_subp.side_effect = iter([("vtoc.....ok", "")]) |
142 | self.assertTrue(block.check_vtoc_signature(self.blockdev)) |
143 | |
144 | - @mock.patch('curtin.block.util.subp') |
145 | - def test_check_vtoc_signature_returns_false_with_no_sig(self, mock_subp): |
146 | - mock_subp.side_effect = [ |
147 | - util.ProcessExecutionError(stdout="", stderr="", exit_code=1)] |
148 | + def test_check_vtoc_signature_returns_false_with_no_sig(self): |
149 | + self.m_subp.side_effect = iter([ |
150 | + util.ProcessExecutionError(stdout="", stderr="", exit_code=1)]) |
151 | self.assertFalse(block.check_vtoc_signature(self.blockdev)) |
152 | |
153 | |
154 | diff --git a/tests/unittests/test_block_zfs.py b/tests/unittests/test_block_zfs.py |
155 | index 3508d4b..e392000 100644 |
156 | --- a/tests/unittests/test_block_zfs.py |
157 | +++ b/tests/unittests/test_block_zfs.py |
158 | @@ -468,6 +468,30 @@ class TestAssertZfsSupported(CiTestCase): |
159 | with self.assertRaises(RuntimeError): |
160 | zfs.zfs_assert_supported() |
161 | |
162 | + @mock.patch('curtin.block.zfs.get_supported_filesystems') |
163 | + @mock.patch('curtin.block.zfs.util.lsb_release') |
164 | + @mock.patch('curtin.block.zfs.util.get_platform_arch') |
165 | + @mock.patch('curtin.block.zfs.util') |
166 | + def test_zfs_assert_supported_raises_exc_on_missing_binaries(self, |
167 | + mock_util, |
168 | + m_arch, |
169 | + m_release, |
170 | + m_supfs): |
171 | + """zfs_assert_supported raises RuntimeError if no zpool or zfs tools""" |
172 | + mock_util.get_platform_arch.return_value = 'amd64' |
173 | + mock_util.lsb_release.return_value = {'codename': 'bionic'} |
174 | + mock_util.subp.return_value = ("", "") |
175 | + m_supfs.return_value = ['zfs'] |
176 | + mock_util.which.return_value = None |
177 | + |
178 | + with self.assertRaises(RuntimeError): |
179 | + zfs.zfs_assert_supported() |
180 | + |
181 | + |
182 | +class TestAssertZfsSupportedSubp(TestAssertZfsSupported): |
183 | + |
184 | + allowed_subp = True |
185 | + |
186 | @mock.patch('curtin.block.zfs.util.subprocess.Popen') |
187 | @mock.patch('curtin.block.zfs.util.is_kmod_loaded') |
188 | @mock.patch('curtin.block.zfs.get_supported_filesystems') |
189 | @@ -481,7 +505,6 @@ class TestAssertZfsSupported(CiTestCase): |
190 | m_popen, |
191 | ): |
192 | """zfs_assert_supported raises RuntimeError modprobe zfs error""" |
193 | - |
194 | m_arch.return_value = 'amd64' |
195 | m_release.return_value = {'codename': 'bionic'} |
196 | m_supfs.return_value = ['ext4'] |
197 | @@ -497,25 +520,6 @@ class TestAssertZfsSupported(CiTestCase): |
198 | with self.assertRaises(RuntimeError): |
199 | zfs.zfs_assert_supported() |
200 | |
201 | - @mock.patch('curtin.block.zfs.get_supported_filesystems') |
202 | - @mock.patch('curtin.block.zfs.util.lsb_release') |
203 | - @mock.patch('curtin.block.zfs.util.get_platform_arch') |
204 | - @mock.patch('curtin.block.zfs.util') |
205 | - def test_zfs_assert_supported_raises_exc_on_missing_binaries(self, |
206 | - mock_util, |
207 | - m_arch, |
208 | - m_release, |
209 | - m_supfs): |
210 | - """zfs_assert_supported raises RuntimeError if no zpool or zfs tools""" |
211 | - mock_util.get_platform_arch.return_value = 'amd64' |
212 | - mock_util.lsb_release.return_value = {'codename': 'bionic'} |
213 | - mock_util.subp.return_value = ("", "") |
214 | - m_supfs.return_value = ['zfs'] |
215 | - mock_util.which.return_value = None |
216 | - |
217 | - with self.assertRaises(RuntimeError): |
218 | - zfs.zfs_assert_supported() |
219 | - |
220 | |
221 | class TestZfsSupported(CiTestCase): |
222 | |
223 | diff --git a/tests/unittests/test_clear_holders.py b/tests/unittests/test_clear_holders.py |
224 | index f73b49d..82b481a 100644 |
225 | --- a/tests/unittests/test_clear_holders.py |
226 | +++ b/tests/unittests/test_clear_holders.py |
227 | @@ -701,13 +701,14 @@ class TestClearHolders(CiTestCase): |
228 | mock_gen_holders_tree.return_value = self.example_holders_trees[1][1] |
229 | clear_holders.assert_clear(device) |
230 | |
231 | + @mock.patch('curtin.block.clear_holders.udev') |
232 | @mock.patch('curtin.block.clear_holders.multipath') |
233 | @mock.patch('curtin.block.clear_holders.lvm') |
234 | @mock.patch('curtin.block.clear_holders.zfs') |
235 | @mock.patch('curtin.block.clear_holders.mdadm') |
236 | @mock.patch('curtin.block.clear_holders.util') |
237 | def test_start_clear_holders_deps(self, mock_util, mock_mdadm, mock_zfs, |
238 | - mock_lvm, mock_mp): |
239 | + mock_lvm, mock_mp, mock_udev): |
240 | mock_zfs.zfs_supported.return_value = True |
241 | clear_holders.start_clear_holders_deps() |
242 | mock_mdadm.mdadm_assemble.assert_called_with( |
243 | @@ -715,13 +716,15 @@ class TestClearHolders(CiTestCase): |
244 | mock_util.load_kernel_module.assert_has_calls([ |
245 | mock.call('bcache')]) |
246 | |
247 | + @mock.patch('curtin.block.clear_holders.udev') |
248 | @mock.patch('curtin.block.clear_holders.multipath') |
249 | @mock.patch('curtin.block.clear_holders.lvm') |
250 | @mock.patch('curtin.block.clear_holders.zfs') |
251 | @mock.patch('curtin.block.clear_holders.mdadm') |
252 | @mock.patch('curtin.block.clear_holders.util') |
253 | def test_start_clear_holders_deps_nozfs(self, mock_util, mock_mdadm, |
254 | - mock_zfs, mock_lvm, mock_mp): |
255 | + mock_zfs, mock_lvm, mock_mp, |
256 | + mock_udev): |
257 | """test that we skip zfs modprobe on unsupported platforms""" |
258 | mock_zfs.zfs_supported.return_value = False |
259 | clear_holders.start_clear_holders_deps() |
260 | diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py |
261 | index aac1a9e..6da6412 100644 |
262 | --- a/tests/unittests/test_commands_block_meta.py |
263 | +++ b/tests/unittests/test_commands_block_meta.py |
264 | @@ -19,6 +19,8 @@ class TestGetPathToStorageVolume(CiTestCase): |
265 | self.add_patch(basepath + 'os.path.exists', 'm_exists') |
266 | self.add_patch(basepath + 'block.lookup_disk', 'm_lookup') |
267 | self.add_patch(basepath + 'devsync', 'm_devsync') |
268 | + self.add_patch(basepath + 'util.subp', 'm_subp') |
269 | + self.add_patch(basepath + 'multipath.is_mpath_member', 'm_mp') |
270 | |
271 | def test_block_lookup_called_with_disk_wwn(self): |
272 | volume = 'mydisk' |
273 | @@ -93,6 +95,8 @@ class TestGetPathToStorageVolume(CiTestCase): |
274 | ValueError('Error'), ValueError('Error')]) |
275 | # no path |
276 | self.m_exists.return_value = False |
277 | + # not multipath |
278 | + self.m_mp.return_value = False |
279 | |
280 | with self.assertRaises(ValueError): |
281 | block_meta.get_path_to_storage_volume(volume, s_cfg) |
282 | diff --git a/tests/unittests/test_commands_collect_logs.py b/tests/unittests/test_commands_collect_logs.py |
283 | index 1feba18..b5df902 100644 |
284 | --- a/tests/unittests/test_commands_collect_logs.py |
285 | +++ b/tests/unittests/test_commands_collect_logs.py |
286 | @@ -288,6 +288,7 @@ class TestCreateTar(CiTestCase): |
287 | |
288 | Configured log_file or post_files which don't exist are ignored. |
289 | """ |
290 | + self.add_patch('curtin.util.subp', 'mock_subp') |
291 | tarfile = self.tmp_path('my.tar', _dir=self.new_root) |
292 | log1 = self.tmp_path('some.log', _dir=self.new_root) |
293 | write_file(log1, 'log content') |
294 | @@ -314,6 +315,7 @@ class TestCreateTar(CiTestCase): |
295 | |
296 | def test_create_log_tarfile_redacts_maas_credentials(self): |
297 | """create_log_tarfile redacts sensitive maas credentials configured.""" |
298 | + self.add_patch('curtin.util.subp', 'mock_subp') |
299 | tarfile = self.tmp_path('my.tar', _dir=self.new_root) |
300 | self.add_patch( |
301 | 'curtin.commands.collect_logs._redact_sensitive_information', |
302 | diff --git a/tests/unittests/test_gpg.py b/tests/unittests/test_gpg.py |
303 | index 2c83ae3..42ff8c4 100644 |
304 | --- a/tests/unittests/test_gpg.py |
305 | +++ b/tests/unittests/test_gpg.py |
306 | @@ -48,50 +48,6 @@ class TestCurtinGpg(CiTestCase): |
307 | "--recv", key], capture=True, |
308 | retries=None) |
309 | |
310 | - @patch('time.sleep') |
311 | - @patch('curtin.util._subp') |
312 | - def test_recv_key_retry_raises(self, mock_under_subp, mock_sleep): |
313 | - key = 'DEADBEEF' |
314 | - keyserver = 'keyserver.ubuntu.com' |
315 | - retries = (1, 2, 5, 10) |
316 | - nr_calls = 5 |
317 | - mock_under_subp.side_effect = iter([ |
318 | - util.ProcessExecutionError()] * nr_calls) |
319 | - |
320 | - with self.assertRaises(ValueError): |
321 | - gpg.recv_key(key, keyserver, retries=retries) |
322 | - |
323 | - print("_subp calls: %s" % mock_under_subp.call_args_list) |
324 | - print("sleep calls: %s" % mock_sleep.call_args_list) |
325 | - expected_calls = nr_calls * [ |
326 | - call(["gpg", "--keyserver", keyserver, "--recv", key], |
327 | - capture=True)] |
328 | - mock_under_subp.assert_has_calls(expected_calls) |
329 | - |
330 | - expected_calls = [call(1), call(2), call(5), call(10)] |
331 | - mock_sleep.assert_has_calls(expected_calls) |
332 | - |
333 | - @patch('time.sleep') |
334 | - @patch('curtin.util._subp') |
335 | - def test_recv_key_retry_works(self, mock_under_subp, mock_sleep): |
336 | - key = 'DEADBEEF' |
337 | - keyserver = 'keyserver.ubuntu.com' |
338 | - nr_calls = 2 |
339 | - mock_under_subp.side_effect = iter([ |
340 | - util.ProcessExecutionError(), # 1 |
341 | - ("", ""), |
342 | - ]) |
343 | - |
344 | - gpg.recv_key(key, keyserver, retries=[1]) |
345 | - |
346 | - print("_subp calls: %s" % mock_under_subp.call_args_list) |
347 | - print("sleep calls: %s" % mock_sleep.call_args_list) |
348 | - expected_calls = nr_calls * [ |
349 | - call(["gpg", "--keyserver", keyserver, "--recv", key], |
350 | - capture=True)] |
351 | - mock_under_subp.assert_has_calls(expected_calls) |
352 | - mock_sleep.assert_has_calls([call(1)]) |
353 | - |
354 | @patch('curtin.util.subp') |
355 | def test_delete_key(self, mock_subp): |
356 | key = 'DEADBEEF' |
357 | @@ -160,4 +116,54 @@ class TestCurtinGpg(CiTestCase): |
358 | call(key, keyserver=keyserver, retries=None)]) |
359 | mock_del.assert_has_calls([call(key)]) |
360 | |
361 | + |
362 | +class TestCurtinGpgSubp(TestCurtinGpg): |
363 | + |
364 | + allowed_subp = True |
365 | + |
366 | + @patch('time.sleep') |
367 | + @patch('curtin.util._subp') |
368 | + def test_recv_key_retry_raises(self, mock_under_subp, mock_sleep): |
369 | + key = 'DEADBEEF' |
370 | + keyserver = 'keyserver.ubuntu.com' |
371 | + retries = (1, 2, 5, 10) |
372 | + nr_calls = 5 |
373 | + mock_under_subp.side_effect = iter([ |
374 | + util.ProcessExecutionError()] * nr_calls) |
375 | + |
376 | + with self.assertRaises(ValueError): |
377 | + gpg.recv_key(key, keyserver, retries=retries) |
378 | + |
379 | + print("_subp calls: %s" % mock_under_subp.call_args_list) |
380 | + print("sleep calls: %s" % mock_sleep.call_args_list) |
381 | + expected_calls = nr_calls * [ |
382 | + call(["gpg", "--keyserver", keyserver, "--recv", key], |
383 | + capture=True)] |
384 | + mock_under_subp.assert_has_calls(expected_calls) |
385 | + |
386 | + expected_calls = [call(1), call(2), call(5), call(10)] |
387 | + mock_sleep.assert_has_calls(expected_calls) |
388 | + |
389 | + @patch('time.sleep') |
390 | + @patch('curtin.util._subp') |
391 | + def test_recv_key_retry_works(self, mock_under_subp, mock_sleep): |
392 | + key = 'DEADBEEF' |
393 | + keyserver = 'keyserver.ubuntu.com' |
394 | + nr_calls = 2 |
395 | + mock_under_subp.side_effect = iter([ |
396 | + util.ProcessExecutionError(), # 1 |
397 | + ("", ""), |
398 | + ]) |
399 | + |
400 | + gpg.recv_key(key, keyserver, retries=[1]) |
401 | + |
402 | + print("_subp calls: %s" % mock_under_subp.call_args_list) |
403 | + print("sleep calls: %s" % mock_sleep.call_args_list) |
404 | + expected_calls = nr_calls * [ |
405 | + call(["gpg", "--keyserver", keyserver, "--recv", key], |
406 | + capture=True)] |
407 | + mock_under_subp.assert_has_calls(expected_calls) |
408 | + mock_sleep.assert_has_calls([call(1)]) |
409 | + |
410 | + |
411 | # vi: ts=4 expandtab syntax=python |
412 | diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py |
413 | index 438f7ee..0377357 100644 |
414 | --- a/tests/unittests/test_util.py |
415 | +++ b/tests/unittests/test_util.py |
416 | @@ -113,6 +113,7 @@ class TestSubp(CiTestCase): |
417 | utf8_invalid = b'ab\xaadef' |
418 | utf8_valid = b'start \xc3\xa9 end' |
419 | utf8_valid_2 = b'd\xc3\xa9j\xc8\xa7' |
420 | + allowed_subp = True |
421 | |
422 | try: |
423 | decode_type = unicode |
424 | @@ -552,7 +553,7 @@ class TestTargetPath(CiTestCase): |
425 | paths.target_path("/target/", "///my/path/")) |
426 | |
427 | |
428 | -class TestRunInChroot(CiTestCase): |
429 | +class TestRunInChrootTestSubp(CiTestCase): |
430 | """Test the legacy 'RunInChroot'. |
431 | |
432 | The test works by mocking ChrootableTarget's __enter__ to do nothing. |
433 | @@ -560,6 +561,7 @@ class TestRunInChroot(CiTestCase): |
434 | a.) RunInChroot is a subclass of ChrootableTarget |
435 | b.) ChrootableTarget's __exit__ only un-does work that its __enter__ |
436 | did. Meaning for our mocked case, it does nothing.""" |
437 | + allowed_subp = True |
438 | |
439 | @mock.patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a) |
440 | def test_run_in_chroot_with_target_slash(self): |
441 | @@ -567,6 +569,16 @@ class TestRunInChroot(CiTestCase): |
442 | out, err = i(['echo', 'HI MOM'], capture=True) |
443 | self.assertEqual('HI MOM\n', out) |
444 | |
445 | + |
446 | +class TestRunInChrootTest(CiTestCase): |
447 | + """Test the legacy 'RunInChroot'. |
448 | + |
449 | + The test works by mocking ChrootableTarget's __enter__ to do nothing. |
450 | + The assumptions made are: |
451 | + a.) RunInChroot is a subclass of ChrootableTarget |
452 | + b.) ChrootableTarget's __exit__ only un-does work that its __enter__ |
453 | + did. Meaning for our mocked case, it does nothing.""" |
454 | + |
455 | @mock.patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a) |
456 | @mock.patch("curtin.util.subp") |
457 | def test_run_in_chroot_with_target(self, m_subp): |
PASSED: Continuous integration, rev:d68faec0fd4 4fd0d7f2b2d5ef3 2cb95d0056e188 /jenkins. ubuntu. com/server/ job/curtin- ci/44/ /jenkins. ubuntu. com/server/ job/curtin- ci/nodes= metal-amd64/ 44/ /jenkins. ubuntu. com/server/ job/curtin- ci/nodes= metal-arm64/ 44/ /jenkins. ubuntu. com/server/ job/curtin- ci/nodes= metal-ppc64el/ 44/ /jenkins. ubuntu. com/server/ job/curtin- ci/nodes= metal-s390x/ 44/
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild: /jenkins. ubuntu. com/server/ job/curtin- ci/44// rebuild
https:/