[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v3 4/5] qemu-iotests: make python tests attempt
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH v3 4/5] qemu-iotests: make python tests attempt to leave intermediate files |
Date: |
Wed, 30 Aug 2017 13:33:19 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 08/30/2017 11:52 AM, Jeff Cody wrote:
> Now that 'check' will clean up after tests, try and make python
> tests leave intermediate files so that they might be inspectable
> on failure.
>
> This isn't perfect; the python unittest framework runs multiple
> tests, even if previous tests failed. So we need to make sure that
> each test still begins with a "clean" slate, to prevent false
> positives or tainted test runs.
>
> Rather than delete images in the unittest tearDown, invert this
> and delete images to be used in that test at the beginning of the
> setUp. This is to make sure that the test run is not inadvertently
> using file droppings from previous runs. We must use 'blind_remove'
> then for these, as the files might not exist yet, but we don't want
> to throw an error for that.
>
> Signed-off-by: Jeff Cody <address@hidden>
> ---
> +++ b/tests/qemu-iotests/030
> @@ -21,7 +21,7 @@
> import time
> import os
> import iotests
> -from iotests import qemu_img, qemu_io
> +from iotests import qemu_img, qemu_io, blind_remove
>
> backing_img = os.path.join(iotests.test_dir, 'backing.img')
> mid_img = os.path.join(iotests.test_dir, 'mid.img')
> @@ -31,6 +31,9 @@ class TestSingleDrive(iotests.QMPTestCase):
> image_len = 1 * 1024 * 1024 # MB
>
> def setUp(self):
> + blind_remove(test_img)
> + blind_remove(mid_img)
> + blind_remove(backing_img)
Would it be any more pythonic to have support for:
blind_remove(test_img, mid_img, backing_img)
built into the previous patch?
> def tearDown(self):
> self.vm.shutdown()
> - os.remove(self.test_img)
> - os.remove(self.mid_img_abs)
> - os.remove(self.backing_img_abs)
> - try:
> - os.rmdir(os.path.join(iotests.test_dir, self.dir1))
> - os.rmdir(os.path.join(iotests.test_dir, self.dir3))
> - os.rmdir(os.path.join(iotests.test_dir, self.dir2))
> - except OSError as exception:
> - if exception.errno != errno.EEXIST and exception.errno !=
> errno.ENOTEMPTY:
> - raise
The code removed here is using a syntax that differs from what you used
in 3/5 when defining blind_remove; does that matter for 3/5?
> +++ b/tests/qemu-iotests/041
> + blind_remove(target_img)
> iotests.create_image(backing_img, self.image_len)
> qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' %
> backing_img, test_img)
> self.vm = iotests.VM().add_drive(test_img,
> "node-name=top,backing.node-name=base")
> @@ -49,12 +52,6 @@ class TestSingleDrive(iotests.QMPTestCase):
>
> def tearDown(self):
> self.vm.shutdown()
> - os.remove(test_img)
> - os.remove(backing_img)
> - try:
> - os.remove(target_img)
> - except OSError:
> - pass
You're changing failures other than ENOENT from ignored to explicit -
nice little bug-fix along the way :) I notice this pattern in multiple
tests; is it worth mentioning in the commit message as intentional?
> @@ -797,6 +788,9 @@ class TestRepairQuorum(iotests.QMPTestCase):
> IMAGES = [ quorum_img1, quorum_img2, quorum_img3 ]
>
> def setUp(self):
> + for i in self.IMAGES + [ quorum_repair_img, quorum_snapshot_file ]:
> + blind_remove(i)
Again, would it be more pythonic if blind_remove() could take a list and
automatically work on each element of the list, rather than having to
make the caller iterate?
> +++ b/tests/qemu-iotests/057
> @@ -23,7 +23,7 @@
> import time
> import os
> import iotests
> -from iotests import qemu_img, qemu_io
> +from iotests import qemu_img, qemu_io, blind_remove
>
> test_drv_base_name = 'drive'
>
> @@ -36,6 +36,8 @@ class ImageSnapshotTestCase(iotests.QMPTestCase):
>
> def _setUp(self, test_img_base_name, image_num):
> self.vm = iotests.VM()
> + for dev_expect in self.expect:
> + blind_remove(dev_expect['image'])
Another place where python magic could make the caller nicer?
> +++ b/tests/qemu-iotests/118
> @@ -411,16 +411,16 @@ class TestFloppyInitiallyEmpty(TestInitiallyEmpty):
>
> class TestChangeReadOnly(ChangeBaseClass):
> def setUp(self):
> - qemu_img('create', '-f', iotests.imgfmt, old_img, '1440k')
> - qemu_img('create', '-f', iotests.imgfmt, new_img, '1440k')
> - self.vm = iotests.VM()
> -
> - def tearDown(self):
> - self.vm.shutdown()
> os.chmod(old_img, 0666)
> os.chmod(new_img, 0666)
> - os.remove(old_img)
> - os.remove(new_img)
> + blind_remove(old_img)
> + blind_remove(new_img)
> + qemu_img('create', '-f', iotests.imgfmt, old_img, '1440k')
> + qemu_img('create', '-f', iotests.imgfmt, new_img, '1440k')
> + self.vm = iotests.VM()
> +
> + def tearDown(self):
> + self.vm.shutdown()
The script framework doesn't have any problem removing left-over
read-only files, correct? (If it does, then earlier in the series you
may need to add 'chmod -R u+rwx scratch/$seq' prior to its removal?)
But overall, I didn't see any problems, so I'm okay with:
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH v3 1/5] qemu-iotests: set TEST_DIR to a unique dir for each test, (continued)
- [Qemu-block] [PATCH v3 1/5] qemu-iotests: set TEST_DIR to a unique dir for each test, Jeff Cody, 2017/08/30
- [Qemu-block] [PATCH v3 3/5] qemu-iotests: add 'blind_remove' for python tests, Jeff Cody, 2017/08/30
- [Qemu-block] [PATCH v3 5/5] qemu-iotests: add option to save temp files on error, Jeff Cody, 2017/08/30
- [Qemu-block] [PATCH v3 4/5] qemu-iotests: make python tests attempt to leave intermediate files, Jeff Cody, 2017/08/30
- Re: [Qemu-block] [PATCH v3 4/5] qemu-iotests: make python tests attempt to leave intermediate files,
Eric Blake <=
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests attempt to leave intermediate files, John Snow, 2017/08/30
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests attempt to leave intermediate files, Eric Blake, 2017/08/30
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests attempt to leave intermediate files, John Snow, 2017/08/30
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests attempt to leave intermediate files, Stefan Hajnoczi, 2017/08/31
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests attempt to leave intermediate files, Jeff Cody, 2017/08/31
[Qemu-block] [PATCH v3 2/5] qemu-iotests: remove file cleanup from bash tests, Jeff Cody, 2017/08/30