Merge lp:~mandel/ubuntuone-client/fix-file-shelf-windows into lp:ubuntuone-client

[Manuel de la Peña]

Ensure that file_shelf tests do pass on windows and that the module uses the os_helper methods.

Ensure that file_shelf tests do pass on windows and that the module uses the os_helper methods.

[Natalia Bidart (nataliabidart)]

This change:

25 - key = fd.name.strip('.new')
26 + key = os.path.split(fd.name.strip('.new'))[1]

does not seem appropriate to me, you're hiding the fact that in windows, somehow, fd.name is a full path and in linux is not.

Like we talked about in IRC, we should just skip the test suite for file_shelf in windows since that code will not be used there.

review: Needs Fixing

[lp:~mandel/ubuntuone-client/fix-file-shelf-windows]

1126. By Manuel de la Peña on 2011-08-05

Skipped problematic test on Windows since the code is deprecated in that platform.

[Manuel de la Peña (mandel)]

I have updated the branch to skip such test so that we ignore the fact that open_file does work in a different way. As food for though I think we should consider update or test case to add an assertEqualPath (or something of the like) so that we can ignore this facts.

[lp:~mandel/ubuntuone-client/fix-file-shelf-windows]

1127. By Manuel de la Peña on 2011-08-05

Merged with trunk.

1128. By Manuel de la Peña on 2011-08-05

Use open_file instead of open in the tests.

1129. By Manuel de la Peña on 2011-08-05

Re add the new line.

[Natalia Bidart (nataliabidart)]

Looks much better now!

review: Approve

=== modified file 'tests/syncdaemon/test_fileshelf.py'
--- tests/syncdaemon/test_fileshelf.py	2011-07-27 14:26:36 +0000
+++ tests/syncdaemon/test_fileshelf.py	2011-08-05 15:23:53 +0000
@@ -25,6 +25,8 @@
 
 from twisted.internet import defer
 
+from ubuntuone.devtools.testcase import skipIfOS
+from ubuntuone.platform import path_exists, open_file
 from ubuntuone.syncdaemon.file_shelf import (
     FileShelf,
     CachedFileShelf,
@@ -74,7 +76,7 @@
             # test __setitem__
             shelf[key] = 'foo'
             key_path = os.path.join(path, *[key[i] for i in xrange(0, idx)])
-            self.assertTrue(os.path.exists(os.path.join(key_path, key)))
+            self.assertTrue(path_exists(os.path.join(key_path, key)))
             # test __getitem__
             self.assertEquals('foo', shelf[key])
             # test __delitem__
@@ -203,7 +205,7 @@
         path = self.shelf.key_file('foo')
         open(self.shelf.key_file('foo'), 'w').close()
         for _ in xrange(20):
-            open(path+'.old', 'w').close()
+            open_file(path+'.old', 'w').close()
             path=path+'.old'
         self.assertRaises(KeyError, self.shelf.__getitem__, 'foo')
 
@@ -221,6 +223,7 @@
         self.assertFalse(os.path.exists(path+'.old'), 'there is a .old file!')
         self.assertFalse(os.path.exists(path+'.new'), 'there is a .new file!')
 
+    @skipIfOS('win32', 'Skipped because code is deprecated on Windows.')
     def test_custom_unpickle(self):
         """Test the _pickle and _unpikle methods."""
         self.mktemp('my_shelf')

=== modified file 'tests/syncdaemon/test_vm.py'
--- tests/syncdaemon/test_vm.py	2011-08-04 16:30:54 +0000
+++ tests/syncdaemon/test_vm.py	2011-08-05 15:23:53 +0000
@@ -3844,10 +3844,8 @@
         os_helper_rename = mocker.replace('ubuntuone.platform.rename')
         is_string = lambda x: isinstance(x, str)
         os_helper_rename(MATCH(is_string), MATCH(is_string))
-        mocker.replay()
-        self.md_upgrader._upgrade_names(dirpath, files)
-        mocker.restore()
-        mocker.verify()
+        with mocker:
+            self.md_upgrader._upgrade_names(dirpath, files)
 
     def test_upgrade_metadata_5_no_os_rename(self):
         """Test that when we upgrade we use the os_helper.rename."""
@@ -3861,12 +3859,10 @@
         os_helper_rename = mocker.replace('ubuntuone.platform.rename')
         is_string = lambda x: isinstance(x, str)
         os_helper_rename(MATCH(is_string), MATCH(is_string))
-        os_helper_rename(MATCH(is_string), MATCH(is_string))
+        mocker.count(3)
         self.md_upgrader._upgrade_metadata_6(6)
-        mocker.replay()
-        self.md_upgrader._upgrade_metadata_5(6)
-        mocker.restore()
-        mocker.verify()
+        with mocker:
+            self.md_upgrader._upgrade_metadata_5(6)
 
 
 class VMTritcaskShelfTests(BaseTwistedTestCase):

=== modified file 'ubuntuone/syncdaemon/file_shelf.py'
--- ubuntuone/syncdaemon/file_shelf.py	2011-06-24 01:47:50 +0000
+++ ubuntuone/syncdaemon/file_shelf.py	2011-08-05 15:23:53 +0000
@@ -26,7 +26,7 @@
 import errno
 from collections import deque
 
-from ubuntuone.platform import open_file
+from ubuntuone.platform import make_dir, open_file, remove_file, rename, stat_path
 
 class FileShelf(object, DictMixin):
     """ File based shelf.
@@ -52,16 +52,16 @@
         creates it
         """
         try:
-            stat_result = os.stat(path)
+            stat_result = stat_path(path)
             # is a regular file?
             if stat.S_ISREG(stat_result.st_mode):
                 os.unlink(path)
-                os.makedirs(path)
+                make_dir(path, True)
             # else, the dir is already there
         except OSError, e:
             if e.errno == errno.ENOENT:
                 # the file or dir don't exist
-                os.makedirs(path)
+                make_dir(path, True)
             else:
                 raise
 
@@ -158,20 +158,20 @@
             self._pickle(value, fh, protocol=2)
             fh.flush()
         if os.path.exists(path):
-            os.rename(path, old_path)
-        os.rename(new_path, path)
+            rename(path, old_path)
+        rename(new_path, path)
 
     def __delitem__(self, key):
         """ delitem backed by the file storage """
         path = self.key_file(key)
         try:
-            os.remove(self.key_file(key))
+            remove_file(self.key_file(key))
         except OSError:
             raise KeyError(key)
         # also delete backup files
         for path in [path+'.old', path+'.new']:
             try:
-                os.remove(path)
+                remove_file(path)
             except OSError:
                 # ignore any OSError
                 pass