| Message ID | 20220526000921.1581503-1-jsnow@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | tests, python: prepare to expand usage of test venv | expand |
On Wed, May 25, 2022, 8:09 PM John Snow <jsnow@redhat.com> wrote: > GitLab CI: https://gitlab.com/jsnow/qemu/-/pipelines/548326343 > > This series collects some of the uncontroversial elements that serve as > pre-requisites for a later series that seeks to generate a testing venv > by default. > > This series makes the following material changes: > > - Install the 'qemu' package into the avocado testing venv > - Use the avocado testing venv to run vm-tests > - Use the avocado testing venv to run device-crash-test > > None of these changes impact 'make check'; these are all specialty > tests that are not run by default. This series also doesn't change how > iotests are run, doesn't add any new dependencies for SRPM builds, etc. > > NOTE: patch 8 isn't strictly required for this series, but including it > here "early" helps the subsequent series. Since the debian docker files > are layered, testing downstream pipelines can fail because the base > image is pulled from the main QEMU repo instead of the downstream. > > In other words: I need this patch in origin/main in order to have the > venv module available for later patches that will actually need it in > our debian10 derivative images. > > (in other-other-words: the 'clang-user' test *will* need it later.) > > John Snow (9): > python: update for mypy 0.950 > tests: add "TESTS_PYTHON" variable to Makefile > tests: use python3 as the python executable name > tests: silence pip upgrade warnings during venv creation > tests: add quiet-venv-pip macro > tests: install "qemu" namespace package into venv > tests: use tests/venv to run basevm.py-based scripts > tests: add python3-venv to debian10.docker > tests: run 'device-crash-test' from tests/venv > > .gitlab-ci.d/buildtest.yml | 8 +++++--- > python/qemu/qmp/util.py | 4 +++- > python/setup.cfg | 1 + > scripts/device-crash-test | 14 +++++++++++--- > tests/Makefile.include | 18 ++++++++++-------- > tests/avocado/avocado_qemu/__init__.py | 11 +++++------ > tests/avocado/virtio_check_params.py | 1 - > tests/avocado/virtio_version.py | 1 - > tests/docker/dockerfiles/debian10.docker | 1 + > tests/requirements.txt | 1 + > tests/vm/Makefile.include | 13 +++++++------ > tests/vm/basevm.py | 6 +++--- > 12 files changed, 47 insertions(+), 32 deletions(-) > > -- > 2.34.1 > Paolo: thanks for reviews. I have a follow-up series that does more adventurous things (the pythonized bootstrapper, sub-dependency groups, and switching iotests over), but it introduces some new problems that are more "rfc" tier again. In short: I allow iotests to bootstrap itself, but this creates two subtle problems: (1) If check is engaged without running check-venv first and iotests creates its own venv, the python binary it uses will be whichever one is your system default, not necessarily the one you configured your build with. This is reasonable behavior IMO, but if you later run "make check", there's no guarantee that Make will re-make the venv with the correct python binary That's a subtle landmine. (2) Similarly, if the venv requirements.txt (or python/setup.cfg) change, iotests doesn't have the logic to notice it ought to re-make the venv. Only the makefile does. However, at least in this case, the makefile is guaranteed to notice if/when we run "check block" again. The odds of these files changing for most people who aren't *me* are pretty low, so it may not really come up much. Still, it's not bullet-proof. (Bonus not-at-all-subtle problem) I need to remove iotest 297, otherwise iotests under a venv that doesn't install mypy/pylint will never run. I discussed this upstream recently w/ Kevin, but my series to address it isn't ready yet. (Just pre-emptively pointing it out to say I'm aware of it!) WIP. Will send new RFC series today predicated on top of this series. --js >
Paolo: I assume this falls under your jurisdiction...ish, unless Cleber (avocado) or Alex (tests more broadly) have any specific inputs. I'm fine with waiting for reviews, but don't know whose bucket this goes to. On Wed, May 25, 2022, 8:09 PM John Snow <jsnow@redhat.com> wrote: > GitLab CI: https://gitlab.com/jsnow/qemu/-/pipelines/548326343 > > This series collects some of the uncontroversial elements that serve as > pre-requisites for a later series that seeks to generate a testing venv > by default. > > This series makes the following material changes: > > - Install the 'qemu' package into the avocado testing venv > - Use the avocado testing venv to run vm-tests > - Use the avocado testing venv to run device-crash-test > > None of these changes impact 'make check'; these are all specialty > tests that are not run by default. This series also doesn't change how > iotests are run, doesn't add any new dependencies for SRPM builds, etc. > > NOTE: patch 8 isn't strictly required for this series, but including it > here "early" helps the subsequent series. Since the debian docker files > are layered, testing downstream pipelines can fail because the base > image is pulled from the main QEMU repo instead of the downstream. > > In other words: I need this patch in origin/main in order to have the > venv module available for later patches that will actually need it in > our debian10 derivative images. > > (in other-other-words: the 'clang-user' test *will* need it later.) > > John Snow (9): > python: update for mypy 0.950 > tests: add "TESTS_PYTHON" variable to Makefile > tests: use python3 as the python executable name > tests: silence pip upgrade warnings during venv creation > tests: add quiet-venv-pip macro > tests: install "qemu" namespace package into venv > tests: use tests/venv to run basevm.py-based scripts > tests: add python3-venv to debian10.docker > tests: run 'device-crash-test' from tests/venv > > .gitlab-ci.d/buildtest.yml | 8 +++++--- > python/qemu/qmp/util.py | 4 +++- > python/setup.cfg | 1 + > scripts/device-crash-test | 14 +++++++++++--- > tests/Makefile.include | 18 ++++++++++-------- > tests/avocado/avocado_qemu/__init__.py | 11 +++++------ > tests/avocado/virtio_check_params.py | 1 - > tests/avocado/virtio_version.py | 1 - > tests/docker/dockerfiles/debian10.docker | 1 + > tests/requirements.txt | 1 + > tests/vm/Makefile.include | 13 +++++++------ > tests/vm/basevm.py | 6 +++--- > 12 files changed, 47 insertions(+), 32 deletions(-) > > -- > 2.34.1 > >
On 5/26/22 16:34, John Snow wrote: > (1) If check is engaged without running check-venv first and iotests > creates its own venv, the python binary it uses will be whichever one is > your system default, not necessarily the one you configured your build with. > > This is reasonable behavior IMO, but if you later run "make check", > there's no guarantee that Make will re-make the venv with the correct > python binary That's a subtle landmine. Yup, that's also a reason to make initial venv creation part of configure. I consider that on the same level as running Meson and setting up git submodules. > (2) Similarly, if the venv requirements.txt (or python/setup.cfg) > change, iotests doesn't have the logic to notice it ought to re-make the > venv. Only the makefile does. However, at least in this case, the > makefile is guaranteed to notice if/when we run "check block" again. The > odds of these files changing for most people who aren't *me* are pretty > low, so it may not really come up much. Still, it's not bullet-proof. Yeah, this is fine. Compare it with e.g. running clang-query: it needs a "make" first to rebuild compile_commands.json. Paolo > (Bonus not-at-all-subtle problem) I need to remove iotest 297, otherwise > iotests under a venv that doesn't install mypy/pylint will never run. I > discussed this upstream recently w/ Kevin, but my series to address it > isn't ready yet. (Just pre-emptively pointing it out to say I'm aware of > it!)
On 5/27/22 16:27, John Snow wrote: > Paolo: I assume this falls under your jurisdiction...ish, unless Cleber > (avocado) or Alex (tests more broadly) have any specific inputs. > > I'm fine with waiting for reviews, but don't know whose bucket this goes to. > I thought it was yours, but I've queued it now. Paolo
On Wed, Jun 1, 2022, 6:06 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > On 5/27/22 16:27, John Snow wrote: > > Paolo: I assume this falls under your jurisdiction...ish, unless Cleber > > (avocado) or Alex (tests more broadly) have any specific inputs. > > > > I'm fine with waiting for reviews, but don't know whose bucket this goes > to. > > > > I thought it was yours, but I've queued it now. > > Paolo > I wanted to be polite since it was build system and tests as well - I don't technically maintain most of these files :) Thank you! >