mbox series

[v2,00/23] Convert avocado tests to normal Python unittests

Message ID 20240724175248.1389201-1-thuth@redhat.com (mailing list archive)
Headers show
Series Convert avocado tests to normal Python unittests | expand

Message

Thomas Huth July 24, 2024, 5:52 p.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 (e.g. the timeout settings work completely differently).

So instead of trying to update the python-based test suite in QEMU
to a newer version of Avocado, we should 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 so
far, but this already proves that many tests only need small modifi-
cations to work without Avocado.

Only tests that use the LinuxTest / LinuxDistro and LinuxSSHMixIn
classes (e.g. based on cloud-init images or using SSH) really depend
on the Avocado framework, so we'd need a solution for those if we
want to continue using them. One solution might be to simply use the
required functions from avocado.utils for these tests, and still run
them via the meson test runner instead, but that needs some further
investigation that will be done later.


Now if you want to try out these patches: Apply the patches, then
recompile and then run:

 make check-functional

You can also run single targets e.g. with:

 make check-functional-ppc

You can also run the tests without any test runner now by
setting the PYTHONPATH environment variable to the "python" folder
of your source tree, and by specifying the build directory via
QEMU_BUILD_ROOT (if autodetection fails) and by specifying the
QEMU binary via QEMU_TEST_QEMU_BINARY. For example:

 export PYTHONPATH=$HOME/qemu/python
 export QEMU_TEST_QEMU_BINARY=qemu-system-x86_64
 export QEMU_BUILD_ROOT=$HOME/qemu/build
 ~/qemu/tests/functional/test_virtio_version.py

The logs of the tests can be found in the build directory under
tests/functional/<arch>/<testname> - console log and general logs will
be put in separate files there.

Still to be done: Update the documentation for this new test framework.

v2:
- Addressed review feedback from v1
- Add pycotap as a wheel instead of trying to install it on demand
  when running "make check-functional" (works much better now!)
- Converted much more tests
- Lots of other small improvements here and there

RFC -> v1:
- Now using pycotap for running the tests instead of "pytest"
- Change the name from "tests/pytest" to "tests/functional" accordingly
- Make it possible to run the tests directly
- Use Python's urllib instead of wget for downloading
- Lots of makefile / meson integration improvements
- Converted more tests
- Update MAINTAINERS file accordingly
- Added a patch to run check-functional in the gitlab-CI
- ... lots of other changes I forgot about ... in fact, I changed so
  many things that I also did not dare to pick up the Reviewed-bys
  from the RFC

Thomas Huth (23):
  python: Install pycotap in our venv if necessary
  tests/functional: Add base classes for the upcoming pytest-based tests
  tests/Makefile.include: Increase the level of indentation in the help
    text
  tests/functional: Prepare the meson build system for the functional
    tests
  tests/functional: Convert simple avocado tests into standalone python
    tests
  tests/functional: Convert avocado tests that just need a small
    adjustment
  tests/functional: Implement fetch_asset() method for downloading
    assets
  tests/functional: Convert some tests that download files via
    fetch_asset()
  tests/functional: Add a function for extracting files from an archive
  tests/functional: Convert some avocado tests that needed
    avocado.utils.archive
  tests/functional: Set up logging
  tests/functional: Convert the s390x avocado tests into standalone
    tests
  tests/functional: Convert the x86_cpu_model_versions test
  tests/functional: Convert the microblaze avocado tests into standalone
    tests
  tests/functional: Convert the riscv_opensbi avocado test into a
    standalone test
  tests/functional: Convert the virtio_gpu avocado test into a
    standalone test
  tests/functional: Convert most ppc avocado tests into standalone tests
  tests/functional: Convert the ppc_amiga avocado test into a standalone
    test
  tests/functional: Convert the ppc_hv avocado test into a standalone
    test
  tests/functional: Convert the m68k nextcube test with tesseract
  tests/functional: Convert the acpi-bits test into a standalone test
  tests/functional: Convert the rx_gdbsim avocado test into a standalone
    test
  gitlab-ci: Add "check-functional" to the build tests

 MAINTAINERS                                   |  32 +-
 .gitlab-ci.d/buildtest-template.yml           |   3 +-
 .gitlab-ci.d/buildtest.yml                    |  60 +--
 python/wheels/pycotap-1.3.1-py3-none-any.whl  | Bin 0 -> 5119 bytes
 pythondeps.toml                               |   1 +
 tests/Makefile.include                        |  41 +-
 tests/avocado/machine_microblaze.py           |  61 ---
 tests/avocado/riscv_opensbi.py                |  63 ---
 tests/avocado/tesseract_utils.py              |  46 --
 .../acpi-bits/bits-config/bits-cfg.txt        |   0
 .../acpi-bits/bits-tests/smbios.py2           |   0
 .../acpi-bits/bits-tests/smilatency.py2       |   0
 .../acpi-bits/bits-tests/testacpi.py2         |   0
 .../acpi-bits/bits-tests/testcpuid.py2        |   0
 tests/functional/meson.build                  | 150 +++++++
 tests/functional/qemu_test/__init__.py        | 393 ++++++++++++++++++
 tests/functional/qemu_test/tesseract.py       |  35 ++
 tests/functional/qemu_test/utils.py           |  47 +++
 .../test_acpi_bits.py}                        |  29 +-
 .../test_arm_canona1100.py}                   |  21 +-
 .../test_arm_n8x0.py}                         |  25 +-
 .../test_avr_mega2560.py}                     |  11 +-
 .../test_cpu_queries.py}                      |   7 +-
 .../test_empty_cpu_model.py}                  |   7 +-
 .../test_info_usernet.py}                     |  11 +-
 .../test_loongarch64_virt.py}                 |  16 +-
 .../test_m68k_nextcube.py}                    |  20 +-
 .../test_mem_addr_space.py}                   |  52 +--
 .../functional/test_microblaze_s3adsp1800.py  |  38 ++
 .../test_microblazeel_s3adsp1800.py           |  41 ++
 .../test_mips64el_loongson3v.py}              |  26 +-
 .../test_netdev_ethtool.py}                   |  32 +-
 .../test_pc_cpu_hotplug_props.py}             |  11 +-
 .../test_ppc64_hv.py}                         |  36 +-
 .../test_ppc64_powernv.py}                    |  45 +-
 .../test_ppc64_pseries.py}                    |  45 +-
 .../ppc_405.py => functional/test_ppc_405.py} |  19 +-
 .../test_ppc_40p.py}                          |  37 +-
 .../test_ppc_74xx.py}                         |  74 ++--
 .../test_ppc_amiga.py}                        |  33 +-
 .../test_ppc_bamboo.py}                       |  23 +-
 .../test_ppc_mpc8544ds.py}                    |  19 +-
 .../test_ppc_virtex_ml507.py}                 |  19 +-
 tests/functional/test_riscv_opensbi.py        |  36 ++
 .../test_rx_gdbsim.py}                        |  34 +-
 .../test_s390x_ccw_virtio.py}                 |  32 +-
 .../test_s390x_topology.py}                   |  70 ++--
 .../test_sparc64_sun4u.py}                    |  25 +-
 .../version.py => functional/test_version.py} |  13 +-
 .../test_virtio_gpu.py}                       |  34 +-
 .../test_virtio_version.py}                   |   8 +-
 .../test_x86_cpu_model_versions.py}           |  63 +--
 tests/meson.build                             |   1 +
 53 files changed, 1187 insertions(+), 758 deletions(-)
 create mode 100644 python/wheels/pycotap-1.3.1-py3-none-any.whl
 delete mode 100644 tests/avocado/machine_microblaze.py
 delete mode 100644 tests/avocado/riscv_opensbi.py
 delete mode 100644 tests/avocado/tesseract_utils.py
 rename tests/{avocado => functional}/acpi-bits/bits-config/bits-cfg.txt (100%)
 rename tests/{avocado => functional}/acpi-bits/bits-tests/smbios.py2 (100%)
 rename tests/{avocado => functional}/acpi-bits/bits-tests/smilatency.py2 (100%)
 rename tests/{avocado => functional}/acpi-bits/bits-tests/testacpi.py2 (100%)
 rename tests/{avocado => functional}/acpi-bits/bits-tests/testcpuid.py2 (100%)
 create mode 100644 tests/functional/meson.build
 create mode 100644 tests/functional/qemu_test/__init__.py
 create mode 100644 tests/functional/qemu_test/tesseract.py
 create mode 100644 tests/functional/qemu_test/utils.py
 rename tests/{avocado/acpi-bits.py => functional/test_acpi_bits.py} (95%)
 mode change 100644 => 100755
 rename tests/{avocado/machine_arm_canona1100.py => functional/test_arm_canona1100.py} (71%)
 mode change 100644 => 100755
 rename tests/{avocado/machine_arm_n8x0.py => functional/test_arm_n8x0.py} (71%)
 mode change 100644 => 100755
 rename tests/{avocado/machine_avr6.py => functional/test_avr_mega2560.py} (90%)
 mode change 100644 => 100755
 rename tests/{avocado/cpu_queries.py => functional/test_cpu_queries.py} (89%)
 mode change 100644 => 100755
 rename tests/{avocado/empty_cpu_model.py => functional/test_empty_cpu_model.py} (84%)
 mode change 100644 => 100755
 rename tests/{avocado/info_usernet.py => functional/test_info_usernet.py} (87%)
 mode change 100644 => 100755
 rename tests/{avocado/machine_loongarch.py => functional/test_loongarch64_virt.py} (89%)
 mode change 100644 => 100755
 rename tests/{avocado/machine_m68k_nextcube.py => functional/test_m68k_nextcube.py} (86%)
 mode change 100644 => 100755
 rename tests/{avocado/mem-addr-space-check.py => functional/test_mem_addr_space.py} (93%)
 mode change 100644 => 100755
 create mode 100755 tests/functional/test_microblaze_s3adsp1800.py
 create mode 100755 tests/functional/test_microblazeel_s3adsp1800.py
 rename tests/{avocado/machine_mips_loongson3v.py => functional/test_mips64el_loongson3v.py} (55%)
 mode change 100644 => 100755
 rename tests/{avocado/netdev-ethtool.py => functional/test_netdev_ethtool.py} (81%)
 mode change 100644 => 100755
 rename tests/{avocado/pc_cpu_hotplug_props.py => functional/test_pc_cpu_hotplug_props.py} (90%)
 mode change 100644 => 100755
 rename tests/{avocado/ppc_hv_tests.py => functional/test_ppc64_hv.py} (92%)
 mode change 100644 => 100755
 rename tests/{avocado/ppc_powernv.py => functional/test_ppc64_powernv.py} (80%)
 mode change 100644 => 100755
 rename tests/{avocado/ppc_pseries.py => functional/test_ppc64_pseries.py} (83%)
 mode change 100644 => 100755
 rename tests/{avocado/ppc_405.py => functional/test_ppc_405.py} (73%)
 mode change 100644 => 100755
 rename tests/{avocado/ppc_prep_40p.py => functional/test_ppc_40p.py} (78%)
 mode change 100644 => 100755
 rename tests/{avocado/ppc_74xx.py => functional/test_ppc_74xx.py} (74%)
 mode change 100644 => 100755
 rename tests/{avocado/ppc_amiga.py => functional/test_ppc_amiga.py} (54%)
 mode change 100644 => 100755
 rename tests/{avocado/ppc_bamboo.py => functional/test_ppc_bamboo.py} (75%)
 mode change 100644 => 100755
 rename tests/{avocado/ppc_mpc8544ds.py => functional/test_ppc_mpc8544ds.py} (75%)
 mode change 100644 => 100755
 rename tests/{avocado/ppc_virtex_ml507.py => functional/test_ppc_virtex_ml507.py} (78%)
 mode change 100644 => 100755
 create mode 100755 tests/functional/test_riscv_opensbi.py
 rename tests/{avocado/machine_rx_gdbsim.py => functional/test_rx_gdbsim.py} (78%)
 mode change 100644 => 100755
 rename tests/{avocado/machine_s390_ccw_virtio.py => functional/test_s390x_ccw_virtio.py} (95%)
 mode change 100644 => 100755
 rename tests/{avocado/s390_topology.py => functional/test_s390x_topology.py} (90%)
 mode change 100644 => 100755
 rename tests/{avocado/machine_sparc64_sun4u.py => functional/test_sparc64_sun4u.py} (60%)
 mode change 100644 => 100755
 rename tests/{avocado/version.py => functional/test_version.py} (78%)
 mode change 100644 => 100755
 rename tests/{avocado/virtio-gpu.py => functional/test_virtio_gpu.py} (88%)
 mode change 100644 => 100755
 rename tests/{avocado/virtio_version.py => functional/test_virtio_version.py} (98%)
 mode change 100644 => 100755
 rename tests/{avocado/x86_cpu_model_versions.py => functional/test_x86_cpu_model_versions.py} (92%)
 mode change 100644 => 100755

Comments

Richard Henderson July 24, 2024, 11:35 p.m. UTC | #1
On 7/25/24 03:52, 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 (e.g. the timeout settings work completely differently).
> 
> So instead of trying to update the python-based test suite in QEMU
> to a newer version of Avocado, we should 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 so
> far, but this already proves that many tests only need small modifi-
> cations to work without Avocado.
> 
> Only tests that use the LinuxTest / LinuxDistro and LinuxSSHMixIn
> classes (e.g. based on cloud-init images or using SSH) really depend
> on the Avocado framework, so we'd need a solution for those if we
> want to continue using them. One solution might be to simply use the
> required functions from avocado.utils for these tests, and still run
> them via the meson test runner instead, but that needs some further
> investigation that will be done later.
> 
> 
> Now if you want to try out these patches: Apply the patches, then
> recompile and then run:
> 
>   make check-functional
> 
> You can also run single targets e.g. with:
> 
>   make check-functional-ppc
> 
> You can also run the tests without any test runner now by
> setting the PYTHONPATH environment variable to the "python" folder
> of your source tree, and by specifying the build directory via
> QEMU_BUILD_ROOT (if autodetection fails) and by specifying the
> QEMU binary via QEMU_TEST_QEMU_BINARY. For example:
> 
>   export PYTHONPATH=$HOME/qemu/python
>   export QEMU_TEST_QEMU_BINARY=qemu-system-x86_64
>   export QEMU_BUILD_ROOT=$HOME/qemu/build
>   ~/qemu/tests/functional/test_virtio_version.py
> 
> The logs of the tests can be found in the build directory under
> tests/functional/<arch>/<testname> - console log and general logs will
> be put in separate files there.
> 
> Still to be done: Update the documentation for this new test framework.

I'll say again that the download *must* be handled separately from the test with timeout. 
This is an absolute show-stopper.

I've tried this twice now, from a decently fast connection in central Brisbane, and have 
had multiple downloads be canceled by the timeout.  Since the download isn't clever enough 
to pick up where it left off, it will never succeed.


r~
Daniel P. Berrangé July 25, 2024, 9:55 a.m. UTC | #2
On Thu, Jul 25, 2024 at 09:35:22AM +1000, Richard Henderson wrote:
> On 7/25/24 03:52, 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 (e.g. the timeout settings work completely differently).
> > 
> > So instead of trying to update the python-based test suite in QEMU
> > to a newer version of Avocado, we should 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 so
> > far, but this already proves that many tests only need small modifi-
> > cations to work without Avocado.
> > 
> > Only tests that use the LinuxTest / LinuxDistro and LinuxSSHMixIn
> > classes (e.g. based on cloud-init images or using SSH) really depend
> > on the Avocado framework, so we'd need a solution for those if we
> > want to continue using them. One solution might be to simply use the
> > required functions from avocado.utils for these tests, and still run
> > them via the meson test runner instead, but that needs some further
> > investigation that will be done later.
> > 
> > 
> > Now if you want to try out these patches: Apply the patches, then
> > recompile and then run:
> > 
> >   make check-functional
> > 
> > You can also run single targets e.g. with:
> > 
> >   make check-functional-ppc
> > 
> > You can also run the tests without any test runner now by
> > setting the PYTHONPATH environment variable to the "python" folder
> > of your source tree, and by specifying the build directory via
> > QEMU_BUILD_ROOT (if autodetection fails) and by specifying the
> > QEMU binary via QEMU_TEST_QEMU_BINARY. For example:
> > 
> >   export PYTHONPATH=$HOME/qemu/python
> >   export QEMU_TEST_QEMU_BINARY=qemu-system-x86_64
> >   export QEMU_BUILD_ROOT=$HOME/qemu/build
> >   ~/qemu/tests/functional/test_virtio_version.py
> > 
> > The logs of the tests can be found in the build directory under
> > tests/functional/<arch>/<testname> - console log and general logs will
> > be put in separate files there.
> > 
> > Still to be done: Update the documentation for this new test framework.
> 
> I'll say again that the download *must* be handled separately from the test
> with timeout. This is an absolute show-stopper.
> 
> I've tried this twice now, from a decently fast connection in central
> Brisbane, and have had multiple downloads be canceled by the timeout.  Since
> the download isn't clever enough to pick up where it left off, it will never
> succeed.

This is a tricky problem the way the tests are currently written, given the
desire for a minimal-change from the old avocado impl.

IIUC, avocado already had a per-test timeout, so would suffer the same
problem with downloads exploding the "normal" running time when cached.

To address this we'll need a refactoring to enable us to declare the
required "assets" externally from the test code.

Taking one simple example

  class LinuxInitrd(QemuSystemTest):

      def test_with_2gib_file_should_exit_error_msg_with_linux_v3_6(self):

          kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora/li'
                        'nux/releases/18/Fedora/x86_64/os/images/pxeboot/vmlinuz')
          kernel_hash = '41464f68efe42b9991250bed86c7081d2ccdbb21'
          kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
          ...snip...

  if __name__ == '__main__':
      QemuSystemTest.main()


Consider if we declared all required assets as class level variable

  class LinuxInitrd(QemuSystemTest):

      ASSETS = {
         "fedora18": {
	    "url": ('https://archives.fedoraproject.org/pub/archive/fedora/li'
                     'nux/releases/18/Fedora/x86_64/os/images/pxeboot/vmlinuz'),
            "hash": "'41464f68efe42b9991250bed86c7081d2ccdbb21'"
	 }
      }

Then, we change the 'fetch_asset' method to take an asset name, not a
URL+hash:

      def test_with_2gib_file_should_exit_error_msg_with_linux_v3_6(self):
          kernel_path = self.fetch_asset("fedora18")

Now, 'fetch_asset' would lookup the URL + hash in the self.__class__.ASSETS
dict, so the test would run exactly as before.

Finally, we modify QemuSystemTest.main() so that knows to look for a
'--fetch-assets' parameter in sys.argv. When it see --fetch-assets,
instead of running each test, it should download everything found in
the ASSETS class variables.

This now gives us the ability to  run a separate '--fetch-assets'
invokation with elevated timeout, while runing tests with a normal
timeout.

This is all a non-trivial amount of work though, so I don't think
it is reasonable todo this as part of the immediate conversion in
this series.

The only short term option is to configure meson run tests with a
massively larger timeout, until we're able to enable some pre-caching
mechansim.

With regards,
Daniel
Thomas Huth July 25, 2024, 10:13 a.m. UTC | #3
On 25/07/2024 01.35, Richard Henderson wrote:
> On 7/25/24 03:52, 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 (e.g. the timeout settings work completely differently).
>>
>> So instead of trying to update the python-based test suite in QEMU
>> to a newer version of Avocado, we should 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 so
>> far, but this already proves that many tests only need small modifi-
>> cations to work without Avocado.
>>
>> Only tests that use the LinuxTest / LinuxDistro and LinuxSSHMixIn
>> classes (e.g. based on cloud-init images or using SSH) really depend
>> on the Avocado framework, so we'd need a solution for those if we
>> want to continue using them. One solution might be to simply use the
>> required functions from avocado.utils for these tests, and still run
>> them via the meson test runner instead, but that needs some further
>> investigation that will be done later.
>>
>>
>> Now if you want to try out these patches: Apply the patches, then
>> recompile and then run:
>>
>>   make check-functional
>>
>> You can also run single targets e.g. with:
>>
>>   make check-functional-ppc
>>
>> You can also run the tests without any test runner now by
>> setting the PYTHONPATH environment variable to the "python" folder
>> of your source tree, and by specifying the build directory via
>> QEMU_BUILD_ROOT (if autodetection fails) and by specifying the
>> QEMU binary via QEMU_TEST_QEMU_BINARY. For example:
>>
>>   export PYTHONPATH=$HOME/qemu/python
>>   export QEMU_TEST_QEMU_BINARY=qemu-system-x86_64
>>   export QEMU_BUILD_ROOT=$HOME/qemu/build
>>   ~/qemu/tests/functional/test_virtio_version.py
>>
>> The logs of the tests can be found in the build directory under
>> tests/functional/<arch>/<testname> - console log and general logs will
>> be put in separate files there.
>>
>> Still to be done: Update the documentation for this new test framework.
> 
> I'll say again that the download *must* be handled separately from the test 
> with timeout. This is an absolute show-stopper.
> 
> I've tried this twice now, from a decently fast connection in central 
> Brisbane, and have had multiple downloads be canceled by the timeout.  Since 
> the download isn't clever enough to pick up where it left off, it will never 
> succeed.

  Hi Richard,

just for my understanding, did you try to run the tests in parallel (i.e. 
something like "make -j$(nproc)")? ... I think in that case you can easily 
clog your internet connection even on modern systems if a lot of tests are 
trying to download the assets in parallel.

For me, it works fine if I use normal serial testing with "-j" (btw. Avocado 
v88 is doing serial testing, too, so you won't lose much time during the 
first run here). But if downloading fails for you without "-j", too, I 
agree, we need to tackle that problem first, e.g. by implementing what 
Daniel suggested. That will take a little bit longer, of course, so I hope 
you meanwhile found a work-around for the problem with the missing "imp" 
package on your system?

  Thomas
Richard Henderson July 25, 2024, 10:42 a.m. UTC | #4
On 7/25/24 19:55, Daniel P. Berrangé wrote:
> On Thu, Jul 25, 2024 at 09:35:22AM +1000, Richard Henderson wrote:
>> On 7/25/24 03:52, 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 (e.g. the timeout settings work completely differently).
>>>
>>> So instead of trying to update the python-based test suite in QEMU
>>> to a newer version of Avocado, we should 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 so
>>> far, but this already proves that many tests only need small modifi-
>>> cations to work without Avocado.
>>>
>>> Only tests that use the LinuxTest / LinuxDistro and LinuxSSHMixIn
>>> classes (e.g. based on cloud-init images or using SSH) really depend
>>> on the Avocado framework, so we'd need a solution for those if we
>>> want to continue using them. One solution might be to simply use the
>>> required functions from avocado.utils for these tests, and still run
>>> them via the meson test runner instead, but that needs some further
>>> investigation that will be done later.
>>>
>>>
>>> Now if you want to try out these patches: Apply the patches, then
>>> recompile and then run:
>>>
>>>    make check-functional
>>>
>>> You can also run single targets e.g. with:
>>>
>>>    make check-functional-ppc
>>>
>>> You can also run the tests without any test runner now by
>>> setting the PYTHONPATH environment variable to the "python" folder
>>> of your source tree, and by specifying the build directory via
>>> QEMU_BUILD_ROOT (if autodetection fails) and by specifying the
>>> QEMU binary via QEMU_TEST_QEMU_BINARY. For example:
>>>
>>>    export PYTHONPATH=$HOME/qemu/python
>>>    export QEMU_TEST_QEMU_BINARY=qemu-system-x86_64
>>>    export QEMU_BUILD_ROOT=$HOME/qemu/build
>>>    ~/qemu/tests/functional/test_virtio_version.py
>>>
>>> The logs of the tests can be found in the build directory under
>>> tests/functional/<arch>/<testname> - console log and general logs will
>>> be put in separate files there.
>>>
>>> Still to be done: Update the documentation for this new test framework.
>>
>> I'll say again that the download *must* be handled separately from the test
>> with timeout. This is an absolute show-stopper.
>>
>> I've tried this twice now, from a decently fast connection in central
>> Brisbane, and have had multiple downloads be canceled by the timeout.  Since
>> the download isn't clever enough to pick up where it left off, it will never
>> succeed.
> 
> This is a tricky problem the way the tests are currently written, given the
> desire for a minimal-change from the old avocado impl.
> 
> IIUC, avocado already had a per-test timeout, so would suffer the same
> problem with downloads exploding the "normal" running time when cached.

Avocado runs a first pass doing all of the downloads, and only afterward runs the actual 
timed tests.  I don't know the specifics of how, but it certainly obvious in the logging.

> Consider if we declared all required assets as class level variable
> 
>    class LinuxInitrd(QemuSystemTest):
> 
>        ASSETS = {
>           "fedora18": {
> 	    "url": ('https://archives.fedoraproject.org/pub/archive/fedora/li'
>                       'nux/releases/18/Fedora/x86_64/os/images/pxeboot/vmlinuz'),
>              "hash": "'41464f68efe42b9991250bed86c7081d2ccdbb21'"
> 	 }
>        }
> 
> Then, we change the 'fetch_asset' method to take an asset name, not a
> URL+hash:
> 
>        def test_with_2gib_file_should_exit_error_msg_with_linux_v3_6(self):
>            kernel_path = self.fetch_asset("fedora18")
> 
> Now, 'fetch_asset' would lookup the URL + hash in the self.__class__.ASSETS
> dict, so the test would run exactly as before.

Sure, that's one possibility.

> This is all a non-trivial amount of work though, so I don't think
> it is reasonable todo this as part of the immediate conversion in
> this series.

I think that if we *don't* do this, then we cannot run in CI *at all* because we will be 
plagued with false timeouts.

And, frankly, any developer more than a few time zones away from the hosting of the asset 
will be continually frustrated.

I repeat: this is a show-stopper.


r~
Richard Henderson July 25, 2024, 10:50 a.m. UTC | #5
On 7/25/24 20:13, Thomas Huth wrote:
>   Hi Richard,
> 
> just for my understanding, did you try to run the tests in parallel (i.e. something like 
> "make -j$(nproc)")?

No, I ran "make check-functional" with zero parallelism.

> For me, it works fine if I use normal serial testing with "-j" (btw. Avocado v88 is doing 
> serial testing, too, so you won't lose much time during the first run here). But if 
> downloading fails for you without "-j", too, I agree, we need to tackle that problem 
> first, e.g. by implementing what Daniel suggested. That will take a little bit longer, of 
> course, so I hope you meanwhile found a work-around for the problem with the missing "imp" 
> package on your system?

Not so far.  I'm using VMs for avocado testing at present.


r~
Daniel P. Berrangé July 25, 2024, 11:07 a.m. UTC | #6
On Thu, Jul 25, 2024 at 08:42:31PM +1000, Richard Henderson wrote:
> On 7/25/24 19:55, Daniel P. Berrangé wrote:
> > On Thu, Jul 25, 2024 at 09:35:22AM +1000, Richard Henderson wrote:
> > > On 7/25/24 03:52, 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 (e.g. the timeout settings work completely differently).
> > > > 
> > > > So instead of trying to update the python-based test suite in QEMU
> > > > to a newer version of Avocado, we should 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 so
> > > > far, but this already proves that many tests only need small modifi-
> > > > cations to work without Avocado.
> > > > 
> > > > Only tests that use the LinuxTest / LinuxDistro and LinuxSSHMixIn
> > > > classes (e.g. based on cloud-init images or using SSH) really depend
> > > > on the Avocado framework, so we'd need a solution for those if we
> > > > want to continue using them. One solution might be to simply use the
> > > > required functions from avocado.utils for these tests, and still run
> > > > them via the meson test runner instead, but that needs some further
> > > > investigation that will be done later.
> > > > 
> > > > 
> > > > Now if you want to try out these patches: Apply the patches, then
> > > > recompile and then run:
> > > > 
> > > >    make check-functional
> > > > 
> > > > You can also run single targets e.g. with:
> > > > 
> > > >    make check-functional-ppc
> > > > 
> > > > You can also run the tests without any test runner now by
> > > > setting the PYTHONPATH environment variable to the "python" folder
> > > > of your source tree, and by specifying the build directory via
> > > > QEMU_BUILD_ROOT (if autodetection fails) and by specifying the
> > > > QEMU binary via QEMU_TEST_QEMU_BINARY. For example:
> > > > 
> > > >    export PYTHONPATH=$HOME/qemu/python
> > > >    export QEMU_TEST_QEMU_BINARY=qemu-system-x86_64
> > > >    export QEMU_BUILD_ROOT=$HOME/qemu/build
> > > >    ~/qemu/tests/functional/test_virtio_version.py
> > > > 
> > > > The logs of the tests can be found in the build directory under
> > > > tests/functional/<arch>/<testname> - console log and general logs will
> > > > be put in separate files there.
> > > > 
> > > > Still to be done: Update the documentation for this new test framework.
> > > 
> > > I'll say again that the download *must* be handled separately from the test
> > > with timeout. This is an absolute show-stopper.
> > > 
> > > I've tried this twice now, from a decently fast connection in central
> > > Brisbane, and have had multiple downloads be canceled by the timeout.  Since
> > > the download isn't clever enough to pick up where it left off, it will never
> > > succeed.
> > 
> > This is a tricky problem the way the tests are currently written, given the
> > desire for a minimal-change from the old avocado impl.
> > 
> > IIUC, avocado already had a per-test timeout, so would suffer the same
> > problem with downloads exploding the "normal" running time when cached.
> 
> Avocado runs a first pass doing all of the downloads, and only afterward
> runs the actual timed tests.  I don't know the specifics of how, but it
> certainly obvious in the logging.

Oh interesting, I found how it does it..

The file avocado/plugins/assets.py will build an AST of the python
code in a test file, look for all 'fetch_asset' calls, then extract
the parameters to these calls, and donwload them. This is clever.
Basically avoids the refactoring that I suggested.

So yeah, that is a gap.

Practically speaking, we have a choice of either  calling into this
avocado python lib as is, or copying tthat python lib into QEMU.

With regards,
Daniel
Thomas Huth July 26, 2024, 1:03 p.m. UTC | #7
On 25/07/2024 13.07, Daniel P. Berrangé wrote:
> On Thu, Jul 25, 2024 at 08:42:31PM +1000, Richard Henderson wrote:
>> On 7/25/24 19:55, Daniel P. Berrangé wrote:
>>> On Thu, Jul 25, 2024 at 09:35:22AM +1000, Richard Henderson wrote:
...
>> Avocado runs a first pass doing all of the downloads, and only afterward
>> runs the actual timed tests.  I don't know the specifics of how, but it
>> certainly obvious in the logging.
> 
> Oh interesting, I found how it does it..
> 
> The file avocado/plugins/assets.py will build an AST of the python
> code in a test file, look for all 'fetch_asset' calls, then extract
> the parameters to these calls, and donwload them. This is clever.
> Basically avoids the refactoring that I suggested.
> 
> So yeah, that is a gap.
> 
> Practically speaking, we have a choice of either  calling into this
> avocado python lib as is, or copying tthat python lib into QEMU.

Honestly, I'd prefer to do some refactoring instead, something like you 
suggested in your earlier mail. Rationale: For the basic tests it would be 
good if we would not depend on the Avacodo framework anymore, otherwise we 
likely will continue to run into the situation that our test framework stops 
working on some random new python versions and nobody within the QEMU 
community has a clue how to fix the situation since nobody is really 
familiar with the Avocado framework. Also, while that 
avocado/plugins/assets.py sounds like a very neat trick done by a skilled 
Python wizard, the average QEMU developer (like me) is just a skilled C 
coder with only basic Python knowledge, so I'd prefer if we could use a 
simpler mechanism instead that is easier to understand and to debug for 
everybody once we run into problems with it.

Thus, I'd suggest to bite the bullet and refactor the tests that download 
assets. I can look into this after my summer vacation - but if somebody 
feels interested and wants to look into this during the next two weeks 
already, that would be very welcome, too, of course!

  Thomas
Cleber Rosa July 26, 2024, 1:50 p.m. UTC | #8
On Fri, Jul 26, 2024 at 9:04 AM Thomas Huth <thuth@redhat.com> wrote:
>
> On 25/07/2024 13.07, Daniel P. Berrangé wrote:
> > On Thu, Jul 25, 2024 at 08:42:31PM +1000, Richard Henderson wrote:
> >> On 7/25/24 19:55, Daniel P. Berrangé wrote:
> >>> On Thu, Jul 25, 2024 at 09:35:22AM +1000, Richard Henderson wrote:
> ...
> >> Avocado runs a first pass doing all of the downloads, and only afterward
> >> runs the actual timed tests.  I don't know the specifics of how, but it
> >> certainly obvious in the logging.
> >
> > Oh interesting, I found how it does it..
> >
> > The file avocado/plugins/assets.py will build an AST of the python
> > code in a test file, look for all 'fetch_asset' calls, then extract
> > the parameters to these calls, and donwload them. This is clever.
> > Basically avoids the refactoring that I suggested.
> >
> > So yeah, that is a gap.
> >
> > Practically speaking, we have a choice of either  calling into this
> > avocado python lib as is, or copying tthat python lib into QEMU.
>
> Honestly, I'd prefer to do some refactoring instead, something like you
> suggested in your earlier mail. Rationale: For the basic tests it would be
> good if we would not depend on the Avacodo framework anymore, otherwise we
> likely will continue to run into the situation that our test framework stops
> working on some random new python versions and nobody within the QEMU
> community has a clue how to fix the situation since nobody is really
> familiar with the Avocado framework. Also, while that
> avocado/plugins/assets.py sounds like a very neat trick done by a skilled
> Python wizard, the average QEMU developer (like me) is just a skilled C
> coder with only basic Python knowledge, so I'd prefer if we could use a
> simpler mechanism instead that is easier to understand and to debug for
> everybody once we run into problems with it.
>

Hi Thomas,

That wizardry is indeed not nice, and has limitations.  It was
replaced in recent Avocado versions for the dependencies mechanism:

   https://avocado-framework.readthedocs.io/en/103.0/guides/user/chapters/dependencies.html

Specifically for the assets (downloadable files), you can find the
documentation here:

   https://avocado-framework.readthedocs.io/en/103.0/guides/user/chapters/dependencies.html#asset

Those are superior to the previous implementation because they compute
a dependency graph that works on the resolution while tests with
dependencies met (or no deps) start running right away.

Regards,
- Cleber.
Philippe Mathieu-Daudé July 29, 2024, 2:44 p.m. UTC | #9
On 25/7/24 01:35, Richard Henderson wrote:
> On 7/25/24 03:52, Thomas Huth wrote:

> I'll say again that the download *must* be handled separately from the 
> test with timeout. This is an absolute show-stopper.

Queuing patches not related to assets downloading
(1-6, 13, 15 and 23).