Re: venv for python qtest bits? (was: Re: [PATCH 11/12] acpi/tests/bits: add README file for bits qtests)

[Ani Sinha]

On Tue, Jun 28, 2022 at 4:00 PM Ani Sinha <ani@anisinha.ca> wrote:
>
> On Tue, Jun 28, 2022 at 3:45 PM Daniel P. Berrangé <berrange@redhat.com> 
> wrote:
> >
> > On Tue, Jun 28, 2022 at 10:28:04AM +0200, Thomas Huth wrote:
> > > On 28/06/2022 10.23, Daniel P. Berrangé wrote:
> > > > On Tue, Jun 28, 2022 at 01:21:35PM +0530, Ani Sinha wrote:
> > > > > On Tue, Jun 28, 2022 at 1:19 PM Daniel P. Berrangé 
> > > > > <berrange@redhat.com> wrote:
> > > > > >
> > > > > > On Tue, Jun 28, 2022 at 09:25:35AM +0200, Thomas Huth wrote:
> > > > > > > On 28/06/2022 09.10, Michael S. Tsirkin wrote:
> > > > > > > > On Tue, Jun 28, 2022 at 09:03:33AM +0200, Thomas Huth wrote:
> > > > > > > > > > > > > > > No problem with that. So that's venv. But do we 
> > > > > > > > > > > > > > > need pip and pulling
> > > > > > > > > > > > > > > packages from the net during testing?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > We do that too. See requirements.txt in tests/
> > > > > > > > > > > > > > Following two are downloaded:
> > > > > > > > > > > > > > avocado-framework==88.1
> > > > > > > > > > > > > > pycdlib==1.11.0
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Also see this line in Makefie.include:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > $(call quiet-venv-pip,install -r $(TESTS_VENV_REQ))
> > > > > > > > > > > > >
> > > > > > > > > > > > > Right but that's avocado since it pulls lots of stuff 
> > > > > > > > > > > > > from
> > > > > > > > > > > > > the net anyway.
> > > > > > > > > > > > > Are the libraries in question not packaged on major 
> > > > > > > > > > > > > distros?
> > > > > > > > > > > >
> > > > > > > > > > > > Currently I only need this:
> > > > > > > > > > > > https://github.com/python-tap/tappy
> > > > > > > > > > > > which is the basic TAP processing library for python.
> > > > > > > > > > > >
> > > > > > > > > > > > It seems its only installed through pip:
> > > > > > > > > > > > https://tappy.readthedocs.io/en/latest/
> > > > > > > > > > > >
> > > > > > > > > > > > I do not think this is packaged by default. It's such a 
> > > > > > > > > > > > basic library
> > > > > > > > > > > > for parsing test output that maybe we can keep this 
> > > > > > > > > > > > somewhere within
> > > > > > > > > > > > the python src tree? Not sure ...
> > > > > > > > > > >
> > > > > > > > > > > It's pretty small for sure. Another submodule?
> > > > > > > > > >
> > > > > > > > > > Unlike BITS, this one is likely going to be maintained for 
> > > > > > > > > > a while and
> > > > > > > > > > will receive new releases through
> > > > > > > > > > https://pypi.org/project/tap.py/
> > > > > > > > > > so forking is OK but someone has to keep this updated.
> > > > > > > > > >
> > > > > > > > > > I am open to anything. Whatever feels right is fine to me.
> > > > > > > > >
> > > > > > > > > John Snow is currently working on the "Pythonification" of 
> > > > > > > > > various QEMU
> > > > > > > > > bits, I think you should loop him into this discussion, too.
> > > > > > > > >
> > > > > > > > >    Thomas
> > > > > > > >
> > > > > > > > submodule does not mean we fork necessarily. We could have
> > > > > > > > all options: check for the module and use it if there, if not
> > > > > > > > use one from system if not there install with pip ..
> > > > > > > > But yea, I'm not sure what's best either.
> > > > > > >
> > > > > > > submodules create a dependency on an internet connection, too. So 
> > > > > > > before you
> > > > > > > add yet another submodule (which have a couple of other 
> > > > > > > disadvantages), I
> > > > > > > think you could also directly use the venv here.
> > > > > >
> > > > > > Definitely not submodules.
> > > > > >
> > > > > > We need to get out of the mindset that submodules are needed for 
> > > > > > every new
> > > > > > dependancy we add. Submodules are only appropriate if the external 
> > > > > > project
> > > > > > is designed to be used as a copylib (eg the keycodemapdb tool), or 
> > > > > > if we
> > > > > > need to bundle in order to prevent a regression for previously 
> > > > > > deployed
> > > > > > QEMU installs where the dependancy is known not to exist on all our
> > > > > > supported platforms.
> > > > > >
> > > > > > This does not apply in this case, because the proposed use of tappy 
> > > > > > is
> > > > > > merely for a test case. Meson just needs to check if tappy exists 
> > > > > > and if
> > > > > > it does, then use it, otherwise skip the tests that need it. The 
> > > > > > user can
> > > > > > arrange to install tappy, as they do with the majority of other 
> > > > > > deps.
> > > > > >
> > > > > > If John's venv stuff is relevant, then we don't even need the meson 
> > > > > > checks,
> > > > > > just delegate to the venv setup.
> > > > > >
> > > > > > Regardless, no submodules are needed or desirable.
> > > > >
> > > > > What about keeping biosbits stuff? Source or pre-built.
> > > >
> > > > Shipping them as pre-built binaries in QEMU is not a viable option
> > > > IMHO, especially for grub as a GPL'd project we need to be extremely
> > > > clear about the exact corresponding source and build process for any
> > > > binary.
> > > >
> > > > For this kind of thing I would generally expect the distro to provide
> > > > packages that we consume. Looking at biosbits I see it is itself
> > > > bundling a bunch more 3rd party projects, libffi, grub2, and including
> > > > even an ancient version of python as a submodule.
> > > >
> > > > So bundling a pre-built biosbits in QEMU appears to mean that we're in
> > > > turn going to unexpectedly bundle a bunch of other 3rd party projects
> > > > too, all with dubious license compliance. I don't think this looks like
> > > > something we should have in qemu.git or qemu tarballs. It will also
> > > > make it challenging for the distro to take biosbits at all, unless
> > > > those 3rd party bundles can be eliminated in favour of using existing
> > > > builds their have packaged for grub, python, libffi, etc.
> > >
> > > So if this depends on some third party binary bits, I think this is pretty
> > > similar to the tests in the avocado directory ... there we download third
> > > party binaries, too... Wouldn't it make sense to adapt your tests to that
> > > framework?
> >
> > Now that you mention it, avocado does feel like a more appropriate fit.
> > IIUC the biosbits project appears to be effectively providing a custom
> > guest OS ISO image. IOW this testing is quite biased towards being
> > integration testing which is the target of avocado, while qtest is much
> > more to the unit testing end of the spectrum.
>
> This is more like unit testing than integration testing, now that you
> mention it. It tests only the bios, very narrowly and does not involve
> any OS at all.

Another thing to consider is that integration testing is further down
the line? Not for once when submitting patches on acpi have I run
them. However, every time I have run make check to make sure
bios-tables-test passes and I am not breaking anything. It's much more
useful to have this kind of thing part of make check before patch
submitters can quickly check for failures either in bios-tables-test
or in bits. Also its lot easier to add new acpi/smbios tests as a part
of this when bios-tables-test and this one are closer together.

>
> This would avoid all the
> > discussion and patches around introducing python to qtest
> >
> > With regards,
> > Daniel
> > --
> > |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange 
> > :|
> > |: https://libvirt.org         -o-            https://fstop138.berrange.com 
> > :|
> > |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange 
> > :|
> >