Francesco Romani has uploaded a new change for review. Change subject: tests: janitorial: factor named temp file creation ......................................................................
tests: janitorial: factor named temp file creation A common pattern in the testsuite is to create a named temporary entry in the filesystem, optionally filled with test data and, less commonly, with adjusted permissions. This patch refactor this behaviour in an handy context manager, named temporaryPath, which also ensures entries are deleted when no longer needed. It is worth to point out that the usage of this context manager can be at least partially replaced with tempfile.NamedTemporyPath; If it's the case, this will be addressed in a future patch. Change-Id: Iddfb6b74f0792e234d58549c6da8fb2ddb5e72fd Signed-off-by: Francesco Romani <[email protected]> --- M tests/fileUtilTests.py M tests/miscTests.py M tests/testrunner.py 3 files changed, 77 insertions(+), 111 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/79/22379/1 diff --git a/tests/fileUtilTests.py b/tests/fileUtilTests.py index 6771727..39d1da9 100644 --- a/tests/fileUtilTests.py +++ b/tests/fileUtilTests.py @@ -17,12 +17,12 @@ # # Refer to the README and COPYING files for full details of the license # -import tempfile import os import storage.fileUtils as fileUtils import testValidation from testrunner import VdsmTestCase as TestCaseBase +from testrunner import temporaryPath class DirectFileTests(TestCaseBase): @@ -35,14 +35,10 @@ platea lacus morbi nisl montes ve. Ac. A, consectetuer erat, justo eu. Elementum et, phasellus fames et rutrum donec magnis eu bibendum. Arcu, ante aliquam ipsum ut facilisis ad.""" - srcFd, srcPath = tempfile.mkstemp() - f = os.fdopen(srcFd, "wb") - f.write(data) - f.flush() - f.close() - with fileUtils.open_ex(srcPath, "dr") as f: - self.assertEquals(f.read(), data) - os.unlink(srcPath) + with temporaryPath(data=data) as srcPath: + # two nested withs to be py2.6 friendly. + with fileUtils.open_ex(srcPath, "dr") as f: + self.assertEquals(f.read(), data) def testSeekRead(self): data = """ @@ -62,29 +58,22 @@ quam. """ self.assertTrue(len(data) > 512) - srcFd, srcPath = tempfile.mkstemp() - f = os.fdopen(srcFd, "wb") - f.write(data) - f.flush() - f.close() - with fileUtils.open_ex(srcPath, "dr") as f: - f.seek(512) - self.assertEquals(f.read(), data[512:]) - os.unlink(srcPath) + with temporaryPath(data=data) as srcPath: + # two nested withs to be py2.6 friendly. + with fileUtils.open_ex(srcPath, "dr") as f: + f.seek(512) + self.assertEquals(f.read(), data[512:]) def testWrite(self): data = """In ut non platea egestas, quisque magnis nunc nostra ac etiam suscipit nec integer sociosqu. Fermentum. Ante orci luctus, ipsum ullamcorper enim arcu class neque inceptos class. Ut, sagittis torquent, commodo facilisi.""" - srcFd, srcPath = tempfile.mkstemp() - os.close(srcFd) - with fileUtils.open_ex(srcPath, "dw") as f: - f.write(data) - - with fileUtils.open_ex(srcPath, "r") as f: - self.assertEquals(f.read(len(data)), data) - os.unlink(srcPath) + with temporaryPath() as srcPath: + with fileUtils.open_ex(srcPath, "dw") as f: + f.write(data) + with fileUtils.open_ex(srcPath, "r") as f: + self.assertEquals(f.read(len(data)), data) def testSmallWrites(self): data = """ @@ -109,15 +98,13 @@ """ self.assertTrue(len(data) > 512) - srcFd, srcPath = tempfile.mkstemp() - os.close(srcFd) - with fileUtils.open_ex(srcPath, "dw") as f: - f.write(data[:512]) - f.write(data[512:]) + with temporaryPath() as srcPath: + with fileUtils.open_ex(srcPath, "dw") as f: + f.write(data[:512]) + f.write(data[512:]) - with fileUtils.open_ex(srcPath, "r") as f: - self.assertEquals(f.read(len(data)), data) - os.unlink(srcPath) + with fileUtils.open_ex(srcPath, "r") as f: + self.assertEquals(f.read(len(data)), data) def testUpdateRead(self): data = """ @@ -137,30 +124,26 @@ """ self.assertTrue(len(data) > 512) - srcFd, srcPath = tempfile.mkstemp() - os.close(srcFd) - with fileUtils.open_ex(srcPath, "wd") as f: - f.write(data[:512]) + with temporaryPath() as srcPath: + with fileUtils.open_ex(srcPath, "wd") as f: + f.write(data[:512]) - with fileUtils.open_ex(srcPath, "r+d") as f: - f.seek(512) - f.write(data[512:]) + with fileUtils.open_ex(srcPath, "r+d") as f: + f.seek(512) + f.write(data[512:]) - with fileUtils.open_ex(srcPath, "r") as f: - self.assertEquals(f.read(len(data)), data) - os.unlink(srcPath) + with fileUtils.open_ex(srcPath, "r") as f: + self.assertEquals(f.read(len(data)), data) class ChownTests(TestCaseBase): @testValidation.ValidateRunningAsRoot def test(self): targetId = 666 - srcFd, srcPath = tempfile.mkstemp() - os.close(srcFd) - fileUtils.chown(srcPath, targetId, targetId) - stat = os.stat(srcPath) - self.assertTrue(stat.st_uid == stat.st_gid == targetId) - os.unlink(srcPath) + with temporaryPath() as srcPath: + fileUtils.chown(srcPath, targetId, targetId) + stat = os.stat(srcPath) + self.assertTrue(stat.st_uid == stat.st_gid == targetId) @testValidation.ValidateRunningAsRoot def testNames(self): @@ -168,15 +151,14 @@ # idea what users are defined and what # there IDs are apart from root tmpId = 666 - srcFd, srcPath = tempfile.mkstemp() - os.close(srcFd) - fileUtils.chown(srcPath, tmpId, tmpId) - stat = os.stat(srcPath) - self.assertTrue(stat.st_uid == stat.st_gid == tmpId) + with temporaryPath() as srcPath: + fileUtils.chown(srcPath, tmpId, tmpId) + stat = os.stat(srcPath) + self.assertTrue(stat.st_uid == stat.st_gid == tmpId) - fileUtils.chown(srcPath, "root", "root") - stat = os.stat(srcPath) - self.assertTrue(stat.st_uid == stat.st_gid == 0) + fileUtils.chown(srcPath, "root", "root") + stat = os.stat(srcPath) + self.assertTrue(stat.st_uid == stat.st_gid == 0) class CopyUserModeToGroupTests(TestCaseBase): @@ -188,13 +170,9 @@ ] def testCopyUserModeToGroup(self): - fd, path = tempfile.mkstemp() - try: - os.close(fd) + with temporaryPath() as path: for initialMode, expectedMode in self.modesList: os.chmod(path, initialMode) fileUtils.copyUserModeToGroup(path) self.assertEquals(os.stat(path).st_mode & self.MODE_MASK, expectedMode) - finally: - os.unlink(path) diff --git a/tests/miscTests.py b/tests/miscTests.py index a8a42f1..fb1191b 100644 --- a/tests/miscTests.py +++ b/tests/miscTests.py @@ -27,6 +27,7 @@ import fcntl import errno from testrunner import VdsmTestCase as TestCaseBase +from testrunner import temporaryPath import inspect from multiprocessing import Process from vdsm import utils @@ -420,26 +421,14 @@ if (len(data) % 512) == 0: data += "!" - srcFd, srcPath = tempfile.mkstemp() - f = os.fdopen(srcFd, "wb") - f.write(data) - f.flush() - f.close() - os.chmod(srcPath, 0o666) + with temporaryPath(perms=0o666, data=data) as srcPath: + with temporaryPath(perms=0o666) as dstPath: + #Copy + rc, out, err = misc.ddWatchCopy(srcPath, dstPath, + None, len(data)) - #Get a tempfilename - dstFd, dstPath = tempfile.mkstemp() - os.chmod(dstPath, 0o666) - - #Copy - rc, out, err = misc.ddWatchCopy(srcPath, dstPath, None, len(data)) - - #Get copied data - readData = open(dstPath).read() - - #clean - os.unlink(dstPath) - os.unlink(srcPath) + #Get copied data + readData = open(dstPath).read() # Compare self.assertEquals(readData, data) @@ -523,26 +512,14 @@ # Makes sure we round up to a complete block size data *= 512 - srcFd, srcPath = tempfile.mkstemp() - f = os.fdopen(srcFd, "wb") - f.write(data) - f.flush() - f.close() - os.chmod(srcPath, 0o666) + with temporaryPath(perms=0o666, data=data) as srcPath: + with temporaryPath(perms=0o666) as dstPath: + #Copy + rc, out, err = misc.ddWatchCopy(srcPath, dstPath, + None, len(data)) - #Get a tempfilename - dstFd, dstPath = tempfile.mkstemp() - os.chmod(dstPath, 0o666) - - #Copy - rc, out, err = misc.ddWatchCopy(srcPath, dstPath, None, len(data)) - - #Get copied data - readData = open(dstPath).read() - - #clean - os.unlink(dstPath) - os.unlink(srcPath) + #Get copied data + readData = open(dstPath).read() #Comapre self.assertEquals(readData, data) @@ -919,16 +896,9 @@ "but to kill myself? That would put a damper on my " "search for answers. Not at all productive.") # (C) Jhonen Vasquez - Johnny the Homicidal Maniac - fd, path = tempfile.mkstemp() - f = os.fdopen(fd, "wb") - f.write(writeData) - f.close() - - #read - readData = misc.readfile(path) - - #clean - os.unlink(path) + with temporaryPath(data=writeData) as path: + #read + readData = misc.readfile(path) self.assertEquals(writeData, readData[0]) diff --git a/tests/testrunner.py b/tests/testrunner.py index f22b67d..773c757 100644 --- a/tests/testrunner.py +++ b/tests/testrunner.py @@ -23,6 +23,8 @@ import unittest from functools import wraps import re +import tempfile +from contextlib import contextmanager from vdsm import utils @@ -123,6 +125,22 @@ stream.write(text) +@contextmanager +def temporaryPath(perms=None, data=None): + fd, src = tempfile.mkstemp() + if data is not None: + f = os.fdopen(fd, "wb") + f.write(data) + f.flush() + f.close() + else: + os.close(fd) + if perms is not None: + os.chmod(src, perms) + yield src + os.unlink(src) + + class VdsmTestCase(unittest.TestCase): def __init__(self, *args, **kwargs): unittest.TestCase.__init__(self, *args, **kwargs) -- To view, visit http://gerrit.ovirt.org/22379 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iddfb6b74f0792e234d58549c6da8fb2ddb5e72fd Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
