launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/launchpad/test-file-creation-oneiric-bug-892955 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #892955 in Launchpad itself: "lp.poppy.tests.test_twistedsftp.TestSFTPServer.test_file_creation fails on oneiric"
  https://bugs.launchpad.net/launchpad/+bug/892955

For more details, see:
https://code.launchpad.net/~allenap/launchpad/test-file-creation-oneiric-bug-892955/+merge/83959

Just use the intended file permissions in writeChunk() in the first place. The test worked before because the umask was 0022. It has now changed to 0002 on Oneiric.
-- 
https://code.launchpad.net/~allenap/launchpad/test-file-creation-oneiric-bug-892955/+merge/83959
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/test-file-creation-oneiric-bug-892955 into lp:launchpad.

=== modified file 'lib/lp/poppy/tests/test_twistedsftp.py'
--- lib/lp/poppy/tests/test_twistedsftp.py	2011-08-12 11:37:08 +0000
+++ lib/lp/poppy/tests/test_twistedsftp.py	2011-11-30 15:00:52 +0000
@@ -6,7 +6,8 @@
 __metaclass__ = type
 
 import os
-import tempfile
+
+from fixtures import TempDir
 
 from lp.poppy.twistedsftp import SFTPServer
 from lp.services.sshserver.sftp import FileIsADirectory
@@ -16,10 +17,16 @@
 class TestSFTPServer(TestCase):
 
     def setUp(self):
-        self.fs_root = tempfile.mkdtemp()
+        self.fs_root = self.useFixture(TempDir()).path
         self.sftp_server = SFTPServer(None, self.fs_root)
         super(TestSFTPServer, self).setUp()
 
+    def assertPermissions(self, expected, file_name):
+        observed = os.stat(file_name).st_mode
+        self.assertEqual(
+            expected, observed, "Expected %07o, got %07o, for %s" % (
+                expected, observed, file_name))
+
     def test_gotVersion(self):
         # gotVersion always returns an empty dict, since the server does not
         # support any extended features. See ISFTPServer.
@@ -33,8 +40,7 @@
             'foo')
         dir_name = os.path.join(self.sftp_server._current_upload, 'foo')
         self.assertEqual(os.listdir(dir_name)[0], 'bar')
-        self.assertEqual(
-            os.stat(os.path.join(dir_name, 'bar')).st_mode, 040775)
+        self.assertPermissions(040775, dir_name)
         self.sftp_server.removeDirectory('foo/bar')
         self.assertEqual(
             os.listdir(os.path.join(self.sftp_server._current_upload,
@@ -50,7 +56,7 @@
         test_file = open(file_name, 'r')
         self.assertEqual(test_file.read(), "This is a test")
         test_file.close()
-        self.assertEqual(os.stat(file_name).st_mode, 0100644)
+        self.assertPermissions(0100644, file_name)
         dir_name = os.path.join(self.sftp_server._current_upload, 'bar/foo')
         os.makedirs(dir_name)
         upload_file = self.sftp_server.openFile('bar/foo', None, None)

=== modified file 'lib/lp/poppy/twistedsftp.py'
--- lib/lp/poppy/twistedsftp.py	2011-05-09 01:57:28 +0000
+++ lib/lp/poppy/twistedsftp.py	2011-11-30 15:00:52 +0000
@@ -127,8 +127,8 @@
 
     def writeChunk(self, offset, data):
         try:
-            chunk_file = os.open(
-                self.filename, os.O_CREAT | os.O_WRONLY, 0664)
+             chunk_file = os.open(
+                self.filename, os.O_CREAT | os.O_WRONLY, 0644)
         except OSError, e:
             if e.errno != errno.EISDIR:
                 raise