Merge lp:~adam-collard/landscape-client/mocker-replace-watchdog-service-tests into lp:~landscape/landscape-client/trunk
- mocker-replace-watchdog-service-tests
- Merge into trunk
Proposed by
Adam Collard
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Adam Collard | ||||
Approved revision: | 897 | ||||
Merged at revision: | 906 | ||||
Proposed branch: | lp:~adam-collard/landscape-client/mocker-replace-watchdog-service-tests | ||||
Merge into: | lp:~landscape/landscape-client/trunk | ||||
Diff against target: |
426 lines (+111/-204) 1 file modified
landscape/tests/test_watchdog.py (+111/-204) |
||||
To merge this branch: | bzr merge lp:~adam-collard/landscape-client/mocker-replace-watchdog-service-tests | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Adam Collard (community) | Approve | ||
Bogdana Vereha (community) | Approve | ||
Alberto Donato (community) | Approve | ||
🤖 Landscape Builder | test results | Approve | |
Review via email:
|
Commit message
Move WatchDogServiceTest from mocker to mock.
Description of the change
Move WatchDogServiceTest from mocker to mock.
For the log rotation test, I moved it from WatchDogServiceTest to WatchDogTest and removed the testing of the log rotation itself (it's covered in test_log.py)
To post a comment you must log in.
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
🤖 Landscape Builder (landscape-builder) : | # |
review:
Abstain
(executing tests)
- 895. By Adam Collard
-
Remove stray mocker bits still leftover
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
🤖 Landscape Builder (landscape-builder) wrote : | # |
review:
Approve
(test results)
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
🤖 Landscape Builder (landscape-builder) : | # |
review:
Abstain
(executing tests)
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
🤖 Landscape Builder (landscape-builder) wrote : | # |
Command: TRIAL_ARGS=-j4 make check
Result: Success
Revno: 895
Branch: lp:~adam-collard/landscape-client/mocker-replace-watchdog-service-tests
Jenkins: https:/
review:
Approve
(test results)
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Bogdana Vereha (bogdana) wrote : | # |
+1 with a minor comment inline
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Bogdana Vereha (bogdana) : | # |
review:
Approve
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Adam Collard (adam-collard) : | # |
review:
Approve
- 896. By Adam Collard
-
mark test as whitebox
- 897. By Adam Collard
-
Add pid_file checks (bogdana's review)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'landscape/tests/test_watchdog.py' |
2 | --- landscape/tests/test_watchdog.py 2016-06-16 20:33:26 +0000 |
3 | +++ landscape/tests/test_watchdog.py 2016-06-16 22:26:10 +0000 |
4 | @@ -3,14 +3,12 @@ |
5 | import sys |
6 | import os |
7 | import signal |
8 | -import logging |
9 | |
10 | import mock |
11 | |
12 | from twisted.internet.utils import getProcessOutput |
13 | from twisted.internet.defer import Deferred, succeed, fail |
14 | from twisted.internet import reactor |
15 | -from twisted.internet.task import deferLater |
16 | from twisted.python.fakepwd import UserDatabase |
17 | |
18 | from landscape.tests.mocker import ARGS, KWARGS |
19 | @@ -238,6 +236,17 @@ |
20 | |
21 | return result.addCallback(check) |
22 | |
23 | + def test_wb_log_notification(self): |
24 | + """ |
25 | + SIGUSR1 should cause logs to be reopened. |
26 | + """ |
27 | + mock_reactor = mock.Mock() |
28 | + watchdog = WatchDog(reactor=mock_reactor, config=self.config) |
29 | + os.kill(os.getpid(), signal.SIGUSR1) |
30 | + |
31 | + mock_reactor.callFromThread.assert_called_once_with( |
32 | + watchdog._notify_rotate_logs) |
33 | + |
34 | |
35 | START = "start" |
36 | STOP = "stop" |
37 | @@ -1018,105 +1027,70 @@ |
38 | "--data-path", self.data_path, |
39 | "--log-dir", self.log_dir]) |
40 | |
41 | - def test_daemonize(self): |
42 | - self.mocker.order() |
43 | - watchdog = self.mocker.patch(WatchDog) |
44 | - watchdog.check_running() |
45 | - self.mocker.result(succeed([])) |
46 | - daemonize = self.mocker.replace("landscape.watchdog.daemonize", |
47 | - passthrough=False) |
48 | - daemonize() |
49 | - watchdog.start() |
50 | - self.mocker.result(succeed(None)) |
51 | - |
52 | - self.mocker.replay() |
53 | + @mock.patch("landscape.watchdog.daemonize") |
54 | + @mock.patch("landscape.watchdog.WatchDog") |
55 | + def test_daemonize(self, mock_watchdog, mock_daemonize): |
56 | + mock_watchdog().check_running.return_value = succeed([]) |
57 | self.configuration.daemon = True |
58 | |
59 | service = WatchDogService(self.configuration) |
60 | service.startService() |
61 | + mock_watchdog().check_running.assert_called_once_with() |
62 | + mock_watchdog().start.assert_called_once_with() |
63 | + mock_daemonize.assert_called_once_with() |
64 | |
65 | - def test_pid_file(self): |
66 | + @mock.patch("landscape.watchdog.daemonize") |
67 | + @mock.patch("landscape.watchdog.WatchDog") |
68 | + def test_pid_file(self, mock_watchdog, mock_daemonize): |
69 | + mock_watchdog().check_running.return_value = succeed([]) |
70 | pid_file = self.makeFile() |
71 | |
72 | - watchdog = self.mocker.patch(WatchDog) |
73 | - watchdog.check_running() |
74 | - self.mocker.result(succeed([])) |
75 | - daemonize = self.mocker.replace("landscape.watchdog.daemonize", |
76 | - passthrough=False) |
77 | - daemonize() |
78 | - watchdog.start() |
79 | - self.mocker.result(succeed(None)) |
80 | - |
81 | - self.mocker.replay() |
82 | self.configuration.daemon = True |
83 | self.configuration.pid_file = pid_file |
84 | |
85 | service = WatchDogService(self.configuration) |
86 | service.startService() |
87 | self.assertEqual(int(open(pid_file, "r").read()), os.getpid()) |
88 | + mock_watchdog().check_running.assert_called_once_with() |
89 | + mock_watchdog().start.assert_called_once_with() |
90 | + mock_daemonize.assert_called_once_with() |
91 | |
92 | - def test_dont_write_pid_file_until_we_really_start(self): |
93 | + @mock.patch("landscape.watchdog.reactor") |
94 | + @mock.patch("landscape.watchdog.daemonize") |
95 | + @mock.patch("landscape.watchdog.WatchDog") |
96 | + def test_dont_write_pid_file_until_we_really_start( |
97 | + self, mock_watchdog, mock_daemonize, mock_reactor): |
98 | """ |
99 | If the client can't be started because another client is still running, |
100 | the client shouldn't be daemonized and the pid file shouldn't be |
101 | written. |
102 | """ |
103 | + mock_watchdog().check_running.return_value = succeed([StubDaemon()]) |
104 | + mock_reactor.crash.return_value = None |
105 | self.log_helper.ignore_errors( |
106 | "ERROR: The following daemons are already running: program-name") |
107 | pid_file = self.makeFile() |
108 | |
109 | - daemonize = self.mocker.replace("landscape.watchdog.daemonize", |
110 | - passthrough=False) |
111 | - daemonize() |
112 | - # daemonize should *not* be called |
113 | - self.mocker.count(0) |
114 | - |
115 | - watchdog = self.mocker.patch(WatchDog) |
116 | - watchdog.check_running() |
117 | - self.mocker.result(succeed([StubDaemon()])) |
118 | - watchdog.start() |
119 | - self.mocker.count(0) |
120 | - |
121 | - reactor = self.mocker.replace("twisted.internet.reactor", |
122 | - passthrough=True) |
123 | - reactor.crash() |
124 | - self.mocker.result(None) |
125 | - |
126 | - self.mocker.replay() |
127 | - |
128 | self.configuration.daemon = True |
129 | self.configuration.pid_file = pid_file |
130 | service = WatchDogService(self.configuration) |
131 | |
132 | - try: |
133 | - service.startService() |
134 | - self.mocker.verify() |
135 | - finally: |
136 | - self.mocker.reset() |
137 | + service.startService() |
138 | self.assertFalse(os.path.exists(pid_file)) |
139 | + mock_daemonize.assert_not_called() |
140 | + mock_watchdog().check_running.assert_called_once_with() |
141 | + mock_watchdog().start.assert_not_called() |
142 | + mock_reactor.crash.assert_called_once_with() |
143 | |
144 | - def test_remove_pid_file(self): |
145 | + @mock.patch("landscape.watchdog.daemonize") |
146 | + @mock.patch("landscape.watchdog.WatchDog") |
147 | + def test_remove_pid_file(self, mock_watchdog, mock_daemonize): |
148 | """ |
149 | When the service is stopped, the pid file is removed. |
150 | """ |
151 | - #don't really daemonize or request an exit |
152 | - daemonize = self.mocker.replace("landscape.watchdog.daemonize", |
153 | - passthrough=False) |
154 | - watchdog_factory = self.mocker.replace("landscape.watchdog.WatchDog", |
155 | - passthrough=False) |
156 | - watchdog = watchdog_factory(ARGS, KWARGS) |
157 | - watchdog.start() |
158 | - self.mocker.result(succeed(None)) |
159 | - |
160 | - watchdog.check_running() |
161 | - self.mocker.result(succeed([])) |
162 | - |
163 | - daemonize() |
164 | - |
165 | - watchdog.request_exit() |
166 | - self.mocker.result(succeed(None)) |
167 | - |
168 | - self.mocker.replay() |
169 | + mock_watchdog().start.return_value = succeed(None) |
170 | + mock_watchdog().check_running.return_value = succeed([]) |
171 | + mock_watchdog().request_exit.return_value = succeed(None) |
172 | |
173 | pid_file = self.makeFile() |
174 | self.configuration.daemon = True |
175 | @@ -1126,143 +1100,109 @@ |
176 | self.assertEqual(int(open(pid_file).read()), os.getpid()) |
177 | service.stopService() |
178 | self.assertFalse(os.path.exists(pid_file)) |
179 | - |
180 | - def test_remove_pid_file_only_when_ours(self): |
181 | - #don't really request an exit |
182 | - watchdog = self.mocker.patch(WatchDog) |
183 | - watchdog.request_exit() |
184 | - self.mocker.result(succeed(None)) |
185 | - |
186 | - self.mocker.replay() |
187 | - |
188 | + self.assertTrue(mock_watchdog.called) |
189 | + mock_watchdog().start.assert_called_once_with() |
190 | + mock_watchdog().check_running.assert_called_once_with() |
191 | + self.assertTrue(mock_daemonize.called) |
192 | + self.assertTrue(mock_watchdog().request_exit.called) |
193 | + |
194 | + @mock.patch("landscape.watchdog.WatchDog") |
195 | + def test_remove_pid_file_only_when_ours(self, mock_watchdog): |
196 | + mock_watchdog().request_exit.return_value = succeed(None) |
197 | pid_file = self.makeFile() |
198 | self.configuration.pid_file = pid_file |
199 | service = WatchDogService(self.configuration) |
200 | open(pid_file, "w").write("abc") |
201 | service.stopService() |
202 | self.assertTrue(os.path.exists(pid_file)) |
203 | + self.assertTrue(mock_watchdog().request_exit.called) |
204 | |
205 | - def test_remove_pid_file_doesnt_explode_on_inaccessibility(self): |
206 | + # Make os.access say that the file isn't writable |
207 | + @mock.patch("landscape.watchdog.os.access", return_value=False) |
208 | + @mock.patch("landscape.watchdog.WatchDog") |
209 | + def test_remove_pid_file_doesnt_explode_on_inaccessibility( |
210 | + self, mock_watchdog, mock_access): |
211 | + mock_watchdog().request_exit.return_value = succeed(None) |
212 | pid_file = self.makeFile() |
213 | - # Make os.access say that the file isn't writable |
214 | - mock_os = self.mocker.replace("os") |
215 | - mock_os.access(pid_file, os.W_OK) |
216 | - self.mocker.result(False) |
217 | - |
218 | - watchdog = self.mocker.patch(WatchDog) |
219 | - watchdog.request_exit() |
220 | - self.mocker.result(succeed(None)) |
221 | - self.mocker.replay() |
222 | |
223 | self.configuration.pid_file = pid_file |
224 | service = WatchDogService(self.configuration) |
225 | open(pid_file, "w").write(str(os.getpid())) |
226 | service.stopService() |
227 | + self.assertTrue(mock_watchdog().request_exit.called) |
228 | self.assertTrue(os.path.exists(pid_file)) |
229 | + mock_access.assert_called_once_with( |
230 | + pid_file, os.W_OK) |
231 | |
232 | - def test_start_service_exits_when_already_running(self): |
233 | + @mock.patch("landscape.watchdog.reactor") |
234 | + @mock.patch("landscape.watchdog.bootstrap_list") |
235 | + def test_start_service_exits_when_already_running( |
236 | + self, mock_bootstrap_list, mock_reactor): |
237 | self.log_helper.ignore_errors( |
238 | "ERROR: The following daemons are already running: program-name") |
239 | - |
240 | - bootstrap_list_mock = self.mocker.patch(bootstrap_list) |
241 | - bootstrap_list_mock.bootstrap(data_path=self.data_path, |
242 | - log_dir=self.log_dir) |
243 | - |
244 | service = WatchDogService(self.configuration) |
245 | |
246 | - self.mocker.order() |
247 | - |
248 | - watchdog_mock = self.mocker.replace(service.watchdog) |
249 | - watchdog_mock.check_running() |
250 | - self.mocker.result(succeed([StubDaemon()])) |
251 | - |
252 | - reactor = self.mocker.replace("twisted.internet.reactor", |
253 | - passthrough=False) |
254 | - reactor.crash() |
255 | - |
256 | - self.mocker.replay() |
257 | - try: |
258 | - result = service.startService() |
259 | - self.mocker.verify() |
260 | - finally: |
261 | - self.mocker.reset() |
262 | + service.watchdog = mock.Mock() |
263 | + service.watchdog.check_running.return_value = succeed([StubDaemon()]) |
264 | + result = service.startService() |
265 | self.assertEqual(service.exit_code, 1) |
266 | + mock_bootstrap_list.bootstrap.assert_called_once_with( |
267 | + data_path=self.data_path, log_dir=self.log_dir) |
268 | + service.watchdog.check_running.assert_called_once_with() |
269 | + self.assertTrue(mock_reactor.crash.called) |
270 | return result |
271 | |
272 | - def test_start_service_exits_when_unknown_errors_occur(self): |
273 | + @mock.patch("landscape.watchdog.reactor") |
274 | + @mock.patch("landscape.watchdog.bootstrap_list") |
275 | + def test_start_service_exits_when_unknown_errors_occur( |
276 | + self, mock_bootstrap_list, mock_reactor): |
277 | self.log_helper.ignore_errors(ZeroDivisionError) |
278 | service = WatchDogService(self.configuration) |
279 | |
280 | - bootstrap_list_mock = self.mocker.patch(bootstrap_list) |
281 | - bootstrap_list_mock.bootstrap(data_path=self.data_path, |
282 | - log_dir=self.log_dir) |
283 | - |
284 | - self.mocker.order() |
285 | - |
286 | - watchdog_mock = self.mocker.replace(service.watchdog) |
287 | - watchdog_mock.check_running() |
288 | - self.mocker.result(succeed([])) |
289 | - watchdog_mock.start() |
290 | + service.watchdog = mock.Mock() |
291 | + service.watchdog.check_running.return_value = succeed([]) |
292 | deferred = fail(ZeroDivisionError("I'm an unknown error!")) |
293 | - self.mocker.result(deferred) |
294 | - |
295 | - reactor = self.mocker.replace("twisted.internet.reactor", |
296 | - passthrough=False) |
297 | - reactor.crash() |
298 | - |
299 | - self.mocker.replay() |
300 | - try: |
301 | - result = service.startService() |
302 | - self.mocker.verify() |
303 | - finally: |
304 | - self.mocker.reset() |
305 | + service.watchdog.start.return_value = deferred |
306 | + |
307 | + result = service.startService() |
308 | self.assertEqual(service.exit_code, 2) |
309 | + mock_bootstrap_list.bootstrap.assert_called_once_with( |
310 | + data_path=self.data_path, log_dir=self.log_dir) |
311 | + service.watchdog.check_running.assert_called_once_with() |
312 | + service.watchdog.start.assert_called_once_with() |
313 | + mock_reactor.crash.assert_called_once_with() |
314 | return result |
315 | |
316 | - def test_bootstrap(self): |
317 | - |
318 | + @mock.patch("landscape.watchdog.os.chown") |
319 | + @mock.patch("landscape.lib.bootstrap.grp.getgrnam") |
320 | + @mock.patch("landscape.lib.bootstrap.os.getuid", return_value=0) |
321 | + def test_bootstrap(self, mock_getuid, mock_getgrnam, mock_chown): |
322 | data_path = self.makeDir() |
323 | log_dir = self.makeDir() |
324 | + fake_pwd = UserDatabase() |
325 | + fake_pwd.addUser("landscape", None, 1234, None, None, None, None) |
326 | + |
327 | + mock_getgrnam("root").gr_gid = 5678 |
328 | + |
329 | + with mock.patch("landscape.lib.bootstrap.pwd", new=fake_pwd): |
330 | + bootstrap_list.bootstrap(data_path=data_path, |
331 | + log_dir=log_dir) |
332 | |
333 | def path(*suffix): |
334 | return os.path.join(data_path, *suffix) |
335 | |
336 | - getuid = self.mocker.replace("os.getuid") |
337 | - getuid() |
338 | - self.mocker.result(0) |
339 | - self.mocker.count(1, None) |
340 | - |
341 | - getpwnam = self.mocker.replace("pwd.getpwnam") |
342 | - value = getpwnam("landscape") |
343 | - self.mocker.count(1, None) |
344 | - value.pw_uid |
345 | - self.mocker.result(1234) |
346 | - self.mocker.count(1, None) |
347 | - |
348 | - getgrnam = self.mocker.replace("grp.getgrnam") |
349 | - value = getgrnam("root") |
350 | - self.mocker.count(1, None) |
351 | - value.gr_gid |
352 | - self.mocker.result(5678) |
353 | - self.mocker.count(1, None) |
354 | - |
355 | - chown = self.mocker.replace("os.chown") |
356 | - chown(path(), 1234, 5678) |
357 | - chown(path("messages"), 1234, 5678) |
358 | - chown(path("sockets"), 1234, 5678) |
359 | - chown(path("package"), 1234, 5678) |
360 | - chown(path("package/hash-id"), 1234, 5678) |
361 | - chown(path("package/binaries"), 1234, 5678) |
362 | - chown(path("package/upgrade-tool"), 1234, 5678) |
363 | - chown(path("custom-graph-scripts"), 1234, 5678) |
364 | - chown(path("package/database"), 1234, 5678) |
365 | - chown(log_dir, 1234, 5678) |
366 | - |
367 | - self.mocker.replay() |
368 | - |
369 | - bootstrap_list.bootstrap(data_path=data_path, |
370 | - log_dir=log_dir) |
371 | - |
372 | + paths = ["package", |
373 | + "package/hash-id", |
374 | + "package/binaries", |
375 | + "package/upgrade-tool", |
376 | + "messages", |
377 | + "sockets", |
378 | + "custom-graph-scripts", |
379 | + log_dir, |
380 | + "package/database"] |
381 | + calls = [mock.call(path(path_comps), 1234, 5678) |
382 | + for path_comps in paths] |
383 | + mock_chown.assert_has_calls([mock.call(path(), 1234, 5678)] + calls) |
384 | self.assertTrue(os.path.isdir(path())) |
385 | self.assertTrue(os.path.isdir(path("package"))) |
386 | self.assertTrue(os.path.isdir(path("messages"))) |
387 | @@ -1282,39 +1222,6 @@ |
388 | self.assertEqual(mode("custom-graph-scripts"), 0755) |
389 | self.assertEqual(mode("package/database"), 0644) |
390 | |
391 | - def test_log_notification(self): |
392 | - """ |
393 | - SIGUSR1 should cause logs to be reopened. |
394 | - """ |
395 | - logging.getLogger().addHandler(logging.FileHandler(self.makeFile())) |
396 | - WatchDogService(self.configuration) |
397 | - # We expect the Watchdog to delegate to each of the sub-processes |
398 | - daemon_mock = self.mocker.patch(Daemon) |
399 | - daemon_mock.rotate_logs() |
400 | - self.mocker.count(3) |
401 | - self.mocker.replay() |
402 | - |
403 | - # Store the initial set of handlers |
404 | - original_streams = [handler.stream for handler in |
405 | - logging.getLogger().handlers if |
406 | - isinstance(handler, logging.FileHandler)] |
407 | - |
408 | - # We fire the signal |
409 | - os.kill(os.getpid(), signal.SIGUSR1) |
410 | - |
411 | - def check(ign): |
412 | - new_streams = [handler.stream for handler in |
413 | - logging.getLogger().handlers if |
414 | - isinstance(handler, logging.FileHandler)] |
415 | - |
416 | - for stream in new_streams: |
417 | - self.assertTrue(stream not in original_streams) |
418 | - |
419 | - # We need to give some room for the callFromThread to run |
420 | - d = deferLater(reactor, 0, lambda: None) |
421 | - return d.addCallback(check) |
422 | - |
423 | - |
424 | STUB_BROKER = """\ |
425 | #!%(executable)s |
426 | import sys |
Command: TRIAL_ARGS=-j4 make check /ci.lscape. net/job/ latch-test/ 5110/
Result: Success
Revno: 894
Branch: lp:~adam-collard/landscape-client/mocker-replace-watchdog-service-tests
Jenkins: https:/