Merge lp:~nataliabidart/ubuntuone-client/unify-os-helper-tests into lp:ubuntuone-client

[Natalia Bidart]

- Unify tests for os_helper in every platform (LP: #823223).

[lp:~nataliabidart/ubuntuone-client/unify-os-helper-tests]

1096. By Natalia Bidart on 2011-08-09

_testMethod is not defined in setUp.

1097. By Natalia Bidart on 2011-08-09

Merged trunk in.

[Manuel de la Peña (mandel)]

+1

review: Approve

[lp:~nataliabidart/ubuntuone-client/unify-os-helper-tests]

1098. By Natalia Bidart on 2011-08-09

Clarification.

[Alejandro J. Cura (alecu)]

Works as expected.

review: Approve

=== modified file 'contrib/testing/testcase.py'
--- contrib/testing/testcase.py	2011-08-04 19:17:11 +0000
+++ contrib/testing/testcase.py	2011-08-09 16:37:00 +0000
@@ -280,16 +280,16 @@
     def tmpdir(self):
         """Default tmpdir: module/class/test_method."""
         # check if we already generated the root path
-        root_dir = getattr(self, '__root', None)
-        if root_dir:
-            return root_dir
+        if self.__root:
+            return self.__root
         base = os.path.join(self.__class__.__module__,
                             self.__class__.__name__,
                             self._testMethodName)
-        # use _trial_temp dir, it should be os.gwtcwd()
+        # use _trial_temp dir, it should be TRIAL_TEMP_DIR or os.getcwd()
         # define the root temp dir of the testcase, pylint: disable=W0201
         root_tmp = os.environ.get('TRIAL_TEMP_DIR', os.getcwd())
         self.__root = os.path.join(root_tmp, '_trial_temp', base)
+        self.addCleanup(self.rmtree, self.__root)
         return self.__root
 
     def rmtree(self, path):
@@ -300,7 +300,7 @@
         if not path_exists(path):
             return
         # change perms to rw, so we can delete the temp dir
-        if path != getattr(self, '__root', None):
+        if path != self.__root:
             set_dir_readwrite(os.path.dirname(path))
         if not can_write(path):
             set_dir_readwrite(path)
@@ -318,8 +318,22 @@
                     adir = adir.encode('utf8')
                 if not can_write(adir):
                     set_dir_readwrite(adir)
-        # XXX: We should not be ignoring errors
-        shutil.rmtree(path, ignore_errors=True)
+
+        if sys.platform == 'win32':
+            # in windows, we need to pass a unicode, literal path to
+            # shutil.rmtree, otherwise we can't remove "deep and wide" paths
+            path = u'\\\\?\\' + path.decode('utf8')
+
+        # Instead of ignoring the errors when removing trees, we are temporarly
+        # printing a message to stdout to caught everyone's attention.
+        # Once the tests are fixed in this regard, we're removing the
+        # try-except block and having shutil.rmtree failing if the path
+        # can not be removed.
+        try:
+            shutil.rmtree(path)
+        except Exception, e:
+            print 'ERROR!! could not recursively remove %r ' \
+                  '(error is %r).' % (path, e)
 
     def makedirs(self, path):
         """Custom makedirs that handle ro parent."""
@@ -331,12 +345,15 @@
     @defer.inlineCallbacks
     def setUp(self):
         yield super(BaseTwistedTestCase, self).setUp()
+        self.__root = None
         # use the config from the branch
-        self.old_get_config_files = config.get_config_files
-        config.get_config_files = lambda: [os.path.join(os.environ['ROOTDIR'],
+        new_get_config_files = lambda: [os.path.join(os.environ['ROOTDIR'],
                                                    'data', 'syncdaemon.conf')]
+        self.patch(config, 'get_config_files', new_get_config_files)
+
         # fake a very basic config file with sane defaults for the tests
-        self.config_file = os.path.join(self.mktemp('config'), 'syncdaemon.conf')
+        config_dir = self.mktemp('config')
+        self.config_file = os.path.join(config_dir, 'syncdaemon.conf')
         with open(self.config_file, 'w') as fp:
             fp.write('[bandwidth_throttling]\n')
             fp.write('on = False\n')
@@ -346,18 +363,6 @@
         config._user_config = None
         config.get_user_config(config_file=self.config_file)
 
-    def tearDown(self):
-        """ cleanup the temp dir. """
-        # invalidate the current config
-        config._user_config = None
-        # restore the old get_config_files
-        config.get_config_files = self.old_get_config_files
-        self.rmtree(os.path.dirname(self.config_file))
-        root_dir = getattr(self, '__root', None)
-        if root_dir:
-            self.rmtree(self.__root)
-        return super(BaseTwistedTestCase, self).tearDown()
-
 
 class FakeMainTestCase(BaseTwistedTestCase):
     """A testcase that starts up a Main instance."""
@@ -376,20 +381,13 @@
         self.shares_dir = self.mktemp('shares_dir')
         self.main = FakeMain(self.root_dir, self.shares_dir,
                              self.data_dir, self.partials_dir)
+        self.addCleanup(self.main.shutdown)
         self.vm = self.main.vm
         self.fs = self.main.fs
         self.event_q = self.main.event_q
         self.action_q = self.main.action_q
         self.event_q.push('SYS_INIT_DONE')
 
-    @defer.inlineCallbacks
-    def tearDown(self):
-        """Shutdown this testcase."""
-        yield super(FakeMainTestCase, self).tearDown()
-        self.main.shutdown()
-        self.log.info("finished test %s.%s", self.__class__.__name__,
-                      self._testMethodName)
-
 
 class FakeVolumeManager(object):
     """ A volume manager that only knows one share, the root"""

=== modified file 'tests/platform/linux/test_os_helper.py'
--- tests/platform/linux/test_os_helper.py	2011-07-27 17:31:25 +0000
+++ tests/platform/linux/test_os_helper.py	2011-08-09 16:37:00 +0000
@@ -27,22 +27,9 @@
 from twisted.internet import defer
 from ubuntuone.devtools.handlers import MementoHandler
 
-from contrib.testing.testcase import BaseTwistedTestCase
+from tests.platform import test_os_helper
 from ubuntuone.platform import (
-    access,
-    allow_writes,
-    listdir,
-    make_dir,
     move_to_trash,
-    open_file,
-    path_exists,
-    remove_dir,
-    remove_file,
-    rename,
-    set_dir_readonly,
-    set_dir_readwrite,
-    set_file_readonly,
-    set_file_readwrite,
     stat_path,
 )
 
@@ -60,186 +47,19 @@
         return self._bad_trash_call
 
 
-class OSWrapperTests(BaseTwistedTestCase):
+class OSWrapperTests(test_os_helper.OSWrapperTests):
     """Tests for os wrapper functions."""
 
     @defer.inlineCallbacks
     def setUp(self):
         """Set up."""
         yield super(OSWrapperTests, self).setUp()
-        self.basedir = self.mktemp('test_root')
-        self.testfile = os.path.join(self.basedir, "test_file")
-        open(self.testfile, 'w').close()
-
         self.handler = MementoHandler()
         self.handler.setLevel(logging.DEBUG)
         self._logger = logging.getLogger('ubuntuone.SyncDaemon')
         self._logger.addHandler(self.handler)
         self.addCleanup(self._logger.removeHandler, self.handler)
 
-    def test_set_dir_readonly(self):
-        """Test for set_dir_readonly."""
-        set_dir_readonly(self.basedir)
-        self.assertRaises(OSError, os.mkdir, os.path.join(self.basedir, 'foo'))
-
-    def test_set_dir_readwrite(self):
-        """Test for set_dir_readwrite."""
-        set_dir_readonly(self.basedir)
-        self.assertRaises(OSError, os.mkdir, os.path.join(self.basedir, 'foo'))
-        set_dir_readwrite(self.basedir)
-        os.mkdir(os.path.join(self.basedir, 'foo'))
-
-    def test_allow_writes(self):
-        """Test for allow_writes."""
-        set_dir_readonly(self.basedir)
-        self.assertRaises(OSError, os.mkdir, os.path.join(self.basedir, 'foo'))
-        with allow_writes(self.basedir):
-            os.mkdir(os.path.join(self.basedir, 'foo'))
-
-    def test_set_file_readonly(self):
-        """Test for set_file_readonly."""
-        set_file_readonly(self.testfile)
-        self.assertRaises(IOError, open, self.testfile, 'w')
-
-    def test_set_file_readwrite(self):
-        """Test for set_file_readwrite."""
-        set_file_readonly(self.testfile)
-        self.assertRaises(IOError, open, self.testfile, 'w')
-        set_file_readwrite(self.testfile)
-        open(self.testfile, 'w')
-
-    def test_path_file_exist_yes(self):
-        """Test that the file exists."""
-        self.assertTrue(path_exists(self.testfile))
-
-    def test_path_file_exist_no(self):
-        """Test that the file doesn't exist."""
-        os.remove(self.testfile)
-        self.assertFalse(path_exists(self.testfile))
-
-    def test_path_dir_exist_yes(self):
-        """Test that the dir exists."""
-        self.assertTrue(path_exists(self.basedir))
-
-    def test_path_dir_exist_no(self):
-        """Test that the dir doesn't exist."""
-        os.remove(self.testfile)
-        self.assertFalse(path_exists(os.path.join(self.basedir, 'nodir')))
-
-    def test_remove_file(self):
-        """Test the remove file."""
-        remove_file(self.testfile)
-        self.assertFalse(os.path.exists(self.testfile))
-
-    def test_remove_dir(self):
-        """Test the remove dir."""
-        testdir = os.path.join(self.basedir, 'foodir')
-        os.mkdir(testdir)
-        assert os.path.exists(testdir)
-        remove_dir(testdir)
-        self.assertFalse(path_exists(testdir))
-
-    def test_make_dir_one(self):
-        """Test the make dir with one dir."""
-        testdir = os.path.join(self.basedir, 'foodir')
-        assert not os.path.exists(testdir)
-        make_dir(testdir)
-        self.assertTrue(os.path.exists(testdir))
-
-    def test_make_dir_already_there(self):
-        """Test the make dir with one dir that exists."""
-        self.assertRaises(OSError, make_dir, self.basedir)
-
-    def test_make_dir_recursive_no(self):
-        """Test the make dir with some dirs, not recursive explicit."""
-        testdir = os.path.join(self.basedir, 'foo', 'bar')
-        assert not os.path.exists(testdir)
-        self.assertRaises(OSError, make_dir, testdir)
-
-    def test_make_dir_recursive_yes(self):
-        """Test the make dir with some dirs, recursive."""
-        testdir = os.path.join(self.basedir, 'foo', 'bar')
-        assert not os.path.exists(testdir)
-        make_dir(testdir, recursive=True)
-        self.assertTrue(os.path.exists(testdir))
-
-    def test_open_file_not_there(self):
-        """Open a file that does not exist."""
-        self.assertRaises(IOError, open_file, os.path.join(self.basedir, 'no'))
-
-    def test_open_file_gets_a_fileobject(self):
-        """Open a file, and get a file object."""
-        testfile = os.path.join(self.basedir, 'testfile')
-        open(testfile, 'w').close()
-        f = open_file(testfile)
-        self.assertTrue(isinstance(f, file))
-
-    def test_open_file_read(self):
-        """Open a file, and read."""
-        testfile = os.path.join(self.basedir, 'testfile')
-        with open(testfile, 'w') as fh:
-            fh.write("foo")
-        f = open_file(testfile, 'r')
-        self.assertTrue(f.read(), "foo")
-
-    def test_open_file_write(self):
-        """Open a file, and write."""
-        testfile = os.path.join(self.basedir, 'testfile')
-        with open_file(testfile, 'w') as fh:
-            fh.write("foo")
-        f = open(testfile)
-        self.assertTrue(f.read(), "foo")
-
-    def test_rename_not_there(self):
-        """Rename something that does not exist."""
-        self.assertRaises(OSError, rename,
-                          os.path.join(self.basedir, 'no'), 'foo')
-
-    def test_rename_file(self):
-        """Rename a file."""
-        testfile1 = os.path.join(self.basedir, 'testfile1')
-        testfile2 = os.path.join(self.basedir, 'testfile2')
-        open(testfile1, 'w').close()
-        rename(testfile1, testfile2)
-        self.assertFalse(os.path.exists(testfile1))
-        self.assertTrue(os.path.exists(testfile2))
-
-    def test_rename_dir(self):
-        """Rename a dir."""
-        testdir1 = os.path.join(self.basedir, 'testdir1')
-        testdir2 = os.path.join(self.basedir, 'testdir2')
-        os.mkdir(testdir1)
-        rename(testdir1, testdir2)
-        self.assertFalse(os.path.exists(testdir1))
-        self.assertTrue(os.path.exists(testdir2))
-
-    def test_listdir(self):
-        """Return a list of the files in a dir."""
-        open(os.path.join(self.basedir, 'foo'), 'w').close()
-        open(os.path.join(self.basedir, 'bar'), 'w').close()
-        l = listdir(self.basedir)
-        self.assertEqual(sorted(l), ['bar', 'foo', 'test_file'])
-
-    def test_access_rw(self):
-        """Test access on a file with full permission."""
-        self.assertTrue(access(self.testfile))
-
-    def test_access_ro(self):
-        """Test access on a file with read only permission."""
-        os.chmod(self.testfile, 0o444)
-        self.assertTrue(access(self.testfile))
-        os.chmod(self.testfile, 0o664)
-
-    def test_access_nothing(self):
-        """Test access on a file with no permission at all."""
-        os.chmod(self.testfile, 0o000)
-        self.assertFalse(access(self.testfile))
-        os.chmod(self.testfile, 0o664)
-
-    def test_stat_normal(self):
-        """Test on a normal file."""
-        self.assertEqual(os.stat(self.testfile), stat_path(self.testfile))
-
     def test_stat_symlink(self):
         """Test that it doesn't follow symlinks.
 
@@ -252,31 +72,6 @@
         self.assertNotEqual(os.stat(link).st_ino, stat_path(link).st_ino)
         self.assertEqual(os.lstat(link).st_ino, stat_path(link).st_ino)
 
-    def test_stat_no_path(self):
-        """Test that it raises proper error when no file is there."""
-        try:
-            return stat_path(os.path.join(self.basedir, 'nofile'))
-        except OSError, e:
-            self.assertEqual(e.errno, errno.ENOENT)
-
-    def test_path_exists_file_yes(self):
-        """The file is there."""
-        self.assertTrue(path_exists(self.testfile))
-
-    def test_path_exists_file_no(self):
-        """The file is not there."""
-        os.remove(self.testfile)
-        self.assertFalse(path_exists(self.testfile))
-
-    def test_path_exists_dir_yes(self):
-        """The dir is there."""
-        self.assertTrue(path_exists(self.basedir))
-
-    def test_path_exists_dir_no(self):
-        """The dir is not there."""
-        self.rmtree(self.basedir)
-        self.assertFalse(path_exists(self.basedir))
-
     def test_movetotrash_file_ok(self):
         """Move a file to trash ok.
 

=== modified file 'tests/platform/test_os_helper.py'
--- tests/platform/test_os_helper.py	2011-07-28 15:35:45 +0000
+++ tests/platform/test_os_helper.py	2011-08-09 16:37:00 +0000
@@ -66,7 +66,7 @@
             self.valid_path = valid_file_path_builder(self.testfile)
 
         # make sure the file exists
-        open(self.valid_path, 'w').close()
+        open_file(self.testfile, 'w').close()
 
     def test_set_dir_readonly(self):
         """Test for set_dir_readonly."""
@@ -98,7 +98,7 @@
         """Test for set_file_readonly."""
         set_file_readonly(self.testfile)
         self.addCleanup(set_dir_readwrite, self.testfile)
-        self.assertRaises(IOError, open, self.valid_path, 'w')
+        self.assertRaises(IOError, open_file, self.testfile, 'w')
 
     def test_set_file_readwrite(self):
         """Test for set_file_readwrite."""
@@ -106,7 +106,7 @@
         self.addCleanup(set_dir_readwrite, self.testfile)
 
         set_file_readwrite(self.testfile)
-        open(self.valid_path, 'w')
+        open_file(self.testfile, 'w')
         self.assertTrue(can_write(self.testfile))
 
     def test_path_file_exist_yes(self):
@@ -115,7 +115,7 @@
 
     def test_path_file_exist_no(self):
         """Test that the file doesn't exist."""
-        os.remove(self.valid_path)
+        remove_file(self.testfile)
         self.assertFalse(path_exists(self.testfile))
 
     def test_path_dir_exist_yes(self):
@@ -129,7 +129,7 @@
     def test_remove_file(self):
         """Test the remove file."""
         remove_file(self.testfile)
-        self.assertFalse(os.path.exists(self.valid_path))
+        self.assertFalse(path_exists(self.testfile))
 
     def test_remove_dir(self):
         """Test the remove dir."""
@@ -174,7 +174,7 @@
 
     def test_open_file_read(self):
         """Open a file, and read."""
-        with open(self.valid_path, 'w') as fh:
+        with open_file(self.testfile, 'w') as fh:
             fh.write("foo")
         f = open_file(self.testfile, 'r')
         self.assertTrue(f.read(), "foo")
@@ -185,7 +185,7 @@
         fh.write("foo")
         fh.close()
 
-        f = open(self.valid_path)
+        f = open_file(self.testfile)
         self.assertTrue(f.read(), "foo")
 
     def test_rename_not_there(self):
@@ -198,10 +198,10 @@
         if target is None:
             target = os.path.join(self.basedir, 'target')
 
-        assert os.path.exists(self.valid_path)
+        assert path_exists(self.testfile)
         rename(self.testfile, target)
 
-        self.assertFalse(os.path.exists(self.valid_path),
+        self.assertFalse(path_exists(self.testfile),
             'Path %r should not exist after rename.' % self.testfile)
         self.assertTrue(path_exists(target),
             'Path %r should exist after rename.' % target)
@@ -242,8 +242,8 @@
 
     def test_access_ro(self):
         """Test access on a file with read only permission."""
-        os.chmod(self.valid_path, 0o444)
-        self.addCleanup(os.chmod, self.valid_path, 0o664)
+        set_file_readonly(self.testfile)
+        self.addCleanup(set_file_readwrite, self.testfile)
         self.assertTrue(access(self.testfile))
 
     def test_access_nothing(self):
@@ -270,7 +270,7 @@
 
     def test_path_exists_file_no(self):
         """The file is not there."""
-        os.remove(self.valid_path)
+        remove_file(self.testfile)
         self.assertFalse(path_exists(self.testfile))
 
     def test_path_exists_dir_yes(self):

=== modified file 'ubuntuone/platform/linux/os_helper.py'
--- ubuntuone/platform/linux/os_helper.py	2011-07-29 13:27:18 +0000
+++ ubuntuone/platform/linux/os_helper.py	2011-08-09 16:37:00 +0000
@@ -47,7 +47,7 @@
 
 def set_file_readwrite(path):
     """Change path permissions to readwrite in a file."""
-    os.chmod(path, 0774)
+    os.chmod(path, 0664)
 
 def set_dir_readonly(path):
     """Change path permissions to readonly in a dir."""