Merge lp:~free.ekanayaka/landscape-client/amp-cleanup-6 into lp:~landscape/landscape-client/trunk
- amp-cleanup-6
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Free Ekanayaka | ||||
Approved revision: | 670 | ||||
Merged at revision: | 666 | ||||
Proposed branch: | lp:~free.ekanayaka/landscape-client/amp-cleanup-6 | ||||
Merge into: | lp:~landscape/landscape-client/trunk | ||||
Diff against target: |
1005 lines (+180/-163) 21 files modified
landscape/amp.py (+7/-11) landscape/broker/amp.py (+3/-3) landscape/broker/tests/test_amp.py (+15/-13) landscape/broker/tests/test_service.py (+5/-1) landscape/configuration.py (+2/-1) landscape/lib/amp.py (+14/-0) landscape/lib/tests/test_amp.py (+17/-19) landscape/manager/tests/test_usermanager.py (+1/-1) landscape/manager/usermanager.py (+2/-4) landscape/monitor/tests/test_service.py (+0/-8) landscape/monitor/usermonitor.py (+2/-2) landscape/package/taskhandler.py (+1/-4) landscape/package/tests/test_changer.py (+1/-1) landscape/package/tests/test_taskhandler.py (+6/-9) landscape/reactor.py (+42/-25) landscape/service.py (+0/-5) landscape/tests/test_amp.py (+16/-20) landscape/tests/test_configuration.py (+42/-22) landscape/tests/test_service.py (+0/-5) landscape/tests/test_watchdog.py (+3/-8) landscape/watchdog.py (+1/-1) |
||||
To merge this branch: | bzr merge lp:~free.ekanayaka/landscape-client/amp-cleanup-6 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Chris Glass (community) | Approve | ||
Christopher Armstrong (community) | Approve | ||
Geoff Teale (community) | Approve | ||
Review via email: mp+161643@code.launchpad.net |
Commit message
Another episode of the AMP-related cleanup:
- Rename landscape.
- Drop the landscape.
- Drop landscape.
- Update tests that were using FakeReactor.
- Make landscape.
- Drop landscape.
Description of the change
Another episode of the AMP-related cleanup:
- Rename landscape.
- Drop the landscape.
- Drop landscape.
- Update tests that were using FakeReactor.
- Make landscape.
- Drop landscape.
Christopher Armstrong (radix) wrote : | # |
Have you filed a bug about Twisted's AMP not supporting synchronous transports? I'm not really sure what that means, that's why I ask, so I can go check out the explanation on that bug.
[1] The code around the call to self._factory.
[2] It doesn't seem right that _socket_paths is a class attribute instead of an instance attribute
Everything else looks fine!
Chris Glass (tribaal) wrote : | # |
Looks good! Thanks for doing this cleanup.
[1]
landscape/
landscape/
Free Ekanayaka (free.ekanayaka) wrote : | # |
Hi Chris, thanks for the careful review. I've filed:
http://
describing the issue.
[1]
You're right, that code is only meant for unit tests. I've renamed MethodCallClien
# XXX support exposing fake asynchronous connections created by tests, so
# they can be flushed transparently and emulate a synchronous behavior. See
# also http://
# hack can be removed.
fake_connection = None
The FakeReactor.
connection = FakeConnection(
factory.
[2]
Yeah, that's indeed a hack. The problem is that FakeReactor instances for clients and servers need to be different. I added a comment:
# XXX probably this shouldn't be a class attribute, but we need client-side
# FakeReactor instaces to be aware of listening sockets created by
# server-side FakeReactor instances.
_socket_paths = {}
Maybe the way to solve it would be to add API for declaring a reactor "aware" of another reactor (so tests would need to call it explicitly). Suggestions are welcome.
- 669. By Free Ekanayaka
-
Address radix's comments
- 670. By Free Ekanayaka
-
Drop unused imports
Free Ekanayaka (free.ekanayaka) wrote : | # |
Hey Chris (Glass), unused imports fixed.
Preview Diff
1 | === modified file 'landscape/amp.py' |
2 | --- landscape/amp.py 2013-04-29 15:46:13 +0000 |
3 | +++ landscape/amp.py 2013-05-02 08:30:35 +0000 |
4 | @@ -17,11 +17,6 @@ |
5 | MethodCallClientFactory, MethodCallServerFactory, RemoteObject) |
6 | |
7 | |
8 | -class ComponentProtocolClientFactory(MethodCallClientFactory): |
9 | - |
10 | - initialDelay = 0.05 |
11 | - |
12 | - |
13 | class ComponentPublisher(object): |
14 | """Publish a Landscape client component using a UNIX socket. |
15 | |
16 | @@ -35,6 +30,7 @@ |
17 | """ |
18 | |
19 | methods = ("ping", "exit") |
20 | + factory = MethodCallServerFactory |
21 | |
22 | def __init__(self, component, reactor, config): |
23 | self._reactor = reactor |
24 | @@ -53,7 +49,7 @@ |
25 | return self._port.stopListening() |
26 | |
27 | |
28 | -class RemoteComponentConnector(object): |
29 | +class ComponentConnector(object): |
30 | """Utility superclass for creating connections with a Landscape component. |
31 | |
32 | @cvar component: The class of the component to connect to, it is expected |
33 | @@ -71,8 +67,8 @@ |
34 | |
35 | @see: L{MethodCallClientFactory}. |
36 | """ |
37 | + factory = MethodCallClientFactory |
38 | component = None # Must be defined by sub-classes |
39 | - factory = ComponentProtocolClientFactory |
40 | remote = RemoteObject |
41 | |
42 | def __init__(self, reactor, config, retry_on_reconnect=False): |
43 | @@ -95,8 +91,8 @@ |
44 | result in a faster reconnection attempts pace. |
45 | @param quiet: A boolean indicating whether to log errors. |
46 | """ |
47 | - reactor = self._reactor._reactor |
48 | - factory = self.factory(reactor) |
49 | + factory = self.factory(self._reactor._reactor) |
50 | + factory.initialDelay = factory.delay = 0.05 |
51 | factory.retryOnReconnect = self._retry_on_reconnect |
52 | factory.remote = self.remote |
53 | |
54 | @@ -115,8 +111,8 @@ |
55 | if factor: |
56 | factory.factor = factor |
57 | socket_path = _get_socket_path(self.component, self._config) |
58 | - self._connector = reactor.connectUNIX(socket_path, factory) |
59 | deferred = factory.getRemoteObject() |
60 | + self._connector = self._reactor.connect_unix(socket_path, factory) |
61 | |
62 | if not quiet: |
63 | deferred.addErrback(log_error) |
64 | @@ -155,7 +151,7 @@ |
65 | def register(cls, connector_class): |
66 | """Register a connector for a Landscape component. |
67 | |
68 | - @param connector_class: A sub-class of L{RemoteComponentConnector} |
69 | + @param connector_class: A sub-class of L{ComponentConnector} |
70 | that can be used to connect to a certain component. |
71 | """ |
72 | cls._by_name[connector_class.component.name] = connector_class |
73 | |
74 | === modified file 'landscape/broker/amp.py' |
75 | --- landscape/broker/amp.py 2013-04-29 11:44:32 +0000 |
76 | +++ landscape/broker/amp.py 2013-05-02 08:30:35 +0000 |
77 | @@ -2,7 +2,7 @@ |
78 | |
79 | from landscape.lib.amp import RemoteObject, MethodCallArgument |
80 | from landscape.amp import ( |
81 | - RemoteComponentConnector, RemoteComponentsRegistry, ComponentPublisher) |
82 | + ComponentConnector, RemoteComponentsRegistry, ComponentPublisher) |
83 | from landscape.broker.server import BrokerServer |
84 | from landscape.broker.client import BrokerClient |
85 | from landscape.monitor.monitor import Monitor |
86 | @@ -95,14 +95,14 @@ |
87 | "message") |
88 | |
89 | |
90 | -class RemoteBrokerConnector(RemoteComponentConnector): |
91 | +class RemoteBrokerConnector(ComponentConnector): |
92 | """Helper to create connections with the L{BrokerServer}.""" |
93 | |
94 | remote = RemoteBroker |
95 | component = BrokerServer |
96 | |
97 | |
98 | -class RemoteClientConnector(RemoteComponentConnector): |
99 | +class RemoteClientConnector(ComponentConnector): |
100 | """Helper to create connections with the L{BrokerServer}.""" |
101 | |
102 | component = BrokerClient |
103 | |
104 | === modified file 'landscape/broker/tests/test_amp.py' |
105 | --- landscape/broker/tests/test_amp.py 2013-04-15 09:03:43 +0000 |
106 | +++ landscape/broker/tests/test_amp.py 2013-05-02 08:30:35 +0000 |
107 | @@ -168,9 +168,11 @@ |
108 | L{RemoteBroker.listen_events} returns a deferred which fires when |
109 | the first of the given events occurs in the broker reactor. |
110 | """ |
111 | - result = self.remote.listen_events(["event1", "event2"]) |
112 | - self.reactor._reactor.callLater(0.05, self.reactor.fire, "event2") |
113 | - return self.assertSuccess(result, "event2") |
114 | + deferred = self.remote.listen_events(["event1", "event2"]) |
115 | + self.reactor.call_later(0.05, self.reactor.fire, "event2") |
116 | + self.reactor.advance(0.05) |
117 | + self.remote._factory.fake_connection.flush() |
118 | + self.assertEqual("event2", self.successResultOf(deferred)) |
119 | |
120 | def test_call_on_events(self): |
121 | """ |
122 | @@ -182,10 +184,12 @@ |
123 | callback2 = self.mocker.mock() |
124 | self.expect(callback2()).result(123) |
125 | self.mocker.replay() |
126 | - result = self.remote.call_on_event({"event1": callback1, |
127 | - "event2": callback2}) |
128 | - self.reactor._reactor.callLater(0.05, self.reactor.fire, "event2") |
129 | - return self.assertSuccess(result, 123) |
130 | + deferred = self.remote.call_on_event({"event1": callback1, |
131 | + "event2": callback2}) |
132 | + self.reactor.call_later(0.05, self.reactor.fire, "event2") |
133 | + self.reactor.advance(0.05) |
134 | + self.remote._factory.fake_connection.flush() |
135 | + self.assertEqual(123, self.successResultOf(deferred)) |
136 | |
137 | def test_fire_event(self): |
138 | """ |
139 | @@ -202,9 +206,8 @@ |
140 | """ |
141 | Trying to call an non-exposed broker method results in a failure. |
142 | """ |
143 | - result = self.remote._sender.send_method_call( |
144 | - method="get_clients", args=[], kwargs={}) |
145 | - return self.assertFailure(result, MethodCallError) |
146 | + deferred = self.remote.get_clients() |
147 | + self.failureResultOf(deferred).trap(MethodCallError) |
148 | |
149 | |
150 | class RemoteClientTest(LandscapeTest): |
151 | @@ -263,6 +266,5 @@ |
152 | """ |
153 | Trying to call an non-exposed client method results in a failure. |
154 | """ |
155 | - result = self.remote_client._sender.send_method_call( |
156 | - method="get_plugins", args=[], kwargs={}) |
157 | - return self.assertFailure(result, MethodCallError) |
158 | + deferred = self.remote_client.get_plugins() |
159 | + self.failureResultOf(deferred).trap(MethodCallError) |
160 | |
161 | === modified file 'landscape/broker/tests/test_service.py' |
162 | --- landscape/broker/tests/test_service.py 2013-04-29 11:37:27 +0000 |
163 | +++ landscape/broker/tests/test_service.py 2013-05-02 08:30:35 +0000 |
164 | @@ -14,7 +14,11 @@ |
165 | |
166 | def setUp(self): |
167 | super(BrokerServiceTest, self).setUp() |
168 | - self.service = BrokerService(self.config) |
169 | + |
170 | + class FakeBrokerService(BrokerService): |
171 | + reactor_factory = FakeReactor |
172 | + |
173 | + self.service = FakeBrokerService(self.config) |
174 | |
175 | def test_persist(self): |
176 | """ |
177 | |
178 | === modified file 'landscape/configuration.py' |
179 | --- landscape/configuration.py 2013-03-27 22:58:20 +0000 |
180 | +++ landscape/configuration.py 2013-05-02 08:30:35 +0000 |
181 | @@ -618,7 +618,8 @@ |
182 | |
183 | 0.05 * (1 - 1.62 ** 14) / (1 - 1.62) = 69 seconds |
184 | """ |
185 | - reactor = TwistedReactor() |
186 | + if reactor is None: |
187 | + reactor = TwistedReactor() |
188 | exit_with_error = [] |
189 | |
190 | def stop(error=None): |
191 | |
192 | === modified file 'landscape/lib/amp.py' |
193 | --- landscape/lib/amp.py 2013-04-30 16:21:01 +0000 |
194 | +++ landscape/lib/amp.py 2013-05-02 08:30:35 +0000 |
195 | @@ -349,6 +349,14 @@ |
196 | result.addCallback(self._handle_result, deferred) |
197 | result.addErrback(self._handle_failure, method, args, kwargs, |
198 | deferred) |
199 | + |
200 | + if self._factory.fake_connection is not None: |
201 | + # Transparently flush the connection after a send_method_call |
202 | + # invokation letting tests simulate a synchronous transport. |
203 | + # This is needed because the Twisted's AMP implementation |
204 | + # assume that the transport is asynchronous. |
205 | + self._factory.fake_connection.flush() |
206 | + |
207 | return deferred |
208 | |
209 | return send_method_call |
210 | @@ -487,6 +495,12 @@ |
211 | retryOnReconnect = False |
212 | retryTimeout = None |
213 | |
214 | + # XXX support exposing fake asynchronous connections created by tests, so |
215 | + # they can be flushed transparently and emulate a synchronous behavior. See |
216 | + # also http://twistedmatrix.com/trac/ticket/6502, once that's fixed this |
217 | + # hack can be removed. |
218 | + fake_connection = None |
219 | + |
220 | def __init__(self, reactor=None): |
221 | """ |
222 | @param object: The object exposed by the L{MethodCallProtocol}s |
223 | |
224 | === modified file 'landscape/lib/tests/test_amp.py' |
225 | --- landscape/lib/tests/test_amp.py 2013-04-30 16:21:01 +0000 |
226 | +++ landscape/lib/tests/test_amp.py 2013-05-02 08:30:35 +0000 |
227 | @@ -1,28 +1,28 @@ |
228 | from twisted.internet import reactor |
229 | +from twisted.internet.error import ConnectError, ConnectionDone |
230 | +from twisted.internet.task import Clock |
231 | from twisted.internet.defer import Deferred, DeferredList |
232 | -from twisted.internet.error import ConnectionDone, ConnectError |
233 | -from twisted.internet.task import Clock |
234 | -from twisted.protocols.amp import AMP |
235 | from twisted.python.failure import Failure |
236 | |
237 | from landscape.lib.amp import ( |
238 | MethodCallError, MethodCallServerProtocol, MethodCallClientProtocol, |
239 | MethodCallServerFactory, MethodCallClientFactory, RemoteObject, |
240 | - RemoteObjectConnector, MethodCallReceiver, MethodCallSender) |
241 | + RemoteObjectConnector, MethodCallSender) |
242 | from landscape.tests.helpers import LandscapeTest |
243 | |
244 | |
245 | class FakeTransport(object): |
246 | """Accumulate written data into a list.""" |
247 | |
248 | - def __init__(self): |
249 | + def __init__(self, connection): |
250 | self.stream = [] |
251 | + self.connection = connection |
252 | |
253 | def write(self, data): |
254 | self.stream.append(data) |
255 | |
256 | def loseConnection(self): |
257 | - raise NotImplemented() |
258 | + self.connection.disconnect() |
259 | |
260 | def getPeer(self): |
261 | pass |
262 | @@ -37,8 +37,16 @@ |
263 | def __init__(self, client, server): |
264 | self.client = client |
265 | self.server = server |
266 | - self.server.makeConnection(FakeTransport()) |
267 | - self.client.makeConnection(FakeTransport()) |
268 | + self.connect() |
269 | + |
270 | + def connect(self): |
271 | + self.server.makeConnection(FakeTransport(self)) |
272 | + self.client.makeConnection(FakeTransport(self)) |
273 | + |
274 | + def disconnect(self): |
275 | + reason = Failure(ConnectionDone()) |
276 | + connector = self |
277 | + self.client.factory.clientConnectionLost(connector, reason) |
278 | |
279 | def flush(self): |
280 | """ |
281 | @@ -338,17 +346,7 @@ |
282 | client.factory = WordsFactory(self.clock) |
283 | self.remote = RemoteObject(client.factory) |
284 | self.connection = FakeConnection(client, server) |
285 | - |
286 | - send_method_call = self.remote._sender.send_method_call |
287 | - |
288 | - def synchronous_send_method_call(method, args=[], kwargs={}): |
289 | - # Transparently flush the connection after a send_method_call |
290 | - # invocation |
291 | - deferred = send_method_call(method, args=args, kwargs=kwargs) |
292 | - self.connection.flush() |
293 | - return deferred |
294 | - |
295 | - self.remote._sender.send_method_call = synchronous_send_method_call |
296 | + client.factory.fake_connection = self.connection |
297 | |
298 | def test_method_call_sender_with_forbidden_method(self): |
299 | """ |
300 | |
301 | === modified file 'landscape/manager/tests/test_usermanager.py' |
302 | --- landscape/manager/tests/test_usermanager.py 2013-03-20 08:58:22 +0000 |
303 | +++ landscape/manager/tests/test_usermanager.py 2013-05-02 08:30:35 +0000 |
304 | @@ -422,7 +422,7 @@ |
305 | def handle_callback(ignored): |
306 | messages = self.broker_service.message_store.get_pending_messages() |
307 | # Ignore the message created by plugin.run. |
308 | - messages = sorted(messages[1:3], |
309 | + messages = sorted([messages[1], messages[3]], |
310 | key=lambda message: message["operation-id"]) |
311 | self.assertMessages(messages, |
312 | [{"type": "operation-result", |
313 | |
314 | === modified file 'landscape/manager/usermanager.py' |
315 | --- landscape/manager/usermanager.py 2013-04-22 09:32:25 +0000 |
316 | +++ landscape/manager/usermanager.py 2013-05-02 08:30:35 +0000 |
317 | @@ -1,9 +1,7 @@ |
318 | -import os |
319 | import logging |
320 | |
321 | -from landscape.lib.amp import RemoteObject |
322 | from landscape.lib.encoding import encode_dict_if_needed |
323 | -from landscape.amp import ComponentPublisher, RemoteComponentConnector |
324 | +from landscape.amp import ComponentPublisher, ComponentConnector |
325 | |
326 | from landscape.user.management import UserManagement |
327 | from landscape.manager.plugin import ManagerPlugin |
328 | @@ -158,6 +156,6 @@ |
329 | methods = ["get_locked_usernames"] |
330 | |
331 | |
332 | -class RemoteUserManagerConnector(RemoteComponentConnector): |
333 | +class RemoteUserManagerConnector(ComponentConnector): |
334 | |
335 | component = UserManager |
336 | |
337 | === modified file 'landscape/monitor/tests/test_service.py' |
338 | --- landscape/monitor/tests/test_service.py 2013-04-29 11:37:27 +0000 |
339 | +++ landscape/monitor/tests/test_service.py 2013-05-02 08:30:35 +0000 |
340 | @@ -1,4 +1,3 @@ |
341 | -from landscape.tests.mocker import ANY |
342 | from landscape.tests.helpers import LandscapeTest, FakeBrokerServiceHelper |
343 | from landscape.reactor import FakeReactor |
344 | from landscape.monitor.config import MonitorConfiguration, ALL_PLUGINS |
345 | @@ -46,13 +45,6 @@ |
346 | starts the plugins and register the monitor as broker client. It also |
347 | start listening on its own socket for incoming connections. |
348 | """ |
349 | - # FIXME: don't actually run the real register method, because at the |
350 | - # moment the UserMonitor plugin still depends on DBus. We can probably |
351 | - # drop this mocking once the AMP migration is completed. |
352 | - for plugin in self.service.plugins: |
353 | - plugin.register = self.mocker.mock() |
354 | - plugin.register(ANY) |
355 | - self.mocker.replay() |
356 | |
357 | def stop_service(ignored): |
358 | [connector] = self.broker_service.broker.get_connectors() |
359 | |
360 | === modified file 'landscape/monitor/usermonitor.py' |
361 | --- landscape/monitor/usermonitor.py 2013-04-22 09:32:25 +0000 |
362 | +++ landscape/monitor/usermonitor.py 2013-05-02 08:30:35 +0000 |
363 | @@ -1,7 +1,7 @@ |
364 | from twisted.internet.defer import maybeDeferred |
365 | |
366 | from landscape.lib.log import log_failure |
367 | -from landscape.amp import ComponentPublisher, RemoteComponentConnector |
368 | +from landscape.amp import ComponentPublisher, ComponentConnector |
369 | |
370 | from landscape.monitor.plugin import MonitorPlugin |
371 | from landscape.user.changes import UserChanges |
372 | @@ -111,6 +111,6 @@ |
373 | methods = ["detect_changes"] |
374 | |
375 | |
376 | -class RemoteUserMonitorConnector(RemoteComponentConnector): |
377 | +class RemoteUserMonitorConnector(ComponentConnector): |
378 | |
379 | component = UserMonitor |
380 | |
381 | === modified file 'landscape/package/taskhandler.py' |
382 | --- landscape/package/taskhandler.py 2013-02-28 10:09:09 +0000 |
383 | +++ landscape/package/taskhandler.py 2013-05-02 08:30:35 +0000 |
384 | @@ -276,10 +276,7 @@ |
385 | |
386 | def finish(): |
387 | connector.disconnect() |
388 | - # For some obscure reason our TwistedReactor.stop method calls |
389 | - # reactor.crash() instead of reactor.stop(), which doesn't work |
390 | - # here. Maybe TwistedReactor.stop should simply use reactor.stop(). |
391 | - reactor.call_later(0, reactor._reactor.stop) |
392 | + reactor.call_later(0, reactor.stop) |
393 | |
394 | def got_error(failure): |
395 | log_failure(failure) |
396 | |
397 | === modified file 'landscape/package/tests/test_changer.py' |
398 | --- landscape/package/tests/test_changer.py 2013-03-18 03:34:11 +0000 |
399 | +++ landscape/package/tests/test_changer.py 2013-05-02 08:30:35 +0000 |
400 | @@ -1479,7 +1479,7 @@ |
401 | self.broker_service.reactor.advance(100) |
402 | self.twisted_reactor.advance(10) |
403 | payloads = self.broker_service.exchanger._transport.payloads |
404 | - self.assertEqual(1, len(payloads)) |
405 | + self.assertEqual(0, len(payloads)) |
406 | |
407 | [arguments] = self.process_factory.spawns |
408 | protocol = arguments[0] |
409 | |
410 | === modified file 'landscape/package/tests/test_taskhandler.py' |
411 | --- landscape/package/tests/test_taskhandler.py 2012-05-10 00:34:13 +0000 |
412 | +++ landscape/package/tests/test_taskhandler.py 2013-05-02 08:30:35 +0000 |
413 | @@ -1,10 +1,9 @@ |
414 | import os |
415 | |
416 | -from twisted.internet import reactor |
417 | from twisted.internet.defer import Deferred, fail |
418 | |
419 | from landscape.lib.lock import lock_path |
420 | -from landscape.reactor import TwistedReactor |
421 | +from landscape.reactor import TwistedReactor, FakeReactor |
422 | from landscape.broker.amp import RemoteBrokerConnector |
423 | from landscape.package.taskhandler import ( |
424 | PackageTaskHandlerConfiguration, PackageTaskHandler, run_task_handler, |
425 | @@ -366,7 +365,8 @@ |
426 | reactor_mock.run() |
427 | self.mocker.call(lambda: call_when_running[0]()) |
428 | connector_mock.disconnect() |
429 | - reactor_mock.call_later(0, reactor.stop) |
430 | + reactor_mock.call_later(0, ANY) |
431 | + self.mocker.result(None) |
432 | |
433 | # Okay, the whole playground is set. |
434 | self.mocker.replay() |
435 | @@ -433,12 +433,10 @@ |
436 | |
437 | def test_errors_in_tasks_are_printed_and_exit_program(self): |
438 | # Ignore a bunch of crap that we don't care about |
439 | - reactor_mock = self.mocker.patch(TwistedReactor) |
440 | init_logging_mock = self.mocker.replace("landscape.deployment" |
441 | ".init_logging", |
442 | passthrough=False) |
443 | init_logging_mock(ARGS) |
444 | - reactor_mock.run() |
445 | |
446 | class MyException(Exception): |
447 | pass |
448 | @@ -450,8 +448,6 @@ |
449 | handler_mock = handler_factory_mock(ARGS) |
450 | self.expect(handler_mock.run()).result(fail(MyException("Hey error"))) |
451 | |
452 | - reactor_mock.call_later(0, reactor.stop) |
453 | - |
454 | self.mocker.replay() |
455 | |
456 | # Ok now for some real stuff |
457 | @@ -460,7 +456,8 @@ |
458 | self.assertIn("MyException", self.logfile.getvalue()) |
459 | |
460 | result = run_task_handler(handler_factory_mock, |
461 | - ["-c", self.config_filename]) |
462 | + ["-c", self.config_filename], |
463 | + reactor=FakeReactor()) |
464 | return result.addCallback(assert_log) |
465 | |
466 | |
467 | @@ -473,7 +470,7 @@ |
468 | The L{LazyRemoteBroker} class doesn't initialize the actual remote |
469 | broker until one of its attributes gets actually accessed. |
470 | """ |
471 | - reactor = TwistedReactor() |
472 | + reactor = FakeReactor() |
473 | connector = RemoteBrokerConnector(reactor, self.broker_service.config) |
474 | self.broker = LazyRemoteBroker(connector) |
475 | self.assertIs(self.broker._remote, None) |
476 | |
477 | === modified file 'landscape/reactor.py' |
478 | --- landscape/reactor.py 2013-04-12 11:49:59 +0000 |
479 | +++ landscape/reactor.py 2013-05-02 08:30:35 +0000 |
480 | @@ -6,6 +6,8 @@ |
481 | import logging |
482 | import bisect |
483 | |
484 | +from twisted.python.failure import Failure |
485 | +from twisted.internet.error import ConnectError |
486 | from twisted.internet.threads import deferToThread |
487 | |
488 | from landscape.log import format_object |
489 | @@ -110,33 +112,13 @@ |
490 | raise InvalidID("EventID instance expected, received %r" % id) |
491 | |
492 | |
493 | -class UnixReactorMixin(object): |
494 | - """Support listening on Unix domain sockets. |
495 | - |
496 | - Note that this mixin uses the *real* Twisted reactor to open a *real* |
497 | - socket. |
498 | - |
499 | - Since it is used by *both* L{TwistedReactor} and L{FakeReactor}, this |
500 | - means that the latter is not really fake in this sense and that unit |
501 | - tests involving calls to this method won't be synchronous anymore. |
502 | - |
503 | - For example, many tests under L{landscape.broker.tests} use C{listen_unix} |
504 | - to setup a "real" remote broker and exercise the RPC/AMP functionality. See |
505 | - in particular L{landscape.broker.tests.helpers.RemoteBrokerHelper}. |
506 | - """ |
507 | - |
508 | - def listen_unix(self, socket, factory): |
509 | - """Start listening on a Unix socket.""" |
510 | - return self._reactor.listenUNIX(socket, factory, wantPID=True) |
511 | - |
512 | - |
513 | class ReactorID(object): |
514 | |
515 | def __init__(self, timeout): |
516 | self._timeout = timeout |
517 | |
518 | |
519 | -class TwistedReactor(EventHandlingReactorMixin, UnixReactorMixin): |
520 | +class TwistedReactor(EventHandlingReactorMixin): |
521 | """Wrap and add functionalities to the Twisted C{reactor}. |
522 | |
523 | This essentially a facade around the C{twisted.internet.reactor} and |
524 | @@ -239,6 +221,14 @@ |
525 | deferred.addCallback(on_success) |
526 | deferred.addErrback(on_failure) |
527 | |
528 | + def listen_unix(self, socket, factory): |
529 | + """Start listening on a Unix socket.""" |
530 | + return self._reactor.listenUNIX(socket, factory, wantPID=True) |
531 | + |
532 | + def connect_unix(self, socket, factory): |
533 | + """Connect to a Unix socket.""" |
534 | + return self._reactor.connectUNIX(socket, factory) |
535 | + |
536 | def run(self): |
537 | """Start the reactor, a C{"run"} event will be fired.""" |
538 | |
539 | @@ -266,7 +256,7 @@ |
540 | self._data = data |
541 | |
542 | |
543 | -class FakeReactor(EventHandlingReactorMixin, UnixReactorMixin): |
544 | +class FakeReactor(EventHandlingReactorMixin): |
545 | """A fake reactor with the same API of L{TwistedReactor}. |
546 | |
547 | This reactor emulates the asychronous interface of L{TwistedReactor}, but |
548 | @@ -279,6 +269,10 @@ |
549 | around Unix sockets), and implement a fake version C{listen_unix}, but this |
550 | hasn't been done yet. |
551 | """ |
552 | + # XXX probably this shouldn't be a class attribute, but we need client-side |
553 | + # FakeReactor instaces to be aware of listening sockets created by |
554 | + # server-side FakeReactor instances. |
555 | + _socket_paths = {} |
556 | |
557 | def __init__(self): |
558 | super(FakeReactor, self).__init__() |
559 | @@ -287,8 +281,8 @@ |
560 | self.hosts = {} |
561 | self._threaded_callbacks = [] |
562 | |
563 | - # We need a reference to the Twisted reactor as well to |
564 | - # let Landscape services listen to Unix sockets |
565 | + # XXX we need a reference to the Twisted reactor as well because |
566 | + # some tests use it |
567 | from twisted.internet import reactor |
568 | self._reactor = reactor |
569 | |
570 | @@ -326,7 +320,8 @@ |
571 | super(FakeReactor, self).cancel_call(id) |
572 | |
573 | def call_when_running(self, f): |
574 | - raise NotImplemented("The FakeReactor doesn't implement this.") |
575 | + # Just schedule a call that will be kicked by the run() method. |
576 | + self.call_later(0, f) |
577 | |
578 | def call_in_main(self, f, *args, **kwargs): |
579 | """Schedule a function for execution in the main thread.""" |
580 | @@ -347,6 +342,28 @@ |
581 | self._in_thread(callback, errback, f, args, kwargs) |
582 | self._run_threaded_callbacks() |
583 | |
584 | + def listen_unix(self, socket_path, factory): |
585 | + |
586 | + class FakePort(object): |
587 | + |
588 | + def stopListening(oself): |
589 | + self._socket_paths.pop(socket_path) |
590 | + |
591 | + self._socket_paths[socket_path] = factory |
592 | + return FakePort() |
593 | + |
594 | + def connect_unix(self, path, factory): |
595 | + server = self._socket_paths.get(path) |
596 | + from landscape.lib.tests.test_amp import FakeConnection |
597 | + if server: |
598 | + connection = FakeConnection(factory.buildProtocol(path), |
599 | + server.buildProtocol(path)) |
600 | + factory.fake_connection = connection |
601 | + else: |
602 | + connector = object() # Fake connector |
603 | + failure = Failure(ConnectError("No such file or directory")) |
604 | + factory.clientConnectionFailed(connector, failure) |
605 | + |
606 | def run(self): |
607 | """Continuously advance this reactor until reactor.stop() is called.""" |
608 | self.fire("run") |
609 | |
610 | === modified file 'landscape/service.py' |
611 | --- landscape/service.py 2013-04-29 11:44:32 +0000 |
612 | +++ landscape/service.py 2013-05-02 08:30:35 +0000 |
613 | @@ -1,4 +1,3 @@ |
614 | -import os |
615 | import logging |
616 | import signal |
617 | |
618 | @@ -42,15 +41,11 @@ |
619 | signal.signal( |
620 | signal.SIGUSR1, |
621 | lambda signal, frame: reactor.callFromThread(rotate_logs)) |
622 | - self.socket = os.path.join(self.config.sockets_path, |
623 | - self.service_name + ".sock") |
624 | |
625 | def startService(self): |
626 | Service.startService(self) |
627 | logging.info("%s started with config %s" % ( |
628 | self.service_name.capitalize(), self.config.get_config_filename())) |
629 | - if hasattr(self, "factory"): |
630 | - self.port = self.reactor.listen_unix(self.socket, self.factory) |
631 | |
632 | def stopService(self): |
633 | # We don't need to call port.stopListening(), because the reactor |
634 | |
635 | === modified file 'landscape/tests/test_amp.py' |
636 | --- landscape/tests/test_amp.py 2013-04-22 09:32:25 +0000 |
637 | +++ landscape/tests/test_amp.py 2013-05-02 08:30:35 +0000 |
638 | @@ -4,9 +4,7 @@ |
639 | from landscape.tests.helpers import LandscapeTest |
640 | from landscape.reactor import FakeReactor |
641 | from landscape.deployment import Configuration |
642 | -from landscape.amp import ( |
643 | - ComponentProtocolClientFactory, RemoteComponentConnector, |
644 | - ComponentPublisher) |
645 | +from landscape.amp import ComponentPublisher, ComponentConnector |
646 | |
647 | |
648 | class TestComponent(object): |
649 | @@ -14,22 +12,21 @@ |
650 | name = "test" |
651 | |
652 | |
653 | -class TestComponentProtocolFactory(ComponentProtocolClientFactory): |
654 | - |
655 | - maxRetries = 0 |
656 | - initialDelay = 0.01 |
657 | - |
658 | - |
659 | -class RemoteTestComponentConnector(RemoteComponentConnector): |
660 | - |
661 | - factory = TestComponentProtocolFactory |
662 | +class TestComponentConnector(ComponentConnector): |
663 | + |
664 | component = TestComponent |
665 | |
666 | |
667 | -class RemoteComponentTest(LandscapeTest): |
668 | +class FakeAMP(object): |
669 | + |
670 | + def __init__(self, locator): |
671 | + self._locator = locator |
672 | + |
673 | + |
674 | +class ComponentPublisherTest(LandscapeTest): |
675 | |
676 | def setUp(self): |
677 | - super(RemoteComponentTest, self).setUp() |
678 | + super(ComponentPublisherTest, self).setUp() |
679 | reactor = FakeReactor() |
680 | config = Configuration() |
681 | config.data_path = self.makeDir() |
682 | @@ -38,7 +35,7 @@ |
683 | self.publisher = ComponentPublisher(self.component, reactor, config) |
684 | self.publisher.start() |
685 | |
686 | - self.connector = RemoteTestComponentConnector(reactor, config) |
687 | + self.connector = TestComponentConnector(reactor, config) |
688 | connected = self.connector.connect() |
689 | connected.addCallback(lambda remote: setattr(self, "remote", remote)) |
690 | return connected |
691 | @@ -46,7 +43,7 @@ |
692 | def tearDown(self): |
693 | self.connector.disconnect() |
694 | self.publisher.stop() |
695 | - super(RemoteComponentTest, self).tearDown() |
696 | + super(ComponentPublisherTest, self).tearDown() |
697 | |
698 | def test_ping(self): |
699 | """ |
700 | @@ -71,16 +68,15 @@ |
701 | return self.assertSuccess(result) |
702 | |
703 | |
704 | -class RemoteComponentConnectorTest(LandscapeTest): |
705 | +class ComponentConnectorTest(LandscapeTest): |
706 | |
707 | def setUp(self): |
708 | - super(RemoteComponentConnectorTest, self).setUp() |
709 | + super(ComponentConnectorTest, self).setUp() |
710 | self.reactor = FakeReactor() |
711 | self.config = Configuration() |
712 | self.config.data_path = self.makeDir() |
713 | self.makeDir(path=self.config.sockets_path) |
714 | - self.connector = RemoteTestComponentConnector(self.reactor, |
715 | - self.config) |
716 | + self.connector = TestComponentConnector(self.reactor, self.config) |
717 | |
718 | def test_connect_logs_errors(self): |
719 | """ |
720 | |
721 | === modified file 'landscape/tests/test_configuration.py' |
722 | --- landscape/tests/test_configuration.py 2013-04-18 09:32:04 +0000 |
723 | +++ landscape/tests/test_configuration.py 2013-05-02 08:30:35 +0000 |
724 | @@ -5,9 +5,10 @@ |
725 | from cStringIO import StringIO |
726 | |
727 | from twisted.internet.defer import succeed, fail |
728 | +from twisted.internet.task import Clock |
729 | |
730 | from landscape.lib.amp import MethodCallSender |
731 | -from landscape.reactor import TwistedReactor |
732 | +from landscape.reactor import TwistedReactor, FakeReactor |
733 | from landscape.lib.fetch import HTTPCodeError, PyCurlError |
734 | from landscape.configuration import ( |
735 | print_text, LandscapeSetupScript, LandscapeSetupConfiguration, |
736 | @@ -1634,7 +1635,7 @@ |
737 | registration_mock = self.mocker.replace(service.registration) |
738 | config_mock = self.mocker.replace(service.config) |
739 | print_text_mock = self.mocker.replace(print_text) |
740 | - reactor_mock = self.mocker.patch(TwistedReactor) |
741 | + reactor_mock = self.mocker.patch(FakeReactor) |
742 | |
743 | # This must necessarily happen in the following order. |
744 | self.mocker.order() |
745 | @@ -1646,8 +1647,6 @@ |
746 | time_mock.sleep(ANY) |
747 | self.mocker.count(1) |
748 | |
749 | - reactor_mock.run() |
750 | - |
751 | # After a nice dance the configuration is reloaded. |
752 | config_mock.reload() |
753 | |
754 | @@ -1665,6 +1664,10 @@ |
755 | |
756 | reactor_mock.stop() |
757 | |
758 | + # This is actually called after everything else since all deferreds |
759 | + # are synchronous and callbacks will be executed immediately. |
760 | + reactor_mock.run() |
761 | + |
762 | # Nothing else is printed! |
763 | print_text_mock(ANY) |
764 | self.mocker.count(0) |
765 | @@ -1672,7 +1675,8 @@ |
766 | self.mocker.replay() |
767 | |
768 | # DO IT! |
769 | - return register(self.config, print_text, sys.exit) |
770 | + return register(self.config, print_text, sys.exit, |
771 | + reactor=FakeReactor()) |
772 | |
773 | def test_register_failure(self): |
774 | """ |
775 | @@ -1685,7 +1689,7 @@ |
776 | registration_mock = self.mocker.replace(service.registration) |
777 | config_mock = self.mocker.replace(service.config) |
778 | print_text_mock = self.mocker.replace(print_text) |
779 | - reactor_mock = self.mocker.patch(TwistedReactor) |
780 | + reactor_mock = self.mocker.patch(FakeReactor) |
781 | |
782 | # This must necessarily happen in the following order. |
783 | self.mocker.order() |
784 | @@ -1697,8 +1701,6 @@ |
785 | time_mock.sleep(ANY) |
786 | self.mocker.count(1) |
787 | |
788 | - reactor_mock.run() |
789 | - |
790 | # After a nice dance the configuration is reloaded. |
791 | config_mock.reload() |
792 | |
793 | @@ -1717,6 +1719,10 @@ |
794 | |
795 | reactor_mock.stop() |
796 | |
797 | + # This is actually called after everything else since all deferreds |
798 | + # are synchronous and callbacks will be executed immediately. |
799 | + reactor_mock.run() |
800 | + |
801 | # Nothing else is printed! |
802 | print_text_mock(ANY) |
803 | self.mocker.count(0) |
804 | @@ -1724,7 +1730,9 @@ |
805 | self.mocker.replay() |
806 | |
807 | # DO IT! |
808 | - return register(self.config, print_text, sys.exit) |
809 | + exit = [] |
810 | + register(self.config, print_text, exit.append, reactor=FakeReactor()) |
811 | + self.assertEqual([2], exit) |
812 | |
813 | def test_register_exchange_failure(self): |
814 | """ |
815 | @@ -1736,7 +1744,7 @@ |
816 | registration_mock = self.mocker.replace(service.registration) |
817 | config_mock = self.mocker.replace(service.config) |
818 | print_text_mock = self.mocker.replace(print_text) |
819 | - reactor_mock = self.mocker.patch(TwistedReactor) |
820 | + reactor_mock = self.mocker.patch(FakeReactor) |
821 | |
822 | # This must necessarily happen in the following order. |
823 | self.mocker.order() |
824 | @@ -1748,8 +1756,6 @@ |
825 | time_mock.sleep(ANY) |
826 | self.mocker.count(1) |
827 | |
828 | - reactor_mock.run() |
829 | - |
830 | # After a nice dance the configuration is reloaded. |
831 | config_mock.reload() |
832 | |
833 | @@ -1767,6 +1773,10 @@ |
834 | |
835 | reactor_mock.stop() |
836 | |
837 | + # This is actually called after everything else since all deferreds |
838 | + # are synchronous and callbacks will be executed immediately. |
839 | + reactor_mock.run() |
840 | + |
841 | # Nothing else is printed! |
842 | print_text_mock(ANY) |
843 | self.mocker.count(0) |
844 | @@ -1774,7 +1784,9 @@ |
845 | self.mocker.replay() |
846 | |
847 | # DO IT! |
848 | - return register(self.config, print_text, sys.exit) |
849 | + exit = [] |
850 | + register(self.config, print_text, exit.append, reactor=FakeReactor()) |
851 | + self.assertEqual([2], exit) |
852 | |
853 | def test_register_timeout_failure(self): |
854 | service = self.broker_service |
855 | @@ -1782,7 +1794,7 @@ |
856 | registration_mock = self.mocker.replace(service.registration) |
857 | config_mock = self.mocker.replace(service.config) |
858 | print_text_mock = self.mocker.replace(print_text) |
859 | - reactor_mock = self.mocker.patch(TwistedReactor) |
860 | + reactor_mock = self.mocker.patch(FakeReactor) |
861 | remote_mock = self.mocker.patch(RemoteBroker) |
862 | |
863 | protocol_mock = self.mocker.patch(MethodCallSender) |
864 | @@ -1800,17 +1812,19 @@ |
865 | time_mock.sleep(ANY) |
866 | self.mocker.count(1) |
867 | |
868 | - reactor_mock.run() |
869 | + # After a nice dance the configuration is reloaded. |
870 | + config_mock.reload() |
871 | |
872 | remote_mock.call_on_event(ANY) |
873 | self.mocker.result(succeed(None)) |
874 | |
875 | - # After a nice dance the configuration is reloaded. |
876 | - config_mock.reload() |
877 | - |
878 | registration_mock.register() |
879 | self.mocker.passthrough() |
880 | |
881 | + # This is actually called after everything else since all deferreds |
882 | + # are synchronous and callbacks will be executed immediately. |
883 | + reactor_mock.run() |
884 | + |
885 | # Nothing else is printed! |
886 | print_text_mock(ANY) |
887 | self.mocker.count(0) |
888 | @@ -1818,7 +1832,12 @@ |
889 | self.mocker.replay() |
890 | |
891 | # DO IT! |
892 | - return register(self.config, print_text, sys.exit) |
893 | + fake_reactor = FakeReactor() |
894 | + fake_reactor._reactor = Clock() |
895 | + deferred = register(self.config, print_text, sys.exit, |
896 | + reactor=fake_reactor) |
897 | + fake_reactor._reactor.advance(100) |
898 | + return deferred |
899 | |
900 | def test_register_bus_connection_failure(self): |
901 | """ |
902 | @@ -1993,14 +2012,13 @@ |
903 | remote_broker = self.mocker.mock() |
904 | |
905 | print_text_mock = self.mocker.replace(print_text) |
906 | - reactor_mock = self.mocker.patch(TwistedReactor) |
907 | + reactor_mock = self.mocker.patch(FakeReactor) |
908 | |
909 | # This is unordered. It's just way too much of a pain. |
910 | print_text_mock("Please wait... ", "") |
911 | time_mock = self.mocker.replace("time") |
912 | time_mock.sleep(ANY) |
913 | self.mocker.count(1) |
914 | - reactor_mock.run() |
915 | |
916 | # SNORE |
917 | connector = connector_factory(ANY, configuration) |
918 | @@ -2025,10 +2043,12 @@ |
919 | # WHOAH DUDE. This waits for callLater(0, reactor.stop). |
920 | connector.disconnect() |
921 | reactor_mock.stop() |
922 | + reactor_mock.run() |
923 | |
924 | self.mocker.replay() |
925 | |
926 | - return register(configuration, print_text, sys.exit, max_retries=0) |
927 | + return register(configuration, print_text, sys.exit, max_retries=0, |
928 | + reactor=FakeReactor()) |
929 | |
930 | |
931 | class SSLCertificateDataTest(LandscapeConfigurationTest): |
932 | |
933 | === modified file 'landscape/tests/test_service.py' |
934 | --- landscape/tests/test_service.py 2013-04-22 09:32:25 +0000 |
935 | +++ landscape/tests/test_service.py 2013-05-02 08:30:35 +0000 |
936 | @@ -7,7 +7,6 @@ |
937 | from landscape.reactor import FakeReactor |
938 | from landscape.deployment import Configuration |
939 | from landscape.service import LandscapeService |
940 | -from landscape.amp import RemoteComponentConnector |
941 | from landscape.tests.helpers import LandscapeTest |
942 | |
943 | |
944 | @@ -15,10 +14,6 @@ |
945 | name = "monitor" |
946 | |
947 | |
948 | -class RemoteTestComponentCreator(RemoteComponentConnector): |
949 | - component = TestComponent |
950 | - |
951 | - |
952 | class TestService(LandscapeService): |
953 | service_name = TestComponent.name |
954 | |
955 | |
956 | === modified file 'landscape/tests/test_watchdog.py' |
957 | --- landscape/tests/test_watchdog.py 2013-04-29 11:44:32 +0000 |
958 | +++ landscape/tests/test_watchdog.py 2013-05-02 08:30:35 +0000 |
959 | @@ -22,7 +22,7 @@ |
960 | from landscape.lib.dns import discover_server |
961 | from landscape.configuration import ( |
962 | fetch_base64_ssl_public_certificate, print_text) |
963 | -from landscape.amp import ComponentProtocolClientFactory, RemoteComponentConnector |
964 | +from landscape.amp import ComponentConnector |
965 | from landscape.broker.amp import RemoteBrokerConnector |
966 | from landscape.deployment import Configuration |
967 | from landscape.reactor import TwistedReactor |
968 | @@ -472,15 +472,9 @@ |
969 | name = "broker" |
970 | |
971 | |
972 | -class StubBrokerProtocolFactory(ComponentProtocolClientFactory): |
973 | - |
974 | - initialDelay = 0.1 |
975 | - |
976 | - |
977 | -class RemoteStubBrokerConnector(RemoteComponentConnector): |
978 | +class RemoteStubBrokerConnector(ComponentConnector): |
979 | |
980 | component = StubBroker |
981 | - factory = StubBrokerProtocolFactory |
982 | |
983 | |
984 | class DaemonTestBase(LandscapeTest): |
985 | @@ -945,6 +939,7 @@ |
986 | return RemoteBrokerConnector |
987 | |
988 | def test_is_running(self): |
989 | + self.daemon._connector._reactor = self.broker_service.reactor |
990 | result = self.daemon.is_running() |
991 | result.addCallback(self.assertTrue) |
992 | return result |
993 | |
994 | === modified file 'landscape/watchdog.py' |
995 | --- landscape/watchdog.py 2013-04-22 09:32:25 +0000 |
996 | +++ landscape/watchdog.py 2013-05-02 08:30:35 +0000 |
997 | @@ -68,7 +68,7 @@ |
998 | @cvar factor: The factor by which the delay between subsequent connection |
999 | attempts will increase. |
1000 | |
1001 | - @param connector: The L{RemoteComponentConnector} of the daemon. |
1002 | + @param connector: The L{ComponentConnector} of the daemon. |
1003 | @param reactor: The reactor used to spawn the process and schedule timed |
1004 | calls. |
1005 | @param verbose: Optionally, report more information when running this |
+1
The duplication of comments in test_configuration is a bit odd, but I guess you're trying to ensure that point isn't missed.