Change in vdsm[ovirt-3.6]: tests: Fix and simplify cleanup after failures

fromani at redhat.com fromani at redhat.com
Thu Oct 15 06:29:08 UTC 2015


Hello Piotr Kliczewski, Sandro Bonazzola, Shmuel Leib Melamud, Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    https://gerrit.ovirt.org/47342

to review the following change.

Change subject: tests: Fix and simplify cleanup after failures
......................................................................

tests: Fix and simplify cleanup after failures

If mount or umount failed, temporary images were not removed because of
incorrect cleanup code. This was not an issue since the temporary files
were created in a temporary directory, but we had two of them, so
failures to remove the first would leave also the second.

This patch simplify cleanup by removing the incorrect cleanup of the
temporary images, as they are removed anyway when the temporary
directory is removed.

Instead of two temporary directories, we use now one directory, and
create the two subdirectories inside it. Now we have to remove only one
directory in tearDown. To make it easier to debug, the temporary
directory is created with a prefix.

Change-Id: Iae829de5ae37008eff3154ad97d5f86c92a41eb2
X-Bug-Url: https://bugzilla.redhat.com/1268229
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
Reviewed-on: https://gerrit.ovirt.org/46387
Continuous-Integration: Jenkins CI
Reviewed-by: Shmuel Leib Melamud <smelamud at redhat.com>
Reviewed-by: Sandro Bonazzola <sbonazzo at redhat.com>
Reviewed-by: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M tests/mkimageTests.py
1 file changed, 6 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/42/47342/1

diff --git a/tests/mkimageTests.py b/tests/mkimageTests.py
index b99dd08..601cad6 100644
--- a/tests/mkimageTests.py
+++ b/tests/mkimageTests.py
@@ -60,8 +60,11 @@
             "DISKIMAGE_GROUP": mkimage.DISKIMAGE_GROUP,
             "_P_PAYLOAD_IMAGES": mkimage._P_PAYLOAD_IMAGES
         }
-        self.workdir = mkdtemp()
-        self.img_dir = mkdtemp()
+        self.tempdir = mkdtemp(prefix="vdsm-mkimage-tests.")
+        self.workdir = os.path.join(self.tempdir, "work")
+        os.mkdir(self.workdir)
+        self.img_dir = os.path.join(self.tempdir, "images")
+        os.mkdir(self.img_dir)
         mkimage.DISKIMAGE_USER = -1
         mkimage.DISKIMAGE_GROUP = -1
         mkimage._P_PAYLOAD_IMAGES = self.img_dir
@@ -82,8 +85,7 @@
         Removes the workdir and its content when finished.
         Restore original values of mkimage constants.
         """
-        rmtree(self.workdir)
-        rmtree(self.img_dir)
+        rmtree(self.tempdir)
         mkimage.DISKIMAGE_USER = self.orig_mkimage["DISKIMAGE_USER"]
         mkimage.DISKIMAGE_GROUP = self.orig_mkimage["DISKIMAGE_GROUP"]
         # pylint: disable=W0212
@@ -174,7 +176,6 @@
             self._check_label(floppy, label)
         finally:
             m.umount(force=True, freeloop=True)
-            os.unlink(floppy)
 
     @permutations([[None], ['fslabel']])
     def test_mkIsoFs(self, label):
@@ -204,7 +205,6 @@
                                      (stat.S_IXOTH, False)))
         finally:
             m.umount(force=True, freeloop=True)
-            os.unlink(iso_img)
 
     def test_removeFs(self):
         """


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iae829de5ae37008eff3154ad97d5f86c92a41eb2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo at redhat.com>
Gerrit-Reviewer: Shmuel Leib Melamud <smelamud at redhat.com>


More information about the vdsm-patches mailing list