Re: [Qemu-devel] [PATCH 08/12] iotests: add testing shim for script-style python tests

From: Max Reitz <mreitz@redhat.com>
To: John Snow <jsnow@redhat.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Fam Zheng <fam@euphon.net>,
	vsementsov@virtuozzo.com, Wen Congyang <wencongyang2@huawei.com>,
	Xie Changlong <xiechanglong.d@gmail.com>,
	Markus Armbruster <armbru@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 08/12] iotests: add testing shim for script-style python tests
Date: Thu, 20 Jun 2019 20:55:59 +0200	[thread overview]
Message-ID: <9f239fc8-adc4-34e4-2d89-c556e7af38df@redhat.com> (raw)
In-Reply-To: <a81fad35-87ab-5af4-7beb-03f0750f4cef@redhat.com>

[-- Attachment #1.1: Type: text/plain, Size: 1897 bytes --]

On 20.06.19 20:47, John Snow wrote:
> 
> 
> On 6/20/19 1:26 PM, Max Reitz wrote:
>> On 20.06.19 19:09, Max Reitz wrote:
>>> On 20.06.19 03:03, John Snow wrote:
>>>> Because the new-style python tests don't use the iotests.main() test
>>>> launcher, we don't turn on the debugger logging for these scripts
>>>> when invoked via ./check -d.
>>>>
>>>> Refactor the launcher shim into new and old style shims so that they
>>>> share environmental configuration.
>>>>
>>>> Two cleanup notes: debug was not actually used as a global, and there
>>>> was no reason to create a class in an inner scope just to achieve
>>>> default variables; we can simply create an instance of the runner with
>>>> the values we want instead.
>>>>
>>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>>> ---
>>>>  tests/qemu-iotests/iotests.py | 40 +++++++++++++++++++++++------------
>>>>  1 file changed, 26 insertions(+), 14 deletions(-)
>>>
>>> I don’t quite get how script_main() works (yes, both my Pythonfu and my
>>> Googlefu are that bad), but it works and looks good, so have a
>>
>> Oh, it doesn’t work (well, not automagically).  I just assumed seeing
>> the log output means it’s working.  Seeing that the test needs to call
>> iotests.script_main() explicitly does clear up my confusion.
>>
>> All OK with me.
>>
>> Max
>>
> 
> Yes. I should convert the others to opt-in to the new format so that
> copy-paste in the future will get us the right paradigm.
> 
> Tests just need to be refactored to have a single point of entry so it
> can be passed as a closure to the test runner.
> 
> If this seems like a good change I will do that as a follow-up series
> with only the churn.

It does seem good to me.  Not even because of the test runner, but maybe
even more so because it seems like better style to split the tests into
one function per case.

Max

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