Re: [PATCH v7 00/11] Rework iotests/check
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
Re: [PATCH v7 00/11] Rework iotests/check
Am 22.01.2021 um 17:08 hat Eric Blake geschrieben: > On 1/22/21 5:27 AM, Kevin Wolf wrote: > > Am 20.01.2021 um 21:52 hat Eric Blake geschrieben: > >> On 1/16/21 7:44 AM, Vladimir Sementsov-Ogievskiy wrote: > >>> 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 > >> > >> I'd really like a second reviewer on 7-11, but I'm queueing 1-6 on my > >> NBD tree now. > > > > Oh, you already sent a pull request? Having 6 in without the rest is not > > a good state. We now have the group info duplicated and one of them is > > ignored, but will become the meaningful copy later. We need to be > > careful to not let them diverge now. > > > > I hope the rest is fine so we can switch over quickly, otherwise I'd > > prefer to revert 6 and redo it from the then current state when we merge > > the whole series. > > Yeah, I probably jumped the gun there. If we don't have v8 in good > working shape soon, I'm happy to send a temporary revert pull request > for patch 6 - let's shoot for late Monday as the cut-off point where I > will act if the revert is needed. I think I'm going to send a pull request anyway, so I could include it there if necessary. Kevin
Re: [PATCH v7 00/11] Rework iotests/check
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 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, no-else-return, too-few-public-methods, too-many-arguments,
Re: [PATCH v7 00/11] Rework iotests/check
On 1/22/21 5:27 AM, Kevin Wolf wrote: > Am 20.01.2021 um 21:52 hat Eric Blake geschrieben: >> On 1/16/21 7:44 AM, Vladimir Sementsov-Ogievskiy wrote: >>> 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 >> >> I'd really like a second reviewer on 7-11, but I'm queueing 1-6 on my >> NBD tree now. > > Oh, you already sent a pull request? Having 6 in without the rest is not > a good state. We now have the group info duplicated and one of them is > ignored, but will become the meaningful copy later. We need to be > careful to not let them diverge now. > > I hope the rest is fine so we can switch over quickly, otherwise I'd > prefer to revert 6 and redo it from the then current state when we merge > the whole series. Yeah, I probably jumped the gun there. If we don't have v8 in good working shape soon, I'm happy to send a temporary revert pull request for patch 6 - let's shoot for late Monday as the cut-off point where I will act if the revert is needed. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v7 00/11] Rework iotests/check
22.01.2021 14:27, Kevin Wolf wrote: Am 20.01.2021 um 21:52 hat Eric Blake geschrieben: On 1/16/21 7:44 AM, Vladimir Sementsov-Ogievskiy wrote: 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 I'd really like a second reviewer on 7-11, but I'm queueing 1-6 on my NBD tree now. Oh, you already sent a pull request? Having 6 in without the rest is not a good state. We now have the group info duplicated and one of them is ignored, but will become the meaningful copy later. We need to be careful to not let them diverge now. I hope the rest is fine so we can switch over quickly, otherwise I'd prefer to revert 6 and redo it from the then current state when we merge the whole series. I can make a new version now with tiny fixes suggested by Eric if it is convenient. (and keep --color suggestion for a separate follow-up) -- Best regards, Vladimir
Re: [PATCH v7 00/11] Rework iotests/check
Am 20.01.2021 um 21:52 hat Eric Blake geschrieben: > On 1/16/21 7:44 AM, Vladimir Sementsov-Ogievskiy wrote: > > 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 > > I'd really like a second reviewer on 7-11, but I'm queueing 1-6 on my > NBD tree now. Oh, you already sent a pull request? Having 6 in without the rest is not a good state. We now have the group info duplicated and one of them is ignored, but will become the meaningful copy later. We need to be careful to not let them diverge now. I hope the rest is fine so we can switch over quickly, otherwise I'd prefer to revert 6 and redo it from the then current state when we merge the whole series. Kevin
Re: [PATCH v7 00/11] Rework iotests/check
On 16/01/21 14:44, Vladimir Sementsov-Ogievskiy wrote: 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 Very nice! I have wondered for some time if we should rewrite check as a wrapper around avocado. Your effort certainly goes in the right direction, since it at least uses the same programming language and introduces abstractions that can be easily turned into avocado wrappers. Paolo 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 findtests: - stop parsing test file after first '# group: ' line - use parse_test_name in add_group_file() - make new list instead of modifying parameter exclude_groups testenv: - fix machine_map - fix self.python testrunner: use env.python to run python tests Vladimir Sementsov-Ogievskiy (11): iotests/277: use dot slash for nbd-fault-injector.py running iotests/303: use dot slash for qcow2.py running iotests: fix some whitespaces in test output files iotests: make tests executable iotests/294: add shebang line iotests: define group in each iotest iotests: add findtests.py iotests: add testenv.py iotests: add testrunner.py iotests: rewrite check into python iotests: rename and move 169 and 199 tests docs/devel/testing.rst| 50 +- Makefile |1 - tests/check-block.sh |2 +- tests/qemu-iotests/001|1 + tests/qemu-iotests/002|1 + tests/qemu-iotests/003|1 + tests/qemu-iotests/004|1 + tests/qemu-iotests/005|1 + tests/qemu-iotests/007|1 + tests/qemu-iotests/008|1 + tests/qemu-iotests/009|1 + tests/qemu-iotests/010|1 + tests/qemu-iotests/011|1 + tests/qemu-iotests/012|1 + tests/qemu-iotests/013|1 + tests/qemu-iotests/014|1 + tests/qemu-iotests/015|1 + tests/qemu-iotests/017|1 + tests/qemu-iotests/018|1 + tests/qemu-iotests/019|1 + tests/qemu-iotests/020|1 + tests/qemu-iotests/021|1 + tests/qemu-iotests/022|1 + tests/qemu-iotests/023|1 + tests/qemu-iotests/024|1 + tests/qemu-iotests/025|1 + tests/qemu-iotests/026|1 + tests/qemu-iotests/027|1 + tests/qemu-iotests/028|1 + tests/qemu-iotests/029|1 + tests/qemu-iotests/030|1 + tests/qemu-iotests/031|1 + tests/qemu-iotests/032|1 + tests/qemu-iotests/033|1 + tests/qemu-iotests/034|1 + tests/qemu-iotests/035|1 + tests/qemu-iotests/036|1 + tests/qemu-iotests/037|1 + tests/qemu-iotests/038|1 + tests/qemu-iotests/039|1 + tests/qemu-iotests/040|1 + tests/qemu-iotests/041|1 + tests/qemu-iotests/042|1 + tests/qemu-iotests/043|1 + tests/qemu-iotests/044|1 + tests/qemu-iotests/045|1 + tests/qemu-iotests/046|1 + tests/qemu-iotests/047|1 + tests/qemu-iotests/048|1 + tests/qemu-iotests/049|1 + tests/qemu-iotests/050|1 + tests/qemu-iotests/051|1 + tests/qemu-iotests/052|1 + tests/qemu-iotests/053|1 + tests/qemu-iotests/054|1 + tests/qemu-iotests/055|1 + tests/qemu-iotests/056|1 + tests/qemu-iotests/057|1 + tests/qemu-iotests/058|1 + tests/qemu-iotests/059|1 + tests/qemu-iotests/060
Re: [PATCH v7 00/11] Rework iotests/check
On 1/16/21 7:44 AM, Vladimir Sementsov-Ogievskiy wrote: > 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 I'd really like a second reviewer on 7-11, but I'm queueing 1-6 on my NBD tree now. > > findtests: - stop parsing test file after first '# group: ' line > - use parse_test_name in add_group_file() > - make new list instead of modifying parameter exclude_groups > > testenv: - fix machine_map > - fix self.python > > testrunner: use env.python to run python tests > > Vladimir Sementsov-Ogievskiy (11): > iotests/277: use dot slash for nbd-fault-injector.py running > iotests/303: use dot slash for qcow2.py running > iotests: fix some whitespaces in test output files > iotests: make tests executable > iotests/294: add shebang line > iotests: define group in each iotest > iotests: add findtests.py > iotests: add testenv.py > iotests: add testrunner.py > iotests: rewrite check into python > iotests: rename and move 169 and 199 tests > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
[PATCH v7 00/11] Rework iotests/check
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 findtests: - stop parsing test file after first '# group: ' line - use parse_test_name in add_group_file() - make new list instead of modifying parameter exclude_groups testenv: - fix machine_map - fix self.python testrunner: use env.python to run python tests Vladimir Sementsov-Ogievskiy (11): iotests/277: use dot slash for nbd-fault-injector.py running iotests/303: use dot slash for qcow2.py running iotests: fix some whitespaces in test output files iotests: make tests executable iotests/294: add shebang line iotests: define group in each iotest iotests: add findtests.py iotests: add testenv.py iotests: add testrunner.py iotests: rewrite check into python iotests: rename and move 169 and 199 tests docs/devel/testing.rst| 50 +- Makefile |1 - tests/check-block.sh |2 +- tests/qemu-iotests/001|1 + tests/qemu-iotests/002|1 + tests/qemu-iotests/003|1 + tests/qemu-iotests/004|1 + tests/qemu-iotests/005|1 + tests/qemu-iotests/007|1 + tests/qemu-iotests/008|1 + tests/qemu-iotests/009|1 + tests/qemu-iotests/010|1 + tests/qemu-iotests/011|1 + tests/qemu-iotests/012|1 + tests/qemu-iotests/013|1 + tests/qemu-iotests/014|1 + tests/qemu-iotests/015|1 + tests/qemu-iotests/017|1 + tests/qemu-iotests/018|1 + tests/qemu-iotests/019|1 + tests/qemu-iotests/020|1 + tests/qemu-iotests/021|1 + tests/qemu-iotests/022|1 + tests/qemu-iotests/023|1 + tests/qemu-iotests/024|1 + tests/qemu-iotests/025|1 + tests/qemu-iotests/026|1 + tests/qemu-iotests/027|1 + tests/qemu-iotests/028|1 + tests/qemu-iotests/029|1 + tests/qemu-iotests/030|1 + tests/qemu-iotests/031|1 + tests/qemu-iotests/032|1 + tests/qemu-iotests/033|1 + tests/qemu-iotests/034|1 + tests/qemu-iotests/035|1 + tests/qemu-iotests/036|1 + tests/qemu-iotests/037|1 + tests/qemu-iotests/038|1 + tests/qemu-iotests/039|1 + tests/qemu-iotests/040|1 + tests/qemu-iotests/041|1 + tests/qemu-iotests/042|1 + tests/qemu-iotests/043|1 + tests/qemu-iotests/044|1 + tests/qemu-iotests/045|1 + tests/qemu-iotests/046|1 + tests/qemu-iotests/047|1 + tests/qemu-iotests/048|1 + tests/qemu-iotests/049|1 + tests/qemu-iotests/050|1 + tests/qemu-iotests/051|1 + tests/qemu-iotests/052|1 + tests/qemu-iotests/053|1 + tests/qemu-iotests/054|1 + tests/qemu-iotests/055|1 + tests/qemu-iotests/056|1 + tests/qemu-iotests/057|1 + tests/qemu-iotests/058|1 + tests/qemu-iotests/059|1 + tests/qemu-iotests/060|1 + tests/qemu-iotests/061|1 + tests/qemu-iotests/062|1 + tests/qemu-iotests/063|1 + tests/qemu-iotests/064|1 + tests/qemu-iotests/065|1 + tests/qemu-iotests/066|1 + tests/qemu-iotests/068|1 +
