Re: [PATCH v7 00/11] Rework iotests/check - Vladimir Sementsov-Ogievskiy

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, mreitz@redhat.com,
	den@openvz.org, jsnow@redhat.com
Subject: Re: [PATCH v7 00/11] Rework iotests/check
Date: Sat, 23 Jan 2021 18:14:32 +0300	[thread overview]
Message-ID: <5ca1af5e-364f-254b-aa44-a07ae62ebcf6@virtuozzo.com> (raw)
In-Reply-To: <20210122161618.GK15866@merkur.fritz.box>

22.01.2021 19:16, Kevin Wolf wrote:
> Am 16.01.2021 um 14:44 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> Hi all!
>>
>> These series has 3 goals:
>>
>>   - get rid of group file (to forget about rebase and in-list conflicts)
>>   - introduce human-readable names for tests
>>   - rewrite check into python
>>
>> v7:
>>    - fix wording and grammar
>>    - satisfy python linters
>>    - move argv interfaces all into one in new check script
>>    - support '-n' == '--dry-run' option
>>    - update check-block to run check with correct PYTHON
> 
> Okay, I think I'm finished with the review for this version.
> 
> I also tried pylint/mypy again and it's mostly clean now (pylint
> complains about the TODO comments, I think we should just disable that
> warning).
> 
> Feel free to include the following as patch 12 in v8.
> 
> Kevin

I remember Max already queued good changes for 297 in his block branch

> 
> 
> diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
> index 85bc1c0c85..8aaa3e455c 100755
> --- a/tests/qemu-iotests/297
> +++ b/tests/qemu-iotests/297
> @@ -31,13 +31,15 @@ if ! type -p "mypy" > /dev/null; then
>       _notrun "mypy not found"
>   fi
> 
> -pylint-3 --score=n iotests.py
> +FILES="findtests.py iotests.py testenv.py testrunner.py check"
> +
> +pylint-3 --score=n $FILES
> 
>   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
> +    --no-implicit-reexport $FILES
> 
>   # success, all done
>   echo "*** done"
> diff --git a/tests/qemu-iotests/297.out b/tests/qemu-iotests/297.out
> index 6acc843649..85213ef96e 100644
> --- a/tests/qemu-iotests/297.out
> +++ b/tests/qemu-iotests/297.out
> @@ -1,3 +1,3 @@
>   QA output created by 297
> -Success: no issues found in 1 source file
> +Success: no issues found in 5 source files
>   *** done
> diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc
> index cd3702e23c..980bfa7b20 100644
> --- a/tests/qemu-iotests/pylintrc
> +++ b/tests/qemu-iotests/pylintrc
> @@ -9,7 +9,8 @@
>   # --enable=similarities". If you want to run only the classes checker, but have
>   # no Warning level messages displayed, use "--disable=all --enable=classes
>   # --disable=W".
> -disable=invalid-name,
> +disable=fixme,
> +        invalid-name,

Max just disable it in 297. I think we should not disable it globally, as additional hint doesn't hurt (except for test output).

>           no-else-return,
>           too-few-public-methods,
>           too-many-arguments,
> 

-- 
Best regards,
Vladimir