Re: [Qemu-devel] [PATCH for-2.6? v2] qemu-iotests: iotests: fail hard if not run via "check"

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2452 bytes --]

On 19.04.2016 21:34, Sascha Silbe wrote:
> Running an iotests-based Python test directly might appear to work,
> but may fail in subtle ways and is insecure:
> 
> - It creates files with predictable file names in a world-writable
>   location (/var/tmp).
> 
> - Tests expect the environment to be set up by check. E.g. 041 and 055
>   may take the wrong code paths if QEMU_DEFAULT_MACHINE is not
>   set. This can lead to false negatives.
> 
> Instead fail hard and tell the user we want to be run via "check".
> 
> The actual environment expected by the tests is currently only defined
> by the implementation of "check". We use two of the environment
> variables set by "check" as indication of whether we're being run via
> "check". Anyone writing their own test runner (replacing "check") will
> need to replicate the full environment (in a broader sense, not just
> environment variables) provided by "check" anyway, including setting
> the two environment variables we check. Whereas a regular developer
> just trying to invoke the tests usually won't have both of these
> defined in their environment so we can catch their mistake and give
> out useful advice.
> 
> Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
> Reviewed-by: Bo Tu <tubo@linux.vnet.ibm.com>
> ---
> v1→v2:
>   - Add comment indicating that it's not just TEST_DIR and
>     QEMU_DEFAULT_MACHINE that need to be set. Amend commit message in
>     a similar way.
> 
> @Tu Bo: I'm assuming your Reviewed-By: still stands as I only added
> more information on the background of the change and the check.
> ---
>  tests/qemu-iotests/iotests.py | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)

Having noted Markus' objections, I have still applied this patch to my
block-next tree (https://github.com/XanClic/qemu/commits/block-next),
for the following reasons:

First, I think it's reasonable to require users to find the source of
this error (which is really trivial, and the environment is
self-explaining) if they wish to write an own iotest platform.

Second, most of the Python tests fail with an exception anyway because
they try to os.path.join() the iotests.test_dir (which is None) with a
string before the main method is even invoked. Therefore, this is
basically only a backup for the few tests that somehow get past this
point, so you generally won't see this error message anyway.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]