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. 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/tests/qapi-flake8.sh | 2 ++
 python/tests/qapi-isort.sh  | 2 ++
 python/tests/qapi-mypy.sh   | 2 ++
 python/tests/qapi-pylint.sh | 4 ++++
 4 files changed, 10 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/tests/qapi-flake8.sh b/python/tests/qapi-flake8.sh
new file mode 100755
index 00000000000..51916a9019d
--- /dev/null
+++ b/python/tests/qapi-flake8.sh
@@ -0,0 +1,2 @@
+#!/bin/sh -e
+python3 -m flake8 ../scripts/qapi/
diff --git a/python/tests/qapi-isort.sh b/python/tests/qapi-isort.sh
new file mode 100755
index 00000000000..60ed5eeb34d
--- /dev/null
+++ b/python/tests/qapi-isort.sh
@@ -0,0 +1,2 @@
+#!/bin/sh -e
+python3 -m isort --sp . -c ../scripts/qapi/
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..d4869578e54
--- /dev/null
+++ b/python/tests/qapi-pylint.sh
@@ -0,0 +1,4 @@
+#!/bin/sh -e
+SETUPTOOLS_USE_DISTUTILS=stdlib python3 -m pylint \
+                                --rcfile=../scripts/qapi/pylintrc \
+                                ../scripts/qapi/
-- 
2.48.1

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. 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

It is for me.

> of setuptools. That's unrelated to this patch and I'll address it
> separately and soon. Thank you for your patience, --mgmt)

Which of these tests, if any, run in "make check"?  In CI?

> Signed-off-by: John Snow <jsnow@redhat.com>

On Mon, Feb 24, 2025 at 7:36 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. 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
>
> It is for me.
>
> > of setuptools. That's unrelated to this patch and I'll address it
> > separately and soon. Thank you for your patience, --mgmt)
>
> Which of these tests, if any, run in "make check"?  In CI?
>

Under "make check", the top-level test in qemu.git, none. I swear on my
future grave I am trying to fix that, but there are barriers to it. Adding
make check support means installing testing dependencies in the configure
venv, which means a slower ./configure invocation. I am trying to figure
out how to minimize this penalty for cases where we either do not want to,
or can't, run the python tests. It's a long story, we can talk about it
later.

In CI, the "check-minreqs" test will run by default as a must-pass test
under the job "check python minreqs".

"check-tox" is an optional job in the CI pipeline that is allowed to fail
as a warning, due to the nature of this test checking bleeding edge
dependencies.

All three local invocations run the exact same tests (literally "make
check" in the python dir), just under different combinations of
dependencies and python versions. "check-minreqs" is more or less the
"canonical" one that *must* succeed, but as a Python maintainer I do my
best to enforce "check-tox" as well, though it does lag behind.

So, this isn't a perfect solution yet but it's certainly much better than
carrying around ad-hoc linter shell scripts and attempting to manage the
dependencies yourself. At least we all have access to the same invocations.


>
> > Signed-off-by: John Snow <jsnow@redhat.com>
>
>

John Snow <jsnow@redhat.com> writes:

> On Mon, Feb 24, 2025 at 7:36 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. 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
>>
>> It is for me.
>>
>> > of setuptools. That's unrelated to this patch and I'll address it
>> > separately and soon. Thank you for your patience, --mgmt)
>>
>> Which of these tests, if any, run in "make check"?  In CI?
>>
>
> Under "make check", the top-level test in qemu.git, none. I swear on my
> future grave

"Not today!"

>              I am trying to fix that,

Also not today.  SCNR!

>                                       but there are barriers to it. Adding
> make check support means installing testing dependencies in the configure
> venv, which means a slower ./configure invocation. I am trying to figure
> out how to minimize this penalty for cases where we either do not want to,
> or can't, run the python tests. It's a long story, we can talk about it
> later.
>
> In CI, the "check-minreqs" test will run by default as a must-pass test
> under the job "check python minreqs".
>
> "check-tox" is an optional job in the CI pipeline that is allowed to fail
> as a warning, due to the nature of this test checking bleeding edge
> dependencies.
>
> All three local invocations run the exact same tests (literally "make
> check" in the python dir), just under different combinations of
> dependencies and python versions. "check-minreqs" is more or less the
> "canonical" one that *must* succeed, but as a Python maintainer I do my
> best to enforce "check-tox" as well, though it does lag behind.
>
> So, this isn't a perfect solution yet but it's certainly much better than
> carrying around ad-hoc linter shell scripts and attempting to manage the
> dependencies yourself. At least we all have access to the same invocations.

So:

* At some point, we'll integrate whatever we want developers to run into
  "make check".  Until then:

* Running "make check-dev" is nice and good enough.  CI might find
  additional problems.  Expected to be rare and no big deal.

* Running "make check-minreqs" locally will get the exact same results
  as the same test in CI will.  Run if you care.

* "make check-tox" is an early warning system.  Don't run unless you're
  interested in preventing potential future problems.

Acked-by: Markus Armbruster <armbru@redhat.com>

[...]

On Wed, Feb 26, 2025 at 4:29 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > On Mon, Feb 24, 2025 at 7:36 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. 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
> >>
> >> It is for me.
> >>
> >> > of setuptools. That's unrelated to this patch and I'll address it
> >> > separately and soon. Thank you for your patience, --mgmt)
> >>
> >> Which of these tests, if any, run in "make check"?  In CI?
> >>
> >
> > Under "make check", the top-level test in qemu.git, none. I swear on my
> > future grave
>
> "Not today!"
>
> >              I am trying to fix that,
>
> Also not today.  SCNR!
>
> >                                       but there are barriers to it.
> Adding
> > make check support means installing testing dependencies in the configure
> > venv, which means a slower ./configure invocation. I am trying to figure
> > out how to minimize this penalty for cases where we either do not want
> to,
> > or can't, run the python tests. It's a long story, we can talk about it
> > later.
> >
> > In CI, the "check-minreqs" test will run by default as a must-pass test
> > under the job "check python minreqs".
> >
> > "check-tox" is an optional job in the CI pipeline that is allowed to fail
> > as a warning, due to the nature of this test checking bleeding edge
> > dependencies.
> >
> > All three local invocations run the exact same tests (literally "make
> > check" in the python dir), just under different combinations of
> > dependencies and python versions. "check-minreqs" is more or less the
> > "canonical" one that *must* succeed, but as a Python maintainer I do my
> > best to enforce "check-tox" as well, though it does lag behind.
> >
> > So, this isn't a perfect solution yet but it's certainly much better than
> > carrying around ad-hoc linter shell scripts and attempting to manage the
> > dependencies yourself. At least we all have access to the same
> invocations.
>
> So:
>
> * At some point, we'll integrate whatever we want developers to run into
>   "make check".  Until then:
>
> * Running "make check-dev" is nice and good enough.  CI might find
>   additional problems.  Expected to be rare and no big deal.
>
> * Running "make check-minreqs" locally will get the exact same results
>   as the same test in CI will.  Run if you care.
>
> * "make check-tox" is an early warning system.  Don't run unless you're
>   interested in preventing potential future problems.
>

More or less; though it does test in every supported python interpreter if
you happen to have multiple installed, so it can be a way to catch errors
that exist between minreqs and $current, but it's still generally only a
test that I think you should run if you are touching the Python stuff in a
major way; i.e. if you're sending something that's a PR for *me*, I think
you should run it. If you're just doing a quick fix to qapi that doesn't do
any deep Python trickery, I'd trust either check-minreqs/CI or check-dev to
be sufficient due diligence.

In other words: *any one* of these tests are likely to catch errors due to
incorrect code and is sufficient due diligence; making sure they *all* pass
is more of a job for *me* to catch ecosystem problems across our wide
platform and python version support matrix.

I very often run "check minreqs" and "check tox" and consider that
exhaustive. the dev test is there only as a quick smoke test if you don't
have a battleship of python interpreters installed, but it's busted at the
moment due to fairly recent setuptools changes @_@

--js


>
> Acked-by: Markus Armbruster <armbru@redhat.com>
>
> [...]
>
>