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

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

13.01.2021 20:57, 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.)
> 
> 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 have a parallel series, "Rework iotests/check", one of its aims is drop
group file, to avoid these endless conflicts in group file when you want
to send series or when you are porting patches to/from downstream.

And you are trying to add one another "group" file :) I don't like the idea.

Why should we loose accidentally the coverage? Logic is extremely simple:
all files except for the list.

> 
> 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!
> +skip_files=(
> +    030 040 041 044 045 055 056 057 065 093 096 118 124 129 132 136 139 147 148
> +    149 151 152 155 163 165 169 194 196 199 202 203 205 206 207 208 210 211 212
> +    213 216 218 219 222 224 228 234 235 236 237 238 240 242 245 246 248 255 256
> +    257 258 260 262 264 266 274 277 280 281 295 296 298 299 300 302 303 304 307
> +    nbd-fault-injector.py qcow2.py qcow2_format.py qed.py
> +)
>   
> -MYPYPATH=../../python/ mypy --warn-unused-configs --disallow-subclassing-any \
> -    --disallow-any-generics --disallow-incomplete-defs \
> -    --disallow-untyped-decorators --no-implicit-optional \
> -    --warn-redundant-casts --warn-unused-ignores \
> -    --no-implicit-reexport iotests.py
> +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'))
> +    then
> +        skip_file=false
> +        for skip in "${skip_files[@]}"; do
> +            if [ "$skip" = "$file" ]; then
> +                skip_file=true
> +                break
> +            fi
> +        done
> +
> +        if ! $skip_file; then
> +            file_list+=("$file")
> +        fi
> +    fi
> +done
> +
> +# Emit list of all files that are checked so we will not accidentally
> +# lose coverage
> +echo 'Files to be checked:'
> +
> +file_list_str=''
> +for file in "${file_list[@]}"; do
> +    echo "  $file"
> +done | sort
> +
> +# We can pass all files to pylint at once...
> +pylint-3 --score=n "${file_list[@]}"
> +
> +# ...but mypy needs to be called once per file.  Otherwise, it will
> +# interpret all given files as belonging together (i.e., they may not
> +# both define the same classes, etc.; most notably, they must not both
> +# define the __main__ module).
> +for file in "${file_list[@]}"; do
> +    mypy_output=$(
> +        MYPYPATH=../../python/ mypy --warn-unused-configs \
> +            --disallow-subclassing-any \
> +            --disallow-any-generics --disallow-incomplete-defs \
> +            --disallow-untyped-decorators --no-implicit-optional \
> +            --warn-redundant-casts --warn-unused-ignores \
> +            --no-implicit-reexport "$file" \
> +            2>&1
> +    )
> +
> +    if [ $? != 0 ]; then
> +        echo "$mypy_output"
> +    fi
> +done
>   
>   # success, all done
>   echo "*** done"
> diff --git a/tests/qemu-iotests/297.out b/tests/qemu-iotests/297.out
> index 6acc843649..c5ebbf6a17 100644
> --- a/tests/qemu-iotests/297.out
> +++ b/tests/qemu-iotests/297.out
> @@ -1,3 +1,7 @@
>   QA output created by 297
> -Success: no issues found in 1 source file
> +Files to be checked:
> +  209
> +  254
> +  283
> +  iotests.py
>   *** done
> 


-- 
Best regards,
Vladimir