John Snow <js...@redhat.com> writes: > Update the python tests to also check qapi. No idea why I didn't do this > before. I guess I was counting on moving it under python/ and then just > forgot after that was NACKed. Oops, this turns out to be really easy. > > flake8, isort and mypy use the tool configuration from the existing > python directory (in setup.cfg). pylint continues to use the special > configuration located in scripts/qapi/ - that configuration is more > permissive. If we wish to unify the two configurations, that's a > separate series and a discussion for a later date. > > As a result of this patch, one would be able to run any of the following > tests locally from the qemu.git/python directory and have it cover the > scripts/qapi/ module as well. All of the following options run the > python tests, static analysis tests, and linter checks; but with > different combinations of dependencies and interpreters. > > - "make check-minreqs" Run tests specifically under our oldest supported > Python and our oldest supported dependencies. This is the test that > runs on GitLab as "check-python-minreqs". This helps ensure we do not > regress support on older platforms accidentally. > > - "make check-tox" Runs the tests under the newest supported > dependencies, but under each supported version of Python in turn. At > time of writing, this is Python 3.8 to 3.13 inclusive. This test helps > catch bleeding-edge problems before they become problems for developer > workstations. This is the GitLab test "check-python-tox" and is an > optionally run, may-fail test due to the unpredictable nature of new > dependencies being released into the ecosystem that may cause > regressions. > > - "make check-dev" Runs the tests under the newest supported > dependencies using whatever version of Python the user happens to have > installed. This is a quick convenience check that does not map to any > particular GitLab test. > > (Note! check-dev may be busted on Fedora 41 and bleeding edge versions > of setuptools. That's unrelated to this patch and I'll address it > separately and soon. Thank you for your patience, --mgmt) > > Signed-off-by: John Snow <js...@redhat.com>
Let's mention this is a step towards having "make check" run the static analysis we want developers to run, but we're not there, yet. > --- > python/setup.cfg | 1 + > python/tests/minreqs.txt | 21 +++++++++++++++++++++ > python/tests/qapi-flake8.sh | 4 ++++ > python/tests/qapi-isort.sh | 6 ++++++ > python/tests/qapi-mypy.sh | 2 ++ > python/tests/qapi-pylint.sh | 6 ++++++ > scripts/qapi/pylintrc | 1 + > 7 files changed, 41 insertions(+) > create mode 100755 python/tests/qapi-flake8.sh > create mode 100755 python/tests/qapi-isort.sh > create mode 100755 python/tests/qapi-mypy.sh > create mode 100755 python/tests/qapi-pylint.sh > > diff --git a/python/setup.cfg b/python/setup.cfg > index cf5af7e6641..84d8a1fd30d 100644 > --- a/python/setup.cfg > +++ b/python/setup.cfg > @@ -47,6 +47,7 @@ devel = > urwid >= 2.1.2 > urwid-readline >= 0.13 > Pygments >= 2.9.0 > + sphinx >= 3.4.3 > > # Provides qom-fuse functionality > fuse = > diff --git a/python/tests/minreqs.txt b/python/tests/minreqs.txt > index 19c0f5e4c50..94928936d44 100644 > --- a/python/tests/minreqs.txt > +++ b/python/tests/minreqs.txt > @@ -11,6 +11,9 @@ > # When adding new dependencies, pin the very oldest non-yanked version > # on PyPI that allows the test suite to pass. > > +# Dependencies for qapidoc/qapi_domain et al > +sphinx==3.4.3 > + > # Dependencies for the TUI addon (Required for successful linting) > urwid==2.1.2 > urwid-readline==0.13 > @@ -49,3 +52,21 @@ platformdirs==2.2.0 > toml==0.10.0 > tomlkit==0.10.1 > wrapt==1.14.0 > + > +# Transitive sphinx dependencies > +Jinja2==2.7 > +MarkupSafe==1.1.0 > +alabaster==0.7.1 > +babel==1.3 > +docutils==0.12 > +imagesize==0.5.0 > +packaging==14.0 > +pytz==2011b0 > +requests==2.5.0 > +snowballstemmer==1.1 > +sphinxcontrib-applehelp==1.0.0 > +sphinxcontrib-devhelp==1.0.0 > +sphinxcontrib-htmlhelp==1.0.0 > +sphinxcontrib-jsmath==1.0.0 > +sphinxcontrib-qthelp==1.0.0 > +sphinxcontrib-serializinghtml==1.0.0 This wasn't there when I last saw this patch. The previous patch also updates this file. How did you decide which updates go where? Or is this an accident? > diff --git a/python/tests/qapi-flake8.sh b/python/tests/qapi-flake8.sh > new file mode 100755 > index 00000000000..7b5983d64a9 > --- /dev/null > +++ b/python/tests/qapi-flake8.sh > @@ -0,0 +1,4 @@ > +#!/bin/sh -e > +python3 -m flake8 ../scripts/qapi/ \ > + ../docs/sphinx/qapidoc.py \ > + ../docs/sphinx/qapi_domain.py Not linting docs/sphinx/qapidoc_legacy.py. This is intentional (see its initial commit message). Since we plan to drop it soon, there's no real need for a comment here, but mentioning it in the commit message wouldn't hurt. > diff --git a/python/tests/qapi-isort.sh b/python/tests/qapi-isort.sh > new file mode 100755 > index 00000000000..f31f12d3425 > --- /dev/null > +++ b/python/tests/qapi-isort.sh > @@ -0,0 +1,6 @@ > +#!/bin/sh -e > +python3 -m isort --sp . -c ../scripts/qapi/ > +# Force isort to recognize "compat" as a local module and not third-party > +python3 -m isort --sp . -c -p compat -p qapidoc_legacy \ > + ../docs/sphinx/qapi_domain.py \ > + ../docs/sphinx/qapidoc.py Comment on flake8 applies. > diff --git a/python/tests/qapi-mypy.sh b/python/tests/qapi-mypy.sh > new file mode 100755 > index 00000000000..377b29b873b > --- /dev/null > +++ b/python/tests/qapi-mypy.sh > @@ -0,0 +1,2 @@ > +#!/bin/sh -e > +python3 -m mypy ../scripts/qapi Not type-checking docs/sphinx/qapi_domain.py and docs/sphinx/qapidoc.py? Impractical due to us targeting an isanely wide Sphinx version range? > diff --git a/python/tests/qapi-pylint.sh b/python/tests/qapi-pylint.sh > new file mode 100755 > index 00000000000..f4bb7a5a795 > --- /dev/null > +++ b/python/tests/qapi-pylint.sh > @@ -0,0 +1,6 @@ > +#!/bin/sh -e > +SETUPTOOLS_USE_DISTUTILS=stdlib python3 -m pylint \ > + --rcfile=../scripts/qapi/pylintrc \ > + ../scripts/qapi/ \ > + ../docs/sphinx/qapidoc.py \ > + ../docs/sphinx/qapi_domain.py Comment on flake8 applies. > diff --git a/scripts/qapi/pylintrc b/scripts/qapi/pylintrc > index d24eece7411..e16283ada3d 100644 > --- a/scripts/qapi/pylintrc > +++ b/scripts/qapi/pylintrc > @@ -19,6 +19,7 @@ disable=consider-using-f-string, > too-many-instance-attributes, > too-many-positional-arguments, > too-many-statements, > + unknown-option-value, > useless-option-value, > > [REPORTS] This wasn't there when I last saw this patch. PATCH 1 also updates this file. How did you decide which updates go where? Or is this an accident?
