Merge lp:~diegosarmentero/ubuntuone-client/darwin4-fsevents into lp:ubuntuone-client
- darwin4-fsevents
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Alejandro J. Cura | ||||
Approved revision: | 1299 | ||||
Merged at revision: | 1274 | ||||
Proposed branch: | lp:~diegosarmentero/ubuntuone-client/darwin4-fsevents | ||||
Merge into: | lp:ubuntuone-client | ||||
Prerequisite: | lp:~mandel/ubuntuone-client/clean-fsevents | ||||
Diff against target: |
783 lines (+647/-9) 4 files modified
tests/platform/filesystem_notifications/test_darwin.py (+637/-0) tests/platform/filesystem_notifications/test_filesystem_notifications.py (+5/-0) tests/platform/filesystem_notifications/test_windows.py (+2/-2) ubuntuone/platform/filesystem_notifications/darwin.py (+3/-7) |
||||
To merge this branch: | bzr merge lp:~diegosarmentero/ubuntuone-client/darwin4-fsevents | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Alejandro J. Cura (community) | Approve | ||
Manuel de la Peña (community) | Approve | ||
Review via email: mp+114405@code.launchpad.net |
This proposal supersedes a proposal from 2012-06-25.
Commit message
- Adding filesystem notifications feature for darwin (LP: #1013323)
Description of the change
Now you can execute:
./run-mac-tests tests/platform/
Alejandro J. Cura (alecu) wrote : Posted in a previous version of this proposal | # |
Manuel de la Peña (mandel) wrote : Posted in a previous version of this proposal | # |
The diff is misleading because it should depend on lp:~mandel/ubuntuone-client/clean-fsevents. Diego can you do a resummit with the correct dependency so that we are not scared regading the size :)
Diego Sarmentero (diegosarmentero) wrote : | # |
Answering alecu:
* Why is test_file_
This is marked for fix in
There is a bug for this, in which i'm working on:
https:/
The rest of the things has been fixed.
Manuel de la Peña (mandel) wrote : | # |
Looks like there is a thread somewhere that is not killed in the tests. I'm running the tests via u1trial and once they are done the process does not stop.
- 1296. By Diego Sarmentero
-
removing time.sleep
Diego Sarmentero (diegosarmentero) wrote : | # |
> Looks like there is a thread somewhere that is not killed in the tests. I'm
> running the tests via u1trial and once they are done the process does not
> stop.
Fixed!
Alejandro J. Cura (alecu) wrote : | # |
The tearDown in TestWatchManager should not be calling self.patch.
In the same tearDown, the deferred returned by self.manager.stop() should be yielded on.
---
typos:
"""Test that we can get a Watch using is wd.""" ->
"""Test that we can get a Watch using its wd."""
---
This line should not be part of the tests; it should be part of the stopping of the watcher thread:
self.manager.
That part of the code (stopping the watcher thread) is particularly dirty: I just found that there's a __del__ in WatchManager that we missed on the review of the previous branch, and that should not be there, since the use of __del__ is discouraged in python, and much more so when it's being used to work around a thread not being correctly finished.
I think that the stop method should be returning a deferred that's fired when the join is done; but please, let's discuss together a better solution to this on IRC or Mumble.
---
Alejandro J. Cura (alecu) : | # |
- 1297. By Diego Sarmentero
-
adding join on stop
- 1298. By Diego Sarmentero
-
moving patch to setUp
- 1299. By Diego Sarmentero
-
reverting
Manuel de la Peña (mandel) wrote : | # |
Looks goo to me knowing that we will merge this tests later.
Alejandro J. Cura (alecu) wrote : | # |
I still would like better handling of the watches being stopped, so for that I'm creating a different bug: #1024102
Otherwise the code here looks ok.
Preview Diff
1 | === modified file 'tests/platform/filesystem_notifications/test_darwin.py' |
2 | --- tests/platform/filesystem_notifications/test_darwin.py 2012-07-10 18:43:56 +0000 |
3 | +++ tests/platform/filesystem_notifications/test_darwin.py 2012-07-12 13:53:19 +0000 |
4 | @@ -32,7 +32,9 @@ |
5 | import os |
6 | import tempfile |
7 | import thread |
8 | +import itertools |
9 | |
10 | +import fsevents |
11 | from twisted.internet import defer |
12 | |
13 | from contrib.testing.testcase import BaseTwistedTestCase |
14 | @@ -41,15 +43,20 @@ |
15 | darwin as filesystem_notifications, |
16 | ) |
17 | from ubuntuone.platform.filesystem_notifications.common import ( |
18 | + NotifyProcessor, |
19 | Watch, |
20 | WatchManager, |
21 | ) |
22 | from ubuntuone.platform.filesystem_notifications.pyinotify_agnostic import ( |
23 | EventsCodes, |
24 | ProcessEvent, |
25 | + IN_CLOSE_WRITE, |
26 | IN_CREATE, |
27 | IN_DELETE, |
28 | + IN_OPEN, |
29 | ) |
30 | +from tests.platform.filesystem_notifications import BaseFSMonitorTestCase |
31 | + |
32 | |
33 | # A reverse mapping for the tests |
34 | REVERSE_MACOS_ACTIONS = {} |
35 | @@ -135,6 +142,7 @@ |
36 | yield super(TestWatch, self).setUp() |
37 | self.basedir = self.mktemp('test_root') |
38 | self.mask = None |
39 | + self.stream = None |
40 | self.memento = MementoHandler() |
41 | self.memento.setLevel(logging.DEBUG) |
42 | self.raw_events = [] |
43 | @@ -747,3 +755,632 @@ |
44 | watch._subdirs.add(path) |
45 | watch._update_subdirs(path, REVERSE_MACOS_ACTIONS[IN_DELETE]) |
46 | self.assertTrue(path not in watch._subdirs) |
47 | + |
48 | + |
49 | +class TestWatchManager(BaseTwistedTestCase): |
50 | + """Test the watch manager.""" |
51 | + |
52 | + @defer.inlineCallbacks |
53 | + def setUp(self): |
54 | + """Set each of the tests.""" |
55 | + yield super(TestWatchManager, self).setUp() |
56 | + self.parent_path = '/Users/username/' |
57 | + self.path = self.parent_path + 'path' |
58 | + self.watch = Watch(1, self.path, None) |
59 | + self.manager = WatchManager(None) |
60 | + self.manager._wdm = {1: self.watch} |
61 | + |
62 | + @defer.inlineCallbacks |
63 | + def test_stop(self): |
64 | + """Test that the different watches are stopped.""" |
65 | + self.was_called = False |
66 | + |
67 | + def fake_stop_watching(watch): |
68 | + """Fake stop watch.""" |
69 | + self.was_called = True |
70 | + return defer.succeed(True) |
71 | + |
72 | + self.patch(Watch, "stop_watching", fake_stop_watching) |
73 | + self.patch(self.manager.manager.observer, "unschedule", lambda x: None) |
74 | + yield self.manager.stop() |
75 | + self.assertTrue(self.was_called, 'The watch stop should be called.') |
76 | + |
77 | + def test_stop_multiple(self): |
78 | + """The watches should became watching=False and the observer stopped.""" |
79 | + self.patch(self.manager.manager.observer, "unschedule", lambda x: None) |
80 | + second_path = self.parent_path + "second_path" |
81 | + second_watch = Watch(2, second_path, None) |
82 | + second_watch._watching = True |
83 | + self.manager._wdm[2] = second_watch |
84 | + self.manager.stop() |
85 | + self.assertFalse(second_watch.platform_watch.watching) |
86 | + self.assertEqual(second_watch._subdirs, set()) |
87 | + # Give time to the thread to be finished. |
88 | + self.manager.manager.observer.join() |
89 | + self.assertFalse(self.manager.manager.observer.is_alive()) |
90 | + |
91 | + def test_get_present_watch(self): |
92 | + """Test that we can get a Watch using is wd.""" |
93 | + self.assertEqual(self.watch, self.manager.get_watch(1)) |
94 | + |
95 | + def test_get_missing_watch(self): |
96 | + """Test that we get an error when trying to get a missing wd.""" |
97 | + self.assertRaises(KeyError, self.manager.get_watch, (1,)) |
98 | + |
99 | + @defer.inlineCallbacks |
100 | + def test_delete_present_watch(self): |
101 | + """Test that we can remove a present watch.""" |
102 | + self.was_called = False |
103 | + |
104 | + def stop_watching(): |
105 | + """Fake stop watch.""" |
106 | + self.was_called = True |
107 | + return defer.succeed(True) |
108 | + |
109 | + def fake_unschedule(s): |
110 | + """Fake function that should receive a Stream object.""" |
111 | + self.stream = s |
112 | + |
113 | + self.patch(self.manager.manager.observer, "unschedule", |
114 | + fake_unschedule) |
115 | + |
116 | + self.watch.stop_watching = stop_watching |
117 | + yield self.manager.del_watch(1) |
118 | + self.assertIsInstance(self.stream, fsevents.Stream) |
119 | + self.assertRaises(KeyError, self.manager.get_watch, (1,)) |
120 | + |
121 | + def test_add_single_watch(self): |
122 | + """Test the addition of a new single watch.""" |
123 | + self.was_called = False |
124 | + |
125 | + def fake_start_watching(*args): |
126 | + """Fake start watch.""" |
127 | + self.was_called = True |
128 | + |
129 | + self.patch(Watch, "start_watching", fake_start_watching) |
130 | + self.manager._wdm = {} |
131 | + |
132 | + mask = 'bit_mask' |
133 | + self.manager.add_watch(self.path, mask) |
134 | + self.assertEqual(1, len(self.manager._wdm)) |
135 | + self.assertTrue(self.was_called, 'The watch start was not called.') |
136 | + self.assertEqual(self.path + os.path.sep, self.manager._wdm[0].path) |
137 | + |
138 | + def test_get_watch_present_wd(self): |
139 | + """Test that the correct path is returned.""" |
140 | + self.assertEqual(self.path + os.path.sep, self.manager.get_path(1)) |
141 | + |
142 | + def test_get_watch_missing_wd(self): |
143 | + """Test that the correct path is returned.""" |
144 | + self.manager._wdm = {} |
145 | + self.assertEqual(None, self.manager.get_path(1)) |
146 | + |
147 | + def test_get_wd_exact_path(self): |
148 | + """Test the wd is returned when there is a watch for the path.""" |
149 | + self.assertEqual(1, self.manager.get_wd(self.path)) |
150 | + |
151 | + def test_get_wd_child_path(self): |
152 | + """Test the wd is returned when we have a child path.""" |
153 | + child = os.path.join(self.path, 'child') |
154 | + self.assertEqual(1, self.manager.get_wd(child)) |
155 | + |
156 | + def test_get_wd_unwatched(self): |
157 | + """A watch on an unwatched path returns None.""" |
158 | + self.assertEqual(None, self.manager.get_wd(self.parent_path)) |
159 | + |
160 | + def test_rm_present_wd(self): |
161 | + """Test the removal of a present watch.""" |
162 | + self.patch(self.watch, "stop_watching", lambda: None) |
163 | + self.patch(self.manager.manager.observer, "unschedule", lambda x: None) |
164 | + self.manager.rm_watch(1) |
165 | + self.assertEqual(None, self.manager._wdm.get(1)) |
166 | + |
167 | + def test_rm_root_path(self): |
168 | + """Test the removal of a root path.""" |
169 | + events = [] |
170 | + |
171 | + def fake_processor(event): |
172 | + """Memorize the processed events.""" |
173 | + events.append(event.pathname) |
174 | + |
175 | + self.watch._processor = fake_processor |
176 | + self.manager.rm_path(self.path) |
177 | + self.assertEqual(self.watch, self.manager._wdm.get(1)) |
178 | + self.watch._watching = True |
179 | + event = FakeFileEvent(256, None, os.path.join(self.path, 'test')) |
180 | + self.watch.platform_watch._process_events(event) |
181 | + self.assertEqual(0, len(events)) |
182 | + |
183 | + def test_rm_child_path(self): |
184 | + """Test the removal of a child path.""" |
185 | + events = [] |
186 | + |
187 | + def fake_processor(event): |
188 | + """Memorize the processed events.""" |
189 | + events.append(event.pathname) |
190 | + |
191 | + self.patch(filesystem_notifications.reactor, 'callFromThread', |
192 | + lambda x, e: x(e)) |
193 | + |
194 | + self.watch._processor = fake_processor |
195 | + child = os.path.join(self.path, 'child') |
196 | + self.manager.rm_path(child) |
197 | + self.assertEqual(self.watch, self.manager._wdm[1]) |
198 | + # assert that the correct event is ignored |
199 | + self.watch.platform_watch.watching = True |
200 | + event = FakeFileEvent(256, None, os.path.join('child', 'test')) |
201 | + self.watch.platform_watch._process_events(event) |
202 | + self.assertEqual(0, len(events)) |
203 | + # assert that other events are not ignored |
204 | + event2 = FakeFileEvent(256, None, 'test') |
205 | + self.watch.platform_watch._process_events(event2) |
206 | + self.assertEqual(1, len(events)) |
207 | + |
208 | + |
209 | +class TestWatchManagerAddWatches(BaseTwistedTestCase): |
210 | + """Test the watch manager.""" |
211 | + timeout = 5 |
212 | + |
213 | + def test_add_watch_twice(self): |
214 | + """Adding a watch twice succeeds when the watch is running.""" |
215 | + self.patch(Watch, "start_watching", lambda self: None) |
216 | + manager = WatchManager(None) |
217 | + # no need to stop watching because start_watching is fake |
218 | + |
219 | + path = '/Users/username/path' |
220 | + mask = 'fake bit mask' |
221 | + d1 = manager.add_watch(path, mask) |
222 | + d2 = manager.add_watch(path, mask) |
223 | + |
224 | + self.assertTrue(d1.result, "Should not be called yet.") |
225 | + self.assertFalse(d2.result, "Should not be called yet.") |
226 | + |
227 | + |
228 | +class FakeEvent(object): |
229 | + """Fake event.""" |
230 | + |
231 | + def __init__(self, wd=0, dir=True, name=None, path=None, pathname=None, |
232 | + cookie=None): |
233 | + """Create fake event.""" |
234 | + self.dir = dir |
235 | + self.wd = wd |
236 | + self.name = name |
237 | + self.path = path |
238 | + self.pathname = pathname |
239 | + self.cookie = cookie |
240 | + |
241 | + |
242 | +class FakeLog(object): |
243 | + """A fake log that is used by the general processor.""" |
244 | + |
245 | + def __init__(self): |
246 | + """Create the fake.""" |
247 | + self.called_methods = [] |
248 | + |
249 | + def info(self, *args): |
250 | + """Fake the info call.""" |
251 | + self.called_methods.append(('info', args)) |
252 | + |
253 | + |
254 | +class FakeGeneralProcessor(object): |
255 | + """Fake implementation of the general processor.""" |
256 | + |
257 | + def __init__(self): |
258 | + """Create the fake.""" |
259 | + self.called_methods = [] |
260 | + self.paths_to_return = [] |
261 | + self.log = FakeLog() |
262 | + self.share_id = None |
263 | + self.ignore = False |
264 | + |
265 | + def rm_from_mute_filter(self, event, paths): |
266 | + """Fake rm_from_mute_filter.""" |
267 | + self.called_methods.append(('rm_from_mute_filter', event, paths)) |
268 | + |
269 | + def add_to_mute_filter(self, event, paths): |
270 | + """Fake add_to_move_filter.""" |
271 | + self.called_methods.append(('add_to_mute_filter', event, paths)) |
272 | + |
273 | + def is_ignored(self, path): |
274 | + """Fake is_ignored.""" |
275 | + self.called_methods.append(('is_ignored', path)) |
276 | + return self.ignore |
277 | + |
278 | + def push_event(self, event): |
279 | + """Fake push event.""" |
280 | + self.called_methods.append(('push_event', event)) |
281 | + |
282 | + def eq_push(self, event, path=None, path_to=None, path_from=None): |
283 | + """Fake event to push event.""" |
284 | + self.called_methods.append(('eq_push', event, path, path_to, |
285 | + path_from)) |
286 | + |
287 | + def get_paths_starting_with(self, fullpath, include_base=False): |
288 | + """Fake get_paths_starting_with.""" |
289 | + self.called_methods.append(('get_paths_starting_with', fullpath, |
290 | + include_base)) |
291 | + return self.paths_to_return |
292 | + |
293 | + def get_path_share_id(self, path): |
294 | + """Fake get_path_share_id.""" |
295 | + self.called_methods.append(('get_path_share_id', path)) |
296 | + return self.share_id |
297 | + |
298 | + def rm_watch(self, path): |
299 | + """Fake the remove watch.""" |
300 | + self.called_methods.append(('rm_watch', path)) |
301 | + |
302 | + def freeze_begin(self, path): |
303 | + """Fake freeze_begin""" |
304 | + self.called_methods.append(('freeze_begin', path)) |
305 | + |
306 | + def freeze_rollback(self): |
307 | + """Fake rollback.""" |
308 | + self.called_methods.append(('freeze_rollback',)) |
309 | + |
310 | + def freeze_commit(self, path): |
311 | + """Fake freeze commit.""" |
312 | + self.called_methods.append(('freeze_commit', path)) |
313 | + |
314 | + |
315 | +class TestNotifyProcessor(BaseTwistedTestCase): |
316 | + """Test the notify processor.""" |
317 | + |
318 | + @defer.inlineCallbacks |
319 | + def setUp(self): |
320 | + """set up the diffeent tests.""" |
321 | + yield super(TestNotifyProcessor, self).setUp() |
322 | + self.processor = NotifyProcessor(None) |
323 | + self.general = FakeGeneralProcessor() |
324 | + self.processor.general_processor = self.general |
325 | + |
326 | + def test_rm_from_mute_filter(self): |
327 | + """Test that we remove the event from the mute filter.""" |
328 | + event = 'event' |
329 | + paths = 'paths' |
330 | + self.processor.rm_from_mute_filter(event, paths) |
331 | + self.assertEqual(1, len(self.general.called_methods)) |
332 | + self.assertEqual('rm_from_mute_filter', |
333 | + self.general.called_methods[0][0]) |
334 | + self.assertEqual(event, self.general.called_methods[0][1]) |
335 | + self.assertEqual(paths, self.general.called_methods[0][2]) |
336 | + |
337 | + def test_add_to_mute_filter(self): |
338 | + """Test that we add the path to the mute filter.""" |
339 | + event = 'event' |
340 | + paths = 'paths' |
341 | + self.processor.add_to_mute_filter(event, paths) |
342 | + self.assertEqual(1, len(self.general.called_methods)) |
343 | + self.assertEqual('add_to_mute_filter', |
344 | + self.general.called_methods[0][0]) |
345 | + self.assertEqual(event, self.general.called_methods[0][1]) |
346 | + self.assertEqual(paths, self.general.called_methods[0][2]) |
347 | + |
348 | + def test_is_ignored(self): |
349 | + """Test that we do ensure that the path is ignored.""" |
350 | + path = 'path' |
351 | + self.processor.is_ignored(path) |
352 | + self.assertEqual(1, len(self.general.called_methods)) |
353 | + self.assertEqual('is_ignored', |
354 | + self.general.called_methods[0][0]) |
355 | + self.assertEqual(path, self.general.called_methods[0][1]) |
356 | + |
357 | + def test_release_held_event(self): |
358 | + """Test that we do release the held event.""" |
359 | + event = 'event' |
360 | + # set the held event to assert that is pushed |
361 | + self.processor.held_event = event |
362 | + self.processor.release_held_event() |
363 | + self.assertEqual('push_event', |
364 | + self.general.called_methods[0][0]) |
365 | + self.assertEqual(event, self.general.called_methods[0][1]) |
366 | + |
367 | + def test_process_IN_MODIFY_dir(self): |
368 | + """Test that the modify works as exepcted with dirs.""" |
369 | + event = FakeEvent(dir=True) |
370 | + self.processor.process_IN_MODIFY(event) |
371 | + # no method should be called |
372 | + self.assertEqual(0, len(self.general.called_methods)) |
373 | + |
374 | + def test_process_IN_MODIFY_file(self): |
375 | + """Test that the modify works as expected with files.""" |
376 | + event = FakeEvent(dir=False, wd=0, name='name', |
377 | + path='path', pathname='pathname') |
378 | + self.processor.process_IN_MODIFY(event) |
379 | + # we should be getting two different method, and open and a close |
380 | + self.assertEqual(2, len(self.general.called_methods)) |
381 | + self.assertEqual('push_event', |
382 | + self.general.called_methods[0][0]) |
383 | + self.assertEqual('push_event', |
384 | + self.general.called_methods[1][0]) |
385 | + self.assertEqual(event.dir, self.general.called_methods[0][1].dir) |
386 | + self.assertEqual(event.wd, self.general.called_methods[0][1].wd) |
387 | + self.assertEqual(event.name, self.general.called_methods[0][1].name) |
388 | + self.assertEqual(event.path, self.general.called_methods[0][1].path) |
389 | + self.assertEqual(event.pathname, |
390 | + self.general.called_methods[0][1].pathname) |
391 | + self.assertEqual(IN_OPEN, |
392 | + self.general.called_methods[0][1].mask) |
393 | + self.assertEqual(event.dir, self.general.called_methods[1][1].dir) |
394 | + self.assertEqual(event.wd, self.general.called_methods[1][1].wd) |
395 | + self.assertEqual(event.name, self.general.called_methods[1][1].name) |
396 | + self.assertEqual(event.path, self.general.called_methods[1][1].path) |
397 | + self.assertEqual(event.pathname, |
398 | + self.general.called_methods[1][1].pathname) |
399 | + self.assertEqual(IN_CLOSE_WRITE, |
400 | + self.general.called_methods[1][1].mask) |
401 | + |
402 | + def test_process_IN_MOVED_FROM(self): |
403 | + """Test that the in moved from works as expected.""" |
404 | + event = FakeEvent(dir=False, wd=0, name='name', |
405 | + path='path', pathname='pathname') |
406 | + self.processor.process_IN_MOVED_FROM(event) |
407 | + self.assertEqual(event, self.processor.held_event) |
408 | + |
409 | + def test_process_IN_MOVED_TO_dir(self): |
410 | + """Test that the in moved to works as expected.""" |
411 | + event = FakeEvent(wd=0, dir=True, name='name', path='path', |
412 | + pathname=os.path.join('test', 'pathname'), |
413 | + cookie='cookie') |
414 | + held_event = FakeEvent(wd=0, dir=True, name='hname', path='hpath', |
415 | + pathname=os.path.join('test', 'hpathname'), |
416 | + cookie='cookie') |
417 | + self.general.share_id = 'my_share_id' |
418 | + self.processor.held_event = held_event |
419 | + self.processor.process_IN_MOVED_TO(event) |
420 | + self.assertEqual(5, len(self.general.called_methods)) |
421 | + # assert that the ignores are called |
422 | + self.assertEqual('is_ignored', self.general.called_methods[0][0]) |
423 | + self.assertEqual(held_event.pathname, |
424 | + self.general.called_methods[0][1]) |
425 | + self.assertEqual('is_ignored', self.general.called_methods[1][0]) |
426 | + self.assertEqual(event.pathname, self.general.called_methods[1][1]) |
427 | + # assert that we do request the share_id |
428 | + self.assertEqual('get_path_share_id', |
429 | + self.general.called_methods[2][0]) |
430 | + self.assertEqual(os.path.split(event.pathname)[0], |
431 | + self.general.called_methods[2][1], |
432 | + 'Get the share_id for event') |
433 | + self.assertEqual('get_path_share_id', |
434 | + self.general.called_methods[3][0]) |
435 | + self.assertEqual(os.path.split(held_event.pathname)[0], |
436 | + self.general.called_methods[3][1], |
437 | + 'Get the share_id for held event.') |
438 | + |
439 | + self.assertEqual('eq_push', self.general.called_methods[4][0]) |
440 | + self.assertEqual('FS_DIR_MOVE', self.general.called_methods[4][1]) |
441 | + self.assertEqual(event.pathname, self.general.called_methods[4][3]) |
442 | + self.assertEqual(held_event.pathname, |
443 | + self.general.called_methods[4][4]) |
444 | + |
445 | + def test_process_IN_MOVED_TO_file(self): |
446 | + """Test that the in moved to works as expected.""" |
447 | + event = FakeEvent(wd=0, dir=False, name='name', path='path', |
448 | + pathname=os.path.join('test', 'pathname'), |
449 | + cookie='cookie') |
450 | + held_event = FakeEvent(wd=0, dir=False, name='hname', path='hpath', |
451 | + pathname=os.path.join('test', 'hpathname'), |
452 | + cookie='cookie') |
453 | + self.general.share_id = 'my_share_id' |
454 | + self.processor.held_event = held_event |
455 | + self.processor.process_IN_MOVED_TO(event) |
456 | + self.assertEqual(5, len(self.general.called_methods)) |
457 | + # assert that the ignores are called |
458 | + self.assertEqual('is_ignored', self.general.called_methods[0][0]) |
459 | + self.assertEqual(held_event.pathname, |
460 | + self.general.called_methods[0][1]) |
461 | + self.assertEqual('is_ignored', self.general.called_methods[1][0]) |
462 | + self.assertEqual(event.pathname, self.general.called_methods[1][1]) |
463 | + # assert that we do request the share_id |
464 | + self.assertEqual('get_path_share_id', |
465 | + self.general.called_methods[2][0]) |
466 | + self.assertEqual(os.path.split(event.pathname)[0], |
467 | + self.general.called_methods[2][1], |
468 | + 'Get the share_id for event') |
469 | + self.assertEqual('get_path_share_id', |
470 | + self.general.called_methods[3][0]) |
471 | + self.assertEqual(os.path.split(held_event.pathname)[0], |
472 | + self.general.called_methods[3][1], |
473 | + 'Get the share_id for held event.') |
474 | + |
475 | + self.assertEqual('eq_push', self.general.called_methods[4][0]) |
476 | + self.assertEqual('FS_FILE_MOVE', self.general.called_methods[4][1]) |
477 | + self.assertEqual(event.pathname, self.general.called_methods[4][3]) |
478 | + self.assertEqual(held_event.pathname, |
479 | + self.general.called_methods[4][4]) |
480 | + |
481 | + def test_fake_create_event_dir(self): |
482 | + """Test that the in moved to works as expected.""" |
483 | + event = FakeEvent(wd=0, dir=True, name='name', path='path', |
484 | + pathname='pathname') |
485 | + self.processor._fake_create_event(event) |
486 | + self.assertEqual(1, len(self.general.called_methods)) |
487 | + self.assertEqual('eq_push', self.general.called_methods[0][0]) |
488 | + self.assertEqual('FS_DIR_CREATE', self.general.called_methods[0][1]) |
489 | + self.assertEqual(event.pathname, self.general.called_methods[0][2]) |
490 | + |
491 | + def test_fake_create_event_file(self): |
492 | + """Test that the in moved to works as expected.""" |
493 | + event = FakeEvent(wd=0, dir=False, name='name', path='path', |
494 | + pathname='pathname') |
495 | + self.processor._fake_create_event(event) |
496 | + self.assertEqual(2, len(self.general.called_methods)) |
497 | + self.assertEqual('eq_push', self.general.called_methods[0][0]) |
498 | + self.assertEqual('FS_FILE_CREATE', self.general.called_methods[0][1]) |
499 | + self.assertEqual(event.pathname, self.general.called_methods[0][2]) |
500 | + self.assertEqual('eq_push', self.general.called_methods[1][0]) |
501 | + self.assertEqual('FS_FILE_CLOSE_WRITE', |
502 | + self.general.called_methods[1][1]) |
503 | + self.assertEqual(event.pathname, self.general.called_methods[1][2]) |
504 | + |
505 | + def test_fake_delete_create_event_dir(self): |
506 | + """Test that we do fake a delete and a later delete.""" |
507 | + event = FakeEvent(wd=0, dir=True, name='name', path='path', |
508 | + pathname='pathname') |
509 | + held_event = FakeEvent(wd=0, dir=True, name='hname', path='hpath', |
510 | + pathname='hpathname') |
511 | + self.processor.held_event = held_event |
512 | + self.processor._fake_delete_create_event(event) |
513 | + self.assertEqual(2, len(self.general.called_methods)) |
514 | + self.assertEqual('eq_push', self.general.called_methods[0][0]) |
515 | + self.assertEqual('FS_DIR_DELETE', self.general.called_methods[0][1]) |
516 | + self.assertEqual(held_event.pathname, |
517 | + self.general.called_methods[0][2]) |
518 | + self.assertEqual('eq_push', self.general.called_methods[1][0]) |
519 | + self.assertEqual('FS_DIR_CREATE', self.general.called_methods[1][1]) |
520 | + self.assertEqual(event.pathname, self.general.called_methods[1][2]) |
521 | + |
522 | + def test_fake_delete_create_event_file(self): |
523 | + """Test that we do fake a delete and a later delete.""" |
524 | + event = FakeEvent(wd=0, dir=False, name='name', path='path', |
525 | + pathname='pathname') |
526 | + held_event = FakeEvent(wd=0, dir=False, name='hname', path='hpath', |
527 | + pathname='hpathname') |
528 | + self.processor.held_event = held_event |
529 | + self.processor._fake_delete_create_event(event) |
530 | + self.assertEqual(3, len(self.general.called_methods)) |
531 | + self.assertEqual('eq_push', self.general.called_methods[0][0]) |
532 | + self.assertEqual('FS_FILE_DELETE', self.general.called_methods[0][1]) |
533 | + self.assertEqual(held_event.pathname, |
534 | + self.general.called_methods[0][2]) |
535 | + self.assertEqual('eq_push', self.general.called_methods[1][0]) |
536 | + self.assertEqual('FS_FILE_CREATE', self.general.called_methods[1][1]) |
537 | + self.assertEqual(event.pathname, self.general.called_methods[1][2]) |
538 | + self.assertEqual('eq_push', self.general.called_methods[2][0]) |
539 | + self.assertEqual('FS_FILE_CLOSE_WRITE', |
540 | + self.general.called_methods[2][1]) |
541 | + self.assertEqual(event.pathname, self.general.called_methods[2][2]) |
542 | + |
543 | + def test_process_default_no_held(self): |
544 | + """Test that the process default works as expected.""" |
545 | + event = 'event' |
546 | + self.processor.process_default(event) |
547 | + self.assertEqual(1, len(self.general.called_methods)) |
548 | + self.assertEqual('push_event', |
549 | + self.general.called_methods[0][0]) |
550 | + self.assertEqual(event, |
551 | + self.general.called_methods[0][1]) |
552 | + |
553 | + def test_process_default_with_held(self): |
554 | + """Test that the process default works as expected.""" |
555 | + event = 'event' |
556 | + held_event = 'held_event' |
557 | + self.processor.held_event = held_event |
558 | + self.processor.process_default(event) |
559 | + self.assertEqual(2, len(self.general.called_methods)) |
560 | + self.assertEqual('push_event', |
561 | + self.general.called_methods[0][0]) |
562 | + self.assertEqual(held_event, |
563 | + self.general.called_methods[0][1]) |
564 | + self.assertEqual('push_event', |
565 | + self.general.called_methods[1][0]) |
566 | + self.assertEqual(event, |
567 | + self.general.called_methods[1][1]) |
568 | + |
569 | + def test_handle_dir_delete_files(self): |
570 | + """Test that the handle dir delete works as expected.""" |
571 | + path = 'path' |
572 | + present_files = 'abcde' |
573 | + # create files and dirs to be returned from the get paths |
574 | + for file_name in present_files: |
575 | + self.general.paths_to_return.append((file_name, False)) |
576 | + self.processor.handle_dir_delete(path) |
577 | + # there are calls for, rm the watch, get paths and then one per file |
578 | + self.assertEqual(len(present_files) + 2, |
579 | + len(self.general.called_methods)) |
580 | + rm_call = self.general.called_methods.pop(0) |
581 | + self.assertEqual('rm_watch', rm_call[0]) |
582 | + self.assertEqual(path, rm_call[1]) |
583 | + paths_call = self.general.called_methods.pop(0) |
584 | + self.assertEqual('get_paths_starting_with', paths_call[0]) |
585 | + self.assertEqual(path, paths_call[1]) |
586 | + self.assertFalse(paths_call[2]) |
587 | + # we need to push the delete events in reverse order because we want |
588 | + # to delete children before we delete parents |
589 | + present_files = present_files[::-1] |
590 | + for i, called_method in enumerate(self.general.called_methods): |
591 | + self.assertEqual('eq_push', called_method[0]) |
592 | + self.assertEqual('FS_FILE_DELETE', called_method[1]) |
593 | + self.assertEqual(present_files[i], called_method[2]) |
594 | + |
595 | + def test_handle_dir_delete_dirs(self): |
596 | + """Test that the handle dir delete works as expected.""" |
597 | + path = 'path' |
598 | + present_files = 'abcde' |
599 | + # create files and dirs to be returned from the get paths |
600 | + for file_name in present_files: |
601 | + self.general.paths_to_return.append((file_name, True)) |
602 | + self.processor.handle_dir_delete(path) |
603 | + # there are calls for, rm the watch, get paths and then one per file |
604 | + self.assertEqual(2 * len(present_files) + 2, |
605 | + len(self.general.called_methods)) |
606 | + rm_call = self.general.called_methods.pop(0) |
607 | + self.assertEqual('rm_watch', rm_call[0]) |
608 | + self.assertEqual(path, rm_call[1]) |
609 | + paths_call = self.general.called_methods.pop(0) |
610 | + self.assertEqual('get_paths_starting_with', paths_call[0]) |
611 | + self.assertEqual(path, paths_call[1]) |
612 | + self.assertFalse(paths_call[2]) |
613 | + # we need to push the delete events in reverse order because we want |
614 | + # to delete children before we delete parents |
615 | + present_files = present_files[::-1] |
616 | + |
617 | + # from http://docs.python.org/library/itertools.html#recipes |
618 | + def grouper(n, iterable, fillvalue=None): |
619 | + "grouper(3, 'ABCDEFG', 'x') --> ABC DEF Gxx" |
620 | + args = [iter(iterable)] * n |
621 | + return itertools.izip_longest(fillvalue=fillvalue, *args) |
622 | + |
623 | + for i, called_methods in enumerate(grouper(2, |
624 | + self.general.called_methods)): |
625 | + rm_call = called_methods[0] |
626 | + self.assertEqual('rm_watch', rm_call[0]) |
627 | + self.assertEqual(present_files[i], rm_call[1]) |
628 | + push_call = called_methods[1] |
629 | + self.assertEqual('eq_push', push_call[0]) |
630 | + self.assertEqual('FS_DIR_DELETE', push_call[1]) |
631 | + self.assertEqual(present_files[i], push_call[2]) |
632 | + |
633 | + def test_freeze_begin(self): |
634 | + """Test that the freeze being works as expected.""" |
635 | + path = 'path' |
636 | + self.processor.freeze_begin(path) |
637 | + self.assertEqual(1, len(self.general.called_methods)) |
638 | + self.assertEqual('freeze_begin', |
639 | + self.general.called_methods[0][0]) |
640 | + self.assertEqual(path, self.general.called_methods[0][1]) |
641 | + |
642 | + def test_freeze_rollback(self): |
643 | + """Test that the freeze rollback works as expected.""" |
644 | + self.processor.freeze_rollback() |
645 | + self.assertEqual(1, len(self.general.called_methods)) |
646 | + self.assertEqual('freeze_rollback', |
647 | + self.general.called_methods[0][0]) |
648 | + |
649 | + def test_freeze_commit(self): |
650 | + """Test that the freeze commit works as expected.""" |
651 | + path = 'path' |
652 | + self.processor.freeze_commit(path) |
653 | + self.assertEqual(1, len(self.general.called_methods)) |
654 | + self.assertEqual('freeze_commit', |
655 | + self.general.called_methods[0][0]) |
656 | + self.assertEqual(path, self.general.called_methods[0][1]) |
657 | + |
658 | + |
659 | +class FilesystemMonitorTestCase(BaseFSMonitorTestCase): |
660 | + """Tests for the FilesystemMonitor.""" |
661 | + |
662 | + def test_add_watch_twice(self): |
663 | + """Check the deferred returned by a second add_watch.""" |
664 | + self.patch(Watch, "start_watching", lambda self: None) |
665 | + self.patch(Watch, "started", lambda self: True) |
666 | + manager = WatchManager(None) |
667 | + # no need to stop watching because start_watching is fake |
668 | + |
669 | + path = '/Users/username/path' |
670 | + mask = 'fake bit mask' |
671 | + d1 = manager.add_watch(path, mask) |
672 | + d2 = manager.add_watch(path, mask) |
673 | + |
674 | + self.assertTrue(d1.result, "Should not be called yet.") |
675 | + self.assertTrue(d2, "Should not be called yet.") |
676 | |
677 | === modified file 'tests/platform/filesystem_notifications/test_filesystem_notifications.py' |
678 | --- tests/platform/filesystem_notifications/test_filesystem_notifications.py 2012-06-28 10:26:50 +0000 |
679 | +++ tests/platform/filesystem_notifications/test_filesystem_notifications.py 2012-07-12 13:53:19 +0000 |
680 | @@ -39,6 +39,7 @@ |
681 | BaseTwistedTestCase, |
682 | FakeVolumeManager, |
683 | skip_if_win32_missing_fs_event, |
684 | + skip_if_darwin_missing_fs_event, |
685 | ) |
686 | from ubuntuone.platform import ( |
687 | remove_file, |
688 | @@ -264,6 +265,7 @@ |
689 | mute_filter = self.monitor._processor.mute_filter |
690 | return sum(len(x) for x in mute_filter._cnt.values()) |
691 | |
692 | + @skip_if_darwin_missing_fs_event |
693 | @skip_if_win32_missing_fs_event |
694 | @defer.inlineCallbacks |
695 | def test_file_open(self): |
696 | @@ -281,6 +283,7 @@ |
697 | test_result = yield self._deferred |
698 | defer.returnValue(test_result) |
699 | |
700 | + @skip_if_darwin_missing_fs_event |
701 | @skip_if_win32_missing_fs_event |
702 | @defer.inlineCallbacks |
703 | def test_file_close_nowrite(self): |
704 | @@ -298,6 +301,7 @@ |
705 | test_result = yield self._deferred |
706 | defer.returnValue(test_result) |
707 | |
708 | + @skip_if_darwin_missing_fs_event |
709 | @defer.inlineCallbacks |
710 | def test_file_create_close_write(self): |
711 | """Test receiving the create and close_write signals on files.""" |
712 | @@ -423,6 +427,7 @@ |
713 | test_result = yield self._deferred |
714 | defer.returnValue(test_result) |
715 | |
716 | + @skip_if_darwin_missing_fs_event |
717 | @defer.inlineCallbacks |
718 | def test_file_moved_from_partial(self): |
719 | """Test the handling of the FILE_MOVE event when source is partial.""" |
720 | |
721 | === modified file 'tests/platform/filesystem_notifications/test_windows.py' |
722 | --- tests/platform/filesystem_notifications/test_windows.py 2012-07-10 20:24:04 +0000 |
723 | +++ tests/platform/filesystem_notifications/test_windows.py 2012-07-12 13:53:19 +0000 |
724 | @@ -1321,7 +1321,7 @@ |
725 | for file_name in present_files: |
726 | self.general.paths_to_return.append((file_name, False)) |
727 | self.processor.handle_dir_delete(path) |
728 | - # there are calls for, rm the wathc, get paths and then one per file |
729 | + # there are calls for, rm the watch, get paths and then one per file |
730 | self.assertEqual(len(present_files) + 2, |
731 | len(self.general.called_methods)) |
732 | rm_call = self.general.called_methods.pop(0) |
733 | @@ -1347,7 +1347,7 @@ |
734 | for file_name in present_files: |
735 | self.general.paths_to_return.append((file_name, True)) |
736 | self.processor.handle_dir_delete(path) |
737 | - # there are calls for, rm the wathc, get paths and then one per file |
738 | + # there are calls for, rm the watch, get paths and then one per file |
739 | self.assertEqual(2 * len(present_files) + 2, |
740 | len(self.general.called_methods)) |
741 | rm_call = self.general.called_methods.pop(0) |
742 | |
743 | === modified file 'ubuntuone/platform/filesystem_notifications/darwin.py' |
744 | --- ubuntuone/platform/filesystem_notifications/darwin.py 2012-07-11 09:48:03 +0000 |
745 | +++ ubuntuone/platform/filesystem_notifications/darwin.py 2012-07-12 13:53:19 +0000 |
746 | @@ -61,7 +61,7 @@ |
747 | } |
748 | |
749 | |
750 | -def path_is_ignored(self, path): |
751 | +def path_is_ignored(path): |
752 | """Should we ignore this path in the current platform.?""" |
753 | # don't support links yet |
754 | if os.path.islink(path): |
755 | @@ -135,10 +135,6 @@ |
756 | self.observer = fsevents.Observer() |
757 | self.observer.start() |
758 | |
759 | - def __del__(self): |
760 | - """Stop the observer.""" |
761 | - self.observer.stop() |
762 | - |
763 | def stop_watch(self, watch): |
764 | """Stop a given watch.""" |
765 | watch.stop_watching() |
766 | @@ -148,10 +144,11 @@ |
767 | def stop(self): |
768 | """Stop the manager.""" |
769 | self.observer.stop() |
770 | + self.observer.join() |
771 | |
772 | def del_watch(self, watch): |
773 | """Delete the watch and clean resources.""" |
774 | - self.observer.unschedule(watch.stream) |
775 | + self.observer.unschedule(watch.platform_watch.stream) |
776 | |
777 | def add_watch(self, watch): |
778 | """This method perform actually the action of registering the watch.""" |
779 | @@ -160,4 +157,3 @@ |
780 | |
781 | def rm_watch(self, watch): |
782 | """Remove the the watch with the given wd.""" |
783 | - self.observer.unschedule(watch.platform_watch.stream) |
The branch looks mostly fine.
A few questions, though:
* Why is test_file_ moved_from_ partial skipped? I think this is a critical operation that should be tested.
* Why is there a test that still uses time.sleep? As mentioned in previous branches we should not be using sleep at all in tests.
And some fixes needed, too:
It looks wrong to import fsevents in /platform/ filesystem_ notifications/ __init_ _.py, so please, move each platform specific definition of EVENT_CODES to ubuntuone. platform. filesystem_ notifications. windows and .darwin
---
Lines 827 and 828 are duplicated.
---
Please move the logic in these few lines to a function, add some unit tests for it, and use it from both places:
837 + index = 0 endswith( os.path. sep): self._path) + index:]
838 + if not self._path.
839 + index = 1
840 + path = path[len(