Merge ~bjornt/maas:refresh-mock-leak into maas:master

Proposed by Björn Tillenius
Status: Merged
Approved by: Björn Tillenius
Approved revision: 6e5b2c1a99f649b09cad8942d8ea48c99ce3b6fe
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~bjornt/maas:refresh-mock-leak
Merge into: maas:master
Diff against target: 310 lines (+71/-30)
1 file modified
src/provisioningserver/refresh/tests/test_refresh.py (+71/-30)
Reviewer Review Type Date Requested Status
Adam Collard (community) Approve
Alberto Donato (community) Approve
MAAS Lander unittests Pending
Review via email: mp+399279@code.launchpad.net

Commit message

Fix refresh tests not to leak changed NODE_INFO_SCRIPTS.

The refresh tests changed the NODE_INFO_SCRIPTS dict, but they didn't
replace it with the original dict after the tests had be ran.

This causes other tests to fail, depending on which order they are run
in.

To post a comment you must log in.
Revision history for this message
Alberto Donato (ack) wrote :

LGTM!

review: Approve
Revision history for this message
Adam Collard (adam-collard) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/src/provisioningserver/refresh/tests/test_refresh.py b/src/provisioningserver/refresh/tests/test_refresh.py
index a11097d..f3fbebd 100644
--- a/src/provisioningserver/refresh/tests/test_refresh.py
+++ b/src/provisioningserver/refresh/tests/test_refresh.py
@@ -60,7 +60,7 @@ class TestRefresh(MAASTestCase):
60 if os.path.exists(path):60 if os.path.exists(path):
61 os.remove(path)61 os.remove(path)
6262
63 def patch_scripts_success(self, script_name=None, script_content=None):63 def create_scripts_success(self, script_name=None, script_content=None):
64 if script_name is None:64 if script_name is None:
65 script_name = factory.make_name("script_name")65 script_name = factory.make_name("script_name")
66 if script_content is None:66 if script_content is None:
@@ -79,11 +79,11 @@ class TestRefresh(MAASTestCase):
79 os.chmod(script_path, st.st_mode | stat.S_IEXEC)79 os.chmod(script_path, st.st_mode | stat.S_IEXEC)
80 self.addCleanup(self._cleanup, script_path)80 self.addCleanup(self._cleanup, script_path)
8181
82 refresh.NODE_INFO_SCRIPTS = OrderedDict(82 return OrderedDict(
83 [(script_name, {"name": script_name, "run_on_controller": True})]83 [(script_name, {"name": script_name, "run_on_controller": True})]
84 )84 )
8585
86 def patch_scripts_failure(self, script_name=None):86 def create_scripts_failure(self, script_name=None):
87 if script_name is None:87 if script_name is None:
88 script_name = factory.make_name("script_name")88 script_name = factory.make_name("script_name")
89 TEST_SCRIPT = dedent(89 TEST_SCRIPT = dedent(
@@ -102,14 +102,14 @@ class TestRefresh(MAASTestCase):
102 os.chmod(script_path, st.st_mode | stat.S_IEXEC)102 os.chmod(script_path, st.st_mode | stat.S_IEXEC)
103 self.addCleanup(self._cleanup, script_path)103 self.addCleanup(self._cleanup, script_path)
104104
105 refresh.NODE_INFO_SCRIPTS = OrderedDict(105 return OrderedDict(
106 [(script_name, {"name": script_name, "run_on_controller": True})]106 [(script_name, {"name": script_name, "run_on_controller": True})]
107 )107 )
108108
109 def test_refresh_accepts_defined_url(self):109 def test_refresh_accepts_defined_url(self):
110 signal = self.patch(refresh, "signal")110 signal = self.patch(refresh, "signal")
111 script_name = factory.make_name("script_name")111 script_name = factory.make_name("script_name")
112 self.patch_scripts_success(script_name)112 info_scripts = self.create_scripts_success(script_name)
113113
114 system_id = factory.make_name("system_id")114 system_id = factory.make_name("system_id")
115 consumer_key = factory.make_name("consumer_key")115 consumer_key = factory.make_name("consumer_key")
@@ -117,7 +117,10 @@ class TestRefresh(MAASTestCase):
117 token_secret = factory.make_name("token_secret")117 token_secret = factory.make_name("token_secret")
118 url = factory.make_url()118 url = factory.make_url()
119119
120 refresh.refresh(system_id, consumer_key, token_key, token_secret, url)120 with patch.dict(refresh.NODE_INFO_SCRIPTS, info_scripts, clear=True):
121 refresh.refresh(
122 system_id, consumer_key, token_key, token_secret, url
123 )
121 self.assertThat(124 self.assertThat(
122 signal,125 signal,
123 MockAnyCall(126 MockAnyCall(
@@ -136,7 +139,7 @@ class TestRefresh(MAASTestCase):
136 def test_refresh_signals_starting(self):139 def test_refresh_signals_starting(self):
137 signal = self.patch(refresh, "signal")140 signal = self.patch(refresh, "signal")
138 script_name = factory.make_name("script_name")141 script_name = factory.make_name("script_name")
139 self.patch_scripts_success(script_name)142 info_scripts = self.create_scripts_success(script_name)
140143
141 system_id = factory.make_name("system_id")144 system_id = factory.make_name("system_id")
142 consumer_key = factory.make_name("consumer_key")145 consumer_key = factory.make_name("consumer_key")
@@ -144,7 +147,10 @@ class TestRefresh(MAASTestCase):
144 token_secret = factory.make_name("token_secret")147 token_secret = factory.make_name("token_secret")
145 url = factory.make_url()148 url = factory.make_url()
146149
147 refresh.refresh(system_id, consumer_key, token_key, token_secret, url)150 with patch.dict(refresh.NODE_INFO_SCRIPTS, info_scripts, clear=True):
151 refresh.refresh(
152 system_id, consumer_key, token_key, token_secret, url
153 )
148 self.assertThat(154 self.assertThat(
149 signal,155 signal,
150 MockAnyCall(156 MockAnyCall(
@@ -170,7 +176,9 @@ class TestRefresh(MAASTestCase):
170 echo '{status: skipped}' > $RESULT_PATH176 echo '{status: skipped}' > $RESULT_PATH
171 """177 """
172 )178 )
173 self.patch_scripts_success(script_name, script_content=script_content)179 info_scripts = self.create_scripts_success(
180 script_name, script_content=script_content
181 )
174182
175 system_id = factory.make_name("system_id")183 system_id = factory.make_name("system_id")
176 consumer_key = factory.make_name("consumer_key")184 consumer_key = factory.make_name("consumer_key")
@@ -178,7 +186,10 @@ class TestRefresh(MAASTestCase):
178 token_secret = factory.make_name("token_secret")186 token_secret = factory.make_name("token_secret")
179 url = factory.make_url()187 url = factory.make_url()
180188
181 refresh.refresh(system_id, consumer_key, token_key, token_secret, url)189 with patch.dict(refresh.NODE_INFO_SCRIPTS, info_scripts, clear=True):
190 refresh.refresh(
191 system_id, consumer_key, token_key, token_secret, url
192 )
182 self.assertThat(193 self.assertThat(
183 signal,194 signal,
184 MockAnyCall(195 MockAnyCall(
@@ -204,7 +215,7 @@ class TestRefresh(MAASTestCase):
204 def test_refresh_sets_env_vars(self):215 def test_refresh_sets_env_vars(self):
205 self.patch(refresh, "signal")216 self.patch(refresh, "signal")
206 script_name = factory.make_name("script_name")217 script_name = factory.make_name("script_name")
207 self.patch_scripts_failure(script_name)218 info_scripts = self.create_scripts_failure(script_name)
208 mock_popen = self.patch(refresh, "Popen")219 mock_popen = self.patch(refresh, "Popen")
209 mock_popen.side_effect = OSError(8, "Exec format error")220 mock_popen.side_effect = OSError(8, "Exec format error")
210221
@@ -214,7 +225,10 @@ class TestRefresh(MAASTestCase):
214 token_secret = factory.make_name("token_secret")225 token_secret = factory.make_name("token_secret")
215 url = factory.make_url()226 url = factory.make_url()
216227
217 refresh.refresh(system_id, consumer_key, token_key, token_secret, url)228 with patch.dict(refresh.NODE_INFO_SCRIPTS, info_scripts, clear=True):
229 refresh.refresh(
230 system_id, consumer_key, token_key, token_secret, url
231 )
218232
219 env = mock_popen.call_args[1]["env"]233 env = mock_popen.call_args[1]["env"]
220 for name in [234 for name in [
@@ -229,7 +243,7 @@ class TestRefresh(MAASTestCase):
229 def test_refresh_signals_failure_on_unexecutable_script(self):243 def test_refresh_signals_failure_on_unexecutable_script(self):
230 signal = self.patch(refresh, "signal")244 signal = self.patch(refresh, "signal")
231 script_name = factory.make_name("script_name")245 script_name = factory.make_name("script_name")
232 self.patch_scripts_failure(script_name)246 info_scripts = self.create_scripts_failure(script_name)
233 mock_popen = self.patch(refresh, "Popen")247 mock_popen = self.patch(refresh, "Popen")
234 mock_popen.side_effect = OSError(8, "Exec format error")248 mock_popen.side_effect = OSError(8, "Exec format error")
235249
@@ -239,7 +253,10 @@ class TestRefresh(MAASTestCase):
239 token_secret = factory.make_name("token_secret")253 token_secret = factory.make_name("token_secret")
240 url = factory.make_url()254 url = factory.make_url()
241255
242 refresh.refresh(system_id, consumer_key, token_key, token_secret, url)256 with patch.dict(refresh.NODE_INFO_SCRIPTS, info_scripts, clear=True):
257 refresh.refresh(
258 system_id, consumer_key, token_key, token_secret, url
259 )
243 self.assertThat(260 self.assertThat(
244 signal,261 signal,
245 MockAnyCall(262 MockAnyCall(
@@ -263,7 +280,7 @@ class TestRefresh(MAASTestCase):
263 def test_refresh_signals_failure_on_unexecutable_script_no_errno(self):280 def test_refresh_signals_failure_on_unexecutable_script_no_errno(self):
264 signal = self.patch(refresh, "signal")281 signal = self.patch(refresh, "signal")
265 script_name = factory.make_name("script_name")282 script_name = factory.make_name("script_name")
266 self.patch_scripts_failure(script_name)283 info_scripts = self.create_scripts_failure(script_name)
267 mock_popen = self.patch(refresh, "Popen")284 mock_popen = self.patch(refresh, "Popen")
268 mock_popen.side_effect = OSError()285 mock_popen.side_effect = OSError()
269286
@@ -273,7 +290,10 @@ class TestRefresh(MAASTestCase):
273 token_secret = factory.make_name("token_secret")290 token_secret = factory.make_name("token_secret")
274 url = factory.make_url()291 url = factory.make_url()
275292
276 refresh.refresh(system_id, consumer_key, token_key, token_secret, url)293 with patch.dict(refresh.NODE_INFO_SCRIPTS, info_scripts, clear=True):
294 refresh.refresh(
295 system_id, consumer_key, token_key, token_secret, url
296 )
277 self.assertThat(297 self.assertThat(
278 signal,298 signal,
279 MockAnyCall(299 MockAnyCall(
@@ -297,7 +317,7 @@ class TestRefresh(MAASTestCase):
297 def test_refresh_signals_failure_on_unexecutable_script_baderrno(self):317 def test_refresh_signals_failure_on_unexecutable_script_baderrno(self):
298 signal = self.patch(refresh, "signal")318 signal = self.patch(refresh, "signal")
299 script_name = factory.make_name("script_name")319 script_name = factory.make_name("script_name")
300 self.patch_scripts_failure(script_name)320 info_scripts = self.create_scripts_failure(script_name)
301 mock_popen = self.patch(refresh, "Popen")321 mock_popen = self.patch(refresh, "Popen")
302 mock_popen.side_effect = OSError(0, "Exec format error")322 mock_popen.side_effect = OSError(0, "Exec format error")
303323
@@ -307,7 +327,10 @@ class TestRefresh(MAASTestCase):
307 token_secret = factory.make_name("token_secret")327 token_secret = factory.make_name("token_secret")
308 url = factory.make_url()328 url = factory.make_url()
309329
310 refresh.refresh(system_id, consumer_key, token_key, token_secret, url)330 with patch.dict(refresh.NODE_INFO_SCRIPTS, info_scripts, clear=True):
331 refresh.refresh(
332 system_id, consumer_key, token_key, token_secret, url
333 )
311 self.assertThat(334 self.assertThat(
312 signal,335 signal,
313 MockAnyCall(336 MockAnyCall(
@@ -331,7 +354,7 @@ class TestRefresh(MAASTestCase):
331 def test_refresh_signals_failure_on_timeout(self):354 def test_refresh_signals_failure_on_timeout(self):
332 signal = self.patch(refresh, "signal")355 signal = self.patch(refresh, "signal")
333 script_name = factory.make_name("script_name")356 script_name = factory.make_name("script_name")
334 self.patch_scripts_failure(script_name)357 info_scripts = self.create_scripts_failure(script_name)
335358
336 system_id = factory.make_name("system_id")359 system_id = factory.make_name("system_id")
337 consumer_key = factory.make_name("consumer_key")360 consumer_key = factory.make_name("consumer_key")
@@ -351,7 +374,10 @@ class TestRefresh(MAASTestCase):
351 refresh, "capture_script_output"374 refresh, "capture_script_output"
352 )375 )
353 mock_capture_script_output.side_effect = timeout_run376 mock_capture_script_output.side_effect = timeout_run
354 refresh.refresh(system_id, consumer_key, token_key, token_secret, url)377 with patch.dict(refresh.NODE_INFO_SCRIPTS, info_scripts, clear=True):
378 refresh.refresh(
379 system_id, consumer_key, token_key, token_secret, url
380 )
355381
356 self.assertThat(382 self.assertThat(
357 signal,383 signal,
@@ -376,7 +402,7 @@ class TestRefresh(MAASTestCase):
376 def test_refresh_signals_finished(self):402 def test_refresh_signals_finished(self):
377 signal = self.patch(refresh, "signal")403 signal = self.patch(refresh, "signal")
378 script_name = factory.make_name("script_name")404 script_name = factory.make_name("script_name")
379 self.patch_scripts_success(script_name)405 info_scripts = self.create_scripts_success(script_name)
380406
381 system_id = factory.make_name("system_id")407 system_id = factory.make_name("system_id")
382 consumer_key = factory.make_name("consumer_key")408 consumer_key = factory.make_name("consumer_key")
@@ -384,7 +410,10 @@ class TestRefresh(MAASTestCase):
384 token_secret = factory.make_name("token_secret")410 token_secret = factory.make_name("token_secret")
385 url = factory.make_url()411 url = factory.make_url()
386412
387 refresh.refresh(system_id, consumer_key, token_key, token_secret, url)413 with patch.dict(refresh.NODE_INFO_SCRIPTS, info_scripts, clear=True):
414 refresh.refresh(
415 system_id, consumer_key, token_key, token_secret, url
416 )
388 self.assertThat(417 self.assertThat(
389 signal,418 signal,
390 MockAnyCall(419 MockAnyCall(
@@ -402,7 +431,7 @@ class TestRefresh(MAASTestCase):
402431
403 def test_refresh_signals_failure(self):432 def test_refresh_signals_failure(self):
404 signal = self.patch(refresh, "signal")433 signal = self.patch(refresh, "signal")
405 self.patch_scripts_failure()434 info_scripts = self.create_scripts_failure()
406435
407 system_id = factory.make_name("system_id")436 system_id = factory.make_name("system_id")
408 consumer_key = factory.make_name("consumer_key")437 consumer_key = factory.make_name("consumer_key")
@@ -410,7 +439,10 @@ class TestRefresh(MAASTestCase):
410 token_secret = factory.make_name("token_secret")439 token_secret = factory.make_name("token_secret")
411 url = factory.make_url()440 url = factory.make_url()
412441
413 refresh.refresh(system_id, consumer_key, token_key, token_secret, url)442 with patch.dict(refresh.NODE_INFO_SCRIPTS, info_scripts, clear=True):
443 refresh.refresh(
444 system_id, consumer_key, token_key, token_secret, url
445 )
414 self.assertThat(446 self.assertThat(
415 signal,447 signal,
416 MockAnyCall(448 MockAnyCall(
@@ -429,7 +461,7 @@ class TestRefresh(MAASTestCase):
429 def test_refresh_executes_lxd_binary(self):461 def test_refresh_executes_lxd_binary(self):
430 signal = self.patch(refresh, "signal")462 signal = self.patch(refresh, "signal")
431 script_name = LXD_OUTPUT_NAME463 script_name = LXD_OUTPUT_NAME
432 self.patch_scripts_success(script_name)464 info_scripts = self.create_scripts_success(script_name)
433465
434 system_id = factory.make_name("system_id")466 system_id = factory.make_name("system_id")
435 consumer_key = factory.make_name("consumer_key")467 consumer_key = factory.make_name("consumer_key")
@@ -437,7 +469,10 @@ class TestRefresh(MAASTestCase):
437 token_secret = factory.make_name("token_secret")469 token_secret = factory.make_name("token_secret")
438 url = factory.make_url()470 url = factory.make_url()
439471
440 refresh.refresh(system_id, consumer_key, token_key, token_secret, url)472 with patch.dict(refresh.NODE_INFO_SCRIPTS, info_scripts, clear=True):
473 refresh.refresh(
474 system_id, consumer_key, token_key, token_secret, url
475 )
441 self.assertThat(476 self.assertThat(
442 signal,477 signal,
443 MockAnyCall(478 MockAnyCall(
@@ -456,7 +491,7 @@ class TestRefresh(MAASTestCase):
456 def test_refresh_executes_lxd_binary_in_snap(self):491 def test_refresh_executes_lxd_binary_in_snap(self):
457 signal = self.patch(refresh, "signal")492 signal = self.patch(refresh, "signal")
458 script_name = LXD_OUTPUT_NAME493 script_name = LXD_OUTPUT_NAME
459 self.patch_scripts_success(script_name)494 info_scripts = self.create_scripts_success(script_name)
460 path = factory.make_name()495 path = factory.make_name()
461 self.patch(os, "environ", {"SNAP": path})496 self.patch(os, "environ", {"SNAP": path})
462497
@@ -466,7 +501,10 @@ class TestRefresh(MAASTestCase):
466 token_secret = factory.make_name("token_secret")501 token_secret = factory.make_name("token_secret")
467 url = factory.make_url()502 url = factory.make_url()
468503
469 refresh.refresh(system_id, consumer_key, token_key, token_secret, url)504 with patch.dict(refresh.NODE_INFO_SCRIPTS, info_scripts, clear=True):
505 refresh.refresh(
506 system_id, consumer_key, token_key, token_secret, url
507 )
470 self.assertThat(508 self.assertThat(
471 signal,509 signal,
472 MockAnyCall(510 MockAnyCall(
@@ -527,7 +565,7 @@ class TestRefresh(MAASTestCase):
527 maaslog = self.patch(refresh.maaslog, "error")565 maaslog = self.patch(refresh.maaslog, "error")
528 error = factory.make_string()566 error = factory.make_string()
529 signal.side_effect = SignalException(error)567 signal.side_effect = SignalException(error)
530 self.patch_scripts_failure()568 info_scripts = self.create_scripts_failure()
531569
532 system_id = factory.make_name("system_id")570 system_id = factory.make_name("system_id")
533 consumer_key = factory.make_name("consumer_key")571 consumer_key = factory.make_name("consumer_key")
@@ -535,7 +573,10 @@ class TestRefresh(MAASTestCase):
535 token_secret = factory.make_name("token_secret")573 token_secret = factory.make_name("token_secret")
536 url = factory.make_url()574 url = factory.make_url()
537575
538 refresh.refresh(system_id, consumer_key, token_key, token_secret, url)576 with patch.dict(refresh.NODE_INFO_SCRIPTS, info_scripts, clear=True):
577 refresh.refresh(
578 system_id, consumer_key, token_key, token_secret, url
579 )
539580
540 self.assertThat(581 self.assertThat(
541 maaslog, MockAnyCall("Error during controller refresh: %s" % error)582 maaslog, MockAnyCall("Error during controller refresh: %s" % error)

Subscribers

People subscribed via source and target branches