[{"id":1760399,"web_url":"http://patchwork.ozlabs.org/comment/1760399/","msgid":"<1646044f-3c57-c415-f261-463c74ea45b8@redhat.com>","list_archive_url":null,"date":"2017-08-30T18:33:19","subject":"Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests\n\tattempt to leave intermediate files","submitter":{"id":6591,"url":"http://patchwork.ozlabs.org/api/people/6591/","name":"Eric Blake","email":"eblake@redhat.com"},"content":"On 08/30/2017 11:52 AM, Jeff Cody wrote:\n> Now that 'check' will clean up after tests, try and make python\n> tests leave intermediate files so that they might be inspectable\n> on failure.\n> \n> This isn't perfect; the python unittest framework runs multiple\n> tests, even if previous tests failed. So we need to make sure that\n> each test still begins with a \"clean\" slate, to prevent false\n> positives or tainted test runs.\n> \n> Rather than delete images in the unittest tearDown, invert this\n> and delete images to be used in that test at the beginning of the\n> setUp. This is to make sure that the test run is not inadvertently\n> using file droppings from previous runs. We must use 'blind_remove'\n> then for these, as the files might not exist yet, but we don't want\n> to throw an error for that.\n> \n> Signed-off-by: Jeff Cody \n> ---\n\n> +++ b/tests/qemu-iotests/030\n> @@ -21,7 +21,7 @@\n> import time\n> import os\n> import iotests\n> -from iotests import qemu_img, qemu_io\n> +from iotests import qemu_img, qemu_io, blind_remove\n> \n> backing_img = os.path.join(iotests.test_dir, 'backing.img')\n> mid_img = os.path.join(iotests.test_dir, 'mid.img')\n> @@ -31,6 +31,9 @@ class TestSingleDrive(iotests.QMPTestCase):\n> image_len = 1 * 1024 * 1024 # MB\n> \n> def setUp(self):\n> + blind_remove(test_img)\n> + blind_remove(mid_img)\n> + blind_remove(backing_img)\n\nWould it be any more pythonic to have support for:\n\nblind_remove(test_img, mid_img, backing_img)\n\nbuilt into the previous patch?\n\n> def tearDown(self):\n> self.vm.shutdown()\n> - os.remove(self.test_img)\n> - os.remove(self.mid_img_abs)\n> - os.remove(self.backing_img_abs)\n> - try:\n> - os.rmdir(os.path.join(iotests.test_dir, self.dir1))\n> - os.rmdir(os.path.join(iotests.test_dir, self.dir3))\n> - os.rmdir(os.path.join(iotests.test_dir, self.dir2))\n> - except OSError as exception:\n> - if exception.errno != errno.EEXIST and exception.errno != errno.ENOTEMPTY:\n> - raise\n\nThe code removed here is using a syntax that differs from what you used\nin 3/5 when defining blind_remove; does that matter for 3/5?\n\n> +++ b/tests/qemu-iotests/041\n\n> + blind_remove(target_img)\n> iotests.create_image(backing_img, self.image_len)\n> qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)\n> self.vm = iotests.VM().add_drive(test_img, \"node-name=top,backing.node-name=base\")\n> @@ -49,12 +52,6 @@ class TestSingleDrive(iotests.QMPTestCase):\n> \n> def tearDown(self):\n> self.vm.shutdown()\n> - os.remove(test_img)\n> - os.remove(backing_img)\n> - try:\n> - os.remove(target_img)\n> - except OSError:\n> - pass\n\nYou're changing failures other than ENOENT from ignored to explicit -\nnice little bug-fix along the way :) I notice this pattern in multiple\ntests; is it worth mentioning in the commit message as intentional?\n\n> @@ -797,6 +788,9 @@ class TestRepairQuorum(iotests.QMPTestCase):\n> IMAGES = [ quorum_img1, quorum_img2, quorum_img3 ]\n> \n> def setUp(self):\n> + for i in self.IMAGES + [ quorum_repair_img, quorum_snapshot_file ]:\n> + blind_remove(i)\n\nAgain, would it be more pythonic if blind_remove() could take a list and\nautomatically work on each element of the list, rather than having to\nmake the caller iterate?\n\n> +++ b/tests/qemu-iotests/057\n> @@ -23,7 +23,7 @@\n> import time\n> import os\n> import iotests\n> -from iotests import qemu_img, qemu_io\n> +from iotests import qemu_img, qemu_io, blind_remove\n> \n> test_drv_base_name = 'drive'\n> \n> @@ -36,6 +36,8 @@ class ImageSnapshotTestCase(iotests.QMPTestCase):\n> \n> def _setUp(self, test_img_base_name, image_num):\n> self.vm = iotests.VM()\n> + for dev_expect in self.expect:\n> + blind_remove(dev_expect['image'])\n\nAnother place where python magic could make the caller nicer?\n\n> +++ b/tests/qemu-iotests/118\n\n> @@ -411,16 +411,16 @@ class TestFloppyInitiallyEmpty(TestInitiallyEmpty):\n> \n> class TestChangeReadOnly(ChangeBaseClass):\n> def setUp(self):\n> - qemu_img('create', '-f', iotests.imgfmt, old_img, '1440k')\n> - qemu_img('create', '-f', iotests.imgfmt, new_img, '1440k')\n> - self.vm = iotests.VM()\n> -\n> - def tearDown(self):\n> - self.vm.shutdown()\n> os.chmod(old_img, 0666)\n> os.chmod(new_img, 0666)\n> - os.remove(old_img)\n> - os.remove(new_img)\n> + blind_remove(old_img)\n> + blind_remove(new_img)\n> + qemu_img('create', '-f', iotests.imgfmt, old_img, '1440k')\n> + qemu_img('create', '-f', iotests.imgfmt, new_img, '1440k')\n> + self.vm = iotests.VM()\n> +\n> + def tearDown(self):\n> + self.vm.shutdown()\n\nThe script framework doesn't have any problem removing left-over\nread-only files, correct? (If it does, then earlier in the series you\nmay need to add 'chmod -R u+rwx scratch/$seq' prior to its removal?)\n\nBut overall, I didn't see any problems, so I'm okay with:\nReviewed-by: Eric Blake ","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=eblake@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjDfz0tNPz9sNw\n\tfor ;\n\tThu, 31 Aug 2017 04:34:01 +1000 (AEST)","from localhost ([::1]:52173 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dn7oN-0001Zp-5h\n\tfor incoming@patchwork.ozlabs.org; Wed, 30 Aug 2017 14:33:59 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:59469)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dn7nx-0001YL-IA\n\tfor qemu-devel@nongnu.org; Wed, 30 Aug 2017 14:33:35 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dn7nw-0002Lu-79\n\tfor qemu-devel@nongnu.org; Wed, 30 Aug 2017 14:33:33 -0400","from mx1.redhat.com ([209.132.183.28]:42552)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dn7nq-0002Jt-8g; Wed, 30 Aug 2017 14:33:26 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 0549483F3E;\n\tWed, 30 Aug 2017 18:33:25 +0000 (UTC)","from [10.10.122.186] (ovpn-122-186.rdu2.redhat.com [10.10.122.186])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 15588708EC;\n\tWed, 30 Aug 2017 18:33:19 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 0549483F3E","To":"Jeff Cody , qemu-devel@nongnu.org","References":"\n\t","From":"Eric Blake ","Openpgp":"url=http://people.redhat.com/eblake/eblake.gpg","Organization":"Red Hat, Inc.","Message-ID":"<1646044f-3c57-c415-f261-463c74ea45b8@redhat.com>","Date":"Wed, 30 Aug 2017 13:33:19 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"Ir754VkvMNOCfRA4h5Cw1rAsarVxTv4l8\"","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.27]);\n\tWed, 30 Aug 2017 18:33:25 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","X-Content-Filtered-By":"Mailman/MimeDel 2.1.21","Subject":"Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests\n\tattempt to leave intermediate files","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"stefanha@redhat.com, kwolf@redhat.com, jsnow@redhat.com,\n\tarmbru@redhat.com, qemu-block@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1760540,"web_url":"http://patchwork.ozlabs.org/comment/1760540/","msgid":"<2690a268-b5be-7974-e8ef-c5a2f09c72a5@redhat.com>","list_archive_url":null,"date":"2017-08-30T22:28:30","subject":"Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests\n\tattempt to leave intermediate files","submitter":{"id":64343,"url":"http://patchwork.ozlabs.org/api/people/64343/","name":"John Snow","email":"jsnow@redhat.com"},"content":"On 08/30/2017 02:33 PM, Eric Blake wrote:\n> On 08/30/2017 11:52 AM, Jeff Cody wrote:\n>> Now that 'check' will clean up after tests, try and make python\n>> tests leave intermediate files so that they might be inspectable\n>> on failure.\n>>\n>> This isn't perfect; the python unittest framework runs multiple\n>> tests, even if previous tests failed. So we need to make sure that\n>> each test still begins with a \"clean\" slate, to prevent false\n>> positives or tainted test runs.\n>>\n>> Rather than delete images in the unittest tearDown, invert this\n>> and delete images to be used in that test at the beginning of the\n>> setUp. This is to make sure that the test run is not inadvertently\n>> using file droppings from previous runs. We must use 'blind_remove'\n>> then for these, as the files might not exist yet, but we don't want\n>> to throw an error for that.\n>>\n>> Signed-off-by: Jeff Cody \n>> ---\n> \n>> +++ b/tests/qemu-iotests/030\n>> @@ -21,7 +21,7 @@\n>> import time\n>> import os\n>> import iotests\n>> -from iotests import qemu_img, qemu_io\n>> +from iotests import qemu_img, qemu_io, blind_remove\n>> \n>> backing_img = os.path.join(iotests.test_dir, 'backing.img')\n>> mid_img = os.path.join(iotests.test_dir, 'mid.img')\n>> @@ -31,6 +31,9 @@ class TestSingleDrive(iotests.QMPTestCase):\n>> image_len = 1 * 1024 * 1024 # MB\n>> \n>> def setUp(self):\n>> + blind_remove(test_img)\n>> + blind_remove(mid_img)\n>> + blind_remove(backing_img)\n> \n> Would it be any more pythonic to have support for:\n> \n> blind_remove(test_img, mid_img, backing_img)\n> \n> built into the previous patch?\n> \n\nIt should probably either take an iterable, or an arbitrary number of\narguments, or both, I dunno. I'm not a python.\n\n>> def tearDown(self):\n>> self.vm.shutdown()\n>> - os.remove(self.test_img)\n>> - os.remove(self.mid_img_abs)\n>> - os.remove(self.backing_img_abs)\n>> - try:\n>> - os.rmdir(os.path.join(iotests.test_dir, self.dir1))\n>> - os.rmdir(os.path.join(iotests.test_dir, self.dir3))\n>> - os.rmdir(os.path.join(iotests.test_dir, self.dir2))\n>> - except OSError as exception:\n>> - if exception.errno != errno.EEXIST and exception.errno != errno.ENOTEMPTY:\n>> - raise\n> \n> The code removed here is using a syntax that differs from what you used\n> in 3/5 when defining blind_remove; does that matter for 3/5?\n> \n>> +++ b/tests/qemu-iotests/041\n> \n>> + blind_remove(target_img)\n>> iotests.create_image(backing_img, self.image_len)\n>> qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)\n>> self.vm = iotests.VM().add_drive(test_img, \"node-name=top,backing.node-name=base\")\n>> @@ -49,12 +52,6 @@ class TestSingleDrive(iotests.QMPTestCase):\n>> \n>> def tearDown(self):\n>> self.vm.shutdown()\n>> - os.remove(test_img)\n>> - os.remove(backing_img)\n>> - try:\n>> - os.remove(target_img)\n>> - except OSError:\n>> - pass\n> \n> You're changing failures other than ENOENT from ignored to explicit -\n> nice little bug-fix along the way :) I notice this pattern in multiple\n> tests; is it worth mentioning in the commit message as intentional?\n> \n>> @@ -797,6 +788,9 @@ class TestRepairQuorum(iotests.QMPTestCase):\n>> IMAGES = [ quorum_img1, quorum_img2, quorum_img3 ]\n>> \n>> def setUp(self):\n>> + for i in self.IMAGES + [ quorum_repair_img, quorum_snapshot_file ]:\n>> + blind_remove(i)\n> \n> Again, would it be more pythonic if blind_remove() could take a list and\n> automatically work on each element of the list, rather than having to\n> make the caller iterate?\n> \n>> +++ b/tests/qemu-iotests/057\n>> @@ -23,7 +23,7 @@\n>> import time\n>> import os\n>> import iotests\n>> -from iotests import qemu_img, qemu_io\n>> +from iotests import qemu_img, qemu_io, blind_remove\n>> \n>> test_drv_base_name = 'drive'\n>> \n>> @@ -36,6 +36,8 @@ class ImageSnapshotTestCase(iotests.QMPTestCase):\n>> \n>> def _setUp(self, test_img_base_name, image_num):\n>> self.vm = iotests.VM()\n>> + for dev_expect in self.expect:\n>> + blind_remove(dev_expect['image'])\n> \n> Another place where python magic could make the caller nicer?\n> \n>> +++ b/tests/qemu-iotests/118\n> \n>> @@ -411,16 +411,16 @@ class TestFloppyInitiallyEmpty(TestInitiallyEmpty):\n>> \n>> class TestChangeReadOnly(ChangeBaseClass):\n>> def setUp(self):\n>> - qemu_img('create', '-f', iotests.imgfmt, old_img, '1440k')\n>> - qemu_img('create', '-f', iotests.imgfmt, new_img, '1440k')\n>> - self.vm = iotests.VM()\n>> -\n>> - def tearDown(self):\n>> - self.vm.shutdown()\n>> os.chmod(old_img, 0666)\n>> os.chmod(new_img, 0666)\n>> - os.remove(old_img)\n>> - os.remove(new_img)\n>> + blind_remove(old_img)\n>> + blind_remove(new_img)\n>> + qemu_img('create', '-f', iotests.imgfmt, old_img, '1440k')\n>> + qemu_img('create', '-f', iotests.imgfmt, new_img, '1440k')\n>> + self.vm = iotests.VM()\n>> +\n>> + def tearDown(self):\n>> + self.vm.shutdown()\n> \n> The script framework doesn't have any problem removing left-over\n> read-only files, correct? (If it does, then earlier in the series you\n> may need to add 'chmod -R u+rwx scratch/$seq' prior to its removal?)\n> \n> But overall, I didn't see any problems, so I'm okay with:\n> Reviewed-by: Eric Blake \n> \n\nI'm a little iffy on this patch; I know that ./check can take care of\nour temp files for us now, but because each python test is itself a\nlittle mini-harness, I'm a little leery of moving the teardown to setup\nand trying to pre-clean the confetti before the test begins.\n\nWhat's the benefit? We still have to clean up these files per-test, but\nnow it's slightly more error-prone and in a weird place.\n\nIf we want to try to preserve the most-recent-failure-files, perhaps we\ncan define a setting in the python test-runner that allows us to\nglobally skip file cleanup.\n\n--js","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=jsnow@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjKtH2DNVz9s83\n\tfor ;\n\tThu, 31 Aug 2017 08:29:08 +1000 (AEST)","from localhost ([::1]:53026 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnBTu-00028W-8l\n\tfor incoming@patchwork.ozlabs.org; Wed, 30 Aug 2017 18:29:06 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:56692)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dnBTV-00026U-Pl\n\tfor qemu-devel@nongnu.org; Wed, 30 Aug 2017 18:28:43 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dnBTU-00024I-EO\n\tfor qemu-devel@nongnu.org; Wed, 30 Aug 2017 18:28:41 -0400","from mx1.redhat.com ([209.132.183.28]:45622)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dnBTP-00022x-GG; Wed, 30 Aug 2017 18:28:35 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 4C6BA883BB;\n\tWed, 30 Aug 2017 22:28:34 +0000 (UTC)","from [10.18.17.130] (dhcp-17-130.bos.redhat.com [10.18.17.130])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 3EB2517494;\n\tWed, 30 Aug 2017 22:28:31 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 4C6BA883BB","To":"Eric Blake , Jeff Cody ,\n\tqemu-devel@nongnu.org","References":"\n\t\n\t<1646044f-3c57-c415-f261-463c74ea45b8@redhat.com>","From":"John Snow ","Message-ID":"<2690a268-b5be-7974-e8ef-c5a2f09c72a5@redhat.com>","Date":"Wed, 30 Aug 2017 18:28:30 -0400","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<1646044f-3c57-c415-f261-463c74ea45b8@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.26]);\n\tWed, 30 Aug 2017 22:28:34 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests\n\tattempt to leave intermediate files","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, qemu-block@nongnu.org, armbru@redhat.com,\n\tstefanha@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1760546,"web_url":"http://patchwork.ozlabs.org/comment/1760546/","msgid":"","list_archive_url":null,"date":"2017-08-30T22:35:51","subject":"Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests\n\tattempt to leave intermediate files","submitter":{"id":6591,"url":"http://patchwork.ozlabs.org/api/people/6591/","name":"Eric Blake","email":"eblake@redhat.com"},"content":"On 08/30/2017 05:28 PM, John Snow wrote:\n\n> I'm a little iffy on this patch; I know that ./check can take care of\n> our temp files for us now, but because each python test is itself a\n> little mini-harness, I'm a little leery of moving the teardown to setup\n> and trying to pre-clean the confetti before the test begins.\n> \n> What's the benefit? We still have to clean up these files per-test, but\n> now it's slightly more error-prone and in a weird place.\n> \n> If we want to try to preserve the most-recent-failure-files, perhaps we\n> can define a setting in the python test-runner that allows us to\n> globally skip file cleanup.\n\nOn the other hand, since each test is a mini-harness, globally skipping\ncleanup will make a two-part test fail on the second because of garbage\nleft behind by the first.\n\nPatch 5 adds a comment with another possible solution: teach the python\nmini-harness to either clean all files in the directory, or to relocate\nthe directory according to test name, so that each mini-test starts with\na fresh location, and cleanup is then handled by the harness rather than\nspaghetti pre-cleanup. But any solution is better than our current\nsituation of nothing, so that's why I'm still okay with this patch as-is\nas offering more (even if not perfect) than before.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=eblake@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjL2p17vQz9s7G\n\tfor ;\n\tThu, 31 Aug 2017 08:36:32 +1000 (AEST)","from localhost ([::1]:53049 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnBb1-0004Zo-P2\n\tfor incoming@patchwork.ozlabs.org; Wed, 30 Aug 2017 18:36:30 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:58073)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dnBae-0004Wy-3S\n\tfor qemu-devel@nongnu.org; Wed, 30 Aug 2017 18:36:05 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dnBac-0004hg-UE\n\tfor qemu-devel@nongnu.org; Wed, 30 Aug 2017 18:36:04 -0400","from mx1.redhat.com ([209.132.183.28]:54016)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dnBaY-0004cw-6B; Wed, 30 Aug 2017 18:35:58 -0400","from smtp.corp.redhat.com\n\t(int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id E1A2B883BB;\n\tWed, 30 Aug 2017 22:35:56 +0000 (UTC)","from [10.10.122.186] (ovpn-122-186.rdu2.redhat.com [10.10.122.186])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 1B0415DD64;\n\tWed, 30 Aug 2017 22:35:51 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com E1A2B883BB","To":"John Snow , Jeff Cody ,\n\tqemu-devel@nongnu.org","References":"\n\t\n\t<1646044f-3c57-c415-f261-463c74ea45b8@redhat.com>\n\t<2690a268-b5be-7974-e8ef-c5a2f09c72a5@redhat.com>","From":"Eric Blake ","Openpgp":"url=http://people.redhat.com/eblake/eblake.gpg","Organization":"Red Hat, Inc.","Message-ID":"","Date":"Wed, 30 Aug 2017 17:35:51 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<2690a268-b5be-7974-e8ef-c5a2f09c72a5@redhat.com>","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"OnT6MpXJeFeaeWROaU7btgmxC72Jeu89C\"","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.14","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.26]);\n\tWed, 30 Aug 2017 22:35:57 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","X-Content-Filtered-By":"Mailman/MimeDel 2.1.21","Subject":"Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests\n\tattempt to leave intermediate files","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, qemu-block@nongnu.org, armbru@redhat.com,\n\tstefanha@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1760551,"web_url":"http://patchwork.ozlabs.org/comment/1760551/","msgid":"<494b96d5-d2a2-957e-3169-7fb2461b59de@redhat.com>","list_archive_url":null,"date":"2017-08-30T22:40:29","subject":"Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests\n\tattempt to leave intermediate files","submitter":{"id":64343,"url":"http://patchwork.ozlabs.org/api/people/64343/","name":"John Snow","email":"jsnow@redhat.com"},"content":"On 08/30/2017 06:35 PM, Eric Blake wrote:\n> On 08/30/2017 05:28 PM, John Snow wrote:\n> \n>> I'm a little iffy on this patch; I know that ./check can take care of\n>> our temp files for us now, but because each python test is itself a\n>> little mini-harness, I'm a little leery of moving the teardown to setup\n>> and trying to pre-clean the confetti before the test begins.\n>>\n>> What's the benefit? We still have to clean up these files per-test, but\n>> now it's slightly more error-prone and in a weird place.\n>>\n>> If we want to try to preserve the most-recent-failure-files, perhaps we\n>> can define a setting in the python test-runner that allows us to\n>> globally skip file cleanup.\n> \n> On the other hand, since each test is a mini-harness, globally skipping\n> cleanup will make a two-part test fail on the second because of garbage\n> left behind by the first.\n> \n\nsubtext was to have per-subtest files.\n\n> Patch 5 adds a comment with another possible solution: teach the python\n> mini-harness to either clean all files in the directory, or to relocate\n> the directory according to test name, so that each mini-test starts with\n> a fresh location, and cleanup is then handled by the harness rather than\n> spaghetti pre-cleanup. But any solution is better than our current\n> situation of nothing, so that's why I'm still okay with this patch as-is\n> as offering more (even if not perfect) than before.\n> \n\nI guess where I am unsure is really if this is better than what we\ncurrently do, which is to (try) to clean up after each test as best as\nwe can. I don't see it as too different from trying to clean up before\neach test.\n\nIt does give us the ability to leave behind a little detritus after a\nfailed run, but it's so imperfect that I wonder if it's worth shifting\nthis code around to change not much.\n\nI won't die on this hill, it just strikes me a slightly less intuitive\nuse of the python unittest framework.\n\n--js","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=jsnow@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjL8D1npKz9s8J\n\tfor ;\n\tThu, 31 Aug 2017 08:41:16 +1000 (AEST)","from localhost ([::1]:53066 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnBfe-0007Yj-Cy\n\tfor incoming@patchwork.ozlabs.org; Wed, 30 Aug 2017 18:41:14 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:59548)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dnBf7-0007XL-N2\n\tfor qemu-devel@nongnu.org; Wed, 30 Aug 2017 18:40:43 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dnBf6-0006So-Tp\n\tfor qemu-devel@nongnu.org; Wed, 30 Aug 2017 18:40:41 -0400","from mx1.redhat.com ([209.132.183.28]:53356)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dnBf0-0006Da-6a; Wed, 30 Aug 2017 18:40:34 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 3D329356C7;\n\tWed, 30 Aug 2017 22:40:33 +0000 (UTC)","from [10.18.17.130] (dhcp-17-130.bos.redhat.com [10.18.17.130])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id BBCD66A317;\n\tWed, 30 Aug 2017 22:40:29 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 3D329356C7","To":"Eric Blake , Jeff Cody ,\n\tqemu-devel@nongnu.org","References":"\n\t\n\t<1646044f-3c57-c415-f261-463c74ea45b8@redhat.com>\n\t<2690a268-b5be-7974-e8ef-c5a2f09c72a5@redhat.com>\n\t","From":"John Snow ","Message-ID":"<494b96d5-d2a2-957e-3169-7fb2461b59de@redhat.com>","Date":"Wed, 30 Aug 2017 18:40:29 -0400","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tWed, 30 Aug 2017 22:40:33 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests\n\tattempt to leave intermediate files","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, qemu-block@nongnu.org, armbru@redhat.com,\n\tstefanha@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1761132,"web_url":"http://patchwork.ozlabs.org/comment/1761132/","msgid":"<20170831153949.GA19001@stefanha-x1.localdomain>","list_archive_url":null,"date":"2017-08-31T15:39:49","subject":"Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests\n\tattempt to leave intermediate files","submitter":{"id":17227,"url":"http://patchwork.ozlabs.org/api/people/17227/","name":"Stefan Hajnoczi","email":"stefanha@redhat.com"},"content":"On Wed, Aug 30, 2017 at 06:40:29PM -0400, John Snow wrote:\n> \n> \n> On 08/30/2017 06:35 PM, Eric Blake wrote:\n> > On 08/30/2017 05:28 PM, John Snow wrote:\n> > \n> >> I'm a little iffy on this patch; I know that ./check can take care of\n> >> our temp files for us now, but because each python test is itself a\n> >> little mini-harness, I'm a little leery of moving the teardown to setup\n> >> and trying to pre-clean the confetti before the test begins.\n> >>\n> >> What's the benefit? We still have to clean up these files per-test, but\n> >> now it's slightly more error-prone and in a weird place.\n> >>\n> >> If we want to try to preserve the most-recent-failure-files, perhaps we\n> >> can define a setting in the python test-runner that allows us to\n> >> globally skip file cleanup.\n> > \n> > On the other hand, since each test is a mini-harness, globally skipping\n> > cleanup will make a two-part test fail on the second because of garbage\n> > left behind by the first.\n> > \n> \n> subtext was to have per-subtest files.\n> \n> > Patch 5 adds a comment with another possible solution: teach the python\n> > mini-harness to either clean all files in the directory, or to relocate\n> > the directory according to test name, so that each mini-test starts with\n> > a fresh location, and cleanup is then handled by the harness rather than\n> > spaghetti pre-cleanup. But any solution is better than our current\n> > situation of nothing, so that's why I'm still okay with this patch as-is\n> > as offering more (even if not perfect) than before.\n> > \n> \n> I guess where I am unsure is really if this is better than what we\n> currently do, which is to (try) to clean up after each test as best as\n> we can. I don't see it as too different from trying to clean up before\n> each test.\n> \n> It does give us the ability to leave behind a little detritus after a\n> failed run, but it's so imperfect that I wonder if it's worth shifting\n> this code around to change not much.\n\nAn alternative is to define iotests.QMPTestCase.setUp() so it clears out\niotests.test_dir. Unfortunately this still requires touching up all\nsetUp() methods so that they call super(TheClass, self).setUp().\n\nAt least there would be no need to delete specific files by name (e.g.\nblind_remove(my_img)).\n\nStefan","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=stefanha@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjmmD653wz9s83\n\tfor ;\n\tFri, 1 Sep 2017 01:40:27 +1000 (AEST)","from localhost ([::1]:56366 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnRZx-0004iy-CN\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 11:40:25 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:38208)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dnRZb-0004hR-RX\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 11:40:07 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dnRZX-0007H7-6L\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 11:40:03 -0400","from mx1.redhat.com ([209.132.183.28]:40214)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dnRZS-0007E6-E4; Thu, 31 Aug 2017 11:39:54 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 62CD7C047B8D;\n\tThu, 31 Aug 2017 15:39:53 +0000 (UTC)","from localhost (ovpn-117-62.ams2.redhat.com [10.36.117.62])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 9B94B9ABBC;\n\tThu, 31 Aug 2017 15:39:50 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 62CD7C047B8D","Date":"Thu, 31 Aug 2017 16:39:49 +0100","From":"Stefan Hajnoczi ","To":"John Snow ","Message-ID":"<20170831153949.GA19001@stefanha-x1.localdomain>","References":"\n\t\n\t<1646044f-3c57-c415-f261-463c74ea45b8@redhat.com>\n\t<2690a268-b5be-7974-e8ef-c5a2f09c72a5@redhat.com>\n\t\n\t<494b96d5-d2a2-957e-3169-7fb2461b59de@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<494b96d5-d2a2-957e-3169-7fb2461b59de@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.31]);\n\tThu, 31 Aug 2017 15:39:53 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests\n\tattempt to leave intermediate files","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, qemu-block@nongnu.org, Jeff Cody ,\n\tqemu-devel@nongnu.org, armbru@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1761135,"web_url":"http://patchwork.ozlabs.org/comment/1761135/","msgid":"<20170831154759.GN4770@localhost.localdomain>","list_archive_url":null,"date":"2017-08-31T15:47:59","subject":"Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests\n\tattempt to leave intermediate files","submitter":{"id":12159,"url":"http://patchwork.ozlabs.org/api/people/12159/","name":"Jeff Cody","email":"jcody@redhat.com"},"content":"On Thu, Aug 31, 2017 at 04:39:49PM +0100, Stefan Hajnoczi wrote:\n> On Wed, Aug 30, 2017 at 06:40:29PM -0400, John Snow wrote:\n> > \n> > \n> > On 08/30/2017 06:35 PM, Eric Blake wrote:\n> > > On 08/30/2017 05:28 PM, John Snow wrote:\n> > > \n> > >> I'm a little iffy on this patch; I know that ./check can take care of\n> > >> our temp files for us now, but because each python test is itself a\n> > >> little mini-harness, I'm a little leery of moving the teardown to setup\n> > >> and trying to pre-clean the confetti before the test begins.\n> > >>\n> > >> What's the benefit? We still have to clean up these files per-test, but\n> > >> now it's slightly more error-prone and in a weird place.\n> > >>\n> > >> If we want to try to preserve the most-recent-failure-files, perhaps we\n> > >> can define a setting in the python test-runner that allows us to\n> > >> globally skip file cleanup.\n> > > \n> > > On the other hand, since each test is a mini-harness, globally skipping\n> > > cleanup will make a two-part test fail on the second because of garbage\n> > > left behind by the first.\n> > > \n> > \n> > subtext was to have per-subtest files.\n> > \n> > > Patch 5 adds a comment with another possible solution: teach the python\n> > > mini-harness to either clean all files in the directory, or to relocate\n> > > the directory according to test name, so that each mini-test starts with\n> > > a fresh location, and cleanup is then handled by the harness rather than\n> > > spaghetti pre-cleanup. But any solution is better than our current\n> > > situation of nothing, so that's why I'm still okay with this patch as-is\n> > > as offering more (even if not perfect) than before.\n> > > \n> > \n> > I guess where I am unsure is really if this is better than what we\n> > currently do, which is to (try) to clean up after each test as best as\n> > we can. I don't see it as too different from trying to clean up before\n> > each test.\n> > \n> > It does give us the ability to leave behind a little detritus after a\n> > failed run, but it's so imperfect that I wonder if it's worth shifting\n> > this code around to change not much.\n> \n> An alternative is to define iotests.QMPTestCase.setUp() so it clears out\n> iotests.test_dir. Unfortunately this still requires touching up all\n> setUp() methods so that they call super(TheClass, self).setUp().\n> \n> At least there would be no need to delete specific files by name (e.g.\n> blind_remove(my_img)).\n> \n\nOne reason to only remove specific files used in the test, is that it\nincreases the chance that intermediate files will be left behind in case of\ntest failure of a different test case.\n\nI think the real long-term solution is to run each unittest test case in its\nown subdirectory, so that no intermediate file removal is necessary, and\neach test case is self-contained.\n\n-Jeff","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx08.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx08.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=jcody@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjmz33Lk2z9s83\n\tfor ;\n\tFri, 1 Sep 2017 01:49:51 +1000 (AEST)","from localhost ([::1]:56386 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnRj3-0007AQ-Kt\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 11:49:49 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:40857)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dnRht-0006kb-CH\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 11:48:45 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dnRhn-0003u3-Ds\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 11:48:37 -0400","from mx1.redhat.com ([209.132.183.28]:60912)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dnRhS-0003ND-5y; Thu, 31 Aug 2017 11:48:10 -0400","from smtp.corp.redhat.com\n\t(int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id ADF68C0546EF;\n\tThu, 31 Aug 2017 15:48:08 +0000 (UTC)","from localhost (ovpn-116-70.phx2.redhat.com [10.3.116.70])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id D1E175D757;\n\tThu, 31 Aug 2017 15:48:00 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com ADF68C0546EF","Date":"Thu, 31 Aug 2017 11:47:59 -0400","From":"Jeff Cody ","To":"Stefan Hajnoczi ","Message-ID":"<20170831154759.GN4770@localhost.localdomain>","References":"\n\t\n\t<1646044f-3c57-c415-f261-463c74ea45b8@redhat.com>\n\t<2690a268-b5be-7974-e8ef-c5a2f09c72a5@redhat.com>\n\t\n\t<494b96d5-d2a2-957e-3169-7fb2461b59de@redhat.com>\n\t<20170831153949.GA19001@stefanha-x1.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170831153949.GA19001@stefanha-x1.localdomain>","User-Agent":"Mutt/1.5.24 (2015-08-30)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.14","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.32]);\n\tThu, 31 Aug 2017 15:48:08 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests\n\tattempt to leave intermediate files","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, qemu-block@nongnu.org, John Snow ,\n\tqemu-devel@nongnu.org, armbru@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1762544,"web_url":"http://patchwork.ozlabs.org/comment/1762544/","msgid":"<20170904095124.GB21280@stefanha-x1.localdomain>","list_archive_url":null,"date":"2017-09-04T09:51:24","subject":"Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests\n\tattempt to leave intermediate files","submitter":{"id":17227,"url":"http://patchwork.ozlabs.org/api/people/17227/","name":"Stefan Hajnoczi","email":"stefanha@redhat.com"},"content":"On Thu, Aug 31, 2017 at 11:47:59AM -0400, Jeff Cody wrote:\n> On Thu, Aug 31, 2017 at 04:39:49PM +0100, Stefan Hajnoczi wrote:\n> > On Wed, Aug 30, 2017 at 06:40:29PM -0400, John Snow wrote:\n> > > \n> > > \n> > > On 08/30/2017 06:35 PM, Eric Blake wrote:\n> > > > On 08/30/2017 05:28 PM, John Snow wrote:\n> > > > \n> > > >> I'm a little iffy on this patch; I know that ./check can take care of\n> > > >> our temp files for us now, but because each python test is itself a\n> > > >> little mini-harness, I'm a little leery of moving the teardown to setup\n> > > >> and trying to pre-clean the confetti before the test begins.\n> > > >>\n> > > >> What's the benefit? We still have to clean up these files per-test, but\n> > > >> now it's slightly more error-prone and in a weird place.\n> > > >>\n> > > >> If we want to try to preserve the most-recent-failure-files, perhaps we\n> > > >> can define a setting in the python test-runner that allows us to\n> > > >> globally skip file cleanup.\n> > > > \n> > > > On the other hand, since each test is a mini-harness, globally skipping\n> > > > cleanup will make a two-part test fail on the second because of garbage\n> > > > left behind by the first.\n> > > > \n> > > \n> > > subtext was to have per-subtest files.\n> > > \n> > > > Patch 5 adds a comment with another possible solution: teach the python\n> > > > mini-harness to either clean all files in the directory, or to relocate\n> > > > the directory according to test name, so that each mini-test starts with\n> > > > a fresh location, and cleanup is then handled by the harness rather than\n> > > > spaghetti pre-cleanup. But any solution is better than our current\n> > > > situation of nothing, so that's why I'm still okay with this patch as-is\n> > > > as offering more (even if not perfect) than before.\n> > > > \n> > > \n> > > I guess where I am unsure is really if this is better than what we\n> > > currently do, which is to (try) to clean up after each test as best as\n> > > we can. I don't see it as too different from trying to clean up before\n> > > each test.\n> > > \n> > > It does give us the ability to leave behind a little detritus after a\n> > > failed run, but it's so imperfect that I wonder if it's worth shifting\n> > > this code around to change not much.\n> > \n> > An alternative is to define iotests.QMPTestCase.setUp() so it clears out\n> > iotests.test_dir. Unfortunately this still requires touching up all\n> > setUp() methods so that they call super(TheClass, self).setUp().\n> > \n> > At least there would be no need to delete specific files by name (e.g.\n> > blind_remove(my_img)).\n> > \n> \n> One reason to only remove specific files used in the test, is that it\n> increases the chance that intermediate files will be left behind in case of\n> test failure of a different test case.\n> \n> I think the real long-term solution is to run each unittest test case in its\n> own subdirectory, so that no intermediate file removal is necessary, and\n> each test case is self-contained.\n\nThat could be achieved in the same way:\n\nModify iotests.QMPTestCase.setUp() to create a new directory and chdir()\ninto it. This still requires touching up all existing setUp() methods\nto call their superclass.\n\nStefan","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=stefanha@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xm4sL1Kttz9s7c\n\tfor ;\n\tMon, 4 Sep 2017 19:52:54 +1000 (AEST)","from localhost ([::1]:48556 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1doo3o-00066e-AP\n\tfor incoming@patchwork.ozlabs.org; Mon, 04 Sep 2017 05:52:52 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:58139)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1doo39-0005zO-G4\n\tfor qemu-devel@nongnu.org; Mon, 04 Sep 2017 05:52:18 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1doo32-00047c-7p\n\tfor qemu-devel@nongnu.org; Mon, 04 Sep 2017 05:52:11 -0400","from mx1.redhat.com ([209.132.183.28]:60136)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1doo2X-0003mN-Q1; Mon, 04 Sep 2017 05:51:34 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 13BD04ACB3;\n\tMon, 4 Sep 2017 09:51:32 +0000 (UTC)","from localhost (ovpn-117-203.ams2.redhat.com [10.36.117.203])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id ED57D88E3D;\n\tMon, 4 Sep 2017 09:51:24 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 13BD04ACB3","Date":"Mon, 4 Sep 2017 10:51:24 +0100","From":"Stefan Hajnoczi ","To":"Jeff Cody ","Message-ID":"<20170904095124.GB21280@stefanha-x1.localdomain>","References":"\n\t\n\t<1646044f-3c57-c415-f261-463c74ea45b8@redhat.com>\n\t<2690a268-b5be-7974-e8ef-c5a2f09c72a5@redhat.com>\n\t\n\t<494b96d5-d2a2-957e-3169-7fb2461b59de@redhat.com>\n\t<20170831153949.GA19001@stefanha-x1.localdomain>\n\t<20170831154759.GN4770@localhost.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170831154759.GN4770@localhost.localdomain>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.38]);\n\tMon, 04 Sep 2017 09:51:32 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests\n\tattempt to leave intermediate files","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, qemu-block@nongnu.org, John Snow ,\n\tqemu-devel@nongnu.org, armbru@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1762698,"web_url":"http://patchwork.ozlabs.org/comment/1762698/","msgid":"<20170904144217.GR4770@localhost.localdomain>","list_archive_url":null,"date":"2017-09-04T14:42:17","subject":"Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests\n\tattempt to leave intermediate files","submitter":{"id":12159,"url":"http://patchwork.ozlabs.org/api/people/12159/","name":"Jeff Cody","email":"jcody@redhat.com"},"content":"On Mon, Sep 04, 2017 at 10:51:24AM +0100, Stefan Hajnoczi wrote:\n> On Thu, Aug 31, 2017 at 11:47:59AM -0400, Jeff Cody wrote:\n> > On Thu, Aug 31, 2017 at 04:39:49PM +0100, Stefan Hajnoczi wrote:\n> > > On Wed, Aug 30, 2017 at 06:40:29PM -0400, John Snow wrote:\n> > > > \n> > > > \n> > > > On 08/30/2017 06:35 PM, Eric Blake wrote:\n> > > > > On 08/30/2017 05:28 PM, John Snow wrote:\n> > > > > \n> > > > >> I'm a little iffy on this patch; I know that ./check can take care of\n> > > > >> our temp files for us now, but because each python test is itself a\n> > > > >> little mini-harness, I'm a little leery of moving the teardown to setup\n> > > > >> and trying to pre-clean the confetti before the test begins.\n> > > > >>\n> > > > >> What's the benefit? We still have to clean up these files per-test, but\n> > > > >> now it's slightly more error-prone and in a weird place.\n> > > > >>\n> > > > >> If we want to try to preserve the most-recent-failure-files, perhaps we\n> > > > >> can define a setting in the python test-runner that allows us to\n> > > > >> globally skip file cleanup.\n> > > > > \n> > > > > On the other hand, since each test is a mini-harness, globally skipping\n> > > > > cleanup will make a two-part test fail on the second because of garbage\n> > > > > left behind by the first.\n> > > > > \n> > > > \n> > > > subtext was to have per-subtest files.\n> > > > \n> > > > > Patch 5 adds a comment with another possible solution: teach the python\n> > > > > mini-harness to either clean all files in the directory, or to relocate\n> > > > > the directory according to test name, so that each mini-test starts with\n> > > > > a fresh location, and cleanup is then handled by the harness rather than\n> > > > > spaghetti pre-cleanup. But any solution is better than our current\n> > > > > situation of nothing, so that's why I'm still okay with this patch as-is\n> > > > > as offering more (even if not perfect) than before.\n> > > > > \n> > > > \n> > > > I guess where I am unsure is really if this is better than what we\n> > > > currently do, which is to (try) to clean up after each test as best as\n> > > > we can. I don't see it as too different from trying to clean up before\n> > > > each test.\n> > > > \n> > > > It does give us the ability to leave behind a little detritus after a\n> > > > failed run, but it's so imperfect that I wonder if it's worth shifting\n> > > > this code around to change not much.\n> > > \n> > > An alternative is to define iotests.QMPTestCase.setUp() so it clears out\n> > > iotests.test_dir. Unfortunately this still requires touching up all\n> > > setUp() methods so that they call super(TheClass, self).setUp().\n> > > \n> > > At least there would be no need to delete specific files by name (e.g.\n> > > blind_remove(my_img)).\n> > > \n> > \n> > One reason to only remove specific files used in the test, is that it\n> > increases the chance that intermediate files will be left behind in case of\n> > test failure of a different test case.\n> > \n> > I think the real long-term solution is to run each unittest test case in its\n> > own subdirectory, so that no intermediate file removal is necessary, and\n> > each test case is self-contained.\n> \n> That could be achieved in the same way:\n> \n> Modify iotests.QMPTestCase.setUp() to create a new directory and chdir()\n> into it. This still requires touching up all existing setUp() methods\n> to call their superclass.\n>\n\nGood idea! I'll send out a v4 to just implement it this way; if I am going\nto touch all the python tests anyway, might as well go all the way.\n\nThanks,\nJeff","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=jcody@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xmCKT0ZNBz9sNc\n\tfor ;\n\tTue, 5 Sep 2017 00:44:13 +1000 (AEST)","from localhost ([::1]:48207 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dosbj-00026r-5V\n\tfor incoming@patchwork.ozlabs.org; Mon, 04 Sep 2017 10:44:11 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:37525)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dosaH-0001HJ-Sn\n\tfor qemu-devel@nongnu.org; Mon, 04 Sep 2017 10:42:46 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dosaC-0007fE-IL\n\tfor qemu-devel@nongnu.org; Mon, 04 Sep 2017 10:42:41 -0400","from mx1.redhat.com ([209.132.183.28]:47170)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dosZx-0007Q6-T8; Mon, 04 Sep 2017 10:42:22 -0400","from smtp.corp.redhat.com\n\t(int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 1DB5967736;\n\tMon, 4 Sep 2017 14:42:20 +0000 (UTC)","from localhost (ovpn-116-70.phx2.redhat.com [10.3.116.70])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id D172560489;\n\tMon, 4 Sep 2017 14:42:17 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 1DB5967736","Date":"Mon, 4 Sep 2017 10:42:17 -0400","From":"Jeff Cody ","To":"Stefan Hajnoczi ","Message-ID":"<20170904144217.GR4770@localhost.localdomain>","References":"\n\t\n\t<1646044f-3c57-c415-f261-463c74ea45b8@redhat.com>\n\t<2690a268-b5be-7974-e8ef-c5a2f09c72a5@redhat.com>\n\t\n\t<494b96d5-d2a2-957e-3169-7fb2461b59de@redhat.com>\n\t<20170831153949.GA19001@stefanha-x1.localdomain>\n\t<20170831154759.GN4770@localhost.localdomain>\n\t<20170904095124.GB21280@stefanha-x1.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170904095124.GB21280@stefanha-x1.localdomain>","User-Agent":"Mutt/1.5.24 (2015-08-30)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.11","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.26]);\n\tMon, 04 Sep 2017 14:42:20 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests\n\tattempt to leave intermediate files","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"kwolf@redhat.com, qemu-block@nongnu.org, John Snow ,\n\tqemu-devel@nongnu.org, armbru@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]