diff options
| -rw-r--r-- | gluster/gfapi.py | 152 | ||||
| -rw-r--r-- | test/unit/gluster/test_gfapi.py | 42 | ||||
| -rw-r--r-- | tools/test-requires | 1 |
3 files changed, 110 insertions, 85 deletions
diff --git a/gluster/gfapi.py b/gluster/gfapi.py index 01fdae7..00a7470 100644 --- a/gluster/gfapi.py +++ b/gluster/gfapi.py @@ -6,6 +6,8 @@ import os import sys import types +from contextlib import contextmanager + # Looks like ctypes is having trouble with dependencies, so just force them to # load with RTLD_GLOBAL until I figure that out. glfs = ctypes.CDLL(find_library("glusterfs"), ctypes.RTLD_GLOBAL, @@ -66,28 +68,35 @@ api.glfs_readdir_r.restype = ctypes.c_int api.glfs_readdir_r.argtypes = [ctypes.c_void_p, ctypes.POINTER(Dirent), ctypes.POINTER(ctypes.POINTER(Dirent))] -# There's a bit of ctypes glitchiness around __del__ functions and module-level -# variables. If we unload the module while we still have references to File or -# Volume objects, the module-level variables might have disappeared by the time -# __del__ gets called. Therefore the objects hold references which they -# release when __del__ is done. We only actually use the object-local values -# in __del__; for clarity, we just use the simpler module-level form elsewhere. - class File(object): def __init__(self, fd): - # Add a reference so the module-level variable "api" doesn't - # get yanked out from under us (see comment above File def'n). - self._api = api self.fd = fd - def __del__(self): - self._api.glfs_close(self.fd) - self._api = None - # File operations, in alphabetical order. + def close(self): + ret = api.glfs_close(self.fd) + if ret < 0: + err = ctypes.get_errno() + raise OSError(err, os.strerror(err)) + return ret + + def discard(self, offset, len): + ret = api.glfs_discard(self.fd, offset, len) + if ret < 0: + err = ctypes.get_errno() + raise OSError(err, os.strerror(err)) + return ret + + def fallocate(self, mode, offset, len): + ret = api.glfs_fallocate(self.fd, mode, offset, len) + if ret < 0: + err = ctypes.get_errno() + raise OSError(err, os.strerror(err)) + return ret + def fsync(self): ret = api.glfs_fsync(self.fd) if ret < 0: @@ -113,20 +122,6 @@ class File(object): raise OSError(err, os.strerror(err)) return ret - def fallocate(self, mode, offset, len): - ret = api.glfs_fallocate(self.fd, mode, offset, len) - if ret < 0: - err = ctypes.get_errno() - raise OSError(err, os.strerror(err)) - return ret - - def discard(self, offset, len): - ret = api.glfs_discard(self.fd, offset, len) - if ret < 0: - err = ctypes.get_errno() - raise OSError(err, os.strerror(err)) - return ret - class Dir(object): @@ -176,12 +171,19 @@ class Volume(object): # File operations, in alphabetical order. + @contextmanager def creat(self, path, flags, mode): fd = api.glfs_creat(self.fs, path, flags, mode) if not fd: err = ctypes.get_errno() raise OSError(err, os.strerror(err)) - return File(fd) + + fileobj = None + try: + fileobj = File(fd) + yield fileobj + finally: + fileobj.close() def getxattr(self, path, key, maxlen): buf = ctypes.create_string_buffer(maxlen) @@ -230,12 +232,19 @@ class Volume(object): raise OSError(err, os.strerror(err)) return ret + @contextmanager def open(self, path, flags): fd = api.glfs_open(self.fs, path, flags) if not fd: err = ctypes.get_errno() raise OSError(err, os.strerror(err)) - return File(fd) + + fileobj = None + try: + fileobj = File(fd) + yield fileobj + finally: + fileobj.close() def opendir(self, path): fd = api.glfs_opendir(self.fs, path) @@ -276,27 +285,27 @@ if __name__ == "__main__": def test_create_write(vol, path, data): mypath = path + ".io" - fd = vol.creat(mypath, os.O_WRONLY | os.O_EXCL, 0644) - if not fd: - return False, "creat error" - rc = fd.write(data) - if rc != len(data): - return False, "wrote %d/%d bytes" % (rc, len(data)) - return True, "wrote %d bytes" % rc + with vol.creat(mypath, os.O_WRONLY | os.O_EXCL, 0644) as fd: + if not fd: + return False, "creat error" + rc = fd.write(data) + if rc != len(data): + return False, "wrote %d/%d bytes" % (rc, len(data)) + return True, "wrote %d bytes" % rc # TBD: this test fails if we do create, open, write, read def test_open_read(vol, path, data): mypath = path + ".io" - fd = vol.open(mypath, os.O_RDONLY) - if not fd: - return False, "open error" - dlen = len(data) * 2 - buf = fd.read(dlen) - if isinstance(buf, types.IntType): - return False, "read error %d" % buf - if len(buf) != len(data): - return False, "read %d/%d bytes" % (len(buf), len(data)) - return True, "read '%s'" % buf + with vol.open(mypath, os.O_RDONLY) as fd: + if not fd: + return False, "open error" + dlen = len(data) * 2 + buf = fd.read(dlen) + if isinstance(buf, types.IntType): + return False, "read error %d" % buf + if len(buf) != len(data): + return False, "read %d/%d bytes" % (len(buf), len(data)) + return True, "read '%s'" % buf def test_lstat(vol, path, data): mypath = path + ".io" @@ -315,16 +324,17 @@ if __name__ == "__main__": if rc < 0: return False, "rename error %d" % rc try: - vol.open(opath, os.O_RDWR) + with vol.open(opath, os.O_RDWR) as fd: + return False, "old path working (%s) after rename" % fd except OSError: pass else: return False, "old path working after rename" - nfd = vol.open(npath, os.O_RDWR) - if not isinstance(nfd, File): - return False, "new path not working after rename" - return True, "rename worked" + with vol.open(npath, os.O_RDWR) as nfd: + if not isinstance(nfd, File): + return False, "new path not working after rename" + return True, "rename worked" def test_unlink(vol, path, data): mypath = path + ".tmp" @@ -333,7 +343,8 @@ if __name__ == "__main__": return False, "unlink error %d" % rc try: - vol.open(mypath, os.O_RDWR) + with vol.open(mypath, os.O_RDWR) as fd: + return False, "old path working (%s) after unlink" % fd except OSError: pass else: @@ -351,10 +362,10 @@ if __name__ == "__main__": def test_create_in_dir(vol, path, data): mypath = path + ".dir/probe" - fd = vol.creat(mypath, os.O_RDWR, 0644) - if not isinstance(fd, File): - return False, "create (in dir) error" - return True, "create (in dir) worked" + with vol.creat(mypath, os.O_RDWR, 0644) as fd: + if not isinstance(fd, File): + return False, "create (in dir) error" + return True, "create (in dir) worked" def test_dir_listing(vol, path, data): mypath = path + ".dir" @@ -394,9 +405,10 @@ if __name__ == "__main__": def test_setxattr(vol, path, data): mypath = path + ".xa" - fd = vol.creat(mypath, os.O_RDWR | os.O_EXCL, 0644) - if not fd: - return False, "creat (xattr test) error" + with vol.creat(mypath, os.O_RDWR | os.O_EXCL, 0644) as fd: + if not fd: + return False, "creat (xattr test) error" + key1, key2 = "hello", "goodbye" if vol.setxattr(mypath, "trusted.key1", key1, len(key1)) < 0: return False, "setxattr (key1) error" @@ -424,16 +436,16 @@ if __name__ == "__main__": def test_fallocate(vol, path, data): mypath = path + ".io" - fd = vol.creat(mypath, os.O_WRONLY | os.O_EXCL, 0644) - if not fd: - return False, "creat error" - rc = fd.fallocate(0, 0, 1024 * 1024) - if rc != 0: - return False, "fallocate error" - rc = fd.discard(4096, 4096) - if rc != 0: - return False, "discard error" - return True, "fallocate/discard worked" + with vol.creat(mypath, os.O_WRONLY | os.O_EXCL, 0644) as fd: + if not fd: + return False, "creat error" + rc = fd.fallocate(0, 0, 1024 * 1024) + if rc != 0: + return False, "fallocate error" + rc = fd.discard(4096, 4096) + if rc != 0: + return False, "discard error" + return True, "fallocate/discard worked" test_list = ( test_create_write, diff --git a/test/unit/gluster/test_gfapi.py b/test/unit/gluster/test_gfapi.py index 78f12ba..07c4e92 100644 --- a/test/unit/gluster/test_gfapi.py +++ b/test/unit/gluster/test_gfapi.py @@ -8,7 +8,7 @@ from nose import SkipTest from mock import Mock, patch def _mock_glfs_close(fd): - return + return 0 def _mock_glfs_closedir(fd): return @@ -150,7 +150,7 @@ class TestDir(unittest.TestCase): with patch("gluster.gfapi.api.glfs_readdir_r", mock_glfs_readdir_r): fd = gfapi.Dir(2) ent = fd.next() - self.assertIsInstance(ent, Dirent) + self.assertTrue(isinstance(ent, Dirent)) class TestVolume(unittest.TestCase): @@ -172,7 +172,6 @@ class TestVolume(unittest.TestCase): self._saved_glfs_closedir = gluster.gfapi.api.glfs_closedir gluster.gfapi.api.glfs_closedir = _mock_glfs_closedir - def tearDown(self): gluster.gfapi.api.glfs_new = self._saved_glfs_new gluster.gfapi.api.glfs_set_volfile_server = \ @@ -187,17 +186,24 @@ class TestVolume(unittest.TestCase): with patch("gluster.gfapi.api.glfs_creat", mock_glfs_creat): vol = gfapi.Volume("localhost", "test") - fd = vol.creat("file.txt", os.O_WRONLY, 0644) - self.assertIsInstance(fd, gfapi.File) + with vol.creat("file.txt", os.O_WRONLY, 0644) as fd: + self.assertTrue(isinstance(fd, gfapi.File)) + self.assertEqual(mock_glfs_creat.call_count, 1) + mock_glfs_creat.assert_called_once_with(2, + "file.txt", os.O_WRONLY, 0644) def test_creat_fail_exception(self): mock_glfs_creat = Mock() mock_glfs_creat.return_value = None + def assert_creat(): + with vol.creat("file.txt", os.O_WRONLY, 0644) as fd: + self.assertEqual(fd, None) + + with patch("gluster.gfapi.api.glfs_creat", mock_glfs_creat): vol = gfapi.Volume("localhost", "test") - self.assertRaises(OSError, vol.creat, "file.txt", os.O_WRONLY, - 0644) + self.assertRaises(OSError, assert_creat) def test_getxattr_success(self): def mock_glfs_getxattr(fs, path, key, buf, maxlen): @@ -225,9 +231,8 @@ class TestVolume(unittest.TestCase): with patch("gluster.gfapi.api.glfs_listxattr", mock_glfs_listxattr): vol = gfapi.Volume("localhost", "test") xattrs = vol.listxattr("file.txt") - self.assertIn("key1", xattrs) - self.assertIn("key2", xattrs) - self.assertNotIn("key", xattrs) + self.assertTrue("key1" in xattrs) + self.assertTrue("key2" in xattrs) def test_listxattr_fail_exception(self): mock_glfs_listxattr = Mock() @@ -244,7 +249,7 @@ class TestVolume(unittest.TestCase): with patch("gluster.gfapi.api.glfs_lstat", mock_glfs_lstat): vol = gfapi.Volume("localhost", "test") stat = vol.lstat("file.txt") - self.assertIsInstance(stat, gfapi.Stat, stat) + self.assertTrue(isinstance(stat, gfapi.Stat)) def test_lstat_fail_exception(self): mock_glfs_lstat = Mock() @@ -277,16 +282,23 @@ class TestVolume(unittest.TestCase): with patch("gluster.gfapi.api.glfs_open", mock_glfs_open): vol = gfapi.Volume("localhost", "test") - f = vol.open("file.txt", os.O_RDONLY) - self.assertIsInstance(f, gfapi.File) + with vol.open("file.txt", os.O_WRONLY) as fd: + self.assertTrue(isinstance(fd, gfapi.File)) + self.assertEqual(mock_glfs_open.call_count, 1) + mock_glfs_open.assert_called_once_with(2, + "file.txt", os.O_WRONLY) def test_open_fail_exception(self): mock_glfs_open = Mock() mock_glfs_open.return_value = None + def assert_open(): + with vol.open("file.txt", os.O_WRONLY) as fd: + self.assertEqual(fd, None) + with patch("gluster.gfapi.api.glfs_open", mock_glfs_open): vol = gfapi.Volume("localhost", "test") - self.assertRaises(OSError, vol.open, "file.txt", os.O_RDONLY) + self.assertRaises(OSError, assert_open) def test_opendir_success(self): mock_glfs_opendir = Mock() @@ -295,7 +307,7 @@ class TestVolume(unittest.TestCase): with patch("gluster.gfapi.api.glfs_opendir", mock_glfs_opendir): vol = gfapi.Volume("localhost", "test") d = vol.opendir("testdir") - self.assertIsInstance(d, gfapi.Dir) + self.assertTrue(isinstance(d, gfapi.Dir)) def test_opendir_fail_exception(self): mock_glfs_opendir = Mock() diff --git a/tools/test-requires b/tools/test-requires index 0d4914c..fdebbb6 100644 --- a/tools/test-requires +++ b/tools/test-requires @@ -6,6 +6,7 @@ hacking>=0.5.6,<0.6 coverage nose nosexcover +openstack.nose_plugin nosehtmloutput sphinx>=1.1.2 mock>=0.8.0 |