From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49460) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1asazL-00072I-AI for qemu-devel@nongnu.org; Tue, 19 Apr 2016 15:07:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1asazK-0001Pw-7X for qemu-devel@nongnu.org; Tue, 19 Apr 2016 15:07:07 -0400 References: <1460633543-7366-1-git-send-email-silbe@linux.vnet.ibm.com> <57101588.3080204@redhat.com> <87fuuh7ppo.fsf@oc4731375738.ibm.com> From: Max Reitz Message-ID: <571681D0.5010707@redhat.com> Date: Tue, 19 Apr 2016 21:06:56 +0200 MIME-Version: 1.0 In-Reply-To: <87fuuh7ppo.fsf@oc4731375738.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="wIPv2KRhCpWrSJDU6tRjomx1vVpN1aiVC" Subject: Re: [Qemu-devel] [PATCH for-2.6?] qemu-iotests: iotests: fail hard if not run via "check" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sascha Silbe , qemu-devel@nongnu.org, qemu-block@nongnu.org Cc: Kevin Wolf This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --wIPv2KRhCpWrSJDU6tRjomx1vVpN1aiVC Content-Type: multipart/mixed; boundary="ql230WUfDhrnGhra7aaHIOwf1NSkuONjl" From: Max Reitz To: Sascha Silbe , qemu-devel@nongnu.org, qemu-block@nongnu.org Cc: Kevin Wolf Message-ID: <571681D0.5010707@redhat.com> Subject: Re: [PATCH for-2.6?] qemu-iotests: iotests: fail hard if not run via "check" References: <1460633543-7366-1-git-send-email-silbe@linux.vnet.ibm.com> <57101588.3080204@redhat.com> <87fuuh7ppo.fsf@oc4731375738.ibm.com> In-Reply-To: <87fuuh7ppo.fsf@oc4731375738.ibm.com> --ql230WUfDhrnGhra7aaHIOwf1NSkuONjl Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 19.04.2016 14:22, Sascha Silbe wrote: > Dear Max, >=20 > Max Reitz writes: >=20 >> On 14.04.2016 13:32, Sascha Silbe wrote: > [tests/qemu-iotests/iotests.py] > [...] >>> def main(supported_fmts=3D[], supported_oses=3D['linux']): >>> '''Run tests''' >>> =20 >>> + if test_dir is None or qemu_default_machine is None: >> >> I think checking test_dir would have been sufficient; this makes it lo= ok >> like it might want to be a complete list of variables that have to be >> set, but then "cachemode" is missing. >=20 > Markus Armbruster basically pointed out the same issue, so how about > this version: >=20 > # We are using TEST_DIR and QEMU_DEFAULT_MACHINE as proxies to > # indicate that we're not being run via "check". There may be > # other things set up by "check" that individual test cases rely > # on. > if test_dir is None or qemu_default_machine is None: > sys.stderr.write('Please run this test via the "check" script\n= ') > sys.exit(os.EX_USAGE) I'm OK with that. I can imagine Markus isn't, because it's implying that you should only run this test through "check", whereas Markus says that maybe you have your own script and that is fine, too. I think two things: 1. I'm not sure why you would want your own script. If the check script isn't good enough, extend it. 2. If you do want your own script, I guess setting up the necessary environment for each test is complicated enough to require you to know what you're doing. And if you know what you're doing, you won't really care about the wording of this warning anyway. I think this is just a warning for unwary users who just try to execute a python test directly because they think that maybe they can. And then this line is just telling them that no, that is not how the test is supposed to be run; instead, it tells them the most convenient and common way to run it. I think it would be confusing if we printed the exact technical details here which basically nobody cares about anyway. So I'm completely fine with this version. >>> + sys.stderr.write('Please run this test via ./check\n') >> >> Or not ./, for people who have separate build and source trees, you >> don't want to invoke check in the source tree. ;-) >=20 > Yeah, was thinking about that, but ultimately considered ./check to be > the best indication of a) "check" being the name of a script and b) > residing in the same directory as the test. But I don't care much about= > this, so see the above version. Yes, that looks fine to me. Regarding b): If you separate the build tree from the source tree, you have to run the check script from the build tree. During the build process, qemu will in fact automatically create a symlink in the build tree. Therefore, in that case, you don't want to execute "./check" in the same directory as the test is in. Max --ql230WUfDhrnGhra7aaHIOwf1NSkuONjl-- --wIPv2KRhCpWrSJDU6tRjomx1vVpN1aiVC Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJXFoHQAAoJEDuxQgLoOKytPaYH+wQPqFJvUKEFzdHk25jXU0Sz bDqpitLUf/ICfQv3WfbzurXiiawZPK1GVe7KtYb9gOL3FePyYqZ+s4ODa3caHg4D oq1NZosMpCpfBJ4GfT9qKYtCPA4uba9CPtSKxX/skCszTU5FWPqrXwkZpC4TpeMB vP5R7tQPLcmggHCQi4zwTBBwk3F34p1pssAgTa9dFPC+PtRmxdbltfdO2fkVyviq EVTw/6WL1yIlNi62krNQDL0a95aK99yL91PLdCqJJMYcn15coPUSkwNbY0k5oLPc hQrMuUyI59SpiCLRUerXklflGXMiX5IwzCA/SrIlAGI0AMNWtFF2zRbrYl54fP8= =hrUT -----END PGP SIGNATURE----- --wIPv2KRhCpWrSJDU6tRjomx1vVpN1aiVC--