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

[Markus Armbruster]

Max Reitz <address@hidden> writes:

> On 19.04.2016 14:22, Sascha Silbe wrote:
>> Dear Max,
>> 
>> Max Reitz <address@hidden> writes:
>> 
>>> On 14.04.2016 13:32, Sascha Silbe wrote:
>> [tests/qemu-iotests/iotests.py]
>> [...]
>>>>  def main(supported_fmts=[], supported_oses=['linux']):
>>>>      '''Run tests'''
>>>>  
>>>> +    if test_dir is None or qemu_default_machine is None:
>>>
>>> I think checking test_dir would have been sufficient; this makes it look
>>> like it might want to be a complete list of variables that have to be
>>> set, but then "cachemode" is missing.
>> 
>> Markus Armbruster basically pointed out the same issue, so how about
>> this version:
>> 
>>     # 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.

As Max predicted, I'm not :)

'Please run this test via the "check" script' isn't an error message,
it's advice.  Advice is appreciated, but an error message must come
first.  I reiterate my proposal to use something like

    <program_name>: TEST_DIR and QEMU_DEFAULT_MACHINE must be set in environment
    The easiest way to do that is running the test via the check script.

or

    <program_name>: TEST_DIR and QEMU_DEFAULT_MACHINE must be set in environment
    Please run this test via the "check" script

[...]