| Message ID | 1461094442-16014-1-git-send-email-silbe@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 0c0b533..054b482 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -46,7 +46,7 @@ if os.environ.get('QEMU_OPTIONS'): imgfmt = os.environ.get('IMGFMT', 'raw') imgproto = os.environ.get('IMGPROTO', 'file') -test_dir = os.environ.get('TEST_DIR', '/var/tmp') +test_dir = os.environ.get('TEST_DIR') output_dir = os.environ.get('OUTPUT_DIR', '.') cachemode = os.environ.get('CACHEMODE') qemu_default_machine = os.environ.get('QEMU_DEFAULT_MACHINE') @@ -441,6 +441,14 @@ def verify_quorum(): def main(supported_fmts=[], supported_oses=['linux']): '''Run tests''' + # 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) + debug = '-d' in sys.argv verbosity = 1 verify_image_format(supported_fmts)