Re: [PATCH v6 10/11] iotests: rewrite check into python

[John Snow <jsnow@redhat.com>]

On 1/13/21 6:20 PM, Eric Blake wrote:
> On 1/9/21 6:26 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Just use classes introduced in previous three commits. Behavior
>> difference is described in these three commits.
>>
>> Drop group file, as it becomes unused.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/check | 994 ++-------------------------------------
>>   tests/qemu-iotests/group | 321 -------------
>>   2 files changed, 28 insertions(+), 1287 deletions(-)
>>   delete mode 100644 tests/qemu-iotests/group
> 
> The bulk of the work was done in the earlier patches, and my python
> review is weak; but I can confirm that with this patch applied, my usual
> attempts at running ./check still appeared to work for me.  That's not
> the same as proving you did 1:1 feature translation (and in fact, your
> commit message documented dropping some features like -v), but if 'make
> check' and CI tools still run, I'm okay leaving it up to developers to
> complain about any other feature that they used but which go missing (or
> to implement it).
> 
> Tested-by: Eric Blake <eblake@redhat.com>
> 

I will *try* to give this a look, but I am still buried under post-PTO 
mail; and I feel comfortable with Kevin's Python review otherwise, so 
don't hold your breath waiting to hear from me.

>>
>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
>> index 952762d5ed..48bb3128c3 100755
>> --- a/tests/qemu-iotests/check
>> +++ b/tests/qemu-iotests/check
>> @@ -1,7 +1,8 @@
>> -#!/usr/bin/env bash
>> +#!/usr/bin/env python3
>>   #
>> -# Copyright (C) 2009 Red Hat, Inc.
>> -# Copyright (c) 2000-2002,2006 Silicon Graphics, Inc.  All Rights Reserved.
>> +# Configure environment and run group of tests in it.
>> +#
>> +# Copyright (c) 2020 Virtuozzo International GmbH
> 
> You may want to claim 2021 as well.
> 
>> +import sys
>> +import os
>> +from findtests import find_tests, TestFinder
>> +from testenv import TestEnv
>> +from testrunner import TestRunner
>> +
>> +if __name__ == '__main__':
>> +    if len(sys.argv) == 2 and sys.argv[1] in ['-h', '--help']:
>> +        print('Usage: ./check [options] [testlist]')
>> +        print()
>> +        TestFinder.get_argparser().print_help()
>> +        print()
>> +        TestEnv.get_argparser().print_help()
>> +        print()
>> +        TestRunner.get_argparser().print_help()
>> +        exit()
>> +
>> +    env = TestEnv(sys.argv[1:])
>> +    tests, remaining_argv = find_tests(env.remaining_argv,
>> +                                       test_dir=env.source_iotests)
>> +
>> +    with TestRunner(remaining_argv, env) as tr:
>> +        assert not tr.remaining_argv
>> +        tr.run_tests([os.path.join(env.source_iotests, t) for t in tests])
> 
> A lot shorter for the main engine ;)
> 
>> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
>> deleted file mode 100644
>> index bc5bc324fe..0000000000
>> --- a/tests/qemu-iotests/group
>> +++ /dev/null
>> @@ -1,321 +0,0 @@
>> -#
>> -# QA groups control file
>> -# Defines test groups
> 
> Happy to see this conflict magnet go!
> 

This rules.

--js