mbox series

[RFC,0/8] Convert avocado tests to normal Python unittests

Message ID 20240711115546.40859-1-thuth@redhat.com
Headers show
Series Convert avocado tests to normal Python unittests | expand

Message

Thomas Huth July 11, 2024, 11:55 a.m. UTC
The Avocado v88 that we use in QEMU is already on a life support
system: It is not supported by upstream anymore, and with the latest
versions of Python, it won't work anymore since it depends on the
"imp" module that has been removed in Python 3.12.

There have been several attempts to update the test suite in QEMU
to a newer version of Avocado, but so far no attempt has successfully
been merged yet.

Additionally, the whole "make check" test suite in QEMU is using the
meson test runner nowadays, so running the python-based tests via the
Avocodo test runner looks and feels quite like an oddball, requiring
the users to deal with the knowledge of multiple test runners in
parallel.

So instead of trying to update the python-based test suite in QEMU
to a newer version of Avocado, we should maybe try to better integrate
it with the meson test runner instead. Indeed most tests work quite
nicely without the Avocado framework already, as you can see with
this patch series - it does not convert all tests, just a subset since
it is just an RFC so far, but as you can see, many tests only need
small modifications to work without Avocado.

If you want to try it: Apply the patches, make sure that you have the
"pytest" program installed, then recompile and then run:

 make check-pytest

Things that need further attention though:

- All tests that use the LinuxTest / LinuxDistro classes (e.g. based
  on cloud-init images) really depend on the Avocado framework,
  thus we'd need a solution for those if we want to continue with
  this approach

- Same for all tests that require the LinuxSSHMixIn class - we'd
  need to provide a solution for ssh-based tests, too.

- We lose the way of running tests via the avocado tags this way...
  single targets can still be tested by running "make check-pytest-arm"
  for example, but running selected tests by other tags does not
  work anymore.

- I haven't looked into logging yet ... this still needs some work
  so that you could e.g. inspect the console output of the guests
  somewhere

- I did not work on documentation updates yet (will do that if we
  agree to continue with this patch series)

What's your thoughts? Is it worth to continue with this approach?
Or shall I rather forget about it and wait for the Avocado version
update?

 Thomas


Ani Sinha (1):
  tests/pytest: add pytest to the meson build system

Thomas Huth (7):
  tests/pytest: Add base classes for the upcoming pytest-based tests
  tests/pytest: Convert some simple avocado tests into pytests
  tests/pytest: Convert info_usernet and version test with small
    adjustments
  tests_pytest: Implement fetch_asset() method for downloading assets
  tests/pytest: Convert some tests that download files via fetch_asset()
  tests/pytest: Add a function for extracting files from an archive
  tests/pytest: Convert avocado test that needed avocado.utils.archive

 tests/Makefile.include                        |   4 +-
 tests/meson.build                             |   1 +
 tests/pytest/meson.build                      |  74 ++++
 tests/pytest/qemu_pytest/__init__.py          | 362 ++++++++++++++++++
 tests/pytest/qemu_pytest/utils.py             |  21 +
 .../test_arm_canona1100.py}                   |  16 +-
 .../test_cpu_queries.py}                      |   2 +-
 .../test_empty_cpu_model.py}                  |   2 +-
 .../test_info_usernet.py}                     |   6 +-
 .../test_machine_arm_n8x0.py}                 |  20 +-
 .../test_machine_avr6.py}                     |   7 +-
 .../test_machine_loongarch.py}                |  11 +-
 .../test_machine_mips_loongson3v.py}          |  19 +-
 .../test_mem_addr_space.py}                   |   3 +-
 .../test_ppc_bamboo.py}                       |  18 +-
 .../version.py => pytest/test_version.py}     |   8 +-
 .../test_virtio_version.py}                   |   2 +-
 17 files changed, 502 insertions(+), 74 deletions(-)
 create mode 100644 tests/pytest/meson.build
 create mode 100644 tests/pytest/qemu_pytest/__init__.py
 create mode 100644 tests/pytest/qemu_pytest/utils.py
 rename tests/{avocado/machine_arm_canona1100.py => pytest/test_arm_canona1100.py} (74%)
 rename tests/{avocado/cpu_queries.py => pytest/test_cpu_queries.py} (96%)
 rename tests/{avocado/empty_cpu_model.py => pytest/test_empty_cpu_model.py} (94%)
 rename tests/{avocado/info_usernet.py => pytest/test_info_usernet.py} (91%)
 rename tests/{avocado/machine_arm_n8x0.py => pytest/test_machine_arm_n8x0.py} (71%)
 rename tests/{avocado/machine_avr6.py => pytest/test_machine_avr6.py} (91%)
 rename tests/{avocado/machine_loongarch.py => pytest/test_machine_loongarch.py} (89%)
 rename tests/{avocado/machine_mips_loongson3v.py => pytest/test_machine_mips_loongson3v.py} (59%)
 rename tests/{avocado/mem-addr-space-check.py => pytest/test_mem_addr_space.py} (99%)
 rename tests/{avocado/ppc_bamboo.py => pytest/test_ppc_bamboo.py} (75%)
 rename tests/{avocado/version.py => pytest/test_version.py} (82%)
 rename tests/{avocado/virtio_version.py => pytest/test_virtio_version.py} (99%)

Comments

Daniel P. Berrangé July 11, 2024, 12:45 p.m. UTC | #1
On Thu, Jul 11, 2024 at 01:55:38PM +0200, Thomas Huth wrote:
> The Avocado v88 that we use in QEMU is already on a life support
> system: It is not supported by upstream anymore, and with the latest
> versions of Python, it won't work anymore since it depends on the
> "imp" module that has been removed in Python 3.12.
> 
> There have been several attempts to update the test suite in QEMU
> to a newer version of Avocado, but so far no attempt has successfully
> been merged yet.
> 
> Additionally, the whole "make check" test suite in QEMU is using the
> meson test runner nowadays, so running the python-based tests via the
> Avocodo test runner looks and feels quite like an oddball, requiring
> the users to deal with the knowledge of multiple test runners in
> parallel.

The fewer / simpler the layers we have in the execution path
of tests the better our life will be in debugging IMHO.

Having each individual test registered with meson has the
particularly strong advantage that we can make use of meson's
timeout feature to force individual tests to abort if they
hang/run too slowly, as we did when converting the iotests
to individual meson tests.

> 
> So instead of trying to update the python-based test suite in QEMU
> to a newer version of Avocado, we should maybe try to better integrate
> it with the meson test runner instead. Indeed most tests work quite
> nicely without the Avocado framework already, as you can see with
> this patch series - it does not convert all tests, just a subset since
> it is just an RFC so far, but as you can see, many tests only need
> small modifications to work without Avocado.
> 
> If you want to try it: Apply the patches, make sure that you have the
> "pytest" program installed, then recompile and then run:
> 
>  make check-pytest
> 
> Things that need further attention though:
> 
> - All tests that use the LinuxTest / LinuxDistro classes (e.g. based
>   on cloud-init images) really depend on the Avocado framework,
>   thus we'd need a solution for those if we want to continue with
>   this approach

Right, avocado is providing 2 distinct things, the test execution
harness and the test framework APIs.

It could be valid to remove use of the harness but keep using
the framework APIs, especially if that's sufficient to unblock
updating to new avocado versions too ? Over the longer term
we can consider whether the framework APIs should remain or
be replaced by something else.

> - Same for all tests that require the LinuxSSHMixIn class - we'd
>   need to provide a solution for ssh-based tests, too.
> 
> - We lose the way of running tests via the avocado tags this way...
>   single targets can still be tested by running "make check-pytest-arm"
>   for example, but running selected tests by other tags does not
>   work anymore.

The meson "suites" concept is the logical equivalent of tags.
You've wired up a suite for each architecture. We could define
more suites if there are other useful criteria for filtering
tests to be run. Perhaps machine type ? "make check-pytest-arm-<machine>"

> - I haven't looked into logging yet ... this still needs some work
>   so that you could e.g. inspect the console output of the guests
>   somewhere

Yep, debuggability is probably the single biggest problem we face
with our tests. Simplifying the test execution harness will help
in this respect, but yeah, we must have a way to capture logs
of stuff executed.

> - I did not work on documentation updates yet (will do that if we
>   agree to continue with this patch series)
> 
> What's your thoughts? Is it worth to continue with this approach?
> Or shall I rather forget about it and wait for the Avocado version
> update?



With regards,
Daniel
Fabiano Rosas July 11, 2024, 2:39 p.m. UTC | #2
Thomas Huth <thuth@redhat.com> writes:

> The Avocado v88 that we use in QEMU is already on a life support
> system: It is not supported by upstream anymore, and with the latest
> versions of Python, it won't work anymore since it depends on the
> "imp" module that has been removed in Python 3.12.
>
> There have been several attempts to update the test suite in QEMU
> to a newer version of Avocado, but so far no attempt has successfully
> been merged yet.
>
> Additionally, the whole "make check" test suite in QEMU is using the
> meson test runner nowadays, so running the python-based tests via the
> Avocodo test runner looks and feels quite like an oddball, requiring
> the users to deal with the knowledge of multiple test runners in
> parallel.
>
> So instead of trying to update the python-based test suite in QEMU
> to a newer version of Avocado, we should maybe try to better integrate
> it with the meson test runner instead. Indeed most tests work quite
> nicely without the Avocado framework already, as you can see with
> this patch series - it does not convert all tests, just a subset since
> it is just an RFC so far, but as you can see, many tests only need
> small modifications to work without Avocado.
>
> If you want to try it: Apply the patches, make sure that you have the
> "pytest" program installed, then recompile and then run:
>
>  make check-pytest
>
> Things that need further attention though:
>
> - All tests that use the LinuxTest / LinuxDistro classes (e.g. based
>   on cloud-init images) really depend on the Avocado framework,
>   thus we'd need a solution for those if we want to continue with
>   this approach
>
> - Same for all tests that require the LinuxSSHMixIn class - we'd
>   need to provide a solution for ssh-based tests, too.

These two seem to be dependent mostly avocado/utils only. Those could
still be used without the whole framework, no? Say we keep importing
avocado.utils, but run everything from meson, would that make sense?
Thomas Huth July 11, 2024, 5:44 p.m. UTC | #3
On 11/07/2024 16.39, Fabiano Rosas wrote:
> Thomas Huth <thuth@redhat.com> writes:
...
>> Things that need further attention though:
>>
>> - All tests that use the LinuxTest / LinuxDistro classes (e.g. based
>>    on cloud-init images) really depend on the Avocado framework,
>>    thus we'd need a solution for those if we want to continue with
>>    this approach
>>
>> - Same for all tests that require the LinuxSSHMixIn class - we'd
>>    need to provide a solution for ssh-based tests, too.
> 
> These two seem to be dependent mostly avocado/utils only. Those could
> still be used without the whole framework, no? Say we keep importing
> avocado.utils, but run everything from meson, would that make sense?

Yes ... maybe ... I can give it a try to see whether that works.

  Thomas
Daniel P. Berrangé July 12, 2024, 7:07 a.m. UTC | #4
On Thu, Jul 11, 2024 at 07:44:39PM +0200, Thomas Huth wrote:
> On 11/07/2024 16.39, Fabiano Rosas wrote:
> > Thomas Huth <thuth@redhat.com> writes:
> ...
> > > Things that need further attention though:
> > > 
> > > - All tests that use the LinuxTest / LinuxDistro classes (e.g. based
> > >    on cloud-init images) really depend on the Avocado framework,
> > >    thus we'd need a solution for those if we want to continue with
> > >    this approach
> > > 
> > > - Same for all tests that require the LinuxSSHMixIn class - we'd
> > >    need to provide a solution for ssh-based tests, too.
> > 
> > These two seem to be dependent mostly avocado/utils only. Those could
> > still be used without the whole framework, no? Say we keep importing
> > avocado.utils, but run everything from meson, would that make sense?
> 
> Yes ... maybe ... I can give it a try to see whether that works.

We only import about 8 modules from avocado.utils. There are probably a
few more indirectly used, but worst case we just clone the bits we need
into the QEMU tree.

With regards,
Daniel
Alex Bennée July 12, 2024, 2:25 p.m. UTC | #5
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Jul 11, 2024 at 07:44:39PM +0200, Thomas Huth wrote:
>> On 11/07/2024 16.39, Fabiano Rosas wrote:
>> > Thomas Huth <thuth@redhat.com> writes:
>> ...
>> > > Things that need further attention though:
>> > > 
>> > > - All tests that use the LinuxTest / LinuxDistro classes (e.g. based
>> > >    on cloud-init images) really depend on the Avocado framework,
>> > >    thus we'd need a solution for those if we want to continue with
>> > >    this approach
>> > > 
>> > > - Same for all tests that require the LinuxSSHMixIn class - we'd
>> > >    need to provide a solution for ssh-based tests, too.
>> > 
>> > These two seem to be dependent mostly avocado/utils only. Those could
>> > still be used without the whole framework, no? Say we keep importing
>> > avocado.utils, but run everything from meson, would that make sense?
>> 
>> Yes ... maybe ... I can give it a try to see whether that works.
>
> We only import about 8 modules from avocado.utils. There are probably a
> few more indirectly used, but worst case we just clone the bits we need
> into the QEMU tree.

Is Avocado still actively developed? I thought you guys used it quite
widely within RedHat?


>
> With regards,
> Daniel
Daniel P. Berrangé July 12, 2024, 2:28 p.m. UTC | #6
On Fri, Jul 12, 2024 at 03:25:23PM +0100, Alex Bennée wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Thu, Jul 11, 2024 at 07:44:39PM +0200, Thomas Huth wrote:
> >> On 11/07/2024 16.39, Fabiano Rosas wrote:
> >> > Thomas Huth <thuth@redhat.com> writes:
> >> ...
> >> > > Things that need further attention though:
> >> > > 
> >> > > - All tests that use the LinuxTest / LinuxDistro classes (e.g. based
> >> > >    on cloud-init images) really depend on the Avocado framework,
> >> > >    thus we'd need a solution for those if we want to continue with
> >> > >    this approach
> >> > > 
> >> > > - Same for all tests that require the LinuxSSHMixIn class - we'd
> >> > >    need to provide a solution for ssh-based tests, too.
> >> > 
> >> > These two seem to be dependent mostly avocado/utils only. Those could
> >> > still be used without the whole framework, no? Say we keep importing
> >> > avocado.utils, but run everything from meson, would that make sense?
> >> 
> >> Yes ... maybe ... I can give it a try to see whether that works.
> >
> > We only import about 8 modules from avocado.utils. There are probably a
> > few more indirectly used, but worst case we just clone the bits we need
> > into the QEMU tree.
> 
> Is Avocado still actively developed? I thought you guys used it quite
> widely within RedHat?

Yes it is active:

https://github.com/avocado-framework/avocado/commits/master/

With regards,
Daniel
John Snow July 16, 2024, 4:45 p.m. UTC | #7
On Thu, Jul 11, 2024, 7:55 AM Thomas Huth <thuth@redhat.com> wrote:

> The Avocado v88 that we use in QEMU is already on a life support
> system: It is not supported by upstream anymore, and with the latest
> versions of Python, it won't work anymore since it depends on the
> "imp" module that has been removed in Python 3.12.
>
> There have been several attempts to update the test suite in QEMU
> to a newer version of Avocado, but so far no attempt has successfully
> been merged yet.
>
> Additionally, the whole "make check" test suite in QEMU is using the
> meson test runner nowadays, so running the python-based tests via the
> Avocodo test runner looks and feels quite like an oddball, requiring
> the users to deal with the knowledge of multiple test runners in
> parallel.
>
> So instead of trying to update the python-based test suite in QEMU
> to a newer version of Avocado, we should maybe try to better integrate
> it with the meson test runner instead. Indeed most tests work quite
> nicely without the Avocado framework already, as you can see with
> this patch series - it does not convert all tests, just a subset since
> it is just an RFC so far, but as you can see, many tests only need
> small modifications to work without Avocado.
>
> If you want to try it: Apply the patches, make sure that you have the
> "pytest" program installed, then recompile and then run:
>
>  make check-pytest
>
> Things that need further attention though:
>
> - All tests that use the LinuxTest / LinuxDistro classes (e.g. based
>   on cloud-init images) really depend on the Avocado framework,
>   thus we'd need a solution for those if we want to continue with
>   this approach
>
> - Same for all tests that require the LinuxSSHMixIn class - we'd
>   need to provide a solution for ssh-based tests, too.
>
> - We lose the way of running tests via the avocado tags this way...
>   single targets can still be tested by running "make check-pytest-arm"
>   for example, but running selected tests by other tags does not
>   work anymore.
>
> - I haven't looked into logging yet ... this still needs some work
>   so that you could e.g. inspect the console output of the guests
>   somewhere
>

This has spilled the most developer blood of any other problem with the
Python-based tests. Be very careful here.

I still have a prototype for replacing QMPMachine with an asyncio variant
that should have more robust logging features, but I put it on the
back-burner.

Avocado tests are the primary user of the QMP Machine interface I hate the
very most, a multi-threaded buffer-reader that works only by the grace of
god. If you do go down this path, I may want to take the opportunity to
abolish that interface once and for all.

I think simplifying the console buffering will help ease debuggability.

(Note, this isn't an avocado exclusive problem so much as it is the
emergent evolution of both qmp machine and avocado developing their own
solutions to console logging problems, resulting in two layers that are
trying to do similar things.)


> - I did not work on documentation updates yet (will do that if we
>   agree to continue with this patch series)
>
> What's your thoughts? Is it worth to continue with this approach?
> Or shall I rather forget about it and wait for the Avocado version
> update?
>

I'm personally ambivalent on avocado; I use it for the python self-tests as
dogfooding but I can likely switch back over to plain pytest if that's the
direction we head. I don't think I use any crazy features except some
asyncio helpers i advocated for. I'm not sure what pytest's asyncio support
looks like, but I have to imagine as the premier testing framework that it
has *something* for me to use.

My only ask is that we keep the tests running in the custom venv
environment we set up at build time. We have some funky post-hoc
initialization of avocado that allows us to use internet packages
post-config for testing purposes. If we move to pytest, it's possible we
can eliminate that funkiness, which would be a win.

I'm also not so sure about recreating all of the framework that pulls vm
images on demand, that sounds like it'd be a lot of work, but maybe I'm
wrong about that.

Tacit ACK from me on this project in general, provided we are still using
the configure venv.


>  Thomas
>
>
> Ani Sinha (1):
>   tests/pytest: add pytest to the meson build system
>
> Thomas Huth (7):
>   tests/pytest: Add base classes for the upcoming pytest-based tests
>   tests/pytest: Convert some simple avocado tests into pytests
>   tests/pytest: Convert info_usernet and version test with small
>     adjustments
>   tests_pytest: Implement fetch_asset() method for downloading assets
>   tests/pytest: Convert some tests that download files via fetch_asset()
>   tests/pytest: Add a function for extracting files from an archive
>   tests/pytest: Convert avocado test that needed avocado.utils.archive
>
>  tests/Makefile.include                        |   4 +-
>  tests/meson.build                             |   1 +
>  tests/pytest/meson.build                      |  74 ++++
>  tests/pytest/qemu_pytest/__init__.py          | 362 ++++++++++++++++++
>  tests/pytest/qemu_pytest/utils.py             |  21 +
>  .../test_arm_canona1100.py}                   |  16 +-
>  .../test_cpu_queries.py}                      |   2 +-
>  .../test_empty_cpu_model.py}                  |   2 +-
>  .../test_info_usernet.py}                     |   6 +-
>  .../test_machine_arm_n8x0.py}                 |  20 +-
>  .../test_machine_avr6.py}                     |   7 +-
>  .../test_machine_loongarch.py}                |  11 +-
>  .../test_machine_mips_loongson3v.py}          |  19 +-
>  .../test_mem_addr_space.py}                   |   3 +-
>  .../test_ppc_bamboo.py}                       |  18 +-
>  .../version.py => pytest/test_version.py}     |   8 +-
>  .../test_virtio_version.py}                   |   2 +-
>  17 files changed, 502 insertions(+), 74 deletions(-)
>  create mode 100644 tests/pytest/meson.build
>  create mode 100644 tests/pytest/qemu_pytest/__init__.py
>  create mode 100644 tests/pytest/qemu_pytest/utils.py
>  rename tests/{avocado/machine_arm_canona1100.py =>
> pytest/test_arm_canona1100.py} (74%)
>  rename tests/{avocado/cpu_queries.py => pytest/test_cpu_queries.py} (96%)
>  rename tests/{avocado/empty_cpu_model.py =>
> pytest/test_empty_cpu_model.py} (94%)
>  rename tests/{avocado/info_usernet.py => pytest/test_info_usernet.py}
> (91%)
>  rename tests/{avocado/machine_arm_n8x0.py =>
> pytest/test_machine_arm_n8x0.py} (71%)
>  rename tests/{avocado/machine_avr6.py => pytest/test_machine_avr6.py}
> (91%)
>  rename tests/{avocado/machine_loongarch.py =>
> pytest/test_machine_loongarch.py} (89%)
>  rename tests/{avocado/machine_mips_loongson3v.py =>
> pytest/test_machine_mips_loongson3v.py} (59%)
>  rename tests/{avocado/mem-addr-space-check.py =>
> pytest/test_mem_addr_space.py} (99%)
>  rename tests/{avocado/ppc_bamboo.py => pytest/test_ppc_bamboo.py} (75%)
>  rename tests/{avocado/version.py => pytest/test_version.py} (82%)
>  rename tests/{avocado/virtio_version.py => pytest/test_virtio_version.py}
> (99%)
>
> --
> 2.45.2
>
>
Paolo Bonzini July 16, 2024, 6:03 p.m. UTC | #8
Il mar 16 lug 2024, 18:45 John Snow <jsnow@redhat.com> ha scritto:

> My only ask is that we keep the tests running in the custom venv
> environment we set up at build time
>

Yes, they do, however pytest should also be added to pythondeps.toml if we
go this way.

If we move to pytest, it's possible we can eliminate that funkiness, which
> would be a win.
>

There is the pycotap dependency to produce TAP from pytest, but that's
probably something small enough to be vendored. And also it depends on what
the dependencies would be for the assets framework.

I'm also not so sure about recreating all of the framework that pulls vm
> images on demand, that sounds like it'd be a lot of work, but maybe I'm
> wrong about that.
>

Yep, that's the part that I am a bit more doubtful about.

Paolo

Tacit ACK from me on this project in general, provided we are still using
> the configure venv.
>
>
>>  Thomas
>>
>>
>> Ani Sinha (1):
>>   tests/pytest: add pytest to the meson build system
>>
>> Thomas Huth (7):
>>   tests/pytest: Add base classes for the upcoming pytest-based tests
>>   tests/pytest: Convert some simple avocado tests into pytests
>>   tests/pytest: Convert info_usernet and version test with small
>>     adjustments
>>   tests_pytest: Implement fetch_asset() method for downloading assets
>>   tests/pytest: Convert some tests that download files via fetch_asset()
>>   tests/pytest: Add a function for extracting files from an archive
>>   tests/pytest: Convert avocado test that needed avocado.utils.archive
>>
>>  tests/Makefile.include                        |   4 +-
>>  tests/meson.build                             |   1 +
>>  tests/pytest/meson.build                      |  74 ++++
>>  tests/pytest/qemu_pytest/__init__.py          | 362 ++++++++++++++++++
>>  tests/pytest/qemu_pytest/utils.py             |  21 +
>>  .../test_arm_canona1100.py}                   |  16 +-
>>  .../test_cpu_queries.py}                      |   2 +-
>>  .../test_empty_cpu_model.py}                  |   2 +-
>>  .../test_info_usernet.py}                     |   6 +-
>>  .../test_machine_arm_n8x0.py}                 |  20 +-
>>  .../test_machine_avr6.py}                     |   7 +-
>>  .../test_machine_loongarch.py}                |  11 +-
>>  .../test_machine_mips_loongson3v.py}          |  19 +-
>>  .../test_mem_addr_space.py}                   |   3 +-
>>  .../test_ppc_bamboo.py}                       |  18 +-
>>  .../version.py => pytest/test_version.py}     |   8 +-
>>  .../test_virtio_version.py}                   |   2 +-
>>  17 files changed, 502 insertions(+), 74 deletions(-)
>>  create mode 100644 tests/pytest/meson.build
>>  create mode 100644 tests/pytest/qemu_pytest/__init__.py
>>  create mode 100644 tests/pytest/qemu_pytest/utils.py
>>  rename tests/{avocado/machine_arm_canona1100.py =>
>> pytest/test_arm_canona1100.py} (74%)
>>  rename tests/{avocado/cpu_queries.py => pytest/test_cpu_queries.py} (96%)
>>  rename tests/{avocado/empty_cpu_model.py =>
>> pytest/test_empty_cpu_model.py} (94%)
>>  rename tests/{avocado/info_usernet.py => pytest/test_info_usernet.py}
>> (91%)
>>  rename tests/{avocado/machine_arm_n8x0.py =>
>> pytest/test_machine_arm_n8x0.py} (71%)
>>  rename tests/{avocado/machine_avr6.py => pytest/test_machine_avr6.py}
>> (91%)
>>  rename tests/{avocado/machine_loongarch.py =>
>> pytest/test_machine_loongarch.py} (89%)
>>  rename tests/{avocado/machine_mips_loongson3v.py =>
>> pytest/test_machine_mips_loongson3v.py} (59%)
>>  rename tests/{avocado/mem-addr-space-check.py =>
>> pytest/test_mem_addr_space.py} (99%)
>>  rename tests/{avocado/ppc_bamboo.py => pytest/test_ppc_bamboo.py} (75%)
>>  rename tests/{avocado/version.py => pytest/test_version.py} (82%)
>>  rename tests/{avocado/virtio_version.py =>
>> pytest/test_virtio_version.py} (99%)
>>
>> --
>> 2.45.2
>>
>>
>>
Daniel P. Berrangé July 16, 2024, 6:10 p.m. UTC | #9
On Tue, Jul 16, 2024 at 08:03:54PM +0200, Paolo Bonzini wrote:
> Il mar 16 lug 2024, 18:45 John Snow <jsnow@redhat.com> ha scritto:
> 
> > My only ask is that we keep the tests running in the custom venv
> > environment we set up at build time
> >
> 
> Yes, they do, however pytest should also be added to pythondeps.toml if we
> go this way.

Done in this patch:

  https://lists.nongnu.org/archive/html/qemu-devel/2024-07/msg03596.html

> 
> > If we move to pytest, it's possible we can eliminate that funkiness, which
> > would be a win.
> >
> 
> There is the pycotap dependency to produce TAP from pytest, but that's
> probably something small enough to be vendored. And also it depends on what
> the dependencies would be for the assets framework.



> 
> > I'm also not so sure about recreating all of the framework that pulls vm
> > images on demand, that sounds like it'd be a lot of work, but maybe I'm
> > wrong about that.
> >
> 
> Yep, that's the part that I am a bit more doubtful about.

Pulling & caching VM images isn't much more than a URL download to 
a local file, not very complex in python. Assuming that's what you
are refering to, then it is already done in this patch:

  https://lists.nongnu.org/archive/html/qemu-devel/2024-07/msg03598.html

With regards,
Daniel
Paolo Bonzini July 16, 2024, 7:34 p.m. UTC | #10
Il mar 16 lug 2024, 20:10 Daniel P. Berrangé <berrange@redhat.com> ha
scritto:

> On Tue, Jul 16, 2024 at 08:03:54PM +0200, Paolo Bonzini wrote:
> > Il mar 16 lug 2024, 18:45 John Snow <jsnow@redhat.com> ha scritto:
> >
> > > My only ask is that we keep the tests running in the custom venv
> > > environment we set up at build time
> > >
> >
> > Yes, they do, however pytest should also be added to pythondeps.toml if
> we
> > go this way.
>
> Done in this patch:
>
>   https://lists.nongnu.org/archive/html/qemu-devel/2024-07/msg03596.html


That adds pycotap, not pytest.

> Yep, that's the part that I am a bit more doubtful about.
>
> Pulling & caching VM images isn't much more than a URL download to
> a local file, not very complex in python. Assuming that's what you
> are refering to, then it is already done in this patch:
>
>   https://lists.nongnu.org/archive/html/qemu-devel/2024-07/msg03598.html


I think there are also compressed assets that have to be passed through
gzip/xzip/zstd. I am worried that Thomas's patches do 90% of the job but
that is not a good estimation of what's left.

Paolo


> 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 :|
>
>
Daniel P. Berrangé July 16, 2024, 7:46 p.m. UTC | #11
On Tue, Jul 16, 2024 at 09:34:41PM +0200, Paolo Bonzini wrote:
> Il mar 16 lug 2024, 20:10 Daniel P. Berrangé <berrange@redhat.com> ha
> scritto:
> 
> > On Tue, Jul 16, 2024 at 08:03:54PM +0200, Paolo Bonzini wrote:
> > > Il mar 16 lug 2024, 18:45 John Snow <jsnow@redhat.com> ha scritto:
> > >
> > > > My only ask is that we keep the tests running in the custom venv
> > > > environment we set up at build time
> > > >
> > >
> > > Yes, they do, however pytest should also be added to pythondeps.toml if
> > we
> > > go this way.
> >
> > Done in this patch:
> >
> >   https://lists.nongnu.org/archive/html/qemu-devel/2024-07/msg03596.html
> 
> 
> That adds pycotap, not pytest.

Yep, the next posting of this series uses only pycotap, there's no
need for using pytest at all, as meson can be the harness directly
when we emit TAP format.

> > Yep, that's the part that I am a bit more doubtful about.
> >
> > Pulling & caching VM images isn't much more than a URL download to
> > a local file, not very complex in python. Assuming that's what you
> > are refering to, then it is already done in this patch:
> >
> >   https://lists.nongnu.org/archive/html/qemu-devel/2024-07/msg03598.html
> 
> 
> I think there are also compressed assets that have to be passed through
> gzip/xzip/zstd. I am worried that Thomas's patches do 90% of the job but
> that is not a good estimation of what's left.



With regards,
Daniel
Thomas Huth July 17, 2024, 6:21 a.m. UTC | #12
On 16/07/2024 18.45, John Snow wrote:
> On Thu, Jul 11, 2024, 7:55 AM Thomas Huth <thuth@redhat.com 
> <mailto:thuth@redhat.com>> wrote:
...
>     - I haven't looked into logging yet ... this still needs some work
>        so that you could e.g. inspect the console output of the guests
>        somewhere

FWIW: This is now done in the next version of the patch series:

  https://lore.kernel.org/qemu-devel/20240716112614.1755692-10-thuth@redhat.com/

> This has spilled the most developer blood of any other problem with the 
> Python-based tests. Be very careful here.

Apart from 1:1 copying the functions from one __init__.py file to the other, 
and from setting up the logger so that it writes its output to a file, I 
didn't have to change anything. It currently simply seems to work.

> I still have a prototype for replacing QMPMachine with an asyncio variant 
> that should have more robust logging features, but I put it on the back-burner.
> 
> Avocado tests are the primary user of the QMP Machine interface I hate the 
> very most, a multi-threaded buffer-reader that works only by the grace of 
> god. If you do go down this path, I may want to take the opportunity to 
> abolish that interface once and for all.
>
 > I think simplifying the console buffering will help ease debuggability.

Feel free to do improvements on top! I think it should be easier now when 
there are no more complicated mixtures with the avocado test runner.

>     What's your thoughts? Is it worth to continue with this approach?
>     Or shall I rather forget about it and wait for the Avocado version
>     update?
> 
> 
> I'm personally ambivalent on avocado; I use it for the python self-tests as 
> dogfooding but I can likely switch back over to plain pytest if that's the 
> direction we head. I don't think I use any crazy features except some 
> asyncio helpers i advocated for. I'm not sure what pytest's asyncio support 
> looks like, but I have to imagine as the premier testing framework that it 
> has *something* for me to use.

There's no more pytest harness in the next iteration of the patch series, 
just the need for pycotap for TAP output. Console logging is completely 
independent of the test runner, I'll simply do normal logging to files there.

> My only ask is that we keep the tests running in the custom venv environment 
> we set up at build time. We have some funky post-hoc initialization of 
> avocado that allows us to use internet packages post-config for testing 
> purposes. If we move to pytest, it's possible we can eliminate that 
> funkiness, which would be a win.

I still need a way for making sure that pycotap is installed, though, so the 
venv is still there.

> I'm also not so sure about recreating all of the framework that pulls vm 
> images on demand, that sounds like it'd be a lot of work, but maybe I'm 
> wrong about that.

It likely does not make sense to rewrite the tests that use these cloud-init 
images (i.e. the ones that depend on the LinuxTest class). But we could 
likely simply continue to use avocado.utils for these, without using the 
avocado test runner.

> Tacit ACK from me on this project in general, provided we are still using 
> the configure venv.

  Thanks,
   Thomas
Thomas Huth July 17, 2024, 7:32 a.m. UTC | #13
On 16/07/2024 20.03, Paolo Bonzini wrote:
> 
> 
> Il mar 16 lug 2024, 18:45 John Snow <jsnow@redhat.com 
> <mailto:jsnow@redhat.com>> ha scritto:
> 
>     My only ask is that we keep the tests running in the custom venv
>     environment we set up at build time
> 
> 
> Yes, they do, however pytest should also be added to pythondeps.toml if we 
> go this way.
> 
>     If we move to pytest, it's possible we can eliminate that funkiness,
>     which would be a win.
> 
> 
> There is the pycotap dependency to produce TAP from pytest, but that's 
> probably something small enough to be vendored.

The next version is only depending on pycotap now. I'm installing it in the 
venv there that we also install when running the old avocado tests. Not sure 
whether that's the best solution, though. Would it be OK to have it in 
python/wheels/ instead?

> And also it depends on what 
> the dependencies would be for the assets framework.
 >
>     I'm also not so sure about recreating all of the framework that pulls vm
>     images on demand, that sounds like it'd be a lot of work, but maybe I'm
>     wrong about that.
> 
> 
> Yep, that's the part that I am a bit more doubtful about.

As I'm mentioned elsewhere, the tests that really have a hard dependency on 
the Avocado framework are only the tests that use the cloud-init images via 
the LinuxTest class. That's currently onle these files:

- boot_linux.py
- hotplug_blk.py
- hotplug_cpu.py
- intel_iommu.py
- replay_linux.py
- smmu.py

I assume we could continue using avocado.utils for the cloud-init stuff 
there, and just run them via the meson test runner, too. I'll give it a try 
when I get some spare time.

  Thomas
Paolo Bonzini July 17, 2024, 7:41 a.m. UTC | #14
On Wed, Jul 17, 2024 at 9:32 AM Thomas Huth <thuth@redhat.com> wrote:
> > There is the pycotap dependency to produce TAP from pytest, but that's
> > probably something small enough to be vendored.
>
> The next version is only depending on pycotap now. I'm installing it in the
> venv there that we also install when running the old avocado tests. Not sure
> whether that's the best solution, though. Would it be OK to have it in
> python/wheels/ instead?

Yes, and you can probably move it to the same group as meson; it's
ridiculously small (5k) and it is indeed used _with_ meson. Then you
don't need any change in "configure".

Paolo