Merge lp:~rainct/zeitgeist/922620 into lp:~zeitgeist/zeitgeist/bluebird
- 922620
- Merge into bluebird
Proposed by
Siegfried Gevatter
Status: | Merged |
---|---|
Merged at revision: | 386 |
Proposed branch: | lp:~rainct/zeitgeist/922620 |
Merge into: | lp:~zeitgeist/zeitgeist/bluebird |
Diff against target: |
279 lines (+94/-28) 4 files modified
python/client.py (+33/-21) test/dbus/blacklist-test.py (+18/-5) test/dbus/monitor-test.py (+29/-0) test/dbus/testutils.py (+14/-2) |
To merge this branch: | bzr merge lp:~rainct/zeitgeist/922620 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Zeitgeist Framework Team | Pending | ||
Review via email: mp+91920@code.launchpad.net |
Commit message
Description of the change
To post a comment you must log in.
lp:~rainct/zeitgeist/922620
updated
- 388. By Siegfried Gevatter
-
Add test case for monitor re-connection
- 389. By Siegfried Gevatter
-
Make bus initialization lazy so tests connect to the correct D-Bus instance
- 390. By Siegfried Gevatter
-
testutils.py: randomize DISPLAY so more than one test can run simultaneously
Revision history for this message
Seif Lotfy (seif) wrote : | # |
lp:~rainct/zeitgeist/922620
updated
- 391. By Siegfried Gevatter
-
Fix monitors being installed twice; python-dbus always gives us a name
owner changed notification, so we want to ignore this first one.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'python/client.py' | |||
2 | --- python/client.py 2011-10-29 13:31:12 +0000 | |||
3 | +++ python/client.py 2012-02-07 22:51:18 +0000 | |||
4 | @@ -40,6 +40,19 @@ | |||
5 | 40 | 40 | ||
6 | 41 | log = logging.getLogger("zeitgeist.client") | 41 | log = logging.getLogger("zeitgeist.client") |
7 | 42 | 42 | ||
8 | 43 | # This is here so testutils.py can override it with a private bus connection. | ||
9 | 44 | # Init needs to be lazy so tests will use the private D-Bus instance. | ||
10 | 45 | global session_bus | ||
11 | 46 | session_bus = None | ||
12 | 47 | def get_bus(): | ||
13 | 48 | global session_bus | ||
14 | 49 | if session_bus is None: | ||
15 | 50 | session_bus = dbus.SessionBus() | ||
16 | 51 | return session_bus | ||
17 | 52 | def _set_bus(bus): | ||
18 | 53 | global session_bus | ||
19 | 54 | session_bus = bus | ||
20 | 55 | |||
21 | 43 | class _DBusInterface(object): | 56 | class _DBusInterface(object): |
22 | 44 | """Wrapper around dbus.Interface adding convenience methods.""" | 57 | """Wrapper around dbus.Interface adding convenience methods.""" |
23 | 45 | 58 | ||
24 | @@ -47,7 +60,6 @@ | |||
25 | 47 | # that here because otherwise all instances would share their state. | 60 | # that here because otherwise all instances would share their state. |
26 | 48 | _disconnect_callbacks = None | 61 | _disconnect_callbacks = None |
27 | 49 | _reconnect_callbacks = None | 62 | _reconnect_callbacks = None |
28 | 50 | _generic_callbacks = None | ||
29 | 51 | 63 | ||
30 | 52 | @staticmethod | 64 | @staticmethod |
31 | 53 | def get_members(introspection_xml): | 65 | def get_members(introspection_xml): |
32 | @@ -69,8 +81,9 @@ | |||
33 | 69 | def reconnect(self): | 81 | def reconnect(self): |
34 | 70 | if not self._reconnect_when_needed: | 82 | if not self._reconnect_when_needed: |
35 | 71 | return | 83 | return |
38 | 72 | self.__proxy = dbus.SessionBus().get_object( | 84 | self.__proxy = get_bus().get_object( |
39 | 73 | self.__iface.requested_bus_name, self.__object_path) | 85 | self.__iface.requested_bus_name, self.__object_path, |
40 | 86 | follow_name_owner_changes=True) | ||
41 | 74 | self.__iface = dbus.Interface(self.__proxy, self.__interface_name) | 87 | self.__iface = dbus.Interface(self.__proxy, self.__interface_name) |
42 | 75 | self._load_introspection_data() | 88 | self._load_introspection_data() |
43 | 76 | 89 | ||
44 | @@ -131,8 +144,7 @@ | |||
45 | 131 | self.reconnect() | 144 | self.reconnect() |
46 | 132 | if signal not in self.__signals: | 145 | if signal not in self.__signals: |
47 | 133 | raise TypeError("Unknown signal name: %s" % signal) | 146 | raise TypeError("Unknown signal name: %s" % signal) |
50 | 134 | self._generic_callbacks.add((signal, callback)) | 147 | return self.__proxy.connect_to_signal( |
49 | 135 | self.__proxy.connect_to_signal( | ||
51 | 136 | signal, | 148 | signal, |
52 | 137 | callback, | 149 | callback, |
53 | 138 | dbus_interface=self.__interface_name, | 150 | dbus_interface=self.__interface_name, |
54 | @@ -167,29 +179,28 @@ | |||
55 | 167 | self._reconnect_when_needed = reconnect | 179 | self._reconnect_when_needed = reconnect |
56 | 168 | self._load_introspection_data() | 180 | self._load_introspection_data() |
57 | 169 | 181 | ||
58 | 182 | self._first_connection = True | ||
59 | 170 | self._disconnect_callbacks = set() | 183 | self._disconnect_callbacks = set() |
60 | 171 | self._reconnect_callbacks = set() | 184 | self._reconnect_callbacks = set() |
61 | 172 | self._generic_callbacks = set() | ||
62 | 173 | 185 | ||
63 | 174 | # Listen to (dis)connection notifications, for connect_exit and connect_join | 186 | # Listen to (dis)connection notifications, for connect_exit and connect_join |
64 | 175 | def name_owner_changed(connection_name): | 187 | def name_owner_changed(connection_name): |
65 | 176 | if connection_name == "": | 188 | if connection_name == "": |
66 | 177 | callbacks = self._disconnect_callbacks | ||
67 | 178 | self.__methods = self.__signals = None | 189 | self.__methods = self.__signals = None |
68 | 190 | for callback in self._disconnect_callbacks: | ||
69 | 191 | callback() | ||
70 | 192 | elif self._first_connection: | ||
71 | 193 | # python-dbus guarantees that it'll call NameOwnerChanged at startup | ||
72 | 194 | # (even if the service was already running). When that happens, we | ||
73 | 195 | # don't want to connect the signals a second time. | ||
74 | 196 | self._first_connection = False | ||
75 | 179 | else: | 197 | else: |
76 | 180 | if not self._reconnect_when_needed: | 198 | if not self._reconnect_when_needed: |
77 | 181 | return | 199 | return |
78 | 182 | self.reconnect() | 200 | self.reconnect() |
89 | 183 | callbacks = self._reconnect_callbacks | 201 | for callback in self._reconnect_callbacks: |
90 | 184 | for signal, callback in self._generic_callbacks: | 202 | callback() |
91 | 185 | try: | 203 | get_bus().watch_name_owner(self.__iface.requested_bus_name, |
82 | 186 | self.connect(signal, callback) | ||
83 | 187 | except TypeError: | ||
84 | 188 | log.exception("Failed to reconnect to signal \"%s\" " | ||
85 | 189 | "after engine disconnection." % signal) | ||
86 | 190 | for callback in callbacks: | ||
87 | 191 | callback() | ||
88 | 192 | dbus.SessionBus().watch_name_owner(self.__iface.requested_bus_name, | ||
92 | 193 | name_owner_changed) | 204 | name_owner_changed) |
93 | 194 | 205 | ||
94 | 195 | class ZeitgeistDBusInterface(object): | 206 | class ZeitgeistDBusInterface(object): |
95 | @@ -233,7 +244,8 @@ | |||
96 | 233 | if not name in cls.__shared_state["extension_interfaces"]: | 244 | if not name in cls.__shared_state["extension_interfaces"]: |
97 | 234 | interface_name = "org.gnome.zeitgeist.%s" % name | 245 | interface_name = "org.gnome.zeitgeist.%s" % name |
98 | 235 | object_path = "/org/gnome/zeitgeist/%s" % path | 246 | object_path = "/org/gnome/zeitgeist/%s" % path |
100 | 236 | proxy = dbus.SessionBus().get_object(busname, object_path) | 247 | proxy = get_bus().get_object(busname, object_path, |
101 | 248 | follow_name_owner_changes=True) | ||
102 | 237 | iface = _DBusInterface(proxy, interface_name, object_path) | 249 | iface = _DBusInterface(proxy, interface_name, object_path) |
103 | 238 | iface.BUS_NAME = busname | 250 | iface.BUS_NAME = busname |
104 | 239 | iface.INTERFACE_NAME = interface_name | 251 | iface.INTERFACE_NAME = interface_name |
105 | @@ -244,8 +256,8 @@ | |||
106 | 244 | def __init__(self, reconnect=True): | 256 | def __init__(self, reconnect=True): |
107 | 245 | if not "dbus_interface" in self.__shared_state: | 257 | if not "dbus_interface" in self.__shared_state: |
108 | 246 | try: | 258 | try: |
111 | 247 | proxy = dbus.SessionBus().get_object(self.BUS_NAME, | 259 | proxy = get_bus().get_object(self.BUS_NAME, |
112 | 248 | self.OBJECT_PATH) | 260 | self.OBJECT_PATH, follow_name_owner_changes=True) |
113 | 249 | except dbus.exceptions.DBusException, e: | 261 | except dbus.exceptions.DBusException, e: |
114 | 250 | if e.get_dbus_name() == "org.freedesktop.DBus.Error.ServiceUnknown": | 262 | if e.get_dbus_name() == "org.freedesktop.DBus.Error.ServiceUnknown": |
115 | 251 | raise RuntimeError( | 263 | raise RuntimeError( |
116 | @@ -292,7 +304,7 @@ | |||
117 | 292 | self._path = monitor_path | 304 | self._path = monitor_path |
118 | 293 | self._insert_callback = insert_callback | 305 | self._insert_callback = insert_callback |
119 | 294 | self._delete_callback = delete_callback | 306 | self._delete_callback = delete_callback |
121 | 295 | dbus.service.Object.__init__(self, dbus.SessionBus(), monitor_path) | 307 | dbus.service.Object.__init__(self, get_bus(), monitor_path) |
122 | 296 | 308 | ||
123 | 297 | def get_path (self): return self._path | 309 | def get_path (self): return self._path |
124 | 298 | path = property(get_path, | 310 | path = property(get_path, |
125 | 299 | 311 | ||
126 | === modified file 'test/dbus/blacklist-test.py' | |||
127 | --- test/dbus/blacklist-test.py 2011-09-07 20:48:27 +0000 | |||
128 | +++ test/dbus/blacklist-test.py 2012-02-07 22:51:18 +0000 | |||
129 | @@ -110,11 +110,13 @@ | |||
130 | 110 | newAllTemplates = blacklist.GetTemplates() | 110 | newAllTemplates = blacklist.GetTemplates() |
131 | 111 | self.assertEquals(len(newAllTemplates), 0) | 111 | self.assertEquals(len(newAllTemplates), 0) |
132 | 112 | 112 | ||
134 | 113 | def testBlacklistSignals(self): | 113 | def testBlacklistSignals(self, mainloop=None, connect_signals=True): |
135 | 114 | self.blacklist = self._get_blacklist_iface() | 114 | self.blacklist = self._get_blacklist_iface() |
137 | 115 | mainloop = self.create_mainloop() | 115 | if mainloop is None: |
138 | 116 | mainloop = self.create_mainloop() | ||
139 | 116 | 117 | ||
140 | 117 | template1 = Event.new_for_values( | 118 | template1 = Event.new_for_values( |
141 | 119 | timestamp=0, | ||
142 | 118 | interpretation=Interpretation.ACCESS_EVENT, | 120 | interpretation=Interpretation.ACCESS_EVENT, |
143 | 119 | subject_uri="http://nothingtoseehere.gov") | 121 | subject_uri="http://nothingtoseehere.gov") |
144 | 120 | 122 | ||
145 | @@ -124,7 +126,6 @@ | |||
146 | 124 | @asyncTestMethod(mainloop) | 126 | @asyncTestMethod(mainloop) |
147 | 125 | def cb_added(template_id, event_template): | 127 | def cb_added(template_id, event_template): |
148 | 126 | global hit | 128 | global hit |
149 | 127 | print "*** cb_added with hit", hit | ||
150 | 128 | self.assertEquals(hit, 0) | 129 | self.assertEquals(hit, 0) |
151 | 129 | hit = 1 | 130 | hit = 1 |
152 | 130 | self.assertEquals(template_id, "TestTemplate") | 131 | self.assertEquals(template_id, "TestTemplate") |
153 | @@ -140,8 +141,9 @@ | |||
154 | 140 | mainloop.quit() | 141 | mainloop.quit() |
155 | 141 | 142 | ||
156 | 142 | # Connect to signals | 143 | # Connect to signals |
159 | 143 | self.blacklist.connect('TemplateAdded', cb_added) | 144 | if connect_signals: |
160 | 144 | self.blacklist.connect('TemplateRemoved', cb_removed) | 145 | self.blacklist.connect('TemplateAdded', cb_added) |
161 | 146 | self.blacklist.connect('TemplateRemoved', cb_removed) | ||
162 | 145 | 147 | ||
163 | 146 | def launch_tests(): | 148 | def launch_tests(): |
164 | 147 | self.blacklist.AddTemplate("TestTemplate", template1) | 149 | self.blacklist.AddTemplate("TestTemplate", template1) |
165 | @@ -150,5 +152,16 @@ | |||
166 | 150 | 152 | ||
167 | 151 | mainloop.run() | 153 | mainloop.run() |
168 | 152 | 154 | ||
169 | 155 | def testBlacklistSignalWithReconnection(self): | ||
170 | 156 | mainloop = self.create_mainloop() | ||
171 | 157 | self.testBlacklistSignals(mainloop) | ||
172 | 158 | |||
173 | 159 | # Restart the Zeitgeist daemon... | ||
174 | 160 | self.kill_daemon() | ||
175 | 161 | self.spawn_daemon() | ||
176 | 162 | |||
177 | 163 | # ... and try again without re-connecting to the signals | ||
178 | 164 | self.testBlacklistSignals(mainloop, connect_signals=False) | ||
179 | 165 | |||
180 | 153 | if __name__ == "__main__": | 166 | if __name__ == "__main__": |
181 | 154 | unittest.main() | 167 | unittest.main() |
182 | 155 | 168 | ||
183 | === modified file 'test/dbus/monitor-test.py' | |||
184 | --- test/dbus/monitor-test.py 2011-09-07 20:48:27 +0000 | |||
185 | +++ test/dbus/monitor-test.py 2012-02-07 22:51:18 +0000 | |||
186 | @@ -286,5 +286,34 @@ | |||
187 | 286 | self.assertEquals(1, len(result)) | 286 | self.assertEquals(1, len(result)) |
188 | 287 | self.assertEquals(1, result.pop()) | 287 | self.assertEquals(1, result.pop()) |
189 | 288 | 288 | ||
190 | 289 | def testMonitorReconnection(self): | ||
191 | 290 | result = [] | ||
192 | 291 | mainloop = self.create_mainloop() | ||
193 | 292 | events = parse_events("test/data/three_events.js") | ||
194 | 293 | |||
195 | 294 | @asyncTestMethod(mainloop) | ||
196 | 295 | def notify_insert_handler(time_range, events): | ||
197 | 296 | result.extend(events) | ||
198 | 297 | mainloop.quit() | ||
199 | 298 | |||
200 | 299 | @asyncTestMethod(mainloop) | ||
201 | 300 | def notify_delete_handler(time_range, event_ids): | ||
202 | 301 | mainloop.quit() | ||
203 | 302 | self.fail("Unexpected delete notification") | ||
204 | 303 | |||
205 | 304 | self.client.install_monitor(TimeRange.always(), [], | ||
206 | 305 | notify_insert_handler, notify_delete_handler) | ||
207 | 306 | |||
208 | 307 | # Restart the Zeitgeist daemon to test automagic monitor re-connection | ||
209 | 308 | self.kill_daemon() | ||
210 | 309 | self.spawn_daemon() | ||
211 | 310 | |||
212 | 311 | # Insert events in idle loop to give the reconnection logic enough time | ||
213 | 312 | gobject.idle_add(lambda *args: self.client.insert_events(events)) | ||
214 | 313 | |||
215 | 314 | mainloop.run() | ||
216 | 315 | |||
217 | 316 | self.assertEquals(3, len(result)) | ||
218 | 317 | |||
219 | 289 | if __name__ == "__main__": | 318 | if __name__ == "__main__": |
220 | 290 | unittest.main() | 319 | unittest.main() |
221 | 291 | 320 | ||
222 | === modified file 'test/dbus/testutils.py' | |||
223 | --- test/dbus/testutils.py 2012-02-07 17:49:49 +0000 | |||
224 | +++ test/dbus/testutils.py 2012-02-07 22:51:18 +0000 | |||
225 | @@ -21,12 +21,14 @@ | |||
226 | 21 | # along with this program. If not, see <http://www.gnu.org/licenses/>. | 21 | # along with this program. If not, see <http://www.gnu.org/licenses/>. |
227 | 22 | 22 | ||
228 | 23 | import unittest | 23 | import unittest |
229 | 24 | import dbus | ||
230 | 24 | import os | 25 | import os |
231 | 25 | import time | 26 | import time |
232 | 26 | import sys | 27 | import sys |
233 | 27 | import signal | 28 | import signal |
234 | 28 | import tempfile | 29 | import tempfile |
235 | 29 | import shutil | 30 | import shutil |
236 | 31 | import random | ||
237 | 30 | from subprocess import Popen, PIPE | 32 | from subprocess import Popen, PIPE |
238 | 31 | 33 | ||
239 | 32 | # DBus setup | 34 | # DBus setup |
240 | @@ -36,7 +38,8 @@ | |||
241 | 36 | 38 | ||
242 | 37 | # Import local Zeitgeist modules | 39 | # Import local Zeitgeist modules |
243 | 38 | sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) | 40 | sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) |
245 | 39 | from zeitgeist.client import ZeitgeistDBusInterface, ZeitgeistClient | 41 | from zeitgeist.client import ZeitgeistDBusInterface, ZeitgeistClient, \ |
246 | 42 | get_bus, _set_bus | ||
247 | 40 | from zeitgeist.datamodel import Event, Subject, Interpretation, Manifestation, \ | 43 | from zeitgeist.datamodel import Event, Subject, Interpretation, Manifestation, \ |
248 | 41 | TimeRange, NULL_EVENT | 44 | TimeRange, NULL_EVENT |
249 | 42 | 45 | ||
250 | @@ -175,11 +178,18 @@ | |||
251 | 175 | 178 | ||
252 | 176 | # hack to clear the state of the interface | 179 | # hack to clear the state of the interface |
253 | 177 | ZeitgeistDBusInterface._ZeitgeistDBusInterface__shared_state = {} | 180 | ZeitgeistDBusInterface._ZeitgeistDBusInterface__shared_state = {} |
254 | 181 | |||
255 | 182 | # Replace the bus connection with a private one for each test case, | ||
256 | 183 | # so that they don't share signals or other state | ||
257 | 184 | _set_bus(dbus.SessionBus(private=True)) | ||
258 | 185 | get_bus().set_exit_on_disconnect(False) | ||
259 | 186 | |||
260 | 178 | self.client = ZeitgeistClient() | 187 | self.client = ZeitgeistClient() |
261 | 179 | 188 | ||
262 | 180 | def tearDown(self): | 189 | def tearDown(self): |
263 | 181 | assert self.daemon is not None | 190 | assert self.daemon is not None |
264 | 182 | assert self.client is not None | 191 | assert self.client is not None |
265 | 192 | get_bus().close() | ||
266 | 183 | self.kill_daemon() | 193 | self.kill_daemon() |
267 | 184 | if 'ZEITGEIST_TESTS_KEEP_TMP' in os.environ: | 194 | if 'ZEITGEIST_TESTS_KEEP_TMP' in os.environ: |
268 | 185 | print '\n\nAll temporary files have been preserved in %s\n' \ | 195 | print '\n\nAll temporary files have been preserved in %s\n' \ |
269 | @@ -389,7 +399,9 @@ | |||
270 | 389 | self.assertEqual(ev1, ev2) | 399 | self.assertEqual(ev1, ev2) |
271 | 390 | 400 | ||
272 | 391 | class DBusPrivateMessageBus(object): | 401 | class DBusPrivateMessageBus(object): |
274 | 392 | DISPLAY = ":27" | 402 | # Choose a random number so it's possible to have more than |
275 | 403 | # one test running at once. | ||
276 | 404 | DISPLAY = ":%d" % random.randint(20, 100) | ||
277 | 393 | 405 | ||
278 | 394 | def _run(self): | 406 | def _run(self): |
279 | 395 | os.environ.update({"DISPLAY": self.DISPLAY}) | 407 | os.environ.update({"DISPLAY": self.DISPLAY}) |
Ok I had to learn this the hard way since you said it is just going to be
more...
Anyhow looks fine for me
On Tue, Feb 7, 2012 at 10:12 PM, Siegfried Gevatter <email address hidden>wrote:
> Siegfried Gevatter has proposed merging lp:~rainct/zeitgeist/922620 into gnals and testDataSourceS ignals" /bugs.launchpad .net/zeitgeist/ +bug/922620 /code.launchpad .net/~rainct/ zeitgeist/ 922620/ +merge/ 91920 /code.launchpad .net/~rainct/ zeitgeist/ 922620/ +merge/ 91920 getLogger( "zeitgeist. client" ) object) : callbacks = None callbacks = None introspection_ xml): _when_needed: ().get_ object( iface.requested _bus_name, ).get_object( iface.requested _bus_name, name_owner_ changes= True) self.__ proxy, interface_ name) introspection_ data() callbacks. add((signal, callback)) proxy.connect_ to_signal( proxy.connect_ to_signal( self.__ interface_ name, t_callbacks = set() _callbacks = set() callbacks = set() changed( connection_ name):
> lp:zeitgeist.
>
> Requested reviews:
> Zeitgeist Framework Team (zeitgeist)
> Related bugs:
> Bug #922620 in Zeitgeist Framework: "Fix failing test cases:
> testBlacklistSi
> https:/
>
> For more details, see:
> https:/
> --
> https:/
> Your team Zeitgeist Framework Team is requested to review the proposed
> merge of lp:~rainct/zeitgeist/922620 into lp:zeitgeist.
>
> === modified file 'python/client.py'
> --- python/client.py 2011-10-29 13:31:12 +0000
> +++ python/client.py 2012-02-07 21:11:18 +0000
> @@ -40,6 +40,15 @@
>
> log = logging.
>
> +# This is here so testutils.py can override it with a private bus
> connection
> +global session_bus
> +session_bus = dbus.SessionBus()
> +def get_bus():
> + return session_bus
> +def _set_bus(bus):
> + global session_bus
> + session_bus = bus
> +
> class _DBusInterface(
> """Wrapper around dbus.Interface adding convenience methods."""
>
> @@ -47,7 +56,6 @@
> # that here because otherwise all instances would share their state.
> _disconnect_
> _reconnect_
> - _generic_callbacks = None
>
> @staticmethod
> def get_members(
> @@ -69,8 +77,9 @@
> def reconnect(self):
> if not self._reconnect
> return
> - self.__proxy = dbus.SessionBus
> - self.__
> self.__object_path)
> + self.__proxy = get_bus(
> + self.__
> self.__object_path,
> + follow_
> self.__iface = dbus.Interface(
> self.__
> self._load_
>
> @@ -131,8 +140,7 @@
> self.reconnect()
> if signal not in self.__signals:
> raise TypeError("Unknown signal name: %s" % signal)
> - self._generic_
> - self.__
> + return self.__
> signal,
> callback,
> dbus_interface=
> @@ -169,7 +177,6 @@
>
> self._disconnec
> self._reconnect
> - self._generic_
>
> # Listen to (dis)connection notifications, for connect_exit
> and connect_join
> def name_owner_
> @@ -181,15 +188,9 @@
> retu...