Max Reitz <mre...@redhat.com> writes:
> On 19.04.2016 14:22, Sascha Silbe wrote:
>> Dear Max,
>>
>> Max Reitz <mre...@redhat.com> 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
[...]
