Re: [PATCH v6 07/11] iotests: add findtests.py

[Eric Blake]

On 1/9/21 6:26 AM, Vladimir Sementsov-Ogievskiy wrote:
> Add python script with new logic of searching for tests:
> 
> Current ./check behavior:
>  - tests are named [0-9][0-9][0-9]
>  - tests must be registered in group file (even if test doesn't belong
>    to any group, like 142)
> 

> 
> This commit don't update check behavior (which will be don in further

done

> commit), still, the documentation changed like new behavior is already
> here.  Let's live with this small inconsistency for the following few
> commits, until final change.
> 
> The file findtests.py is self-executable and may be used for debugging
> purposes.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  docs/devel/testing.rst          |  50 ++++++-
>  tests/qemu-iotests/findtests.py | 229 ++++++++++++++++++++++++++++++++
>  2 files changed, 278 insertions(+), 1 deletion(-)
>  create mode 100755 tests/qemu-iotests/findtests.py

In addition to Kevin's review,


> +Additional way of defining groups is creating tests/qemu-iotests/group.local
> +file. This should be used only for downstream (this file should never appear
> +in upstream). This file may be used for defining some downstream test groups
> +or for temporary disable tests, like this:

Kevin suggested s/disable/disabling/; I'd go one step further:
/temporary disable/temporarily disabling/


> +
> +- quick : Tests in this group should finish within some few seconds.

s/some/a/

Also, for each group name, the space before ':' looks odd.

> +++ b/tests/qemu-iotests/findtests.py
> @@ -0,0 +1,229 @@
> +#!/usr/bin/env python3
> +#
> +# Parse command line options to define set of tests to run.
> +#
> +# Copyright (c) 2020 Virtuozzo International GmbH

Worth claiming 2021 as well?

Otherwise, my python review is weak, but I didn't spot anything
blatantly wrong.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org