Change in vdsm[master]: tests: Fix unsafe monkeypatching

nsoffer at redhat.com nsoffer at redhat.com
Sat Sep 19 00:14:39 UTC 2015


Nir Soffer has uploaded a new change for review.

Change subject: tests: Fix unsafe monkeypatching
......................................................................

tests: Fix unsafe monkeypatching

mkimage module could be left in bad state if setup failed after it was
patched, or removing temporary files in tearDown failed.

This patch ensure that the module is patched only if all other setUp
code was successful, and is restored first in tearDown.

The manual patching code was replaced with the monkeypatch.Patch.

Change-Id: I479fbd03db65e4c9cfa63fe4fe101f1b4b4c3876
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
M tests/mkimageTests.py
1 file changed, 11 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/88/46388/1

diff --git a/tests/mkimageTests.py b/tests/mkimageTests.py
index f32cb73..3805c19 100644
--- a/tests/mkimageTests.py
+++ b/tests/mkimageTests.py
@@ -32,6 +32,7 @@
 from tempfile import mkdtemp
 
 from nose.plugins.skip import SkipTest
+from monkeypatch import Patch
 from testlib import VdsmTestCase, permutations, expandPermutations
 from testValidation import checkSudo, ValidateRunningAsRoot
 
@@ -54,18 +55,9 @@
         by mkimage._commonCleanFs.
         Avoid errors creating _P_PAYLOAD_IMAGES
         """
-        # pylint: disable=W0212
-        self.orig_mkimage = {
-            "DISKIMAGE_USER": mkimage.DISKIMAGE_USER,
-            "DISKIMAGE_GROUP": mkimage.DISKIMAGE_GROUP,
-            "_P_PAYLOAD_IMAGES": mkimage._P_PAYLOAD_IMAGES
-        }
         self.tempdir = mkdtemp(prefix="vdsm-mkimage-tests")
         self.workdir = os.path.join(self.tempdir, "work")
         self.img_dir = os.path.join(self.tempdir, "images")
-        mkimage.DISKIMAGE_USER = -1
-        mkimage.DISKIMAGE_GROUP = -1
-        mkimage._P_PAYLOAD_IMAGES = self.img_dir
         self.files = {}
         self.expected_results = {}
         self.subdir = os.path.join('a', 'subdirectory', 'for', 'testing')
@@ -78,16 +70,22 @@
             self.expected_results[longpath] = content
             self.files[longpath] = b64encode(content)
 
+        self.patch = Patch([
+            (mkimage, "DISKIMAGE_USER", -1),
+            (mkimage, "DISKIMAGE_GROUP", -1),
+            (mkimage, "_P_PAYLOAD_IMAGES", self.img_dir),
+        ])
+
+        # Must be last; if setUp fails, tearDown is not invoked
+        self.patch.apply()
+
     def tearDown(self):
         """
         Removes the workdir and its content when finished.
         Restore original values of mkimage constants.
         """
+        self.patch.revert()
         rmtree(self.tempdir)
-        mkimage.DISKIMAGE_USER = self.orig_mkimage["DISKIMAGE_USER"]
-        mkimage.DISKIMAGE_GROUP = self.orig_mkimage["DISKIMAGE_GROUP"]
-        # pylint: disable=W0212
-        mkimage._P_PAYLOAD_IMAGES = self.orig_mkimage["_P_PAYLOAD_IMAGES"]
 
     def _check_permissions(self, filepath, permsMask):
         """


-- 
To view, visit https://gerrit.ovirt.org/46388
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I479fbd03db65e4c9cfa63fe4fe101f1b4b4c3876
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>


More information about the vdsm-patches mailing list