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 <jsnow@redhat.com>
---
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
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
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
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
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
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]
--
2.48.1John Snow <jsnow@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 <jsnow@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?
On Tue, Mar 25, 2025 at 3:53 AM Markus Armbruster <armbru@redhat.com> wrote: > John Snow <jsnow@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 <jsnow@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. > It both is and isn't. That we can now check qapi and the qapi sphinx extensions from the same place as we do linting for python/ is sufficient justification in and of itself, regardless of how we improve and integrate this testing later on. > > > --- > > 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? > The previous patch pins dependencies that already existed, but we neglected to pin in this file. It's fixing an existing oversight. This patch adds a bunch of new pinned dependencies for Sphinx, which we need for type-checking Sphinx extensions. > > > 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. > Alright. > > > 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? > Yes. > > > 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? I didn't add the Sphinx extensions last time you saw this series, so that's new. This winds up being needed to tolerate the "too many positional arguments" option which only applies to newer pylint versions - older versions will complain about the option being unrecognized. In order to continue allowing a wide version of pylint versions, we need this option.
John Snow <jsnow@redhat.com> writes: > On Tue, Mar 25, 2025 at 3:53 AM Markus Armbruster <armbru@redhat.com> wrote: > >> John Snow <jsnow@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 <jsnow@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. >> > > It both is and isn't. That we can now check qapi and the qapi sphinx > extensions from the same place as we do linting for python/ is sufficient > justification in and of itself, regardless of how we improve and integrate > this testing later on. Alright. >> > --- >> > 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? >> > > The previous patch pins dependencies that already existed, but we neglected > to pin in this file. It's fixing an existing oversight. > > This patch adds a bunch of new pinned dependencies for Sphinx, which we > need for type-checking Sphinx extensions. So... the previous patch fixes existing tests, and this one extends their coverage to the modern parts of docs/sphinx/. Correct? Which tests exactly? I just asked that on the previous patch. [...] >> > 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? > > > I didn't add the Sphinx extensions last time you saw this series, so that's > new. This winds up being needed to tolerate the "too many positional > arguments" option which only applies to newer pylint versions - older > versions will complain about the option being unrecognized. In order to > continue allowing a wide version of pylint versions, we need this option. Got it. Worth a comment?
On Wed, Mar 26, 2025 at 2:13 AM Markus Armbruster <armbru@redhat.com> wrote: > John Snow <jsnow@redhat.com> writes: > > > On Tue, Mar 25, 2025 at 3:53 AM Markus Armbruster <armbru@redhat.com> > wrote: > > > >> John Snow <jsnow@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 <jsnow@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. > >> > > > > It both is and isn't. That we can now check qapi and the qapi sphinx > > extensions from the same place as we do linting for python/ is sufficient > > justification in and of itself, regardless of how we improve and > integrate > > this testing later on. > > Alright. > > >> > --- > >> > 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? > >> > > > > The previous patch pins dependencies that already existed, but we > neglected > > to pin in this file. It's fixing an existing oversight. > > > > This patch adds a bunch of new pinned dependencies for Sphinx, which we > > need for type-checking Sphinx extensions. > > So... the previous patch fixes existing tests, and this one extends > their coverage to the modern parts of docs/sphinx/. Correct? > Yep. > > Which tests exactly? I just asked that on the previous patch. > minreqs.txt concerns only make check-minreqs, but the shell script tests that just invoke a linter concern all launch avenues: make check, make check-dev, make check-tox, make check-minreqs. > > [...] > > >> > 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? > > > > > > I didn't add the Sphinx extensions last time you saw this series, so > that's > > new. This winds up being needed to tolerate the "too many positional > > arguments" option which only applies to newer pylint versions - older > > versions will complain about the option being unrecognized. In order to > > continue allowing a wide version of pylint versions, we need this option. > > Got it. Worth a comment? > Yep, I can update the commit message.
© 2016 - 2025 Red Hat, Inc.