Re: [PATCH 02/10] python: add qapi static analysis tests

[Markus Armbruster <armbru@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>

[...]