* [PATCH RFC 0/3] python/iotests: Run iotest linters during Python CI
@ 2021-06-04 16:39 John Snow
2021-06-04 16:39 ` [PATCH RFC 1/3] python: expose typing information via PEP 561 John Snow
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: John Snow @ 2021-06-04 16:39 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
Since iotests are such a heavy and prominent user of the Python qemu.qmp
and qemu.machine packages, it would be convenient if the Python linting
suite also checked this client for any possible regressions introduced
by shifting around signatures, types, or interfaces in these packages.
(Of course, we'd eventually find those problems when iotest 297 ran, but
with increasing distance between Python development and Block
development, the risk of an accidental breakage in this regard
increases. I, personally, know to run iotests (and especially 297) after
changing Python code, but not everyone in the future might.)
Add the ability for the Python CI to run the iotest linters too, which
means that iotests would be checked against:
- Python 3.6, using a frozen set of packages using 'pipenv'
- Python 3.6 through Python 3.10 inclusive, using 'tox' and the latest
versions of mypy/pylint that happen to be installed during test
time. (This CI test is allowed to fail with a warning, and can serve
as a bellwether for when new incompatible changes may disrupt the
linters. Testing against old and new Python interpreters alike can
help surface incompatibility issues we may need to be aware of.)
It also means that you can cd to ./python and:
- "make venv-check", if you have Python 3.6 and pipenv installed. (On
Fedora: `dnf install python36` or `dnf install python3.6`) This will
set up a venv with exactly the same versions of all packages and their
dependencies as the CI test does. After this series, it will run the
iotest linters, too.
- "make check-tox", if you have Python 3.6 through Python 3.10
installed. (On Fedora: `dnf install python3-tox python3.10`) This will
set up five different venvs, one for each Python version, and run all
of the Python linters against each. After this series, it will also
include the iotest linters.
"John, that's annoying. None of those invocations are free from some
kind of annoying dependency. Not everyone runs Fedora!"
Yeah, yeah. This series doesn't *remove* iotest 297 either. It continues
to work just fine! There's also a slightly more involved method that
will run on "any version you happen to have", but the setup is more
laborious, and I haven't made a Makefile invocation to canonize it yet:
> cd /python
> python3 -m venv ~/.cache/qemu-venv/
> source ~/.cache/qemu-venv/bin/activate
> make develop
> make check
> deactivate
- This uses whatever version of Python you happen to have, and doesn't
require pipenv or tox.
- It should work on any distro with any python3 >= 3.6.0
- use 'activate.[fish|csh] as desired to enter the venv. (I use FiSH!)
- This will run the linters with correct versions against the qemu
packages installed into this venv.
Example outputs from the three different local execution methods, in
order as outlined above:
jsnow@scv ~/s/q/python (python-package-iotest)> make venv-check
make[1]: Entering directory '/home/jsnow/src/qemu/python'
JOB ID : f5f383275da6b9d5eb5fe717e463f47f18980d07
JOB LOG : /home/jsnow/avocado/job-results/job-2021-06-04T12.28-f5f3832/job.log
(1/5) tests/flake8.sh: PASS (0.43 s)
(2/5) tests/iotests.sh: PASS (9.93 s)
(3/5) tests/isort.sh: PASS (0.24 s)
(4/5) tests/mypy.sh: PASS (0.25 s)
(5/5) tests/pylint.sh: PASS (3.66 s)
RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB TIME : 14.85 s
make[1]: Leaving directory '/home/jsnow/src/qemu/python'
jsnow@scv ~/s/q/python (python-package-iotest)> make check-tox
GLOB sdist-make: /home/jsnow/src/qemu/python/setup.py
py36 inst-nodeps: /home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip
py36 installed: appdirs==1.4.4,astroid==2.5.6,avocado-framework==88.1,distlib==0.3.2,filelock==3.0.12,flake8==3.9.2,fusepy==3.0.1,importlib-metadata==4.5.0,importlib-resources==5.1.4,isort==5.8.0,lazy-object-proxy==1.6.0,mccabe==0.6.1,mypy==0.812,mypy-extensions==0.4.3,packaging==20.9,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pyflakes==2.3.1,pylint==2.8.3,pyparsing==2.4.7,qemu @ file:///home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip,six==1.16.0,toml==0.10.2,tox==3.23.1,typed-ast==1.4.3,typing-extensions==3.10.0.0,virtualenv==20.4.7,wrapt==1.12.1,zipp==3.4.1
py36 run-test-pre: PYTHONHASHSEED='1077404307'
py36 run-test: commands[0] | make check
JOB ID : 8d6a98b947956794e83943950a66dea2e2ee2f0b
JOB LOG : /home/jsnow/avocado/job-results/job-2021-06-04T12.29-8d6a98b/job.log
(1/5) tests/flake8.sh: PASS (0.36 s)
(2/5) tests/iotests.sh: PASS (9.64 s)
(3/5) tests/isort.sh: PASS (0.19 s)
(4/5) tests/mypy.sh: PASS (0.24 s)
(5/5) tests/pylint.sh: PASS (3.64 s)
RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB TIME : 14.38 s
py37 inst-nodeps: /home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip
py37 installed: appdirs==1.4.4,astroid==2.5.6,avocado-framework==88.1,distlib==0.3.2,filelock==3.0.12,flake8==3.9.2,fusepy==3.0.1,importlib-metadata==4.5.0,isort==5.8.0,lazy-object-proxy==1.6.0,mccabe==0.6.1,mypy==0.812,mypy-extensions==0.4.3,packaging==20.9,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pyflakes==2.3.1,pylint==2.8.3,pyparsing==2.4.7,qemu @ file:///home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip,six==1.16.0,toml==0.10.2,tox==3.23.1,typed-ast==1.4.3,typing-extensions==3.10.0.0,virtualenv==20.4.7,wrapt==1.12.1,zipp==3.4.1
py37 run-test-pre: PYTHONHASHSEED='1077404307'
py37 run-test: commands[0] | make check
JOB ID : 97419c5769a56797e1a9b4d91586d6face9be5a2
JOB LOG : /home/jsnow/avocado/job-results/job-2021-06-04T12.29-97419c5/job.log
(1/5) tests/flake8.sh: PASS (0.34 s)
(2/5) tests/iotests.sh: PASS (10.42 s)
(3/5) tests/isort.sh: PASS (0.16 s)
(4/5) tests/mypy.sh: PASS (0.20 s)
(5/5) tests/pylint.sh: PASS (3.52 s)
RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB TIME : 15.01 s
py38 inst-nodeps: /home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip
py38 installed: appdirs==1.4.4,astroid==2.5.6,avocado-framework==88.1,distlib==0.3.2,filelock==3.0.12,flake8==3.9.2,fusepy==3.0.1,isort==5.8.0,lazy-object-proxy==1.6.0,mccabe==0.6.1,mypy==0.812,mypy-extensions==0.4.3,packaging==20.9,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pyflakes==2.3.1,pylint==2.8.3,pyparsing==2.4.7,qemu @ file:///home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip,six==1.16.0,toml==0.10.2,tox==3.23.1,typed-ast==1.4.3,typing-extensions==3.10.0.0,virtualenv==20.4.7,wrapt==1.12.1
py38 run-test-pre: PYTHONHASHSEED='1077404307'
py38 run-test: commands[0] | make check
JOB ID : 1be3a502bea18cdf537426778719dce1d0c9c3a0
JOB LOG : /home/jsnow/avocado/job-results/job-2021-06-04T12.30-1be3a50/job.log
(1/5) tests/flake8.sh: PASS (0.29 s)
(2/5) tests/iotests.sh: PASS (9.17 s)
(3/5) tests/isort.sh: PASS (0.14 s)
(4/5) tests/mypy.sh: PASS (0.20 s)
(5/5) tests/pylint.sh: PASS (3.21 s)
RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB TIME : 13.32 s
py39 inst-nodeps: /home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip
py39 installed: appdirs==1.4.4,astroid==2.5.6,avocado-framework==88.1,distlib==0.3.2,filelock==3.0.12,flake8==3.9.2,fusepy==3.0.1,isort==5.8.0,lazy-object-proxy==1.6.0,mccabe==0.6.1,mypy==0.812,mypy-extensions==0.4.3,packaging==20.9,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pyflakes==2.3.1,pylint==2.8.3,pyparsing==2.4.7,qemu @ file:///home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip,six==1.16.0,toml==0.10.2,tox==3.23.1,typed-ast==1.4.3,typing-extensions==3.10.0.0,virtualenv==20.4.7,wrapt==1.12.1
py39 run-test-pre: PYTHONHASHSEED='1077404307'
py39 run-test: commands[0] | make check
JOB ID : 0323fcaf5137caab9fbca3e91bc0338ae6cb81dc
JOB LOG : /home/jsnow/avocado/job-results/job-2021-06-04T12.30-0323fca/job.log
(1/5) tests/flake8.sh: PASS (0.26 s)
(2/5) tests/iotests.sh: PASS (10.03 s)
(3/5) tests/isort.sh: PASS (0.14 s)
(4/5) tests/mypy.sh: PASS (0.19 s)
(5/5) tests/pylint.sh: PASS (3.39 s)
RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB TIME : 14.37 s
py310 inst-nodeps: /home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip
py310 installed: appdirs==1.4.4,astroid==2.5.6,avocado-framework==88.1,distlib==0.3.2,filelock==3.0.12,flake8==3.9.2,fusepy==3.0.1,isort==5.8.0,lazy-object-proxy==1.6.0,mccabe==0.6.1,mypy==0.812,mypy-extensions==0.4.3,packaging==20.9,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pyflakes==2.3.1,pylint==2.8.3,pyparsing==2.4.7,qemu @ file:///home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip,six==1.16.0,toml==0.10.2,tox==3.23.1,typed-ast==1.4.3,typing-extensions==3.10.0.0,virtualenv==20.4.7,wrapt==1.12.1
py310 run-test-pre: PYTHONHASHSEED='1077404307'
py310 run-test: commands[0] | make check
JOB ID : 88f99ef4b76af4e48e9b1cd845d276d1c29d32dd
JOB LOG : /home/jsnow/avocado/job-results/job-2021-06-04T12.30-88f99ef/job.log
(1/5) tests/flake8.sh: PASS (0.26 s)
(2/5) tests/iotests.sh: PASS (13.34 s)
(3/5) tests/isort.sh: PASS (0.15 s)
(4/5) tests/mypy.sh: PASS (0.33 s)
(5/5) tests/pylint.sh: PASS (3.40 s)
RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB TIME : 17.76 s
_______________________________________________________________ summary ________________________________________________________________
py36: commands succeeded
py37: commands succeeded
py38: commands succeeded
py39: commands succeeded
py310: commands succeeded
congratulations :)
(qemu-venv) jsnow@scv ~/s/q/python (python-package-iotest)> make check
JOB ID : d4d3abff53bef6f41b5e2d10d889040d3a698208
JOB LOG : /home/jsnow/avocado/job-results/job-2021-06-04T12.22-d4d3abf/job.log
(1/5) tests/flake8.sh: PASS (0.27 s)
(2/5) tests/iotests.sh: PASS (10.30 s)
(3/5) tests/isort.sh: PASS (0.15 s)
(4/5) tests/mypy.sh: PASS (0.19 s)
(5/5) tests/pylint.sh: PASS (3.40 s)
RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB TIME : 14.65 s
John Snow (3):
python: expose typing information via PEP 561
iotests: split 'linters.py' off from 297
python: Add iotest linters to test suite
python/qemu/machine/py.typed | 0
python/qemu/qmp/py.typed | 0
python/qemu/utils/py.typed | 0
python/setup.cfg | 3 +
python/tests/iotests.sh | 2 +
tests/qemu-iotests/297 | 88 ++++-------------------
tests/qemu-iotests/linters.py | 130 ++++++++++++++++++++++++++++++++++
7 files changed, 148 insertions(+), 75 deletions(-)
create mode 100644 python/qemu/machine/py.typed
create mode 100644 python/qemu/qmp/py.typed
create mode 100644 python/qemu/utils/py.typed
create mode 100755 python/tests/iotests.sh
create mode 100644 tests/qemu-iotests/linters.py
--
2.31.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH RFC 1/3] python: expose typing information via PEP 561 2021-06-04 16:39 [PATCH RFC 0/3] python/iotests: Run iotest linters during Python CI John Snow @ 2021-06-04 16:39 ` John Snow 2021-06-04 16:39 ` [PATCH RFC 2/3] iotests: split 'linters.py' off from 297 John Snow ` (2 subsequent siblings) 3 siblings, 0 replies; 8+ messages in thread From: John Snow @ 2021-06-04 16:39 UTC (permalink / raw) To: qemu-devel Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost, qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow Create 'py.typed' files in each subpackage that indicate to mypy that this is a typed module, so that users of any of these packages can use mypy to check their code as well. (Note: theoretically it's possible to ditch MANIFEST.in in favor of using package_data, but I genuinely could not figure out how to get it to include things from the source root into the package root. I tried!...) https: //www.python.org/dev/peps/pep-0561/#specification Signed-off-by: John Snow <jsnow@redhat.com> --- python/qemu/machine/py.typed | 0 python/qemu/qmp/py.typed | 0 python/qemu/utils/py.typed | 0 python/setup.cfg | 3 +++ 4 files changed, 3 insertions(+) create mode 100644 python/qemu/machine/py.typed create mode 100644 python/qemu/qmp/py.typed create mode 100644 python/qemu/utils/py.typed diff --git a/python/qemu/machine/py.typed b/python/qemu/machine/py.typed new file mode 100644 index 0000000000..e69de29bb2 diff --git a/python/qemu/qmp/py.typed b/python/qemu/qmp/py.typed new file mode 100644 index 0000000000..e69de29bb2 diff --git a/python/qemu/utils/py.typed b/python/qemu/utils/py.typed new file mode 100644 index 0000000000..e69de29bb2 diff --git a/python/setup.cfg b/python/setup.cfg index 0fcdec6f32..a1f9e9d76d 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -37,6 +37,9 @@ devel = pylint >= 2.8.0 tox >= 3.18.0 +[options.package_data] +* = py.typed + [flake8] extend-ignore = E722 # Prefer pylint's bare-except checks to flake8's exclude = __pycache__, -- 2.31.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH RFC 2/3] iotests: split 'linters.py' off from 297 2021-06-04 16:39 [PATCH RFC 0/3] python/iotests: Run iotest linters during Python CI John Snow 2021-06-04 16:39 ` [PATCH RFC 1/3] python: expose typing information via PEP 561 John Snow @ 2021-06-04 16:39 ` John Snow 2021-06-05 14:27 ` Vladimir Sementsov-Ogievskiy 2021-06-04 16:39 ` [PATCH RFC 3/3] python: Add iotest linters to test suite John Snow 2021-06-05 14:08 ` [PATCH RFC 0/3] python/iotests: Run iotest linters during Python CI Vladimir Sementsov-Ogievskiy 3 siblings, 1 reply; 8+ messages in thread From: John Snow @ 2021-06-04 16:39 UTC (permalink / raw) To: qemu-devel Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost, qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow Refactor the core function of the linting configuration out of 297 and into a new file called linters.py. Now, linters.py represents an invocation of the linting scripts that more resembles a "normal" execution of pylint/mypy, like you'd expect to use if 'qemu' was a bona-fide package you obtained from PyPI. 297, by contrast, now represents the iotests-specific configuration bits you need to get it to function correctly as a part of iotests, and with 'qemu' as a namespace package that isn't "installed" to the current environment, but just lives elsewhere in our source tree. By doing this, we will able to run the same linting configuration from the Python CI tests without calling iotest logging functions or messing around with PYTHONPATH / MYPYPATH. iotest 297 continues to operate in a standalone fashion for now -- presumably, it's convenient for block maintainers and contributors to run in this manner. See the following commit for how this is used from the Python packaging side. Signed-off-by: John Snow <jsnow@redhat.com> --- - It's a big glob of a patch. Sorry. I can work it into smaller pieces if the idea is well received. - I change the invocations of mypy/pylint to "python3 -m pylint" and "python3 -m mypy" respectively, which causes these linters to use the virtual environment's preferred version. This forces the test to use the test environments curated by the CI jobs. - If you have installed Fedora's pylint package that provides "pylint-3", the above trick will still work correctly. - Checking for "pylint-3" specifically in 297 was left alone. Theoretically, this check could be broadened to simply look for the presence of a 'pylint' module to allow it to be more permissive. Signed-off-by: John Snow <jsnow@redhat.com> --- tests/qemu-iotests/297 | 88 ++++------------------- tests/qemu-iotests/linters.py | 130 ++++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 75 deletions(-) create mode 100644 tests/qemu-iotests/linters.py diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297 index 433b732336..5c753279fc 100755 --- a/tests/qemu-iotests/297 +++ b/tests/qemu-iotests/297 @@ -17,98 +17,36 @@ # along with this program. If not, see <http://www.gnu.org/licenses/>. import os -import re import shutil -import subprocess -import sys import iotests +import linters -# TODO: Empty this list! -SKIP_FILES = ( - '030', '040', '041', '044', '045', '055', '056', '057', '065', '093', - '096', '118', '124', '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', '302', '303', '304', '307', - 'nbd-fault-injector.py', 'qcow2.py', 'qcow2_format.py', 'qed.py' -) - - -def is_python_file(filename): - if not os.path.isfile(filename): - return False - - if filename.endswith('.py'): - return True - - with open(filename) as f: - try: - first_line = f.readline() - return re.match('^#!.*python', first_line) is not None - except UnicodeDecodeError: # Ignore binary files - return False - - -def run_linters(): - files = [filename for filename in (set(os.listdir('.')) - set(SKIP_FILES)) - if is_python_file(filename)] +def main(): + files = linters.get_test_files() iotests.logger.debug('Files to be checked:') iotests.logger.debug(', '.join(sorted(files))) - print('=== pylint ===') - sys.stdout.flush() - - # Todo notes are fine, but fixme's or xxx's should probably just be - # fixed (in tests, at least) env = os.environ.copy() - qemu_module_path = os.path.join(os.path.dirname(__file__), - '..', '..', 'python') + qemu_module_path = os.path.join( + os.path.dirname(__file__), + '..', '..', 'python' + ) + try: env['PYTHONPATH'] += os.pathsep + qemu_module_path except KeyError: env['PYTHONPATH'] = qemu_module_path - subprocess.run(('pylint-3', '--score=n', '--notes=FIXME,XXX', *files), - env=env, check=False) - print('=== mypy ===') - sys.stdout.flush() - - # We have to call mypy separately for each 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). env['MYPYPATH'] = env['PYTHONPATH'] - for filename in files: - p = subprocess.run(('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', - '--namespace-packages', - filename), - env=env, - check=False, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - universal_newlines=True) - if p.returncode != 0: - print(p.stdout) + for linter in ('pylint-3', 'mypy'): + if shutil.which(linter) is None: + iotests.notrun(f'{linter} not found') + iotests.script_main(lambda: linters.run_linters(files, env=env)) -for linter in ('pylint-3', 'mypy'): - if shutil.which(linter) is None: - iotests.notrun(f'{linter} not found') -iotests.script_main(run_linters) +main() diff --git a/tests/qemu-iotests/linters.py b/tests/qemu-iotests/linters.py new file mode 100644 index 0000000000..1bbcfd1088 --- /dev/null +++ b/tests/qemu-iotests/linters.py @@ -0,0 +1,130 @@ +# Copyright (C) 2020 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +import os +import re +import subprocess +import sys +from typing import List, Mapping, Optional + + +# TODO: Empty this list! +SKIP_FILES = ( + '030', '040', '041', '044', '045', '055', '056', '057', '065', '093', + '096', '118', '124', '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', '302', '303', '304', '307', + 'nbd-fault-injector.py', 'qcow2.py', 'qcow2_format.py', 'qed.py' +) + + +def is_python_file(filename: str, directory: str = '.') -> bool: + filepath = os.path.join(directory, filename) + + if not os.path.isfile(filepath): + return False + + if filename.endswith('.py'): + return True + + with open(filepath) as f: + try: + first_line = f.readline() + return re.match('^#!.*python', first_line) is not None + except UnicodeDecodeError: # Ignore binary files + return False + + +def get_test_files(directory: str = '.') -> List[str]: + return [ + f for f in (set(os.listdir(directory)) - set(SKIP_FILES)) + if is_python_file(f, directory) + ] + + +def run_linters( + files: List[str], + directory: str = '.', + env: Optional[Mapping[str, str]] = None, +) -> int: + ret = 0 + + print('=== pylint ===') + sys.stdout.flush() + + # Todo notes are fine, but fixme's or xxx's should probably just be + # fixed (in tests, at least) + p = subprocess.run( + ('python3', '-m', 'pylint', '--score=n', '--notes=FIXME,XXX', *files), + cwd=directory, + env=env, + check=False, + universal_newlines=True, + ) + ret += p.returncode + + print('=== mypy ===') + sys.stdout.flush() + + # We have to call mypy separately for each 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 filename in files: + p = subprocess.run( + ( + 'python3', '-m', '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', + '--namespace-packages', + filename, + ), + cwd=directory, + env=env, + check=False, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + universal_newlines=True + ) + + ret += p.returncode + if p.returncode != 0: + print(p.stdout) + + return ret + + +def main() -> int: + """ + Used by the Python CI system as an entry point to run these linters. + """ + directory = os.path.dirname(os.path.realpath(__file__)) + files = get_test_files(directory) + return run_linters(files, directory) + + +if __name__ == '__main__': + sys.exit(main()) -- 2.31.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH RFC 2/3] iotests: split 'linters.py' off from 297 2021-06-04 16:39 ` [PATCH RFC 2/3] iotests: split 'linters.py' off from 297 John Snow @ 2021-06-05 14:27 ` Vladimir Sementsov-Ogievskiy 2021-06-07 15:40 ` John Snow 0 siblings, 1 reply; 8+ messages in thread From: Vladimir Sementsov-Ogievskiy @ 2021-06-05 14:27 UTC (permalink / raw) To: John Snow, qemu-devel Cc: Max Reitz, Markus Armbruster, Kevin Wolf, qemu-block, Cleber Rosa, Eduardo Habkost 04.06.2021 19:39, John Snow wrote: > Refactor the core function of the linting configuration out of 297 and > into a new file called linters.py. > > Now, linters.py represents an invocation of the linting scripts that > more resembles a "normal" execution of pylint/mypy, like you'd expect to > use if 'qemu' was a bona-fide package you obtained from PyPI. > > 297, by contrast, now represents the iotests-specific configuration bits > you need to get it to function correctly as a part of iotests, and with > 'qemu' as a namespace package that isn't "installed" to the current > environment, but just lives elsewhere in our source tree. > > By doing this, we will able to run the same linting configuration from > the Python CI tests without calling iotest logging functions or messing > around with PYTHONPATH / MYPYPATH. > > iotest 297 continues to operate in a standalone fashion for now -- > presumably, it's convenient for block maintainers and contributors to > run in this manner. > > See the following commit for how this is used from the Python packaging side. > > Signed-off-by: John Snow <jsnow@redhat.com> > > --- > > - It's a big glob of a patch. Sorry. I can work it into smaller pieces > if the idea is well received. If at least split movement from other refactoring, would be good > > - I change the invocations of mypy/pylint to "python3 -m pylint" and > "python3 -m mypy" respectively, which causes these linters to use the > virtual environment's preferred version. This forces the test to use the > test environments curated by the CI jobs. > > - If you have installed Fedora's pylint package that provides > "pylint-3", the above trick will still work correctly. > > - Checking for "pylint-3" specifically in 297 was left > alone. Theoretically, this check could be broadened to simply look for > the presence of a 'pylint' module to allow it to be more permissive. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > tests/qemu-iotests/297 | 88 ++++------------------- > tests/qemu-iotests/linters.py | 130 ++++++++++++++++++++++++++++++++++ > 2 files changed, 143 insertions(+), 75 deletions(-) > create mode 100644 tests/qemu-iotests/linters.py > > diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297 > index 433b732336..5c753279fc 100755 > --- a/tests/qemu-iotests/297 > +++ b/tests/qemu-iotests/297 > @@ -17,98 +17,36 @@ > # along with this program. If not, see <http://www.gnu.org/licenses/>. > > import os > -import re > import shutil [..] > - # fixed (in tests, at least) > env = os.environ.copy() > - qemu_module_path = os.path.join(os.path.dirname(__file__), > - '..', '..', 'python') > + qemu_module_path = os.path.join( > + os.path.dirname(__file__), > + '..', '..', 'python' > + ) Hmm, you made 3 lines from 2 :) ... If rename to python_path it will fit into one line. I'm not sure that it's better. > + > try: > env['PYTHONPATH'] += os.pathsep + qemu_module_path > except KeyError: > env['PYTHONPATH'] = qemu_module_path > - subprocess.run(('pylint-3', '--score=n', '--notes=FIXME,XXX', *files), > - env=env, check=False) > > - print('=== mypy ===') > - sys.stdout.flush() > - > - # We have to call mypy separately for each 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). > env['MYPYPATH'] = env['PYTHONPATH'] > - for filename in files: > - p = subprocess.run(('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', > - '--namespace-packages', > - filename), > - env=env, > - check=False, > - stdout=subprocess.PIPE, > - stderr=subprocess.STDOUT, > - universal_newlines=True) > > - if p.returncode != 0: > - print(p.stdout) > + for linter in ('pylint-3', 'mypy'): > + if shutil.which(linter) is None: > + iotests.notrun(f'{linter} not found') > > + iotests.script_main(lambda: linters.run_linters(files, env=env)) Why to use lambda, and not just pass main to script_main? Or, may be, use iotests.script_initialize() at top, and keep the whole script at top indentation level? -- Best regards, Vladimir ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RFC 2/3] iotests: split 'linters.py' off from 297 2021-06-05 14:27 ` Vladimir Sementsov-Ogievskiy @ 2021-06-07 15:40 ` John Snow 0 siblings, 0 replies; 8+ messages in thread From: John Snow @ 2021-06-07 15:40 UTC (permalink / raw) To: Vladimir Sementsov-Ogievskiy, qemu-devel Cc: Kevin Wolf, Eduardo Habkost, qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa On 6/5/21 10:27 AM, Vladimir Sementsov-Ogievskiy wrote: > 04.06.2021 19:39, John Snow wrote: >> Refactor the core function of the linting configuration out of 297 and >> into a new file called linters.py. >> >> Now, linters.py represents an invocation of the linting scripts that >> more resembles a "normal" execution of pylint/mypy, like you'd expect to >> use if 'qemu' was a bona-fide package you obtained from PyPI. >> >> 297, by contrast, now represents the iotests-specific configuration bits >> you need to get it to function correctly as a part of iotests, and with >> 'qemu' as a namespace package that isn't "installed" to the current >> environment, but just lives elsewhere in our source tree. >> >> By doing this, we will able to run the same linting configuration from >> the Python CI tests without calling iotest logging functions or messing >> around with PYTHONPATH / MYPYPATH. >> >> iotest 297 continues to operate in a standalone fashion for now -- >> presumably, it's convenient for block maintainers and contributors to >> run in this manner. >> >> See the following commit for how this is used from the Python >> packaging side. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> >> --- >> >> - It's a big glob of a patch. Sorry. I can work it into smaller pieces >> if the idea is well received. > > If at least split movement from other refactoring, would be good > Sure. Refactor first and then move seems like the way to go here. I'll do that. >> >> - I change the invocations of mypy/pylint to "python3 -m pylint" and >> "python3 -m mypy" respectively, which causes these linters to use the >> virtual environment's preferred version. This forces the test to >> use the >> test environments curated by the CI jobs. >> >> - If you have installed Fedora's pylint package that provides >> "pylint-3", the above trick will still work correctly. >> >> - Checking for "pylint-3" specifically in 297 was left >> alone. Theoretically, this check could be broadened to simply look for >> the presence of a 'pylint' module to allow it to be more permissive. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> tests/qemu-iotests/297 | 88 ++++------------------- >> tests/qemu-iotests/linters.py | 130 ++++++++++++++++++++++++++++++++++ >> 2 files changed, 143 insertions(+), 75 deletions(-) >> create mode 100644 tests/qemu-iotests/linters.py >> >> diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297 >> index 433b732336..5c753279fc 100755 >> --- a/tests/qemu-iotests/297 >> +++ b/tests/qemu-iotests/297 >> @@ -17,98 +17,36 @@ >> # along with this program. If not, see <http://www.gnu.org/licenses/>. >> import os >> -import re >> import shutil > > [..] > >> - # fixed (in tests, at least) >> env = os.environ.copy() >> - qemu_module_path = os.path.join(os.path.dirname(__file__), >> - '..', '..', 'python') >> + qemu_module_path = os.path.join( >> + os.path.dirname(__file__), >> + '..', '..', 'python' >> + ) > > Hmm, you made 3 lines from 2 :) ... If rename to python_path it will fit > into one line. I'm not sure that it's better. > I'll try it. I don't expect these args to change often so I don't insist on the args-as-code-block thing here. >> + >> try: >> env['PYTHONPATH'] += os.pathsep + qemu_module_path >> except KeyError: >> env['PYTHONPATH'] = qemu_module_path >> - subprocess.run(('pylint-3', '--score=n', '--notes=FIXME,XXX', >> *files), >> - env=env, check=False) >> - print('=== mypy ===') >> - sys.stdout.flush() >> - >> - # We have to call mypy separately for each 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). >> env['MYPYPATH'] = env['PYTHONPATH'] >> - for filename in files: >> - p = subprocess.run(('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', >> - '--namespace-packages', >> - filename), >> - env=env, >> - check=False, >> - stdout=subprocess.PIPE, >> - stderr=subprocess.STDOUT, >> - universal_newlines=True) >> - if p.returncode != 0: >> - print(p.stdout) >> + for linter in ('pylint-3', 'mypy'): >> + if shutil.which(linter) is None: >> + iotests.notrun(f'{linter} not found') >> + iotests.script_main(lambda: linters.run_linters(files, env=env)) > > Why to use lambda, and not just pass main to script_main? > No strong reason, just a messy draft. I can clean it up as you suggest. > Or, may be, use iotests.script_initialize() at top, and keep the whole > script at top indentation level? That works too, but it's more churn. --- I need to look into the mypy invocation and see if I can't figure out a way for it to work for everything at once instead of needing to run one at a time. Maybe that's something to worry about later, though. Eventually, the custom linter invocation here should be stored in more traditional configuration files (pylintrc, mypy.ini) as much as is possible. The environment hacking stuff will need to remain here as long as iotests does not run in a virtual environment, however. I'd like to eventually change that, too. --js ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH RFC 3/3] python: Add iotest linters to test suite 2021-06-04 16:39 [PATCH RFC 0/3] python/iotests: Run iotest linters during Python CI John Snow 2021-06-04 16:39 ` [PATCH RFC 1/3] python: expose typing information via PEP 561 John Snow 2021-06-04 16:39 ` [PATCH RFC 2/3] iotests: split 'linters.py' off from 297 John Snow @ 2021-06-04 16:39 ` John Snow 2021-06-05 14:08 ` [PATCH RFC 0/3] python/iotests: Run iotest linters during Python CI Vladimir Sementsov-Ogievskiy 3 siblings, 0 replies; 8+ messages in thread From: John Snow @ 2021-06-04 16:39 UTC (permalink / raw) To: qemu-devel Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost, qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow As a convenience, since iotests is an extremely prominent user of the qemu.qmp and qemu.machine packages and already implements a linting regime, run those tests as well so that it's very hard to miss regressions caused by changes to the python library. Signed-off-by: John Snow <jsnow@redhat.com> --- python/tests/iotests.sh | 2 ++ 1 file changed, 2 insertions(+) create mode 100755 python/tests/iotests.sh diff --git a/python/tests/iotests.sh b/python/tests/iotests.sh new file mode 100755 index 0000000000..ec2fc58066 --- /dev/null +++ b/python/tests/iotests.sh @@ -0,0 +1,2 @@ +#!/bin/sh -e +PYTHONPATH=../tests/qemu-iotests/ python3 -m linters -- 2.31.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH RFC 0/3] python/iotests: Run iotest linters during Python CI 2021-06-04 16:39 [PATCH RFC 0/3] python/iotests: Run iotest linters during Python CI John Snow ` (2 preceding siblings ...) 2021-06-04 16:39 ` [PATCH RFC 3/3] python: Add iotest linters to test suite John Snow @ 2021-06-05 14:08 ` Vladimir Sementsov-Ogievskiy 2021-06-07 15:51 ` John Snow 3 siblings, 1 reply; 8+ messages in thread From: Vladimir Sementsov-Ogievskiy @ 2021-06-05 14:08 UTC (permalink / raw) To: John Snow, qemu-devel Cc: Max Reitz, Markus Armbruster, Kevin Wolf, qemu-block, Cleber Rosa, Eduardo Habkost 04.06.2021 19:39, John Snow wrote: > Since iotests are such a heavy and prominent user of the Python qemu.qmp > and qemu.machine packages, it would be convenient if the Python linting > suite also checked this client for any possible regressions introduced > by shifting around signatures, types, or interfaces in these packages. I think that's a right idea. > > (Of course, we'd eventually find those problems when iotest 297 ran, but > with increasing distance between Python development and Block > development, the risk of an accidental breakage in this regard > increases. I, personally, know to run iotests (and especially 297) after > changing Python code, but not everyone in the future might.) > > Add the ability for the Python CI to run the iotest linters too, which > means that iotests would be checked against: > > - Python 3.6, using a frozen set of packages using 'pipenv' > - Python 3.6 through Python 3.10 inclusive, using 'tox' and the latest > versions of mypy/pylint that happen to be installed during test > time. (This CI test is allowed to fail with a warning, and can serve > as a bellwether for when new incompatible changes may disrupt the > linters. Testing against old and new Python interpreters alike can > help surface incompatibility issues we may need to be aware of.) > > It also means that you can cd to ./python and: > > - "make venv-check", if you have Python 3.6 and pipenv installed. (On > Fedora: `dnf install python36` or `dnf install python3.6`) This will > set up a venv with exactly the same versions of all packages and their > dependencies as the CI test does. After this series, it will run the > iotest linters, too. > > - "make check-tox", if you have Python 3.6 through Python 3.10 > installed. (On Fedora: `dnf install python3-tox python3.10`) This will > set up five different venvs, one for each Python version, and run all > of the Python linters against each. After this series, it will also > include the iotest linters. So, it doesn't run from "make check"? > > "John, that's annoying. None of those invocations are free from some > kind of annoying dependency. Not everyone runs Fedora!" > > Yeah, yeah. This series doesn't *remove* iotest 297 either. It continues > to work just fine! There's also a slightly more involved method that > will run on "any version you happen to have", but the setup is more > laborious, and I haven't made a Makefile invocation to canonize it yet: > >> cd /python >> python3 -m venv ~/.cache/qemu-venv/ >> source ~/.cache/qemu-venv/bin/activate >> make develop >> make check >> deactivate > > - This uses whatever version of Python you happen to have, and doesn't > require pipenv or tox. > - It should work on any distro with any python3 >= 3.6.0 > - use 'activate.[fish|csh] as desired to enter the venv. (I use FiSH!) > - This will run the linters with correct versions against the qemu > packages installed into this venv. > > Example outputs from the three different local execution methods, in > order as outlined above: > > jsnow@scv ~/s/q/python (python-package-iotest)> make venv-check > make[1]: Entering directory '/home/jsnow/src/qemu/python' > JOB ID : f5f383275da6b9d5eb5fe717e463f47f18980d07 > JOB LOG : /home/jsnow/avocado/job-results/job-2021-06-04T12.28-f5f3832/job.log > (1/5) tests/flake8.sh: PASS (0.43 s) > (2/5) tests/iotests.sh: PASS (9.93 s) > (3/5) tests/isort.sh: PASS (0.24 s) > (4/5) tests/mypy.sh: PASS (0.25 s) > (5/5) tests/pylint.sh: PASS (3.66 s) > RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0 > JOB TIME : 14.85 s > make[1]: Leaving directory '/home/jsnow/src/qemu/python' > > jsnow@scv ~/s/q/python (python-package-iotest)> make check-tox > GLOB sdist-make: /home/jsnow/src/qemu/python/setup.py > py36 inst-nodeps: /home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip > py36 installed: appdirs==1.4.4,astroid==2.5.6,avocado-framework==88.1,distlib==0.3.2,filelock==3.0.12,flake8==3.9.2,fusepy==3.0.1,importlib-metadata==4.5.0,importlib-resources==5.1.4,isort==5.8.0,lazy-object-proxy==1.6.0,mccabe==0.6.1,mypy==0.812,mypy-extensions==0.4.3,packaging==20.9,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pyflakes==2.3.1,pylint==2.8.3,pyparsing==2.4.7,qemu @ file:///home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip,six==1.16.0,toml==0.10.2,tox==3.23.1,typed-ast==1.4.3,typing-extensions==3.10.0.0,virtualenv==20.4.7,wrapt==1.12.1,zipp==3.4.1 > py36 run-test-pre: PYTHONHASHSEED='1077404307' > py36 run-test: commands[0] | make check > JOB ID : 8d6a98b947956794e83943950a66dea2e2ee2f0b > JOB LOG : /home/jsnow/avocado/job-results/job-2021-06-04T12.29-8d6a98b/job.log > (1/5) tests/flake8.sh: PASS (0.36 s) > (2/5) tests/iotests.sh: PASS (9.64 s) > (3/5) tests/isort.sh: PASS (0.19 s) > (4/5) tests/mypy.sh: PASS (0.24 s) > (5/5) tests/pylint.sh: PASS (3.64 s) > RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0 > JOB TIME : 14.38 s > py37 inst-nodeps: /home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip > py37 installed: appdirs==1.4.4,astroid==2.5.6,avocado-framework==88.1,distlib==0.3.2,filelock==3.0.12,flake8==3.9.2,fusepy==3.0.1,importlib-metadata==4.5.0,isort==5.8.0,lazy-object-proxy==1.6.0,mccabe==0.6.1,mypy==0.812,mypy-extensions==0.4.3,packaging==20.9,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pyflakes==2.3.1,pylint==2.8.3,pyparsing==2.4.7,qemu @ file:///home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip,six==1.16.0,toml==0.10.2,tox==3.23.1,typed-ast==1.4.3,typing-extensions==3.10.0.0,virtualenv==20.4.7,wrapt==1.12.1,zipp==3.4.1 > py37 run-test-pre: PYTHONHASHSEED='1077404307' > py37 run-test: commands[0] | make check > JOB ID : 97419c5769a56797e1a9b4d91586d6face9be5a2 > JOB LOG : /home/jsnow/avocado/job-results/job-2021-06-04T12.29-97419c5/job.log > (1/5) tests/flake8.sh: PASS (0.34 s) > (2/5) tests/iotests.sh: PASS (10.42 s) > (3/5) tests/isort.sh: PASS (0.16 s) > (4/5) tests/mypy.sh: PASS (0.20 s) > (5/5) tests/pylint.sh: PASS (3.52 s) > RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0 > JOB TIME : 15.01 s > py38 inst-nodeps: /home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip > py38 installed: appdirs==1.4.4,astroid==2.5.6,avocado-framework==88.1,distlib==0.3.2,filelock==3.0.12,flake8==3.9.2,fusepy==3.0.1,isort==5.8.0,lazy-object-proxy==1.6.0,mccabe==0.6.1,mypy==0.812,mypy-extensions==0.4.3,packaging==20.9,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pyflakes==2.3.1,pylint==2.8.3,pyparsing==2.4.7,qemu @ file:///home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip,six==1.16.0,toml==0.10.2,tox==3.23.1,typed-ast==1.4.3,typing-extensions==3.10.0.0,virtualenv==20.4.7,wrapt==1.12.1 > py38 run-test-pre: PYTHONHASHSEED='1077404307' > py38 run-test: commands[0] | make check > JOB ID : 1be3a502bea18cdf537426778719dce1d0c9c3a0 > JOB LOG : /home/jsnow/avocado/job-results/job-2021-06-04T12.30-1be3a50/job.log > (1/5) tests/flake8.sh: PASS (0.29 s) > (2/5) tests/iotests.sh: PASS (9.17 s) > (3/5) tests/isort.sh: PASS (0.14 s) > (4/5) tests/mypy.sh: PASS (0.20 s) > (5/5) tests/pylint.sh: PASS (3.21 s) > RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0 > JOB TIME : 13.32 s > py39 inst-nodeps: /home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip > py39 installed: appdirs==1.4.4,astroid==2.5.6,avocado-framework==88.1,distlib==0.3.2,filelock==3.0.12,flake8==3.9.2,fusepy==3.0.1,isort==5.8.0,lazy-object-proxy==1.6.0,mccabe==0.6.1,mypy==0.812,mypy-extensions==0.4.3,packaging==20.9,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pyflakes==2.3.1,pylint==2.8.3,pyparsing==2.4.7,qemu @ file:///home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip,six==1.16.0,toml==0.10.2,tox==3.23.1,typed-ast==1.4.3,typing-extensions==3.10.0.0,virtualenv==20.4.7,wrapt==1.12.1 > py39 run-test-pre: PYTHONHASHSEED='1077404307' > py39 run-test: commands[0] | make check > JOB ID : 0323fcaf5137caab9fbca3e91bc0338ae6cb81dc > JOB LOG : /home/jsnow/avocado/job-results/job-2021-06-04T12.30-0323fca/job.log > (1/5) tests/flake8.sh: PASS (0.26 s) > (2/5) tests/iotests.sh: PASS (10.03 s) > (3/5) tests/isort.sh: PASS (0.14 s) > (4/5) tests/mypy.sh: PASS (0.19 s) > (5/5) tests/pylint.sh: PASS (3.39 s) > RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0 > JOB TIME : 14.37 s > py310 inst-nodeps: /home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip > py310 installed: appdirs==1.4.4,astroid==2.5.6,avocado-framework==88.1,distlib==0.3.2,filelock==3.0.12,flake8==3.9.2,fusepy==3.0.1,isort==5.8.0,lazy-object-proxy==1.6.0,mccabe==0.6.1,mypy==0.812,mypy-extensions==0.4.3,packaging==20.9,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pyflakes==2.3.1,pylint==2.8.3,pyparsing==2.4.7,qemu @ file:///home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip,six==1.16.0,toml==0.10.2,tox==3.23.1,typed-ast==1.4.3,typing-extensions==3.10.0.0,virtualenv==20.4.7,wrapt==1.12.1 > py310 run-test-pre: PYTHONHASHSEED='1077404307' > py310 run-test: commands[0] | make check > JOB ID : 88f99ef4b76af4e48e9b1cd845d276d1c29d32dd > JOB LOG : /home/jsnow/avocado/job-results/job-2021-06-04T12.30-88f99ef/job.log > (1/5) tests/flake8.sh: PASS (0.26 s) > (2/5) tests/iotests.sh: PASS (13.34 s) > (3/5) tests/isort.sh: PASS (0.15 s) > (4/5) tests/mypy.sh: PASS (0.33 s) > (5/5) tests/pylint.sh: PASS (3.40 s) > RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0 > JOB TIME : 17.76 s > _______________________________________________________________ summary ________________________________________________________________ > py36: commands succeeded > py37: commands succeeded > py38: commands succeeded > py39: commands succeeded > py310: commands succeeded > congratulations :) > > (qemu-venv) jsnow@scv ~/s/q/python (python-package-iotest)> make check > JOB ID : d4d3abff53bef6f41b5e2d10d889040d3a698208 > JOB LOG : /home/jsnow/avocado/job-results/job-2021-06-04T12.22-d4d3abf/job.log > (1/5) tests/flake8.sh: PASS (0.27 s) > (2/5) tests/iotests.sh: PASS (10.30 s) > (3/5) tests/isort.sh: PASS (0.15 s) > (4/5) tests/mypy.sh: PASS (0.19 s) > (5/5) tests/pylint.sh: PASS (3.40 s) > RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0 > JOB TIME : 14.65 s > > John Snow (3): > python: expose typing information via PEP 561 > iotests: split 'linters.py' off from 297 > python: Add iotest linters to test suite > > python/qemu/machine/py.typed | 0 > python/qemu/qmp/py.typed | 0 > python/qemu/utils/py.typed | 0 > python/setup.cfg | 3 + > python/tests/iotests.sh | 2 + > tests/qemu-iotests/297 | 88 ++++------------------- > tests/qemu-iotests/linters.py | 130 ++++++++++++++++++++++++++++++++++ > 7 files changed, 148 insertions(+), 75 deletions(-) > create mode 100644 python/qemu/machine/py.typed > create mode 100644 python/qemu/qmp/py.typed > create mode 100644 python/qemu/utils/py.typed > create mode 100755 python/tests/iotests.sh > create mode 100644 tests/qemu-iotests/linters.py > -- Best regards, Vladimir ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RFC 0/3] python/iotests: Run iotest linters during Python CI 2021-06-05 14:08 ` [PATCH RFC 0/3] python/iotests: Run iotest linters during Python CI Vladimir Sementsov-Ogievskiy @ 2021-06-07 15:51 ` John Snow 0 siblings, 0 replies; 8+ messages in thread From: John Snow @ 2021-06-07 15:51 UTC (permalink / raw) To: Vladimir Sementsov-Ogievskiy, qemu-devel Cc: Kevin Wolf, Eduardo Habkost, qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa On 6/5/21 10:08 AM, Vladimir Sementsov-Ogievskiy wrote: > 04.06.2021 19:39, John Snow wrote: >> Since iotests are such a heavy and prominent user of the Python qemu.qmp >> and qemu.machine packages, it would be convenient if the Python linting >> suite also checked this client for any possible regressions introduced >> by shifting around signatures, types, or interfaces in these packages. > > I think that's a right idea. > Yep. It will help me to stabilize the qemu.qmp interface and make sure there are no breakages. >> >> (Of course, we'd eventually find those problems when iotest 297 ran, but >> with increasing distance between Python development and Block >> development, the risk of an accidental breakage in this regard >> increases. I, personally, know to run iotests (and especially 297) after >> changing Python code, but not everyone in the future might.) >> >> Add the ability for the Python CI to run the iotest linters too, which >> means that iotests would be checked against: >> >> - Python 3.6, using a frozen set of packages using 'pipenv' >> - Python 3.6 through Python 3.10 inclusive, using 'tox' and the latest >> versions of mypy/pylint that happen to be installed during test >> time. (This CI test is allowed to fail with a warning, and can serve >> as a bellwether for when new incompatible changes may disrupt the >> linters. Testing against old and new Python interpreters alike can >> help surface incompatibility issues we may need to be aware of.) >> >> It also means that you can cd to ./python and: >> >> - "make venv-check", if you have Python 3.6 and pipenv installed. (On >> Fedora: `dnf install python36` or `dnf install python3.6`) This will >> set up a venv with exactly the same versions of all packages and their >> dependencies as the CI test does. After this series, it will run the >> iotest linters, too. >> >> - "make check-tox", if you have Python 3.6 through Python 3.10 >> installed. (On Fedora: `dnf install python3-tox python3.10`) This will >> set up five different venvs, one for each Python version, and run all >> of the Python linters against each. After this series, it will also >> include the iotest linters. > > So, it doesn't run from "make check"? > Well.. it would, but it wouldn't necessarily succeed. It depends on the environment in which you run it. "make venv-check" and "make check-tox" both set up their own VENV for running the linters, handling dependencies for you. "make check" does not. Both venv-check and check-tox simply run "make check" as their only command after they set up their venv(s). It requires some kind of setup. I avoided suggesting it in this cover letter for that reason. Cleber said during review that maybe the fact that "make check" isn't a "handle things for you" command is confusing, and with this review comment from you, I am starting to agree. Maybe we want this instead: make venv-check [pipenv] make raw-check make check-tox [tox] make raw-check make check [venv] make raw-check with "make raw-check" being the setup-less check step that everything else uses, and "make check" representing the third type of test I outlined below in this cover letter. >> >> "John, that's annoying. None of those invocations are free from some >> kind of annoying dependency. Not everyone runs Fedora!" >> >> Yeah, yeah. This series doesn't *remove* iotest 297 either. It continues >> to work just fine! There's also a slightly more involved method that >> will run on "any version you happen to have", but the setup is more >> laborious, and I haven't made a Makefile invocation to canonize it yet: >> >>> cd /python >>> python3 -m venv ~/.cache/qemu-venv/ >>> source ~/.cache/qemu-venv/bin/activate >>> make develop >>> make check >>> deactivate >> >> - This uses whatever version of Python you happen to have, and doesn't >> require pipenv or tox. >> - It should work on any distro with any python3 >= 3.6.0 >> - use 'activate.[fish|csh] as desired to enter the venv. (I use FiSH!) >> - This will run the linters with correct versions against the qemu >> packages installed into this venv. >> >> Example outputs from the three different local execution methods, in >> order as outlined above: >> >> jsnow@scv ~/s/q/python (python-package-iotest)> make venv-check >> make[1]: Entering directory '/home/jsnow/src/qemu/python' >> JOB ID : f5f383275da6b9d5eb5fe717e463f47f18980d07 >> JOB LOG : >> /home/jsnow/avocado/job-results/job-2021-06-04T12.28-f5f3832/job.log >> (1/5) tests/flake8.sh: PASS (0.43 s) >> (2/5) tests/iotests.sh: PASS (9.93 s) >> (3/5) tests/isort.sh: PASS (0.24 s) >> (4/5) tests/mypy.sh: PASS (0.25 s) >> (5/5) tests/pylint.sh: PASS (3.66 s) >> RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 >> | CANCEL 0 >> JOB TIME : 14.85 s >> make[1]: Leaving directory '/home/jsnow/src/qemu/python' >> >> jsnow@scv ~/s/q/python (python-package-iotest)> make check-tox >> GLOB sdist-make: /home/jsnow/src/qemu/python/setup.py >> py36 inst-nodeps: >> /home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip >> py36 installed: >> appdirs==1.4.4,astroid==2.5.6,avocado-framework==88.1,distlib==0.3.2,filelock==3.0.12,flake8==3.9.2,fusepy==3.0.1,importlib-metadata==4.5.0,importlib-resources==5.1.4,isort==5.8.0,lazy-object-proxy==1.6.0,mccabe==0.6.1,mypy==0.812,mypy-extensions==0.4.3,packaging==20.9,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pyflakes==2.3.1,pylint==2.8.3,pyparsing==2.4.7,qemu >> @ >> file:///home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip,six==1.16.0,toml==0.10.2,tox==3.23.1,typed-ast==1.4.3,typing-extensions==3.10.0.0,virtualenv==20.4.7,wrapt==1.12.1,zipp==3.4.1 >> >> py36 run-test-pre: PYTHONHASHSEED='1077404307' >> py36 run-test: commands[0] | make check >> JOB ID : 8d6a98b947956794e83943950a66dea2e2ee2f0b >> JOB LOG : >> /home/jsnow/avocado/job-results/job-2021-06-04T12.29-8d6a98b/job.log >> (1/5) tests/flake8.sh: PASS (0.36 s) >> (2/5) tests/iotests.sh: PASS (9.64 s) >> (3/5) tests/isort.sh: PASS (0.19 s) >> (4/5) tests/mypy.sh: PASS (0.24 s) >> (5/5) tests/pylint.sh: PASS (3.64 s) >> RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 >> | CANCEL 0 >> JOB TIME : 14.38 s >> py37 inst-nodeps: >> /home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip >> py37 installed: >> appdirs==1.4.4,astroid==2.5.6,avocado-framework==88.1,distlib==0.3.2,filelock==3.0.12,flake8==3.9.2,fusepy==3.0.1,importlib-metadata==4.5.0,isort==5.8.0,lazy-object-proxy==1.6.0,mccabe==0.6.1,mypy==0.812,mypy-extensions==0.4.3,packaging==20.9,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pyflakes==2.3.1,pylint==2.8.3,pyparsing==2.4.7,qemu >> @ >> file:///home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip,six==1.16.0,toml==0.10.2,tox==3.23.1,typed-ast==1.4.3,typing-extensions==3.10.0.0,virtualenv==20.4.7,wrapt==1.12.1,zipp==3.4.1 >> >> py37 run-test-pre: PYTHONHASHSEED='1077404307' >> py37 run-test: commands[0] | make check >> JOB ID : 97419c5769a56797e1a9b4d91586d6face9be5a2 >> JOB LOG : >> /home/jsnow/avocado/job-results/job-2021-06-04T12.29-97419c5/job.log >> (1/5) tests/flake8.sh: PASS (0.34 s) >> (2/5) tests/iotests.sh: PASS (10.42 s) >> (3/5) tests/isort.sh: PASS (0.16 s) >> (4/5) tests/mypy.sh: PASS (0.20 s) >> (5/5) tests/pylint.sh: PASS (3.52 s) >> RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 >> | CANCEL 0 >> JOB TIME : 15.01 s >> py38 inst-nodeps: >> /home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip >> py38 installed: >> appdirs==1.4.4,astroid==2.5.6,avocado-framework==88.1,distlib==0.3.2,filelock==3.0.12,flake8==3.9.2,fusepy==3.0.1,isort==5.8.0,lazy-object-proxy==1.6.0,mccabe==0.6.1,mypy==0.812,mypy-extensions==0.4.3,packaging==20.9,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pyflakes==2.3.1,pylint==2.8.3,pyparsing==2.4.7,qemu >> @ >> file:///home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip,six==1.16.0,toml==0.10.2,tox==3.23.1,typed-ast==1.4.3,typing-extensions==3.10.0.0,virtualenv==20.4.7,wrapt==1.12.1 >> >> py38 run-test-pre: PYTHONHASHSEED='1077404307' >> py38 run-test: commands[0] | make check >> JOB ID : 1be3a502bea18cdf537426778719dce1d0c9c3a0 >> JOB LOG : >> /home/jsnow/avocado/job-results/job-2021-06-04T12.30-1be3a50/job.log >> (1/5) tests/flake8.sh: PASS (0.29 s) >> (2/5) tests/iotests.sh: PASS (9.17 s) >> (3/5) tests/isort.sh: PASS (0.14 s) >> (4/5) tests/mypy.sh: PASS (0.20 s) >> (5/5) tests/pylint.sh: PASS (3.21 s) >> RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 >> | CANCEL 0 >> JOB TIME : 13.32 s >> py39 inst-nodeps: >> /home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip >> py39 installed: >> appdirs==1.4.4,astroid==2.5.6,avocado-framework==88.1,distlib==0.3.2,filelock==3.0.12,flake8==3.9.2,fusepy==3.0.1,isort==5.8.0,lazy-object-proxy==1.6.0,mccabe==0.6.1,mypy==0.812,mypy-extensions==0.4.3,packaging==20.9,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pyflakes==2.3.1,pylint==2.8.3,pyparsing==2.4.7,qemu >> @ >> file:///home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip,six==1.16.0,toml==0.10.2,tox==3.23.1,typed-ast==1.4.3,typing-extensions==3.10.0.0,virtualenv==20.4.7,wrapt==1.12.1 >> >> py39 run-test-pre: PYTHONHASHSEED='1077404307' >> py39 run-test: commands[0] | make check >> JOB ID : 0323fcaf5137caab9fbca3e91bc0338ae6cb81dc >> JOB LOG : >> /home/jsnow/avocado/job-results/job-2021-06-04T12.30-0323fca/job.log >> (1/5) tests/flake8.sh: PASS (0.26 s) >> (2/5) tests/iotests.sh: PASS (10.03 s) >> (3/5) tests/isort.sh: PASS (0.14 s) >> (4/5) tests/mypy.sh: PASS (0.19 s) >> (5/5) tests/pylint.sh: PASS (3.39 s) >> RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 >> | CANCEL 0 >> JOB TIME : 14.37 s >> py310 inst-nodeps: >> /home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip >> py310 installed: >> appdirs==1.4.4,astroid==2.5.6,avocado-framework==88.1,distlib==0.3.2,filelock==3.0.12,flake8==3.9.2,fusepy==3.0.1,isort==5.8.0,lazy-object-proxy==1.6.0,mccabe==0.6.1,mypy==0.812,mypy-extensions==0.4.3,packaging==20.9,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pyflakes==2.3.1,pylint==2.8.3,pyparsing==2.4.7,qemu >> @ >> file:///home/jsnow/src/qemu/python/.tox/.tmp/package/1/qemu-0.6.1.0a1.zip,six==1.16.0,toml==0.10.2,tox==3.23.1,typed-ast==1.4.3,typing-extensions==3.10.0.0,virtualenv==20.4.7,wrapt==1.12.1 >> >> py310 run-test-pre: PYTHONHASHSEED='1077404307' >> py310 run-test: commands[0] | make check >> JOB ID : 88f99ef4b76af4e48e9b1cd845d276d1c29d32dd >> JOB LOG : >> /home/jsnow/avocado/job-results/job-2021-06-04T12.30-88f99ef/job.log >> (1/5) tests/flake8.sh: PASS (0.26 s) >> (2/5) tests/iotests.sh: PASS (13.34 s) >> (3/5) tests/isort.sh: PASS (0.15 s) >> (4/5) tests/mypy.sh: PASS (0.33 s) >> (5/5) tests/pylint.sh: PASS (3.40 s) >> RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 >> | CANCEL 0 >> JOB TIME : 17.76 s >> _______________________________________________________________ >> summary ________________________________________________________________ >> py36: commands succeeded >> py37: commands succeeded >> py38: commands succeeded >> py39: commands succeeded >> py310: commands succeeded >> congratulations :) >> >> (qemu-venv) jsnow@scv ~/s/q/python (python-package-iotest)> make check >> JOB ID : d4d3abff53bef6f41b5e2d10d889040d3a698208 >> JOB LOG : >> /home/jsnow/avocado/job-results/job-2021-06-04T12.22-d4d3abf/job.log >> (1/5) tests/flake8.sh: PASS (0.27 s) >> (2/5) tests/iotests.sh: PASS (10.30 s) >> (3/5) tests/isort.sh: PASS (0.15 s) >> (4/5) tests/mypy.sh: PASS (0.19 s) >> (5/5) tests/pylint.sh: PASS (3.40 s) >> RESULTS : PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 >> | CANCEL 0 >> JOB TIME : 14.65 s >> >> John Snow (3): >> python: expose typing information via PEP 561 >> iotests: split 'linters.py' off from 297 >> python: Add iotest linters to test suite >> >> python/qemu/machine/py.typed | 0 >> python/qemu/qmp/py.typed | 0 >> python/qemu/utils/py.typed | 0 >> python/setup.cfg | 3 + >> python/tests/iotests.sh | 2 + >> tests/qemu-iotests/297 | 88 ++++------------------- >> tests/qemu-iotests/linters.py | 130 ++++++++++++++++++++++++++++++++++ >> 7 files changed, 148 insertions(+), 75 deletions(-) >> create mode 100644 python/qemu/machine/py.typed >> create mode 100644 python/qemu/qmp/py.typed >> create mode 100644 python/qemu/utils/py.typed >> create mode 100755 python/tests/iotests.sh >> create mode 100644 tests/qemu-iotests/linters.py >> > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-06-07 15:52 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-04 16:39 [PATCH RFC 0/3] python/iotests: Run iotest linters during Python CI John Snow 2021-06-04 16:39 ` [PATCH RFC 1/3] python: expose typing information via PEP 561 John Snow 2021-06-04 16:39 ` [PATCH RFC 2/3] iotests: split 'linters.py' off from 297 John Snow 2021-06-05 14:27 ` Vladimir Sementsov-Ogievskiy 2021-06-07 15:40 ` John Snow 2021-06-04 16:39 ` [PATCH RFC 3/3] python: Add iotest linters to test suite John Snow 2021-06-05 14:08 ` [PATCH RFC 0/3] python/iotests: Run iotest linters during Python CI Vladimir Sementsov-Ogievskiy 2021-06-07 15:51 ` John Snow
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.