Merge lp:~mikemc/ubuntuone-client/clean-up-fsmonitor-in-tests into lp:ubuntuone-client

[Mike McCracken]

- Clean up usage of Filesystem Monitor in test suite.

- Clean up usage of Filesystem Monitor in test suite.

All:
- Add fake FilesystemMonitor implementation to avoid unnecessary
  threads or IPC connection overhead for tests that don't inspect
  events.

test_eq_inotify specifically:
- Skip some tests which assume linux-specific inotify events
- Skip tests for UDF ancestors on non-linux, clarify skip message.
- Skip tests for non-utf8 names, as those are ignored in the fsevent daemon on darwin.

TO TEST: full test suite still not totally working on darwin, run
trial on the specific test files that are changed, eg:

% u1trial --reactor=twisted tests/syncdaemon/test_eq_inotify.py tests/syncdaemon/test_sync.py tests/syncdaemon/test_vm.py tests/syncdaemon/test_eventqueue.py

NOTE: several tests do still require the fsevents daemon (on darwin) to be running. Otherwise they will fail by timing out when connecting to it. This is a minor annoyance, and I'll file a bug about fixing it via e.g. skipIf, like we do with Squid.

[dobey (dobey)]

review: Approve

=== modified file 'contrib/testing/testcase.py'
--- contrib/testing/testcase.py	2012-10-22 17:18:47 +0000
+++ contrib/testing/testcase.py	2013-01-10 22:19:25 +0000
@@ -217,12 +217,52 @@
         return {RECENT_TRANSFERS: [], UPLOADING: []}
 
 
+class FakeMonitor(object):
+    """A fake FilesystemMonitor."""
+
+    def __init__(self, eq, fs, ignore_config=None, timeout=1):
+        """Do nothing."""
+
+    def freeze_begin(self, path):
+        """Succeed quietly."""
+
+    def freeze_rollback(self):
+        """Succeed quietly."""
+
+    def freeze_commit(self, events):
+        """Never report dirty commits for fake events."""
+        return defer.succeed(False)
+
+    def add_to_mute_filter(self, event, **info):
+        """Do nothing."""
+
+    def rm_from_mute_filter(self, event, **info):
+        """Do nothing."""
+
+    def add_watches_to_udf_ancestors(self, volume):
+        """Just report success."""
+        return defer.succeed(True)
+
+    def add_watch(self, dirpath):
+        """Just report success."""
+        defer.succeed(True)
+
+    def rm_watch(self, dirpath):
+        """Just report success."""
+        return defer.succeed(True)
+
+    def shutdown(self):
+        """Just report success."""
+        return defer.succeed(True)
+
+
 class FakeMain(main.Main):
     """ A fake Main class to setup the tests """
 
     _fake_AQ_class = FakeActionQueue
     _fake_AQ_params = ()
     _sync_class = None
+    _monitor_class = FakeMonitor
 
     # don't call Main.__init__ we take care of creating a fake main and
     # all its attributes. pylint: disable=W0231
@@ -238,7 +278,8 @@
         self.vm = volume_manager.VolumeManager(self)
         self.fs = fs_manager.FileSystemManager(
             self.data_dir, self.partials_dir, self.vm, self.db)
-        self.event_q = event_queue.EventQueue(self.fs)
+        self.event_q = event_queue.EventQueue(self.fs,
+                                              monitor_class=self._monitor_class)
         self.fs.register_eq(self.event_q)
         self.action_q = self._fake_AQ_class(self.event_q, self,
                                             *self._fake_AQ_params)

=== modified file 'tests/syncdaemon/test_eq_inotify.py'
--- tests/syncdaemon/test_eq_inotify.py	2012-04-09 20:07:05 +0000
+++ tests/syncdaemon/test_eq_inotify.py	2013-01-10 22:19:25 +0000
@@ -34,15 +34,17 @@
 import functools
 import logging
 import os
+import sys
 
 from twisted.internet import defer, reactor
 from ubuntuone.devtools.handlers import MementoHandler
-from ubuntuone.devtools.testcases import skipIfOS
+from ubuntuone.devtools.testcases import skipIfOS, skipIfNotOS
 
 from contrib.testing.testcase import (
     BaseTwistedTestCase,
     FakeMain,
     Listener,
+    skip_if_darwin_missing_fs_event,
     skip_if_win32_missing_fs_event,
 )
 from tests.syncdaemon.test_eventqueue import BaseEQTestCase
@@ -182,6 +184,9 @@
     # this timeout must be bigger than the one used in event_queue
     timeout = 2
 
+    # use the default FSMonitor
+    _monitor_class = None
+
     @defer.inlineCallbacks
     def setUp(self):
         """Setup the test."""
@@ -330,6 +335,7 @@
         yield self._deferred
 
     @skip_if_win32_missing_fs_event
+    @skip_if_darwin_missing_fs_event
     @defer.inlineCallbacks
     def test_commit_middle_events(self):
         """Commit behaviour when something happened in the middle."""
@@ -396,6 +402,7 @@
         yield self._deferred
 
     @skip_if_win32_missing_fs_event
+    @skip_if_darwin_missing_fs_event
     @defer.inlineCallbacks
     def test_selective(self):
         """Check that it's frozen only for a path."""
@@ -633,6 +640,8 @@
         make_dir(root_dir)
         open_file(fromfile, "w").close()
         self.eq.add_to_mute_filter("FS_FILE_CREATE", path=tofile)
+        if sys.platform == 'darwin':
+            self.eq.add_to_mute_filter("FS_FILE_OPEN", path=tofile)
         self.eq.add_to_mute_filter("FS_FILE_CLOSE_WRITE", path=tofile)
 
         yield self.eq.add_watch(root_dir)
@@ -646,7 +655,7 @@
         yield self._deferred
 
 
-@skip_if_win32_missing_fs_event
+@skipIfNotOS('linux2', "Only Linux watches UDF ancestors")
 class AncestorsUDFTestCase(BaseTwistedTestCase):
     """Events over UDF's ancestor are properly handled."""
 
@@ -661,6 +670,7 @@
         self.data_dir = self.mktemp('data_dir')
         self.shares_dir = self.mktemp('shares_dir')
         self.partials_dir = self.mktemp('partials_dir')
+        self.patch(FakeMain, '_monitor_class', None)
         self.main = FakeMain(self.root_dir, self.shares_dir,
                              self.data_dir, self.partials_dir)
         self.addCleanup(self.main.shutdown)
@@ -988,6 +998,7 @@
 
 
 @skipIfOS('win32', "we can't create files with invalid utf8 byte sequences.")
+@skipIfOS('darwin', "fsevents daemon ignores events with invalid filenames")
 class NonUTF8NamesTests(BaseTwisted):
     """Test the non-utf8 name handling."""
 
@@ -1160,6 +1171,7 @@
 
 
 @skip_if_win32_missing_fs_event
+@skip_if_darwin_missing_fs_event
 class SignalingTests(BaseTwisted):
     """Test the whole stuff to receive signals."""
 

=== modified file 'tests/syncdaemon/test_eventqueue.py'
--- tests/syncdaemon/test_eventqueue.py	2012-07-26 22:00:58 +0000
+++ tests/syncdaemon/test_eventqueue.py	2013-01-10 22:19:25 +0000
@@ -36,7 +36,9 @@
 from twisted.internet import defer
 from twisted.trial.unittest import TestCase
 
-from contrib.testing.testcase import BaseTwistedTestCase, FakeVolumeManager
+from contrib.testing.testcase import (BaseTwistedTestCase,
+                                      FakeMonitor,
+                                      FakeVolumeManager)
 from ubuntuone.platform.filesystem_notifications.monitor import FilesystemMonitor
 from ubuntuone.syncdaemon import (
     event_queue,
@@ -49,6 +51,8 @@
 class BaseEQTestCase(BaseTwistedTestCase):
     """ Setup an EQ for test. """
 
+    _monitor_class = FakeMonitor
+
     @defer.inlineCallbacks
     def setUp(self):
         """Setup the test."""
@@ -66,7 +70,8 @@
                        share_id='', is_dir=True)
         self.fs.set_by_path(path=self.root_dir,
                               local_hash=None, server_hash=None)
-        self.eq = event_queue.EventQueue(self.fs)
+        self.eq = event_queue.EventQueue(self.fs,
+                                         monitor_class=self._monitor_class)
         self.eq.listener_map = {}
         self.addCleanup(self.eq.shutdown)
         self.fs.register_eq(self.eq)
@@ -395,7 +400,7 @@
         return d
 
 
-class FakeMonitor(object):
+class SimpleFakeMonitor(object):
     """A fake FilesystemMonitor."""
 
     def __init__(self, *args):
@@ -431,7 +436,7 @@
     @defer.inlineCallbacks
     def test_shutdown_defers(self):
         """The shutdown method in eq defers on the shutdown of the monitor."""
-        self.patch(event_queue, "FilesystemMonitor", FakeMonitor)
+        self.patch(event_queue, "FilesystemMonitor", SimpleFakeMonitor)
         eq = event_queue.EventQueue(None)
         d = eq.shutdown()
         self.assertFalse(d.called, "shutdown is fired after the monitor.")

=== modified file 'tests/syncdaemon/test_fsm.py'
--- tests/syncdaemon/test_fsm.py	2012-11-02 13:49:39 +0000
+++ tests/syncdaemon/test_fsm.py	2013-01-10 22:19:25 +0000
@@ -42,6 +42,7 @@
     BaseTwistedTestCase,
     FakeVolumeManager,
     FakeMain,
+    FakeMonitor,
     Listener,
     skip_if_win32_and_uses_metadata_older_than_5,
     skip_if_win32_and_uses_readonly,
@@ -116,7 +117,7 @@
         self.addCleanup(self.db.shutdown)
         self.fsm = FileSystemManager(self.fsmdir, self.partials_dir,
                                      FakeVolumeManager(self.root_dir), self.db)
-        self.eq = EventQueue(self.fsm)
+        self.eq = EventQueue(self.fsm, monitor_class=FakeMonitor)
         self.addCleanup(self.eq.shutdown)
         self.fsm.register_eq(self.eq)
         self.share = yield self.create_share('share', u'share_name')
@@ -4424,4 +4425,4 @@
         result = self.fsm.get_paths_by_pattern(search)
         self.assertNotEqual(result, expected)
         expected = sorted(expected)
-        self.assertEqual(result, expected)
\ No newline at end of file
+        self.assertEqual(result, expected)

=== modified file 'tests/syncdaemon/test_sync.py'
--- tests/syncdaemon/test_sync.py	2012-10-02 19:52:03 +0000
+++ tests/syncdaemon/test_sync.py	2013-01-10 22:19:25 +0000
@@ -297,19 +297,21 @@
         yield self.fsm.vm.add_share(share)
         defer.returnValue(share)
 
-
-class TestSync(BaseSync):
-    """Test for Sync."""
-
+class TestUsingRealFSMonitor(BaseSync):
+    """Class for tests that require a real FS monitor."""
     timeout = 3
 
     @defer.inlineCallbacks
     def setUp(self):
-        """Set up."""
-        yield super(TestSync, self).setUp()
-        self.sync = Sync(main=self.main)
-        self.fsm = self.main.fs
-        self.handler.setLevel(logging.DEBUG)
+        """Override self.main from BaseSync."""
+        yield super(TestUsingRealFSMonitor, self).setUp()
+        # FakeMain sends _monitor_class to EventQueue, which
+        # uses platform default monitor when given None:
+        self.patch(FakeMain, "_monitor_class", None)
+        self.main = FakeMain(root_dir=self.root, shares_dir=self.shares,
+                             data_dir=self.data,
+                             partials_dir=self.partials_dir)
+        self.addCleanup(self.main.shutdown)
 
     @skipIfOS('win32', 'In windows we can not unlink opened files.')
     def test_deleting_open_files_is_no_cause_for_despair(self):
@@ -331,6 +333,19 @@
         d.addCallback(cb)
         return d
 
+class TestSync(BaseSync):
+    """Test for Sync."""
+
+    timeout = 3
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        """Set up."""
+        yield super(TestSync, self).setUp()
+        self.sync = Sync(main=self.main)
+        self.fsm = self.main.fs
+        self.handler.setLevel(logging.DEBUG)
+
     def test_handle_AQ_DOWNLOAD_DOES_NOT_EXIST(self):
         """handle_AQ_DOWNLOAD_DOES_NOT_EXIST."""
         self.called = False

=== modified file 'tests/syncdaemon/test_vm.py'
--- tests/syncdaemon/test_vm.py	2012-08-06 19:18:57 +0000
+++ tests/syncdaemon/test_vm.py	2013-01-10 22:19:25 +0000
@@ -1468,34 +1468,6 @@
                       'watch for %r should be present.' % udf.path)
 
     @defer.inlineCallbacks
-    def test_add_udf_with_content(self):
-        """Test for VolumeManager.add_udf with content on disk."""
-        # create a sync instance
-        from ubuntuone.syncdaemon import sync
-        sync = sync.Sync(self.main)
-        suggested_path = u"~/suggested_path"
-        udf = self._create_udf(suggested_path=suggested_path,
-                               subscribed=True)
-        # create some files inside it
-        make_dir(udf.path)
-        for i in range(10):
-            with open_file(os.path.join(udf.path, 'file_%d' % (i,)), 'wb') as f:
-                f.write(os.urandom(10))
-        self.assertEqual(len(os.listdir(udf.path)), 10)
-        # patch the fake action queue to intercept make_file calls
-        called = []
-        self.main.action_q.make_file = lambda *a: called.append(a)
-        yield self.vm.add_udf(udf)
-        self.assertEqual(len(called), 10)
-        # check that the UDF is in the fsm metadata
-        mdobj = self.main.fs.get_by_path(udf.path)
-        self.assertEqual(mdobj.node_id, udf.node_id)
-        self.assertEqual(mdobj.share_id, udf.volume_id)
-        # check that there is a watch in the UDF
-        self.assertIn(udf.path, self.watches,
-                      'watch for %r should be present.' % udf.path)
-
-    @defer.inlineCallbacks
     def test_add_udf_calls_AQ(self):
         """Test that VolumeManager.add_udf calls AQ.rescan_from_scratch."""
         udf = self._create_udf(subscribed=True)
@@ -1927,6 +1899,69 @@
         self.assertEqual(expected, actual)
 
 
+class VolumeManagerOpTestsRequiringRealFSMonitor(BaseVolumeManagerTests):
+    """Tests of UDF/Volumes operations which require a real FSMonitor."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        """Setup the test."""
+        yield super(VolumeManagerOpTestsRequiringRealFSMonitor, self).setUp()
+        # FakeMain sends _monitor_class to EventQueue, which
+        # uses platform default monitor when given None:
+        self.patch(FakeMain, "_monitor_class", None)
+        self.main = FakeMain(root_dir=self.root_dir,
+                             shares_dir=self.shares_dir,
+                             data_dir=self.data_dir,
+                             partials_dir=self.partials_dir)
+        # re-do the patching from super that alters main or its
+        # event_q:
+        orig_add_watch = self.main.event_q.add_watch
+
+        def fake_add_watch(path):
+            self.watches.add(path)
+            return orig_add_watch(path)
+
+        orig_rm_watch = self.main.event_q.rm_watch
+
+        def fake_rm_watch(path):
+            self.watches.remove(path)
+            return orig_rm_watch(path)
+
+        self.patch(self.main.event_q, 'add_watch', fake_add_watch)
+        self.patch(self.main.event_q, 'rm_watch', fake_rm_watch)
+        self.vm = self.main.vm
+        self.main.event_q.push('SYS_INIT_DONE')
+        self.addCleanup(self.main.shutdown)
+
+    @defer.inlineCallbacks
+    def test_add_udf_with_content(self):
+        """Test for VolumeManager.add_udf with content on disk."""
+        # create a sync instance
+        from ubuntuone.syncdaemon import sync
+        sync = sync.Sync(self.main)
+        suggested_path = u"~/suggested_path"
+        udf = self._create_udf(suggested_path=suggested_path,
+                               subscribed=True)
+        # create some files inside it
+        make_dir(udf.path)
+        for i in range(10):
+            with open_file(os.path.join(udf.path, 'file_%d' % (i,)), 'wb') as f:
+                f.write(os.urandom(10))
+        self.assertEqual(len(os.listdir(udf.path)), 10)
+        # patch the fake action queue to intercept make_file calls
+        called = []
+        self.main.action_q.make_file = lambda *a: called.append(a)
+        yield self.vm.add_udf(udf)
+        self.assertEqual(len(called), 10)
+        # check that the UDF is in the fsm metadata
+        mdobj = self.main.fs.get_by_path(udf.path)
+        self.assertEqual(mdobj.node_id, udf.node_id)
+        self.assertEqual(mdobj.share_id, udf.volume_id)
+        # check that there is a watch in the UDF
+        self.assertIn(udf.path, self.watches,
+                      'watch for %r should be present.' % udf.path)
+                    
+
 class VolumeManagerOperationsTests(BaseVolumeManagerTests):
     """Test UDF/Volumes operations."""