Re: [PATCH v2 1/8] iotests/297: Allow checking all Python test files

[Eric Blake <eblake@redhat.com>]

On 1/13/21 11:57 AM, Max Reitz wrote:
> I.e., all Python files in the qemu-iotests/ directory.
> 
> Most files of course do not pass, so there is an extensive skip list for
> now.  (The only files that do pass are 209, 254, 283, and iotests.py.)
> 
> (Alternatively, we could have the opposite, i.e. an explicit list of
> files that we do want to check, but I think it is better to check files
> by default.)

Concur with the choice for default.

> 
> I decided to include the list of files checked in the reference output,
> so we do not accidentally lose coverage of anything.  That means adding
> new Python tests will require a change to 297.out, but that should not
> be a problem.

I'm okay with that.

> 
> On the other hand, I decided to hide mypy's "Success" lines from the
> reference output, because they do not add anything useful.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/297     | 66 ++++++++++++++++++++++++++++++++++----
>  tests/qemu-iotests/297.out |  6 +++-
>  2 files changed, 65 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
> index 5c5420712b..b1a7d6d5e8 100755
> --- a/tests/qemu-iotests/297
> +++ b/tests/qemu-iotests/297
> @@ -30,13 +30,67 @@ if ! type -p "mypy" > /dev/null; then
>      _notrun "mypy not found"
>  fi
>  
> -pylint-3 --score=n iotests.py
> +# TODO: Empty this list!

:)


> +file_list=()
> +for file in *; do
> +    # Check files with a .py extension or a Python shebang
> +    # (Unless they are in the skip_files list)
> +    if [ -f "$file" ] && ((echo "$file" | grep -q '\.py$') ||
> +                          (head -n 1 "$file" | grep -q '^#!.*python'))

Bash has an (obsolete) operator (()) (behaves like a mix of $(()) and
'if'); when nesting subshells, POSIX recommends inserting a space to
avoid inadvertent triggering of the alternate semantics of the operator.
 But why do you need nested subshells?  This is equivalent:

if [ -f "$file" ] && (echo  "$file" | grep -q '\.py$' ||
                      head -n 1 "$file" | grep -q '^#!.*python')

> +    then
> +        skip_file=false
> +        for skip in "${skip_files[@]}"; do

bashism, but iotests require bash, so fine.

> +            if [ "$skip" = "$file" ]; then
> +                skip_file=true
> +                break
> +            fi
> +        done
> +
> +        if ! $skip_file; then
> +            file_list+=("$file")

Likewise.

Whether or not you strip the extra (),
Reviewed-by: Eric Blake <eblake@redhat.com>

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