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

[Kevin Wolf <kwolf@redhat.com>]

Am 16.01.2021 um 14:44 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 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)
> 
> Behavior of findtests.py:
>  - group file is dropped
>  - tests are all files in tests/ subdirectory (except for .out files),
>    so it's not needed more to "register the test", just create it with
>    appropriate name in tests/ subdirectory. Old names like
>    [0-9][0-9][0-9] (in root iotests directory) are supported too, but
>    not recommended for new tests
>  - groups are parsed from '# group: ' line inside test files
>  - optional file group.local may be used to define some additional
>    groups for downstreams
>  - 'disabled' group is used to temporary disable tests. So instead of
>    commenting tests in old 'group' file you now can add them to
>    disabled group with help of 'group.local' file
>  - selecting test ranges like 5-15 are not supported more
>    (to support restarting failed ./check command from the middle of the
>     process, new argument is added: --start-from)
> 
> Benefits:
>  - no rebase conflicts in group file on patch porting from branch to
>    branch
>  - no conflicts in upstream, when different series want to occupy same
>    test number
>  - meaningful names for test files
>    For example, with digital number, when some person wants to add some
>    test about block-stream, he most probably will just create a new
>    test. But if there would be test-block-stream test already, he will
>    at first look at it and may be just add a test-case into it.
>    And anyway meaningful names are better.
> 
> This commit don't update check behavior (which will be done in further
> 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.

As Eric mentioned, this isn't accurate any more.

You mentioned using it as a way to debug things. I assume this is now
covered by the dry run option?

> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  docs/devel/testing.rst          |  50 +++++++++-
>  tests/qemu-iotests/findtests.py | 159 ++++++++++++++++++++++++++++++++
>  2 files changed, 208 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemu-iotests/findtests.py

> +    def add_group_file(self, fname: str) -> None:
> +        with open(fname) as f:
> +            for line in f:
> +                line = line.strip()
> +
> +                if (not line) or line[0] == '#':
> +                    continue
> +
> +                words = line.split()
> +                test_file = self.parse_test_name(words[0])
> +                groups = words[1:]

The previous version still had this:

+                if test_file not in self.all_tests:
+                    print(f'Warning: {fname}: "{test_file}" test is not found.'
+                          ' Skip.')
+                    continue

Why did you remove it? I found this useful when I had a wrong test name
in my group.local. Now it's silently ignored.

> +                for g in groups:
> +                    self.groups[g].add(test_file)

Kevin