Change in vdsm[master]: tests: janitorial: factor named temp file creation

fromani at redhat.com fromani at redhat.com
Fri Dec 13 16:54:59 UTC 2013


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 <fromani at redhat.com>
---
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)
 
 
+ at 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 <fromani at redhat.com>


More information about the vdsm-patches mailing list