On 19.10.21 16:49, John Snow wrote:
> Now, 297 is just the iotests-specific incantations and linters.py is as
> minimal as I can think to make it. The only remaining element in here
> that ought to be configuration and not code is the list of skip files,
> but they're still numerous enough that repeating them for mypy and
> pylint configurations both would be ... a hassle.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> tests/qemu-iotests/297 | 72 +++++----------------------------
> tests/qemu-iotests/linters.py | 76 +++++++++++++++++++++++++++++++++++
> 2 files changed, 87 insertions(+), 61 deletions(-)
> create mode 100644 tests/qemu-iotests/linters.py
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Thanks!
I wonder about `check_linter()`, though. By not moving it to
linters.py, we can’t use it in its entry point, and so the Python test
infrastructure will have a strong dependency on these linters. Though
then again, it probably already does, and I suppose that’s one of the
points hindering us from running this from make check?
That one is left behind because it uses iotests API to skip a test. Environment setup that guarantees we won't *need* to skip the test is handled by the virtual environment setup magic in qemu/python/Makefile.
Hanna
More info than you require:
There's maybe about four ways you could run the python tests that might make sense depending on who you are and what you're trying to accomplish; they're documented in "make help" and again in qemu/python/README.rst;
(1) make check-dev -- makes a venv with whatever python you happen to have, installs the latest packages, runs the tests
(2) make check-pipenv -- requires python 3.6 specifically, installs the *oldest* packages, runs the tests
(3) make check-tox -- requires python 3.6 through 3.10, installs the newest packages, runs the tests per each python version
(4) make check -- perform no setup at all, just run the tests in the current environment. (Used directly by all three prior invocations)
In general, I assume that human beings that aren't working on Python stuff actively will be using (1) at their interactive console, if they decide to run any of these at all. It imposes the least pre-requirements while still endeavoring to be a target that will "just work".
Options (2) and (3) are what power the CI tests 'check-python-pipenv'
and 'check-python-tox', respectively; with(2) being
the one that actually gates the CI.
Option (4) is only really a convenience for bypassing the venv-setup stuff if you want to construct your own (or not use one at all). So the tests just assume that the tools they have available will work. It's kind of a 'power user' target.
The way the CI works at the moment is that it uses a "fedora:latest" base image and installs python interpreters 3.6 through 3.10 inclusive, pipenv, and tox. From there, it has enough juice to run the makefile targets which take care of all of the actual linting dependencies and their different versions to get a wider matrix on the version testing to ensure we're still keeping compatibility with the 3.6 platform while also checking for new problems that show up on the bleeding edge.
iotests 297 right now doesn't do any python environment setup at all, so we can't guarantee that the linters are present, so we decide to allow the test to be skipped. My big hesitation of adding this test directly into 'make check' is that I will need to do some environment probing to make sure that the 'pylint' version isn't too old -- or otherwise set up a venv in the build directory that can be used to run tests. I know we already set one up for the acceptance tests, so maybe there's an opportunity to re-use that venv, but I need to make sure that the dependencies between the two sets of tests are aligned. I don't know if they agree, currently.
I think I probably want to minimize the number of different virtual python environments we create during the build, so I will look into what problems might exist in re-purposing the acceptance test venv. If that can get squared away easily, there's likely no real big barrier to adding more tests that rely on a python venv to get cooking during the normal build/check process, including iotest 297.
--js