On Wed, Feb 26, 2025 at 4:29 AM Markus Armbruster <arm...@redhat.com> wrote:
> John Snow <js...@redhat.com> writes: > > > On Mon, Feb 24, 2025 at 7:36 AM Markus Armbruster <arm...@redhat.com> > wrote: > > > >> 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. 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 <arm...@redhat.com> > > [...] > >
