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

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

[Thomas Huth]

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%)

-- 
2.45.2

[RFC PATCH 1/8] tests/pytest: Add base classes for the upcoming pytest-based tests

[Thomas Huth]

The file is a copy of the tests/avocado/avocado_qemu/__init__.py file
with some adjustments to get rid of the Avocado dependencies (i.e.
we also have to drop the LinuxSSHMixIn and LinuxTest for now).

The emulator binary, source and build directory are now passed via
environment variables that will be set via meson.build later.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/pytest/qemu_pytest/__init__.py | 344 +++++++++++++++++++++++++++
 1 file changed, 344 insertions(+)
 create mode 100644 tests/pytest/qemu_pytest/__init__.py

diff --git a/tests/pytest/qemu_pytest/__init__.py b/tests/pytest/qemu_pytest/__init__.py
new file mode 100644
index 0000000000..e3ed32e3de
--- /dev/null
+++ b/tests/pytest/qemu_pytest/__init__.py
@@ -0,0 +1,344 @@
+# Test class and utilities for functional tests
+#
+# Copyright 2018, 2024 Red Hat, Inc.
+#
+# Original Author (Avocado-based tests):
+#  Cleber Rosa <crosa@redhat.com>
+#
+# Adaption for pytest based version:
+#  Thomas Huth <thuth@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+import logging
+import os
+import shutil
+import subprocess
+import sys
+import tempfile
+import time
+import uuid
+import unittest
+
+from qemu.machine import QEMUMachine
+from qemu.utils import (get_info_usernet_hostfwd_port, kvm_available,
+                        tcg_available)
+
+BUILD_DIR = os.getenv('PYTEST_BUILD_ROOT')
+SOURCE_DIR = os.getenv('PYTEST_SOURCE_ROOT')
+
+def has_cmd(name, args=None):
+    """
+    This function is for use in a @skipUnless decorator, e.g.:
+
+        @skipUnless(*has_cmd('sudo -n', ('sudo', '-n', 'true')))
+        def test_something_that_needs_sudo(self):
+            ...
+    """
+
+    if args is None:
+        args = ('which', name)
+
+    try:
+        _, stderr, exitcode = run_cmd(args)
+    except Exception as e:
+        exitcode = -1
+        stderr = str(e)
+
+    if exitcode != 0:
+        cmd_line = ' '.join(args)
+        err = f'{name} required, but "{cmd_line}" failed: {stderr.strip()}'
+        return (False, err)
+    else:
+        return (True, '')
+
+def has_cmds(*cmds):
+    """
+    This function is for use in a @skipUnless decorator and
+    allows checking for the availability of multiple commands, e.g.:
+
+        @skipUnless(*has_cmds(('cmd1', ('cmd1', '--some-parameter')),
+                              'cmd2', 'cmd3'))
+        def test_something_that_needs_cmd1_and_cmd2(self):
+            ...
+    """
+
+    for cmd in cmds:
+        if isinstance(cmd, str):
+            cmd = (cmd,)
+
+        ok, errstr = has_cmd(*cmd)
+        if not ok:
+            return (False, errstr)
+
+    return (True, '')
+
+def run_cmd(args):
+    subp = subprocess.Popen(args,
+                            stdout=subprocess.PIPE,
+                            stderr=subprocess.PIPE,
+                            universal_newlines=True)
+    stdout, stderr = subp.communicate()
+    ret = subp.returncode
+
+    return (stdout, stderr, ret)
+
+def is_readable_executable_file(path):
+    return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
+
+def _console_interaction(test, success_message, failure_message,
+                         send_string, keep_sending=False, vm=None):
+    assert not keep_sending or send_string
+    if vm is None:
+        vm = test.vm
+    console = vm.console_file
+    console_logger = logging.getLogger('console')
+    while True:
+        if send_string:
+            vm.console_socket.sendall(send_string.encode())
+            if not keep_sending:
+                send_string = None # send only once
+        try:
+            msg = console.readline().decode().strip()
+        except UnicodeDecodeError:
+            msg = None
+        if not msg:
+            continue
+        console_logger.debug(msg)
+        if success_message is None or success_message in msg:
+            break
+        if failure_message and failure_message in msg:
+            console.close()
+            fail = 'Failure message found in console: "%s". Expected: "%s"' % \
+                    (failure_message, success_message)
+            test.fail(fail)
+
+def interrupt_interactive_console_until_pattern(test, success_message,
+                                                failure_message=None,
+                                                interrupt_string='\r'):
+    """
+    Keep sending a string to interrupt a console prompt, while logging the
+    console output. Typical use case is to break a boot loader prompt, such:
+
+        Press a key within 5 seconds to interrupt boot process.
+        5
+        4
+        3
+        2
+        1
+        Booting default image...
+
+    :param test: a  test containing a VM that will have its console
+                 read and probed for a success or failure message
+    :type test: :class:`qemu_pytest.QemuSystemTest`
+    :param success_message: if this message appears, test succeeds
+    :param failure_message: if this message appears, test fails
+    :param interrupt_string: a string to send to the console before trying
+                             to read a new line
+    """
+    _console_interaction(test, success_message, failure_message,
+                         interrupt_string, True)
+
+def wait_for_console_pattern(test, success_message, failure_message=None,
+                             vm=None):
+    """
+    Waits for messages to appear on the console, while logging the content
+
+    :param test: a test containing a VM that will have its console
+                 read and probed for a success or failure message
+    :type test: :class:`qemu_pytest.QemuSystemTest`
+    :param success_message: if this message appears, test succeeds
+    :param failure_message: if this message appears, test fails
+    """
+    _console_interaction(test, success_message, failure_message, None, vm=vm)
+
+def exec_command(test, command):
+    """
+    Send a command to a console (appending CRLF characters), while logging
+    the content.
+
+    :param test: a test containing a VM.
+    :type test: :class:`qemu_pytest.QemuSystemTest`
+    :param command: the command to send
+    :type command: str
+    """
+    _console_interaction(test, None, None, command + '\r')
+
+def exec_command_and_wait_for_pattern(test, command,
+                                      success_message, failure_message=None):
+    """
+    Send a command to a console (appending CRLF characters), then wait
+    for success_message to appear on the console, while logging the.
+    content. Mark the test as failed if failure_message is found instead.
+
+    :param test: a test containing a VM that will have its console
+                 read and probed for a success or failure message
+    :type test: :class:`qemu_pytest.QemuSystemTest`
+    :param command: the command to send
+    :param success_message: if this message appears, test succeeds
+    :param failure_message: if this message appears, test fails
+    """
+    _console_interaction(test, success_message, failure_message, command + '\r')
+
+class QemuBaseTest(unittest.TestCase):
+
+    # default timeout for all tests, can be overridden
+    timeout = 120
+
+    qemu_bin = os.getenv('PYTEST_QEMU_BINARY')
+
+    workdir = os.path.join(BUILD_DIR, 'tests/pytest')
+    logdir = os.path.join(BUILD_DIR, 'tests/pytest')
+
+    cpu = None
+    machine = None
+
+    log = logging.getLogger('qemu-pytest')
+
+    def setUp(self, bin_prefix):
+        self.assertIsNotNone(BUILD_DIR, 'PYTEST_BUILD_ROOT must be set')
+        self.assertIsNotNone(SOURCE_DIR,'PYTEST_SOURCE_ROOT must be set')
+        self.assertIsNotNone(self.qemu_bin, 'PYTEST_QEMU_BINARY must be set')
+
+    def fetch_asset(self, name,
+                    asset_hash, algorithm=None,
+                    locations=None, expire=None,
+                    find_only=False, cancel_on_missing=True):
+        return super().fetch_asset(name,
+                        asset_hash=asset_hash,
+                        algorithm=algorithm,
+                        locations=locations,
+                        expire=expire,
+                        find_only=find_only,
+                        cancel_on_missing=cancel_on_missing)
+
+
+class QemuSystemTest(QemuBaseTest):
+    """Facilitates system emulation tests."""
+
+    def setUp(self):
+        self._vms = {}
+
+        super().setUp('qemu-system-')
+
+    def require_accelerator(self, accelerator):
+        """
+        Requires an accelerator to be available for the test to continue
+
+        It takes into account the currently set qemu binary.
+
+        If the check fails, the test is canceled.  If the check itself
+        for the given accelerator is not available, the test is also
+        canceled.
+
+        :param accelerator: name of the accelerator, such as "kvm" or "tcg"
+        :type accelerator: str
+        """
+        checker = {'tcg': tcg_available,
+                   'kvm': kvm_available}.get(accelerator)
+        if checker is None:
+            self.cancel("Don't know how to check for the presence "
+                        "of accelerator %s" % accelerator)
+        if not checker(qemu_bin=self.qemu_bin):
+            self.cancel("%s accelerator does not seem to be "
+                        "available" % accelerator)
+
+    def require_netdev(self, netdevname):
+        netdevhelp = run_cmd([self.qemu_bin,
+                             '-M', 'none', '-netdev', 'help'])[0];
+        if netdevhelp.find('\n' + netdevname + '\n') < 0:
+            self.cancel('no support for user networking')
+
+    def require_multiprocess(self):
+        """
+        Test for the presence of the x-pci-proxy-dev which is required
+        to support multiprocess.
+        """
+        devhelp = run_cmd([self.qemu_bin,
+                           '-M', 'none', '-device', 'help'])[0];
+        if devhelp.find('x-pci-proxy-dev') < 0:
+            self.cancel('no support for multiprocess device emulation')
+
+    def _new_vm(self, name, *args):
+        self._sd = tempfile.TemporaryDirectory(prefix="qemu_")
+        vm = QEMUMachine(self.qemu_bin, base_temp_dir=self.workdir,
+                         log_dir=self.logdir)
+        self.log.debug('QEMUMachine "%s" created', name)
+        self.log.debug('QEMUMachine "%s" temp_dir: %s', name, vm.temp_dir)
+        self.log.debug('QEMUMachine "%s" log_dir: %s', name, vm.log_dir)
+        if args:
+            vm.add_args(*args)
+        return vm
+
+    def get_qemu_img(self):
+        self.log.debug('Looking for and selecting a qemu-img binary')
+
+        # If qemu-img has been built, use it, otherwise the system wide one
+        # will be used.
+        qemu_img = os.path.join(BUILD_DIR, 'qemu-img')
+        if not os.path.exists(qemu_img):
+            qemu_img = find_command('qemu-img', False)
+        if qemu_img is False:
+            self.cancel('Could not find "qemu-img"')
+
+        return qemu_img
+
+    @property
+    def vm(self):
+        return self.get_vm(name='default')
+
+    def get_vm(self, *args, name=None):
+        if not name:
+            name = str(uuid.uuid4())
+        if self._vms.get(name) is None:
+            self._vms[name] = self._new_vm(name, *args)
+            if self.cpu is not None:
+                self._vms[name].add_args('-cpu', self.cpu)
+            if self.machine is not None:
+                self._vms[name].set_machine(self.machine)
+        return self._vms[name]
+
+    def set_vm_arg(self, arg, value):
+        """
+        Set an argument to list of extra arguments to be given to the QEMU
+        binary. If the argument already exists then its value is replaced.
+
+        :param arg: the QEMU argument, such as "-cpu" in "-cpu host"
+        :type arg: str
+        :param value: the argument value, such as "host" in "-cpu host"
+        :type value: str
+        """
+        if not arg or not value:
+            return
+        if arg not in self.vm.args:
+            self.vm.args.extend([arg, value])
+        else:
+            idx = self.vm.args.index(arg) + 1
+            if idx < len(self.vm.args):
+                self.vm.args[idx] = value
+            else:
+                self.vm.args.append(value)
+
+    def tearDown(self):
+        for vm in self._vms.values():
+            vm.shutdown()
+        self._sd = None
+        super().tearDown()
+
+
+class QemuUserTest(QemuBaseTest):
+    """Facilitates user-mode emulation tests."""
+
+    def setUp(self):
+        self._ldpath = []
+        super().setUp('qemu-')
+
+    def add_ldpath(self, ldpath):
+        self._ldpath.append(os.path.abspath(ldpath))
+
+    def run(self, bin_path, args=[]):
+        qemu_args = " ".join(["-L %s" % ldpath for ldpath in self._ldpath])
+        bin_args = " ".join(args)
+        return process.run("%s %s %s %s" % (self.qemu_bin, qemu_args,
+                                            bin_path, bin_args))
-- 
2.45.2

[RFC PATCH 2/8] tests/pytest: Convert some simple avocado tests into pytests

[Thomas Huth]

These test are rather simple and don't need any modifications apart
from adjusting the "from avocado_qemu" line. These tests can now
be run directly via "pytest" by setting the PYTHONPATH environment
variable to the python folder of QEMU and by providing the QEMU
binary via the PYTEST_QEMU_BINARY environment variable, and the source
and build directories via the PYTEST_SOURCE_ROOTand PYTEST_BUILD_ROOT
environment variables.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/{avocado/cpu_queries.py => pytest/test_cpu_queries.py}   | 2 +-
 .../empty_cpu_model.py => pytest/test_empty_cpu_model.py}      | 2 +-
 .../mem-addr-space-check.py => pytest/test_mem_addr_space.py}  | 3 +--
 .../virtio_version.py => pytest/test_virtio_version.py}        | 2 +-
 4 files changed, 4 insertions(+), 5 deletions(-)
 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/mem-addr-space-check.py => pytest/test_mem_addr_space.py} (99%)
 rename tests/{avocado/virtio_version.py => pytest/test_virtio_version.py} (99%)

diff --git a/tests/avocado/cpu_queries.py b/tests/pytest/test_cpu_queries.py
similarity index 96%
rename from tests/avocado/cpu_queries.py
rename to tests/pytest/test_cpu_queries.py
index d3faa14720..b300447121 100644
--- a/tests/avocado/cpu_queries.py
+++ b/tests/pytest/test_cpu_queries.py
@@ -8,7 +8,7 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later.  See the COPYING file in the top-level directory.
 
-from avocado_qemu import QemuSystemTest
+from qemu_pytest import QemuSystemTest
 
 class QueryCPUModelExpansion(QemuSystemTest):
     """
diff --git a/tests/avocado/empty_cpu_model.py b/tests/pytest/test_empty_cpu_model.py
similarity index 94%
rename from tests/avocado/empty_cpu_model.py
rename to tests/pytest/test_empty_cpu_model.py
index d906ef3d3c..113740bc82 100644
--- a/tests/avocado/empty_cpu_model.py
+++ b/tests/pytest/test_empty_cpu_model.py
@@ -7,7 +7,7 @@
 #
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later.  See the COPYING file in the top-level directory.
-from avocado_qemu import QemuSystemTest
+from qemu_pytest import QemuSystemTest
 
 class EmptyCPUModel(QemuSystemTest):
     def test(self):
diff --git a/tests/avocado/mem-addr-space-check.py b/tests/pytest/test_mem_addr_space.py
similarity index 99%
rename from tests/avocado/mem-addr-space-check.py
rename to tests/pytest/test_mem_addr_space.py
index 85541ea051..6ae7ba5e6b 100644
--- a/tests/avocado/mem-addr-space-check.py
+++ b/tests/pytest/test_mem_addr_space.py
@@ -8,8 +8,7 @@
 #
 # SPDX-License-Identifier: GPL-2.0-or-later
 
-from avocado_qemu import QemuSystemTest
-import signal
+from qemu_pytest import QemuSystemTest
 import time
 
 class MemAddrCheck(QemuSystemTest):
diff --git a/tests/avocado/virtio_version.py b/tests/pytest/test_virtio_version.py
similarity index 99%
rename from tests/avocado/virtio_version.py
rename to tests/pytest/test_virtio_version.py
index afe5e828b5..ca3aa806df 100644
--- a/tests/avocado/virtio_version.py
+++ b/tests/pytest/test_virtio_version.py
@@ -12,7 +12,7 @@
 import os
 
 from qemu.machine import QEMUMachine
-from avocado_qemu import QemuSystemTest
+from qemu_pytest import QemuSystemTest
 
 # Virtio Device IDs:
 VIRTIO_NET = 1
-- 
2.45.2

[RFC PATCH 3/8] tests/pytest: Convert info_usernet and version test with small adjustments

[Thomas Huth]

These two simple tests can be converted to a pytest quite easily,
we just have to set the machine to 'none' now manually since we
don't support the avocado tags here yet.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .../info_usernet.py => pytest/test_info_usernet.py}       | 6 ++----
 tests/{avocado/version.py => pytest/test_version.py}      | 8 +++-----
 2 files changed, 5 insertions(+), 9 deletions(-)
 rename tests/{avocado/info_usernet.py => pytest/test_info_usernet.py} (91%)
 rename tests/{avocado/version.py => pytest/test_version.py} (82%)

diff --git a/tests/avocado/info_usernet.py b/tests/pytest/test_info_usernet.py
similarity index 91%
rename from tests/avocado/info_usernet.py
rename to tests/pytest/test_info_usernet.py
index e1aa7a6e0a..0cc3697c0b 100644
--- a/tests/avocado/info_usernet.py
+++ b/tests/pytest/test_info_usernet.py
@@ -8,18 +8,16 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later.  See the COPYING file in the top-level directory.
 
-from avocado_qemu import QemuSystemTest
+from qemu_pytest import QemuSystemTest
 
 from qemu.utils import get_info_usernet_hostfwd_port
 
 
 class InfoUsernet(QemuSystemTest):
-    """
-    :avocado: tags=machine:none
-    """
 
     def test_hostfwd(self):
         self.require_netdev('user')
+        self.machine = 'none'
         self.vm.add_args('-netdev', 'user,id=vnet,hostfwd=:127.0.0.1:0-:22')
         self.vm.launch()
         res = self.vm.cmd('human-monitor-command',
diff --git a/tests/avocado/version.py b/tests/pytest/test_version.py
similarity index 82%
rename from tests/avocado/version.py
rename to tests/pytest/test_version.py
index c6139568a1..2d16b4075d 100644
--- a/tests/avocado/version.py
+++ b/tests/pytest/test_version.py
@@ -9,15 +9,13 @@
 # later.  See the COPYING file in the top-level directory.
 
 
-from avocado_qemu import QemuSystemTest
+from qemu_pytest import QemuSystemTest
 
 
 class Version(QemuSystemTest):
-    """
-    :avocado: tags=quick
-    :avocado: tags=machine:none
-    """
+
     def test_qmp_human_info_version(self):
+        self.machine = 'none'
         self.vm.add_args('-nodefaults')
         self.vm.launch()
         res = self.vm.cmd('human-monitor-command',
-- 
2.45.2

[RFC PATCH 4/8] tests/pytest: add pytest to the meson build system

[Thomas Huth]

From: Ani Sinha <ani@anisinha.ca>

Integrate the pytest framework with the meson build system. This
will make meson run all the pytests under the pytest directory.

Signed-off-by: Ani Sinha <ani@anisinha.ca>
[thuth: Removed the acpi-bits and adjusted for converted avocado tests instead]
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include   |  4 ++-
 tests/meson.build        |  1 +
 tests/pytest/meson.build | 53 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 1 deletion(-)
 create mode 100644 tests/pytest/meson.build

diff --git a/tests/Makefile.include b/tests/Makefile.include
index d39d5dd6a4..68151717d7 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -3,12 +3,14 @@
 .PHONY: check-help
 check-help:
 	@echo "Regression testing targets:"
-	@echo " $(MAKE) check                  Run block, qapi-schema, unit, softfloat, qtest and decodetree tests"
+	@echo " $(MAKE) check                  Run block, qapi-schema, unit, softfloat, qtest, pytest and decodetree tests"
 	@echo " $(MAKE) bench                  Run speed tests"
 	@echo
 	@echo "Individual test suites:"
 	@echo " $(MAKE) check-qtest-TARGET     Run qtest tests for given target"
 	@echo " $(MAKE) check-qtest            Run qtest tests"
+	@echo " $(MAKE) check-pytest           Run pytest tests"
+	@echo " $(MAKE) check-pytest-TARGET    Run pytest for a given target"
 	@echo " $(MAKE) check-unit             Run qobject tests"
 	@echo " $(MAKE) check-qapi-schema      Run QAPI schema tests"
 	@echo " $(MAKE) check-block            Run block tests"
diff --git a/tests/meson.build b/tests/meson.build
index acb6807094..17510a468e 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -85,3 +85,4 @@ subdir('unit')
 subdir('qapi-schema')
 subdir('qtest')
 subdir('migration')
+subdir('pytest')
diff --git a/tests/pytest/meson.build b/tests/pytest/meson.build
new file mode 100644
index 0000000000..1486628d45
--- /dev/null
+++ b/tests/pytest/meson.build
@@ -0,0 +1,53 @@
+slow_pytests = {
+  'mem_addr_space' : 90,
+}
+
+pytests_generic = [
+  'empty_cpu_model',
+  'info_usernet',
+  'version',
+]
+
+pytests_x86_64 = [
+  'cpu_queries',
+  'mem_addr_space',
+  'virtio_version',
+]
+
+pytest = find_program('pytest', required: false)
+if not pytest.found()
+  message('pytest not available ==> Disabled the qemu-pytests.')
+  subdir_done()
+endif
+
+foreach dir : target_dirs
+  if not dir.endswith('-softmmu')
+    continue
+  endif
+
+  target_base = dir.split('-')[0]
+  pytest_emulator = emulators['qemu-system-' + target_base]
+  target_pytests = get_variable('pytests_' + target_base, []) + pytests_generic
+
+  test_deps = roms
+  pytest_env = environment()
+  if have_tools
+    pytest_env.set('PYTEST_QEMU_IMG', './qemu-img')
+    test_deps += [qemu_img]
+  endif
+  pytest_env.set('PYTEST_QEMU_BINARY', meson.global_build_root() / 'qemu-system-' + target_base)
+  pytest_env.set('PYTEST_SOURCE_ROOT', meson.project_source_root())
+  pytest_env.set('PYTEST_BUILD_ROOT', meson.project_build_root())
+  pytest_env.set('PYTHONPATH', meson.project_source_root() / 'python')
+
+  foreach test : target_pytests
+    test('pytest-@0@/@1@'.format(target_base, test),
+         pytest,
+         depends: [test_deps, pytest_emulator, emulator_modules],
+         env: pytest_env,
+         args: [meson.current_source_dir() / 'test_' + test + '.py'],
+         timeout: slow_pytests.get(test, 60),
+         priority: slow_pytests.get(test, 60),
+         suite: ['pytest', 'pytest-' + target_base])
+  endforeach
+endforeach
-- 
2.45.2

[RFC PATCH 5/8] tests_pytest: Implement fetch_asset() method for downloading assets

[Thomas Huth]

In the pytests, we cannot use the fetch_asset() function from Avocado
anymore, so we have to provide our own implementation now instead.
Thus add such a function based on the _download_with_cache() function
from tests/vm/basevm.py for this purpose.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/pytest/qemu_pytest/__init__.py | 40 ++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/tests/pytest/qemu_pytest/__init__.py b/tests/pytest/qemu_pytest/__init__.py
index e3ed32e3de..73d80b3828 100644
--- a/tests/pytest/qemu_pytest/__init__.py
+++ b/tests/pytest/qemu_pytest/__init__.py
@@ -11,6 +11,7 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later.  See the COPYING file in the top-level directory.
 
+import hashlib
 import logging
 import os
 import shutil
@@ -201,17 +202,34 @@ def setUp(self, bin_prefix):
         self.assertIsNotNone(SOURCE_DIR,'PYTEST_SOURCE_ROOT must be set')
         self.assertIsNotNone(self.qemu_bin, 'PYTEST_QEMU_BINARY must be set')
 
-    def fetch_asset(self, name,
-                    asset_hash, algorithm=None,
-                    locations=None, expire=None,
-                    find_only=False, cancel_on_missing=True):
-        return super().fetch_asset(name,
-                        asset_hash=asset_hash,
-                        algorithm=algorithm,
-                        locations=locations,
-                        expire=expire,
-                        find_only=find_only,
-                        cancel_on_missing=cancel_on_missing)
+    def check_hash(self, file_name, expected_hash):
+        if not expected_hash:
+            return True
+        if len(expected_hash) == 32:
+            sum_prog = 'md5sum'
+        elif len(expected_hash) == 40:
+            sum_prog = 'sha1sum'
+        elif len(expected_hash) == 64:
+            sum_prog = 'sha256sum'
+        elif len(expected_hash) == 128:
+            sum_prog = 'sha512sum'
+        else:
+            raise Exception("unknown hash type")
+        checksum = subprocess.check_output([sum_prog, file_name]).split()[0]
+        return expected_hash == checksum.decode("utf-8")
+
+    def fetch_asset(self, url, asset_hash):
+        cache_dir = os.path.expanduser("~/.cache/qemu/download")
+        if not os.path.exists(cache_dir):
+            os.makedirs(cache_dir)
+        fname = os.path.join(cache_dir,
+                             hashlib.sha1(url.encode("utf-8")).hexdigest())
+        if os.path.exists(fname) and self.check_hash(fname, asset_hash):
+            return fname
+        logging.debug("Downloading %s to %s...", url, fname)
+        subprocess.check_call(["wget", "-c", url, "-O", fname + ".download"])
+        os.rename(fname + ".download", fname)
+        return fname
 
 
 class QemuSystemTest(QemuBaseTest):
-- 
2.45.2

[RFC PATCH 6/8] tests/pytest: Convert some tests that download files via fetch_asset()

[Thomas Huth]

Now that we've got a working fetch_asset() function, we can convert
some Avocado tests that use this function for downloading their
required files.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/pytest/meson.build                      | 16 +++++++++++++++
 .../test_machine_arm_n8x0.py}                 | 20 +++++++------------
 .../test_machine_avr6.py}                     |  7 ++-----
 .../test_machine_loongarch.py}                | 11 ++++------
 .../test_machine_mips_loongson3v.py}          | 19 ++++++------------
 5 files changed, 35 insertions(+), 38 deletions(-)
 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%)

diff --git a/tests/pytest/meson.build b/tests/pytest/meson.build
index 1486628d45..d3607db362 100644
--- a/tests/pytest/meson.build
+++ b/tests/pytest/meson.build
@@ -8,6 +8,22 @@ pytests_generic = [
   'version',
 ]
 
+pytests_arm = [
+  'machine_arm_n8x0',
+]
+
+pytests_avr = [
+  'machine_avr6',
+]
+
+pytests_loongarch64 = [
+  'machine_loongarch',
+]
+
+pytests_mips64el = [
+  'machine_mips_loongson3v',
+]
+
 pytests_x86_64 = [
   'cpu_queries',
   'mem_addr_space',
diff --git a/tests/avocado/machine_arm_n8x0.py b/tests/pytest/test_machine_arm_n8x0.py
similarity index 71%
rename from tests/avocado/machine_arm_n8x0.py
rename to tests/pytest/test_machine_arm_n8x0.py
index 12e9a6803b..082284b975 100644
--- a/tests/avocado/machine_arm_n8x0.py
+++ b/tests/pytest/test_machine_arm_n8x0.py
@@ -10,9 +10,9 @@
 
 import os
 
-from avocado import skipUnless
-from avocado_qemu import QemuSystemTest
-from avocado_qemu import wait_for_console_pattern
+from unittest import skipUnless
+from qemu_pytest import QemuSystemTest
+from qemu_pytest import wait_for_console_pattern
 
 class N8x0Machine(QemuSystemTest):
     """Boots the Linux kernel and checks that the console is operational"""
@@ -32,18 +32,12 @@ def __do_test_n8x0(self):
         self.vm.launch()
         wait_for_console_pattern(self, 'TSC2005 driver initializing')
 
-    @skipUnless(os.getenv('AVOCADO_ALLOW_UNTRUSTED_CODE'), 'untrusted code')
+    @skipUnless(os.getenv('QEMU_TEST_ALLOW_UNTRUSTED_CODE'), 'untrusted code')
     def test_n800(self):
-        """
-        :avocado: tags=arch:arm
-        :avocado: tags=machine:n800
-        """
+        self.machine = 'n800'
         self.__do_test_n8x0()
 
-    @skipUnless(os.getenv('AVOCADO_ALLOW_UNTRUSTED_CODE'), 'untrusted code')
+    @skipUnless(os.getenv('QEMU_TEST_ALLOW_UNTRUSTED_CODE'), 'untrusted code')
     def test_n810(self):
-        """
-        :avocado: tags=arch:arm
-        :avocado: tags=machine:n810
-        """
+        self.machine = 'n810'
         self.__do_test_n8x0()
diff --git a/tests/avocado/machine_avr6.py b/tests/pytest/test_machine_avr6.py
similarity index 91%
rename from tests/avocado/machine_avr6.py
rename to tests/pytest/test_machine_avr6.py
index 5485db79c6..479038a221 100644
--- a/tests/avocado/machine_avr6.py
+++ b/tests/pytest/test_machine_avr6.py
@@ -19,16 +19,12 @@
 
 import time
 
-from avocado_qemu import QemuSystemTest
+from qemu_pytest import QemuSystemTest
 
 class AVR6Machine(QemuSystemTest):
     timeout = 5
 
     def test_freertos(self):
-        """
-        :avocado: tags=arch:avr
-        :avocado: tags=machine:arduino-mega-2560-v3
-        """
         """
         https://github.com/seharris/qemu-avr-tests/raw/master/free-rtos/Demo/AVR_ATMega2560_GCC/demo.elf
         constantly prints out 'ABCDEFGHIJKLMNOPQRSTUVWXABCDEFGHIJKLMNOPQRSTUVWX'
@@ -39,6 +35,7 @@ def test_freertos(self):
         rom_hash = '7eb521f511ca8f2622e0a3c5e8dd686efbb911d4'
         rom_path = self.fetch_asset(rom_url, asset_hash=rom_hash)
 
+        self.machine = 'arduino-mega-2560-v3'
         self.vm.add_args('-bios', rom_path)
         self.vm.add_args('-nographic')
         self.vm.launch()
diff --git a/tests/avocado/machine_loongarch.py b/tests/pytest/test_machine_loongarch.py
similarity index 89%
rename from tests/avocado/machine_loongarch.py
rename to tests/pytest/test_machine_loongarch.py
index 8de308f2d6..3eb68745ac 100644
--- a/tests/avocado/machine_loongarch.py
+++ b/tests/pytest/test_machine_loongarch.py
@@ -5,9 +5,9 @@
 # Copyright (c) 2023 Loongson Technology Corporation Limited
 #
 
-from avocado_qemu import QemuSystemTest
-from avocado_qemu import exec_command_and_wait_for_pattern
-from avocado_qemu import wait_for_console_pattern
+from qemu_pytest import QemuSystemTest
+from qemu_pytest import exec_command_and_wait_for_pattern
+from qemu_pytest import wait_for_console_pattern
 
 class LoongArchMachine(QemuSystemTest):
     KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
@@ -21,10 +21,7 @@ def wait_for_console_pattern(self, success_message, vm=None):
 
     def test_loongarch64_devices(self):
 
-        """
-        :avocado: tags=arch:loongarch64
-        :avocado: tags=machine:virt
-        """
+        self.machine = 'virt'
 
         kernel_url = ('https://github.com/yangxiaojuan-loongson/qemu-binary/'
                       'releases/download/2024-05-30/vmlinuz.efi')
diff --git a/tests/avocado/machine_mips_loongson3v.py b/tests/pytest/test_machine_mips_loongson3v.py
similarity index 59%
rename from tests/avocado/machine_mips_loongson3v.py
rename to tests/pytest/test_machine_mips_loongson3v.py
index 5194cf18c9..189b22c04e 100644
--- a/tests/avocado/machine_mips_loongson3v.py
+++ b/tests/pytest/test_machine_mips_loongson3v.py
@@ -10,28 +10,21 @@
 import os
 import time
 
-from avocado import skipUnless
-from avocado_qemu import QemuSystemTest
-from avocado_qemu import wait_for_console_pattern
+from unittest import skipUnless
+from qemu_pytest import QemuSystemTest
+from qemu_pytest import wait_for_console_pattern
 
 class MipsLoongson3v(QemuSystemTest):
     timeout = 60
 
-    @skipUnless(os.getenv('AVOCADO_ALLOW_UNTRUSTED_CODE'), 'untrusted code')
+    @skipUnless(os.getenv('QEMU_TEST_ALLOW_UNTRUSTED_CODE'), 'untrusted code')
     def test_pmon_serial_console(self):
-        """
-        :avocado: tags=arch:mips64el
-        :avocado: tags=endian:little
-        :avocado: tags=machine:loongson3-virt
-        :avocado: tags=cpu:Loongson-3A1000
-        :avocado: tags=device:liointc
-        :avocado: tags=device:goldfish_rtc
-        """
+        self.machine = 'loongson3-virt'
 
         pmon_hash = '7c8b45dd81ccfc55ff28f5aa267a41c3'
         pmon_path = self.fetch_asset('https://github.com/loongson-community/pmon/'
                                     'releases/download/20210112/pmon-3avirt.bin',
-                                     asset_hash=pmon_hash, algorithm='md5')
+                                     asset_hash=pmon_hash)
 
         self.vm.set_console()
         self.vm.add_args('-bios', pmon_path)
-- 
2.45.2

[RFC PATCH 7/8] tests/pytest: Add a function for extracting files from an archive

[Thomas Huth]

Some Avocado-based tests use the "archive" module from avocado.utils
to extract files from an archive. To be able to use these tests
without Avocado, we have to provide our own function for extracting
files. Fortunately, there is already the tarfile module that will
provide us with this functionality, so let's just add a nice wrapper
function around that.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/pytest/qemu_pytest/utils.py | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 tests/pytest/qemu_pytest/utils.py

diff --git a/tests/pytest/qemu_pytest/utils.py b/tests/pytest/qemu_pytest/utils.py
new file mode 100644
index 0000000000..4eb5e5d5e5
--- /dev/null
+++ b/tests/pytest/qemu_pytest/utils.py
@@ -0,0 +1,21 @@
+# Utilities for python-based QEMU tests
+#
+# Copyright 2024 Red Hat, Inc.
+#
+# Authors:
+#  Thomas Huth <thuth@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+import tarfile
+
+def archive_extract(archive, dest_dir, member=None):
+    with tarfile.open(archive) as tf:
+        if hasattr(tarfile, 'data_filter'):
+            tf.extraction_filter = getattr(tarfile, 'data_filter',
+                                           (lambda member, path: member))
+        if member:
+            tf.extract(member=member, path=dest_dir)
+        else:
+            tf.extractall(path=dest_dir)
-- 
2.45.2

[RFC PATCH 8/8] tests/pytest: Convert avocado test that needed avocado.utils.archive

[Thomas Huth]

Instead of using the "archive" module from avocado.utils, switch
these tests to use the new wrapper function that is based on the
"tarfile" module instead.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/pytest/meson.build                       |  5 +++++
 .../test_arm_canona1100.py}                    | 16 +++++++---------
 .../test_ppc_bamboo.py}                        | 18 ++++++------------
 3 files changed, 18 insertions(+), 21 deletions(-)
 rename tests/{avocado/machine_arm_canona1100.py => pytest/test_arm_canona1100.py} (74%)
 rename tests/{avocado/ppc_bamboo.py => pytest/test_ppc_bamboo.py} (75%)

diff --git a/tests/pytest/meson.build b/tests/pytest/meson.build
index d3607db362..baaacf00cc 100644
--- a/tests/pytest/meson.build
+++ b/tests/pytest/meson.build
@@ -9,6 +9,7 @@ pytests_generic = [
 ]
 
 pytests_arm = [
+  'arm_canona1100',
   'machine_arm_n8x0',
 ]
 
@@ -24,6 +25,10 @@ pytests_mips64el = [
   'machine_mips_loongson3v',
 ]
 
+pytests_ppc = [
+  'ppc_bamboo',
+]
+
 pytests_x86_64 = [
   'cpu_queries',
   'mem_addr_space',
diff --git a/tests/avocado/machine_arm_canona1100.py b/tests/pytest/test_arm_canona1100.py
similarity index 74%
rename from tests/avocado/machine_arm_canona1100.py
rename to tests/pytest/test_arm_canona1100.py
index a42d8b0f2b..296df41a06 100644
--- a/tests/avocado/machine_arm_canona1100.py
+++ b/tests/pytest/test_arm_canona1100.py
@@ -8,9 +8,9 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later.  See the COPYING file in the top-level directory.
 
-from avocado_qemu import QemuSystemTest
-from avocado_qemu import wait_for_console_pattern
-from avocado.utils import archive
+from qemu_pytest import QemuSystemTest
+from qemu_pytest import wait_for_console_pattern
+from qemu_pytest.utils import archive_extract
 
 class CanonA1100Machine(QemuSystemTest):
     """Boots the barebox firmware and checks that the console is operational"""
@@ -18,16 +18,14 @@ class CanonA1100Machine(QemuSystemTest):
     timeout = 90
 
     def test_arm_canona1100(self):
-        """
-        :avocado: tags=arch:arm
-        :avocado: tags=machine:canon-a1100
-        :avocado: tags=device:pflash_cfi02
-        """
+        self.machine = 'canon-a1100'
+
         tar_url = ('https://qemu-advcal.gitlab.io'
                    '/qac-best-of-multiarch/download/day18.tar.xz')
         tar_hash = '068b5fc4242b29381acee94713509f8a876e9db6'
         file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
-        archive.extract(file_path, self.workdir)
+        archive_extract(file_path, dest_dir=self.workdir,
+                        member="day18/barebox.canon-a1100.bin")
         self.vm.set_console()
         self.vm.add_args('-bios',
                          self.workdir + '/day18/barebox.canon-a1100.bin')
diff --git a/tests/avocado/ppc_bamboo.py b/tests/pytest/test_ppc_bamboo.py
similarity index 75%
rename from tests/avocado/ppc_bamboo.py
rename to tests/pytest/test_ppc_bamboo.py
index a81be3d608..4964dedbfe 100644
--- a/tests/avocado/ppc_bamboo.py
+++ b/tests/pytest/test_ppc_bamboo.py
@@ -5,30 +5,24 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later.  See the COPYING file in the top-level directory.
 
-from avocado.utils import archive
-from avocado_qemu import QemuSystemTest
-from avocado_qemu import wait_for_console_pattern
-from avocado_qemu import exec_command_and_wait_for_pattern
+from qemu_pytest.utils import archive_extract
+from qemu_pytest import QemuSystemTest
+from qemu_pytest import wait_for_console_pattern
+from qemu_pytest import exec_command_and_wait_for_pattern
 
 class BambooMachine(QemuSystemTest):
 
     timeout = 90
 
     def test_ppc_bamboo(self):
-        """
-        :avocado: tags=arch:ppc
-        :avocado: tags=machine:bamboo
-        :avocado: tags=cpu:440epb
-        :avocado: tags=device:rtl8139
-        :avocado: tags=accel:tcg
-        """
+        self.machine = 'bamboo'
         self.require_accelerator("tcg")
         self.require_netdev('user')
         tar_url = ('http://landley.net/aboriginal/downloads/binaries/'
                    'system-image-powerpc-440fp.tar.gz')
         tar_hash = '53e5f16414b195b82d2c70272f81c2eedb39bad9'
         file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
-        archive.extract(file_path, self.workdir)
+        archive_extract(file_path, self.workdir)
         self.vm.set_console()
         self.vm.add_args('-kernel', self.workdir +
                                    '/system-image-powerpc-440fp/linux',
-- 
2.45.2

Re: [RFC PATCH 0/8] Convert avocado tests to normal Python unittests

[Daniel P. Berrangé]

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
-- 
|: 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 :|

Re: [RFC PATCH 0/8] Convert avocado tests to normal Python unittests

[Fabiano Rosas]

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?

Re: [RFC PATCH 5/8] tests_pytest: Implement fetch_asset() method for downloading assets

[Richard Henderson]

On 7/11/24 04:55, Thomas Huth wrote:
> +    def fetch_asset(self, url, asset_hash):
> +        cache_dir = os.path.expanduser("~/.cache/qemu/download")
> +        if not os.path.exists(cache_dir):
> +            os.makedirs(cache_dir)
> +        fname = os.path.join(cache_dir,
> +                             hashlib.sha1(url.encode("utf-8")).hexdigest())
> +        if os.path.exists(fname) and self.check_hash(fname, asset_hash):
> +            return fname
> +        logging.debug("Downloading %s to %s...", url, fname)
> +        subprocess.check_call(["wget", "-c", url, "-O", fname + ".download"])
> +        os.rename(fname + ".download", fname)
> +        return fname

Download failure via exception?
Check hash on downloaded asset?


r~

Re: [RFC PATCH 0/8] Convert avocado tests to normal Python unittests

[Thomas Huth]

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

Re: [RFC PATCH 5/8] tests_pytest: Implement fetch_asset() method for downloading assets

[Richard Henderson]

On 7/11/24 09:45, Richard Henderson wrote:
> On 7/11/24 04:55, Thomas Huth wrote:
>> +    def fetch_asset(self, url, asset_hash):
>> +        cache_dir = os.path.expanduser("~/.cache/qemu/download")
>> +        if not os.path.exists(cache_dir):
>> +            os.makedirs(cache_dir)
>> +        fname = os.path.join(cache_dir,
>> +                             hashlib.sha1(url.encode("utf-8")).hexdigest())
>> +        if os.path.exists(fname) and self.check_hash(fname, asset_hash):
>> +            return fname
>> +        logging.debug("Downloading %s to %s...", url, fname)
>> +        subprocess.check_call(["wget", "-c", url, "-O", fname + ".download"])
>> +        os.rename(fname + ".download", fname)
>> +        return fname
> 
> Download failure via exception?
> Check hash on downloaded asset?

I would prefer to see assets, particularly downloading, handled in a separate pass from tests.

(1) Asset download should not count against test timeout.
(2) Running tests while disconnected should skip unavailable assets.

Avocado kinda does this, but still generates errors instead of skips.


r~

Re: [RFC PATCH 5/8] tests_pytest: Implement fetch_asset() method for downloading assets

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 7/11/24 09:45, Richard Henderson wrote:
>> On 7/11/24 04:55, Thomas Huth wrote:
>>> +    def fetch_asset(self, url, asset_hash):
>>> +        cache_dir = os.path.expanduser("~/.cache/qemu/download")
>>> +        if not os.path.exists(cache_dir):
>>> +            os.makedirs(cache_dir)
>>> +        fname = os.path.join(cache_dir,
>>> +                             hashlib.sha1(url.encode("utf-8")).hexdigest())
>>> +        if os.path.exists(fname) and self.check_hash(fname, asset_hash):
>>> +            return fname
>>> +        logging.debug("Downloading %s to %s...", url, fname)
>>> +        subprocess.check_call(["wget", "-c", url, "-O", fname + ".download"])
>>> +        os.rename(fname + ".download", fname)
>>> +        return fname
>> Download failure via exception?
>> Check hash on downloaded asset?
>
> I would prefer to see assets, particularly downloading, handled in a
> separate pass from tests.

And I assume cachable?

>
> (1) Asset download should not count against test timeout.
> (2) Running tests while disconnected should skip unavailable assets.
>
> Avocado kinda does this, but still generates errors instead of skips.
>
>
> r~

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [RFC PATCH 5/8] tests_pytest: Implement fetch_asset() method for downloading assets

[Richard Henderson]

On 7/11/24 12:23, Alex Bennée wrote:
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> On 7/11/24 09:45, Richard Henderson wrote:
>>> On 7/11/24 04:55, Thomas Huth wrote:
>>>> +    def fetch_asset(self, url, asset_hash):
>>>> +        cache_dir = os.path.expanduser("~/.cache/qemu/download")
>>>> +        if not os.path.exists(cache_dir):
>>>> +            os.makedirs(cache_dir)
>>>> +        fname = os.path.join(cache_dir,
>>>> +                             hashlib.sha1(url.encode("utf-8")).hexdigest())
>>>> +        if os.path.exists(fname) and self.check_hash(fname, asset_hash):
>>>> +            return fname
>>>> +        logging.debug("Downloading %s to %s...", url, fname)
>>>> +        subprocess.check_call(["wget", "-c", url, "-O", fname + ".download"])
>>>> +        os.rename(fname + ".download", fname)
>>>> +        return fname
>>> Download failure via exception?
>>> Check hash on downloaded asset?
>>
>> I would prefer to see assets, particularly downloading, handled in a
>> separate pass from tests.
> 
> And I assume cachable?

The cache is already handled here.  But downloading after cache miss is non-optional, may 
not fail, and is accounted against the meson test timeout.


r~

Re: [RFC PATCH 5/8] tests_pytest: Implement fetch_asset() method for downloading assets

[Thomas Huth]

On 11/07/2024 18.45, Richard Henderson wrote:
> On 7/11/24 04:55, Thomas Huth wrote:
>> +    def fetch_asset(self, url, asset_hash):
>> +        cache_dir = os.path.expanduser("~/.cache/qemu/download")
>> +        if not os.path.exists(cache_dir):
>> +            os.makedirs(cache_dir)
>> +        fname = os.path.join(cache_dir,
>> +                             hashlib.sha1(url.encode("utf-8")).hexdigest())
>> +        if os.path.exists(fname) and self.check_hash(fname, asset_hash):
>> +            return fname
>> +        logging.debug("Downloading %s to %s...", url, fname)
>> +        subprocess.check_call(["wget", "-c", url, "-O", fname + 
>> ".download"])
>> +        os.rename(fname + ".download", fname)
>> +        return fname
> 
> Download failure via exception?
> Check hash on downloaded asset?

Yes, you're right, I'll add that. But that means it's also missing from 
_download_with_cache in tests/vm/basevm.py so far...

  Thomas

Re: [RFC PATCH 5/8] tests_pytest: Implement fetch_asset() method for downloading assets

[Thomas Huth]

On 11/07/2024 20.49, Richard Henderson wrote:
> On 7/11/24 09:45, Richard Henderson wrote:
>> On 7/11/24 04:55, Thomas Huth wrote:
>>> +    def fetch_asset(self, url, asset_hash):
>>> +        cache_dir = os.path.expanduser("~/.cache/qemu/download")
>>> +        if not os.path.exists(cache_dir):
>>> +            os.makedirs(cache_dir)
>>> +        fname = os.path.join(cache_dir,
>>> +                             hashlib.sha1(url.encode("utf-8")).hexdigest())
>>> +        if os.path.exists(fname) and self.check_hash(fname, asset_hash):
>>> +            return fname
>>> +        logging.debug("Downloading %s to %s...", url, fname)
>>> +        subprocess.check_call(["wget", "-c", url, "-O", fname + 
>>> ".download"])
>>> +        os.rename(fname + ".download", fname)
>>> +        return fname
>>
>> Download failure via exception?
>> Check hash on downloaded asset?
> 
> I would prefer to see assets, particularly downloading, handled in a 
> separate pass from tests.
> 
> (1) Asset download should not count against test timeout.
> (2) Running tests while disconnected should skip unavailable assets.

Sounds good, but honestly, that's above my primitive python-fu. I guess that 
would need some kind of introspection of the tests to retrieve the 
information what they want to download?

  Thomas

Re: [RFC PATCH 5/8] tests_pytest: Implement fetch_asset() method for downloading assets

[Thomas Huth]

On 11/07/2024 23.35, Richard Henderson wrote:
> On 7/11/24 12:23, Alex Bennée wrote:
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>>> On 7/11/24 09:45, Richard Henderson wrote:
>>>> On 7/11/24 04:55, Thomas Huth wrote:
>>>>> +    def fetch_asset(self, url, asset_hash):
>>>>> +        cache_dir = os.path.expanduser("~/.cache/qemu/download")
>>>>> +        if not os.path.exists(cache_dir):
>>>>> +            os.makedirs(cache_dir)
>>>>> +        fname = os.path.join(cache_dir,
>>>>> +                             
>>>>> hashlib.sha1(url.encode("utf-8")).hexdigest())
>>>>> +        if os.path.exists(fname) and self.check_hash(fname, asset_hash):
>>>>> +            return fname
>>>>> +        logging.debug("Downloading %s to %s...", url, fname)
>>>>> +        subprocess.check_call(["wget", "-c", url, "-O", fname + 
>>>>> ".download"])
>>>>> +        os.rename(fname + ".download", fname)
>>>>> +        return fname
>>>> Download failure via exception?
>>>> Check hash on downloaded asset?
>>>
>>> I would prefer to see assets, particularly downloading, handled in a
>>> separate pass from tests.
>>
>> And I assume cachable?
> 
> The cache is already handled here.  But downloading after cache miss is 
> non-optional, may not fail, and is accounted against the meson test timeout.

Unless someone has a really good idea how to implement that download before 
running the tests, I think we can start by simply giving enough headroom in 
the initial timeout settings. We can then hopefully easily fine-tune later - 
since this time the framework is under our control, so we don't have to sync 
in a cumbersome way with the avocado test runner any more.

  Thomas

Re: [RFC PATCH 0/8] Convert avocado tests to normal Python unittests

[Daniel P. Berrangé]

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
-- 
|: 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 :|

Re: [RFC PATCH 1/8] tests/pytest: Add base classes for the upcoming pytest-based tests

[Daniel P. Berrangé]

On Thu, Jul 11, 2024 at 01:55:39PM +0200, Thomas Huth wrote:
> The file is a copy of the tests/avocado/avocado_qemu/__init__.py file
> with some adjustments to get rid of the Avocado dependencies (i.e.
> we also have to drop the LinuxSSHMixIn and LinuxTest for now).
> 
> The emulator binary, source and build directory are now passed via
> environment variables that will be set via meson.build later.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/pytest/qemu_pytest/__init__.py | 344 +++++++++++++++++++++++++++
>  1 file changed, 344 insertions(+)
>  create mode 100644 tests/pytest/qemu_pytest/__init__.py
> 
> diff --git a/tests/pytest/qemu_pytest/__init__.py b/tests/pytest/qemu_pytest/__init__.py
> new file mode 100644
> index 0000000000..e3ed32e3de
> --- /dev/null
> +++ b/tests/pytest/qemu_pytest/__init__.py
> @@ -0,0 +1,344 @@
> +# Test class and utilities for functional tests
> +#
> +# Copyright 2018, 2024 Red Hat, Inc.
> +#
> +# Original Author (Avocado-based tests):
> +#  Cleber Rosa <crosa@redhat.com>
> +#
> +# Adaption for pytest based version:
> +#  Thomas Huth <thuth@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or
> +# later.  See the COPYING file in the top-level directory.
> +
> +import logging
> +import os
> +import shutil
> +import subprocess
> +import sys
> +import tempfile
> +import time
> +import uuid
> +import unittest
> +
> +from qemu.machine import QEMUMachine
> +from qemu.utils import (get_info_usernet_hostfwd_port, kvm_available,
> +                        tcg_available)
> +
> +BUILD_DIR = os.getenv('PYTEST_BUILD_ROOT')
> +SOURCE_DIR = os.getenv('PYTEST_SOURCE_ROOT')

We can make life slightly nicer for developers running tests directly
without meson, by figuring this out automatically if the env vars are
omitted. To enable devs to do

  PYTEST_QEMU_BINARY=./build/qemu-system-aarch64 \
  PYTHONPATH=./python \
  pytest build/tests/pytest/test_info_usernet.py

I propose the following additional logic on top of your patch:

diff --git a/tests/pytest/qemu_pytest/__init__.py b/tests/pytest/qemu_pytest/__init__.py
index 73d80b3828..711cb06012 100644
--- a/tests/pytest/qemu_pytest/__init__.py
+++ b/tests/pytest/qemu_pytest/__init__.py
@@ -21,13 +21,41 @@
 import time
 import uuid
 import unittest
+from pathlib import Path
 
 from qemu.machine import QEMUMachine
 from qemu.utils import (get_info_usernet_hostfwd_port, kvm_available,
                         tcg_available)
 
-BUILD_DIR = os.getenv('PYTEST_BUILD_ROOT')
-SOURCE_DIR = os.getenv('PYTEST_SOURCE_ROOT')
+def _pytest_dir():
+    if sys.argv[0].startswith("pytest") or "bin/pytest" in sys.argv[0]:
+        if sys.argv[1].endswith(".py"):
+            # Assume 'pytest ./build/tests/pytest/test_blah.py ....'
+            return Path(sys.argv[1]).absolute().parent
+        else:
+            # Assume 'pytest ./build/tests/pytest'
+            return Path(sys.argv[1]).absolute()
+
+    # Assume './build/tests/pytest/test_NAME.py'
+    if sys.argv[0].endswith(".py"):
+        return Path(sys.argv[0]).absolute().parent
+
+    raise Exception("Cannot identify pytest build dir, set PYTEST_BUILD_ROOT")
+
+def _build_dir():
+    root = os.getenv('PYTEST_BUILD_ROOT')
+    if root is not None:
+        return Path(root)
+    
+    return _pytest_dir().parent.parent
+
+def _source_dir():
+    root = os.getenv('PYTEST_SOURCE_ROOT')
+    if root is not None:
+        return Path(root)
+
+    # Assume build/tests/pytest is a symlink to the source root
+    return _pytest_dir().resolve().parent.parent
 
 def has_cmd(name, args=None):
     """
@@ -189,8 +217,8 @@ class QemuBaseTest(unittest.TestCase):
 
     qemu_bin = os.getenv('PYTEST_QEMU_BINARY')
 
-    workdir = os.path.join(BUILD_DIR, 'tests/pytest')
-    logdir = os.path.join(BUILD_DIR, 'tests/pytest')
+    workdir = str(Path(_build_dir(), 'tests', 'pytest'))
+    logdir = str(Path(_build_dir(), 'tests', 'pytest'))
 
     cpu = None
     machine = None
@@ -198,8 +226,6 @@ class QemuBaseTest(unittest.TestCase):
     log = logging.getLogger('qemu-pytest')
 
     def setUp(self, bin_prefix):
-        self.assertIsNotNone(BUILD_DIR, 'PYTEST_BUILD_ROOT must be set')
-        self.assertIsNotNone(SOURCE_DIR,'PYTEST_SOURCE_ROOT must be set')
         self.assertIsNotNone(self.qemu_bin, 'PYTEST_QEMU_BINARY must be set')
 
     def check_hash(self, file_name, expected_hash):
@@ -294,9 +320,11 @@ def get_qemu_img(self):
 
         # If qemu-img has been built, use it, otherwise the system wide one
         # will be used.
-        qemu_img = os.path.join(BUILD_DIR, 'qemu-img')
-        if not os.path.exists(qemu_img):
-            qemu_img = find_command('qemu-img', False)
+        qemu_img = Path(_build_dir(), 'qemu-img')
+        if qemu_img.exists():
+            return str(qemu_img)
+
+        qemu_img = find_command('qemu-img', False)
         if qemu_img is False:
             self.cancel('Could not find "qemu-img"')
 

This also allows for executing the tests directly without even involving
pytest if we make one slight further change to each individual tests:

diff --git a/tests/pytest/test_info_usernet.py b/tests/pytest/test_info_usernet.py
index 0cc3697c0b..aa7abb6a5e 100644
--- a/tests/pytest/test_info_usernet.py
+++ b/tests/pytest/test_info_usernet.py
@@ -8,6 +8,8 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later.  See the COPYING file in the top-level directory.
 
+import unittest
+
 from qemu_pytest import QemuSystemTest
 
 from qemu.utils import get_info_usernet_hostfwd_port
@@ -29,3 +31,6 @@ def test_hostfwd(self):
         self.assertGreater(port, 0,
                            ('Found a redirected port that is not greater than'
                             ' zero'))
+
+if __name__ == '__main__':
+    unittest.main()


and chmod +x tests/pytest/*.py, then that lets us simply run

  PYTEST_QEMU_BINARY=./build/qemu-system-aarch64 \
  PYTHONPATH=./python \
  build/tests/pytest/test_info_usernet.py 

being able to run with zero harness is nice for debugging problems,
especially if you want to be strace'ing tests without the harness
in the middle, which is something I've often wanted.

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 :|

Re: [RFC PATCH 2/8] tests/pytest: Convert some simple avocado tests into pytests

[Daniel P. Berrangé]

On Thu, Jul 11, 2024 at 01:55:40PM +0200, Thomas Huth wrote:
> These test are rather simple and don't need any modifications apart
> from adjusting the "from avocado_qemu" line. These tests can now
> be run directly via "pytest" by setting the PYTHONPATH environment
> variable to the python folder of QEMU and by providing the QEMU
> binary via the PYTEST_QEMU_BINARY environment variable, and the source
> and build directories via the PYTEST_SOURCE_ROOTand PYTEST_BUILD_ROOT
> environment variables.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/{avocado/cpu_queries.py => pytest/test_cpu_queries.py}   | 2 +-
>  .../empty_cpu_model.py => pytest/test_empty_cpu_model.py}      | 2 +-
>  .../mem-addr-space-check.py => pytest/test_mem_addr_space.py}  | 3 +--
>  .../virtio_version.py => pytest/test_virtio_version.py}        | 2 +-
>  4 files changed, 4 insertions(+), 5 deletions(-)
>  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/mem-addr-space-check.py => pytest/test_mem_addr_space.py} (99%)
>  rename tests/{avocado/virtio_version.py => pytest/test_virtio_version.py} (99%)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

though if you take my suggestion in the previous patch, then this
patch should 'chmod +x' all the test files, and add the __main__
magic to call 'unittest.main().

> 
> diff --git a/tests/avocado/cpu_queries.py b/tests/pytest/test_cpu_queries.py
> similarity index 96%
> rename from tests/avocado/cpu_queries.py
> rename to tests/pytest/test_cpu_queries.py
> index d3faa14720..b300447121 100644
> --- a/tests/avocado/cpu_queries.py
> +++ b/tests/pytest/test_cpu_queries.py
> @@ -8,7 +8,7 @@
>  # This work is licensed under the terms of the GNU GPL, version 2 or
>  # later.  See the COPYING file in the top-level directory.
>  
> -from avocado_qemu import QemuSystemTest
> +from qemu_pytest import QemuSystemTest
>  
>  class QueryCPUModelExpansion(QemuSystemTest):
>      """
> diff --git a/tests/avocado/empty_cpu_model.py b/tests/pytest/test_empty_cpu_model.py
> similarity index 94%
> rename from tests/avocado/empty_cpu_model.py
> rename to tests/pytest/test_empty_cpu_model.py
> index d906ef3d3c..113740bc82 100644
> --- a/tests/avocado/empty_cpu_model.py
> +++ b/tests/pytest/test_empty_cpu_model.py
> @@ -7,7 +7,7 @@
>  #
>  # This work is licensed under the terms of the GNU GPL, version 2 or
>  # later.  See the COPYING file in the top-level directory.
> -from avocado_qemu import QemuSystemTest
> +from qemu_pytest import QemuSystemTest
>  
>  class EmptyCPUModel(QemuSystemTest):
>      def test(self):
> diff --git a/tests/avocado/mem-addr-space-check.py b/tests/pytest/test_mem_addr_space.py
> similarity index 99%
> rename from tests/avocado/mem-addr-space-check.py
> rename to tests/pytest/test_mem_addr_space.py
> index 85541ea051..6ae7ba5e6b 100644
> --- a/tests/avocado/mem-addr-space-check.py
> +++ b/tests/pytest/test_mem_addr_space.py
> @@ -8,8 +8,7 @@
>  #
>  # SPDX-License-Identifier: GPL-2.0-or-later
>  
> -from avocado_qemu import QemuSystemTest
> -import signal
> +from qemu_pytest import QemuSystemTest
>  import time
>  
>  class MemAddrCheck(QemuSystemTest):
> diff --git a/tests/avocado/virtio_version.py b/tests/pytest/test_virtio_version.py
> similarity index 99%
> rename from tests/avocado/virtio_version.py
> rename to tests/pytest/test_virtio_version.py
> index afe5e828b5..ca3aa806df 100644
> --- a/tests/avocado/virtio_version.py
> +++ b/tests/pytest/test_virtio_version.py
> @@ -12,7 +12,7 @@
>  import os
>  
>  from qemu.machine import QEMUMachine
> -from avocado_qemu import QemuSystemTest
> +from qemu_pytest import QemuSystemTest
>  
>  # Virtio Device IDs:
>  VIRTIO_NET = 1
> -- 
> 2.45.2
> 

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 :|

Re: [RFC PATCH 3/8] tests/pytest: Convert info_usernet and version test with small adjustments

[Daniel P. Berrangé]

On Thu, Jul 11, 2024 at 01:55:41PM +0200, Thomas Huth wrote:
> These two simple tests can be converted to a pytest quite easily,
> we just have to set the machine to 'none' now manually since we
> don't support the avocado tags here yet.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  .../info_usernet.py => pytest/test_info_usernet.py}       | 6 ++----
>  tests/{avocado/version.py => pytest/test_version.py}      | 8 +++-----
>  2 files changed, 5 insertions(+), 9 deletions(-)
>  rename tests/{avocado/info_usernet.py => pytest/test_info_usernet.py} (91%)
>  rename tests/{avocado/version.py => pytest/test_version.py} (82%)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


> diff --git a/tests/avocado/version.py b/tests/pytest/test_version.py
> similarity index 82%
> rename from tests/avocado/version.py
> rename to tests/pytest/test_version.py
> index c6139568a1..2d16b4075d 100644
> --- a/tests/avocado/version.py
> +++ b/tests/pytest/test_version.py
> @@ -9,15 +9,13 @@
>  # later.  See the COPYING file in the top-level directory.
>  
>  
> -from avocado_qemu import QemuSystemTest
> +from qemu_pytest import QemuSystemTest
>  
>  
>  class Version(QemuSystemTest):
> -    """
> -    :avocado: tags=quick

I was going to suggest we expose 'quick' in the meson.build a suite,
but it seems we've barely used this tag, only 2 other examples, so
its pointless.

> -    :avocado: tags=machine:none
> -    """
> +
>      def test_qmp_human_info_version(self):
> +        self.machine = 'none'
>          self.vm.add_args('-nodefaults')
>          self.vm.launch()
>          res = self.vm.cmd('human-monitor-command',
> -- 
> 2.45.2
> 

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 :|

Re: [RFC PATCH 4/8] tests/pytest: add pytest to the meson build system

[Daniel P. Berrangé]

On Thu, Jul 11, 2024 at 01:55:42PM +0200, Thomas Huth wrote:
> From: Ani Sinha <ani@anisinha.ca>
> 
> Integrate the pytest framework with the meson build system. This
> will make meson run all the pytests under the pytest directory.

Lets add a note about the compelling benefit of this new approach

  With this change, each functional test becomes subject
  to an individual execution timeout, defaulting to 60
  seconds, but overridable per-test.

> 
> Signed-off-by: Ani Sinha <ani@anisinha.ca>
> [thuth: Removed the acpi-bits and adjusted for converted avocado tests instead]
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/Makefile.include   |  4 ++-
>  tests/meson.build        |  1 +
>  tests/pytest/meson.build | 53 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 57 insertions(+), 1 deletion(-)
>  create mode 100644 tests/pytest/meson.build

For CI purposes we'll need to add 'python3-pytest' to
tests/lcitool/projects/qemu.yml, and re-generate the
the dockerfiles. Some of the other non-gitlab CI
integrations probably need manual additions of pytest
packages.

> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index d39d5dd6a4..68151717d7 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -3,12 +3,14 @@
>  .PHONY: check-help
>  check-help:
>  	@echo "Regression testing targets:"
> -	@echo " $(MAKE) check                  Run block, qapi-schema, unit, softfloat, qtest and decodetree tests"
> +	@echo " $(MAKE) check                  Run block, qapi-schema, unit, softfloat, qtest, pytest and decodetree tests"
>  	@echo " $(MAKE) bench                  Run speed tests"
>  	@echo
>  	@echo "Individual test suites:"
>  	@echo " $(MAKE) check-qtest-TARGET     Run qtest tests for given target"
>  	@echo " $(MAKE) check-qtest            Run qtest tests"
> +	@echo " $(MAKE) check-pytest           Run pytest tests"
> +	@echo " $(MAKE) check-pytest-TARGET    Run pytest for a given target"

Or name it after the type of test rather than harness ?

 eg  check-functional / check-functional-TARGET

For that matter perhaps also for the dir name ?

   tests/functional/*.py

>  	@echo " $(MAKE) check-unit             Run qobject tests"
>  	@echo " $(MAKE) check-qapi-schema      Run QAPI schema tests"
>  	@echo " $(MAKE) check-block            Run block tests"
> diff --git a/tests/meson.build b/tests/meson.build
> index acb6807094..17510a468e 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -85,3 +85,4 @@ subdir('unit')
>  subdir('qapi-schema')
>  subdir('qtest')
>  subdir('migration')
> +subdir('pytest')
> diff --git a/tests/pytest/meson.build b/tests/pytest/meson.build
> new file mode 100644
> index 0000000000..1486628d45
> --- /dev/null
> +++ b/tests/pytest/meson.build
> @@ -0,0 +1,53 @@
> +slow_pytests = {
> +  'mem_addr_space' : 90,
> +}
> +
> +pytests_generic = [
> +  'empty_cpu_model',
> +  'info_usernet',
> +  'version',
> +]
> +
> +pytests_x86_64 = [
> +  'cpu_queries',
> +  'mem_addr_space',
> +  'virtio_version',
> +]
> +
> +pytest = find_program('pytest', required: false)
> +if not pytest.found()
> +  message('pytest not available ==> Disabled the qemu-pytests.')
> +  subdir_done()
> +endif
> +
> +foreach dir : target_dirs
> +  if not dir.endswith('-softmmu')
> +    continue
> +  endif
> +
> +  target_base = dir.split('-')[0]
> +  pytest_emulator = emulators['qemu-system-' + target_base]
> +  target_pytests = get_variable('pytests_' + target_base, []) + pytests_generic
> +
> +  test_deps = roms
> +  pytest_env = environment()
> +  if have_tools
> +    pytest_env.set('PYTEST_QEMU_IMG', './qemu-img')
> +    test_deps += [qemu_img]
> +  endif
> +  pytest_env.set('PYTEST_QEMU_BINARY', meson.global_build_root() / 'qemu-system-' + target_base)
> +  pytest_env.set('PYTEST_SOURCE_ROOT', meson.project_source_root())
> +  pytest_env.set('PYTEST_BUILD_ROOT', meson.project_build_root())
> +  pytest_env.set('PYTHONPATH', meson.project_source_root() / 'python')
> +
> +  foreach test : target_pytests
> +    test('pytest-@0@/@1@'.format(target_base, test),
> +         pytest,
> +         depends: [test_deps, pytest_emulator, emulator_modules],
> +         env: pytest_env,
> +         args: [meson.current_source_dir() / 'test_' + test + '.py'],
> +         timeout: slow_pytests.get(test, 60),
> +         priority: slow_pytests.get(test, 60),
> +         suite: ['pytest', 'pytest-' + target_base])
> +  endforeach
> +endforeach
> -- 
> 2.45.2
> 

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 :|

Re: [RFC PATCH 5/8] tests_pytest: Implement fetch_asset() method for downloading assets

[Daniel P. Berrangé]

On Thu, Jul 11, 2024 at 01:55:43PM +0200, Thomas Huth wrote:
> In the pytests, we cannot use the fetch_asset() function from Avocado
> anymore, so we have to provide our own implementation now instead.
> Thus add such a function based on the _download_with_cache() function
> from tests/vm/basevm.py for this purpose.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/pytest/qemu_pytest/__init__.py | 40 ++++++++++++++++++++--------
>  1 file changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/tests/pytest/qemu_pytest/__init__.py b/tests/pytest/qemu_pytest/__init__.py
> index e3ed32e3de..73d80b3828 100644
> --- a/tests/pytest/qemu_pytest/__init__.py
> +++ b/tests/pytest/qemu_pytest/__init__.py
> @@ -11,6 +11,7 @@
>  # This work is licensed under the terms of the GNU GPL, version 2 or
>  # later.  See the COPYING file in the top-level directory.
>  
> +import hashlib
>  import logging
>  import os
>  import shutil
> @@ -201,17 +202,34 @@ def setUp(self, bin_prefix):
>          self.assertIsNotNone(SOURCE_DIR,'PYTEST_SOURCE_ROOT must be set')
>          self.assertIsNotNone(self.qemu_bin, 'PYTEST_QEMU_BINARY must be set')
>  
> -    def fetch_asset(self, name,
> -                    asset_hash, algorithm=None,
> -                    locations=None, expire=None,
> -                    find_only=False, cancel_on_missing=True):
> -        return super().fetch_asset(name,
> -                        asset_hash=asset_hash,
> -                        algorithm=algorithm,
> -                        locations=locations,
> -                        expire=expire,
> -                        find_only=find_only,
> -                        cancel_on_missing=cancel_on_missing)
> +    def check_hash(self, file_name, expected_hash):
> +        if not expected_hash:
> +            return True
> +        if len(expected_hash) == 32:
> +            sum_prog = 'md5sum'
> +        elif len(expected_hash) == 40:
> +            sum_prog = 'sha1sum'
> +        elif len(expected_hash) == 64:
> +            sum_prog = 'sha256sum'
> +        elif len(expected_hash) == 128:
> +            sum_prog = 'sha512sum'
> +        else:
> +            raise Exception("unknown hash type")

Why shouldn't we just standardize on sha256 as we convert each test
to pytest ? sha512 is overkill, and md5/sha1 shouldn't really be used
anymore.

> +        checksum = subprocess.check_output([sum_prog, file_name]).split()[0]
> +        return expected_hash == checksum.decode("utf-8")
> +
> +    def fetch_asset(self, url, asset_hash):
> +        cache_dir = os.path.expanduser("~/.cache/qemu/download")
> +        if not os.path.exists(cache_dir):
> +            os.makedirs(cache_dir)
> +        fname = os.path.join(cache_dir,
> +                             hashlib.sha1(url.encode("utf-8")).hexdigest())
> +        if os.path.exists(fname) and self.check_hash(fname, asset_hash):
> +            return fname
> +        logging.debug("Downloading %s to %s...", url, fname)
> +        subprocess.check_call(["wget", "-c", url, "-O", fname + ".download"])
> +        os.rename(fname + ".download", fname)
> +        return fname

To avoid a dep on an external command that may not be installed,
I think we could replace wget with native python code:

 import urllib
 from shutil import copyfileobj
 
 with urllib.request.urlopen(url) as src:
    with open(fname + ".download", "w+") as dst
       copyfileobj(src, dst)


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 :|

Re: [RFC PATCH 6/8] tests/pytest: Convert some tests that download files via fetch_asset()

[Daniel P. Berrangé]

On Thu, Jul 11, 2024 at 01:55:44PM +0200, Thomas Huth wrote:
> Now that we've got a working fetch_asset() function, we can convert
> some Avocado tests that use this function for downloading their
> required files.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/pytest/meson.build                      | 16 +++++++++++++++
>  .../test_machine_arm_n8x0.py}                 | 20 +++++++------------
>  .../test_machine_avr6.py}                     |  7 ++-----
>  .../test_machine_loongarch.py}                | 11 ++++------
>  .../test_machine_mips_loongson3v.py}          | 19 ++++++------------
>  5 files changed, 35 insertions(+), 38 deletions(-)
>  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%)

If you take my suggestion in the previous patch, the sha1 hashes
would need updating to sha256 hashes here. Aside from that

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|

Re: [RFC PATCH 7/8] tests/pytest: Add a function for extracting files from an archive

[Daniel P. Berrangé]

On Thu, Jul 11, 2024 at 01:55:45PM +0200, Thomas Huth wrote:
> Some Avocado-based tests use the "archive" module from avocado.utils
> to extract files from an archive. To be able to use these tests
> without Avocado, we have to provide our own function for extracting
> files. Fortunately, there is already the tarfile module that will
> provide us with this functionality, so let's just add a nice wrapper
> function around that.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/pytest/qemu_pytest/utils.py | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 tests/pytest/qemu_pytest/utils.py
> 
> diff --git a/tests/pytest/qemu_pytest/utils.py b/tests/pytest/qemu_pytest/utils.py
> new file mode 100644
> index 0000000000..4eb5e5d5e5
> --- /dev/null
> +++ b/tests/pytest/qemu_pytest/utils.py
> @@ -0,0 +1,21 @@
> +# Utilities for python-based QEMU tests
> +#
> +# Copyright 2024 Red Hat, Inc.
> +#
> +# Authors:
> +#  Thomas Huth <thuth@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or
> +# later.  See the COPYING file in the top-level directory.
> +
> +import tarfile
> +
> +def archive_extract(archive, dest_dir, member=None):
> +    with tarfile.open(archive) as tf:
> +        if hasattr(tarfile, 'data_filter'):

Not convinced this is still needed. The python docs don't say anything
about 'data_filter' being introduced after 3.0, so can likely
assume it always exists.

> +            tf.extraction_filter = getattr(tarfile, 'data_filter',
> +                                           (lambda member, path: member))
> +        if member:
> +            tf.extract(member=member, path=dest_dir)
> +        else:
> +            tf.extractall(path=dest_dir)
> -- 
> 2.45.2
> 

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 :|

Re: [RFC PATCH 5/8] tests_pytest: Implement fetch_asset() method for downloading assets

[Thomas Huth]

On 12/07/2024 11.09, Daniel P. Berrangé wrote:
> On Thu, Jul 11, 2024 at 01:55:43PM +0200, Thomas Huth wrote:
>> In the pytests, we cannot use the fetch_asset() function from Avocado
>> anymore, so we have to provide our own implementation now instead.
>> Thus add such a function based on the _download_with_cache() function
>> from tests/vm/basevm.py for this purpose.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   tests/pytest/qemu_pytest/__init__.py | 40 ++++++++++++++++++++--------
>>   1 file changed, 29 insertions(+), 11 deletions(-)
>>
>> diff --git a/tests/pytest/qemu_pytest/__init__.py b/tests/pytest/qemu_pytest/__init__.py
>> index e3ed32e3de..73d80b3828 100644
>> --- a/tests/pytest/qemu_pytest/__init__.py
>> +++ b/tests/pytest/qemu_pytest/__init__.py
>> @@ -11,6 +11,7 @@
>>   # This work is licensed under the terms of the GNU GPL, version 2 or
>>   # later.  See the COPYING file in the top-level directory.
>>   
>> +import hashlib
>>   import logging
>>   import os
>>   import shutil
>> @@ -201,17 +202,34 @@ def setUp(self, bin_prefix):
>>           self.assertIsNotNone(SOURCE_DIR,'PYTEST_SOURCE_ROOT must be set')
>>           self.assertIsNotNone(self.qemu_bin, 'PYTEST_QEMU_BINARY must be set')
>>   
>> -    def fetch_asset(self, name,
>> -                    asset_hash, algorithm=None,
>> -                    locations=None, expire=None,
>> -                    find_only=False, cancel_on_missing=True):
>> -        return super().fetch_asset(name,
>> -                        asset_hash=asset_hash,
>> -                        algorithm=algorithm,
>> -                        locations=locations,
>> -                        expire=expire,
>> -                        find_only=find_only,
>> -                        cancel_on_missing=cancel_on_missing)
>> +    def check_hash(self, file_name, expected_hash):
>> +        if not expected_hash:
>> +            return True
>> +        if len(expected_hash) == 32:
>> +            sum_prog = 'md5sum'
>> +        elif len(expected_hash) == 40:
>> +            sum_prog = 'sha1sum'
>> +        elif len(expected_hash) == 64:
>> +            sum_prog = 'sha256sum'
>> +        elif len(expected_hash) == 128:
>> +            sum_prog = 'sha512sum'
>> +        else:
>> +            raise Exception("unknown hash type")
> 
> Why shouldn't we just standardize on sha256 as we convert each test
> to pytest ? sha512 is overkill, and md5/sha1 shouldn't really be used
> anymore.

I mainly added that for minimizing the changes that I need to do on the 
existing tests. Updating all the hashsums there is certainly some additional 
work... If you want to help, feel free to send patches for the existing 
avocado tests to update the md5 and sha1 sums there!

>> +        checksum = subprocess.check_output([sum_prog, file_name]).split()[0]
>> +        return expected_hash == checksum.decode("utf-8")
>> +
>> +    def fetch_asset(self, url, asset_hash):
>> +        cache_dir = os.path.expanduser("~/.cache/qemu/download")
>> +        if not os.path.exists(cache_dir):
>> +            os.makedirs(cache_dir)
>> +        fname = os.path.join(cache_dir,
>> +                             hashlib.sha1(url.encode("utf-8")).hexdigest())
>> +        if os.path.exists(fname) and self.check_hash(fname, asset_hash):
>> +            return fname
>> +        logging.debug("Downloading %s to %s...", url, fname)
>> +        subprocess.check_call(["wget", "-c", url, "-O", fname + ".download"])
>> +        os.rename(fname + ".download", fname)
>> +        return fname
> 
> To avoid a dep on an external command that may not be installed,
> I think we could replace wget with native python code:
> 
>   import urllib
...

Yes, I came across urllib after I sent out the patches, too, sounds like the 
right way to go. We should also update tests/vm/basevm.py accordingly!

  Thomas

Re: [RFC PATCH 4/8] tests/pytest: add pytest to the meson build system

[Thomas Huth]

On 12/07/2024 11.01, Daniel P. Berrangé wrote:
> On Thu, Jul 11, 2024 at 01:55:42PM +0200, Thomas Huth wrote:
>> From: Ani Sinha <ani@anisinha.ca>
>>
>> Integrate the pytest framework with the meson build system. This
>> will make meson run all the pytests under the pytest directory.
> 
> Lets add a note about the compelling benefit of this new approach
> 
>    With this change, each functional test becomes subject
>    to an individual execution timeout, defaulting to 60
>    seconds, but overridable per-test.

The avocado runner uses timeouts, too, so it's not really an additional 
benefit that we get here.

> For CI purposes we'll need to add 'python3-pytest' to
> tests/lcitool/projects/qemu.yml, and re-generate the
> the dockerfiles. Some of the other non-gitlab CI
> integrations probably need manual additions of pytest
> packages.

I'm currently rather looking into getting rid of pytest and to use pycotap 
instead: Using the TAP protocol for running the tests, you get a much nicer 
output from the meson test runner, which can then count the subtests and 
properly report SKIPs for tests that have not been run.

>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index d39d5dd6a4..68151717d7 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -3,12 +3,14 @@
>>   .PHONY: check-help
>>   check-help:
>>   	@echo "Regression testing targets:"
>> -	@echo " $(MAKE) check                  Run block, qapi-schema, unit, softfloat, qtest and decodetree tests"
>> +	@echo " $(MAKE) check                  Run block, qapi-schema, unit, softfloat, qtest, pytest and decodetree tests"
>>   	@echo " $(MAKE) bench                  Run speed tests"
>>   	@echo
>>   	@echo "Individual test suites:"
>>   	@echo " $(MAKE) check-qtest-TARGET     Run qtest tests for given target"
>>   	@echo " $(MAKE) check-qtest            Run qtest tests"
>> +	@echo " $(MAKE) check-pytest           Run pytest tests"
>> +	@echo " $(MAKE) check-pytest-TARGET    Run pytest for a given target"
> 
> Or name it after the type of test rather than harness ?
> 
>   eg  check-functional / check-functional-TARGET
> 
> For that matter perhaps also for the dir name ?
> 
>     tests/functional/*.py

I almost expected that discussion again ... (see 
https://lists.gnu.org/archive/html/qemu-devel/2021-05/msg06553.html ) ... 
last time we couldn't really agree on such a name and decided to go with the 
name of the framework...

I agree that "pytest" is likely not the best name here, especially if 
switching to the pycotap test runner instead of using the "pytest" program, 
but "functional" might trigger the same discussion again as last time ... 
should it rather be "functional" or "validation" or "integration" etc.?

Maybe best if we come up with a new fictional name for the "new" test 
framework... something like "pyqe" - PYthon-based Qemu test Environment"? 
... could be considered as a play on the word "pike", too, i.e. something 
that makes sure that not everything gets in ... ? WDYT?

  Thomas

Re: [RFC PATCH 4/8] tests/pytest: add pytest to the meson build system

[Daniel P. Berrangé]

On Fri, Jul 12, 2024 at 12:14:45PM +0200, Thomas Huth wrote:
> On 12/07/2024 11.01, Daniel P. Berrangé wrote:
> > On Thu, Jul 11, 2024 at 01:55:42PM +0200, Thomas Huth wrote:
> > > From: Ani Sinha <ani@anisinha.ca>
> > > 
> > > Integrate the pytest framework with the meson build system. This
> > > will make meson run all the pytests under the pytest directory.
> > 
> > Lets add a note about the compelling benefit of this new approach
> > 
> >    With this change, each functional test becomes subject
> >    to an individual execution timeout, defaulting to 60
> >    seconds, but overridable per-test.
> 
> The avocado runner uses timeouts, too, so it's not really an additional
> benefit that we get here.

At the meson level though, we can't put an overall cap on
the execution time, as there's only 1 huge test visible,
and thus the meson timeout multiplier also won't get
honoured IIUC.

> 
> > For CI purposes we'll need to add 'python3-pytest' to
> > tests/lcitool/projects/qemu.yml, and re-generate the
> > the dockerfiles. Some of the other non-gitlab CI
> > integrations probably need manual additions of pytest
> > packages.
> 
> I'm currently rather looking into getting rid of pytest and to use pycotap
> instead: Using the TAP protocol for running the tests, you get a much nicer
> output from the meson test runner, which can then count the subtests and
> properly report SKIPs for tests that have not been run.
> 
> > > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > > index d39d5dd6a4..68151717d7 100644
> > > --- a/tests/Makefile.include
> > > +++ b/tests/Makefile.include
> > > @@ -3,12 +3,14 @@
> > >   .PHONY: check-help
> > >   check-help:
> > >   	@echo "Regression testing targets:"
> > > -	@echo " $(MAKE) check                  Run block, qapi-schema, unit, softfloat, qtest and decodetree tests"
> > > +	@echo " $(MAKE) check                  Run block, qapi-schema, unit, softfloat, qtest, pytest and decodetree tests"
> > >   	@echo " $(MAKE) bench                  Run speed tests"
> > >   	@echo
> > >   	@echo "Individual test suites:"
> > >   	@echo " $(MAKE) check-qtest-TARGET     Run qtest tests for given target"
> > >   	@echo " $(MAKE) check-qtest            Run qtest tests"
> > > +	@echo " $(MAKE) check-pytest           Run pytest tests"
> > > +	@echo " $(MAKE) check-pytest-TARGET    Run pytest for a given target"
> > 
> > Or name it after the type of test rather than harness ?
> > 
> >   eg  check-functional / check-functional-TARGET
> > 
> > For that matter perhaps also for the dir name ?
> > 
> >     tests/functional/*.py
> 
> I almost expected that discussion again ... (see
> https://lists.gnu.org/archive/html/qemu-devel/2021-05/msg06553.html ) ...
> last time we couldn't really agree on such a name and decided to go with the
> name of the framework...
> 
> I agree that "pytest" is likely not the best name here, especially if
> switching to the pycotap test runner instead of using the "pytest" program,
> but "functional" might trigger the same discussion again as last time ...
> should it rather be "functional" or "validation" or "integration" etc.?

IMHO you can just make an executive decision and pick one of those
three. None of them are terrible, any would be a valid choice.

> Maybe best if we come up with a new fictional name for the "new" test
> framework... something like "pyqe" - PYthon-based Qemu test Environment"?
> ... could be considered as a play on the word "pike", too, i.e. something
> that makes sure that not everything gets in ... ? WDYT?

A wierd acronym isn't really going to tell contributors anything about
what this is, compared to calling it 'functional' or 'integration', etc.

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 :|

Re: [RFC PATCH 4/8] tests/pytest: add pytest to the meson build system

[Daniel P. Berrangé]

On Fri, Jul 12, 2024 at 12:14:45PM +0200, Thomas Huth wrote:
> On 12/07/2024 11.01, Daniel P. Berrangé wrote:
> > On Thu, Jul 11, 2024 at 01:55:42PM +0200, Thomas Huth wrote:
> > > From: Ani Sinha <ani@anisinha.ca>
> > > 
> > > Integrate the pytest framework with the meson build system. This
> > > will make meson run all the pytests under the pytest directory.
> > 
> > Lets add a note about the compelling benefit of this new approach
> > 
> >    With this change, each functional test becomes subject
> >    to an individual execution timeout, defaulting to 60
> >    seconds, but overridable per-test.
> 
> The avocado runner uses timeouts, too, so it's not really an additional
> benefit that we get here.
> 
> > For CI purposes we'll need to add 'python3-pytest' to
> > tests/lcitool/projects/qemu.yml, and re-generate the
> > the dockerfiles. Some of the other non-gitlab CI
> > integrations probably need manual additions of pytest
> > packages.
> 
> I'm currently rather looking into getting rid of pytest and to use pycotap
> instead: Using the TAP protocol for running the tests, you get a much nicer
> output from the meson test runner, which can then count the subtests and
> properly report SKIPs for tests that have not been run.

I've just looked at pycotap and IIUC, there's no command line
tool equivalent to '/usr/bin/pytest' at all. Each test case
is expected to provide a stub for "__main__" to invoke the
tests. As such each individual test is directly executable.
This meshes nicely with what I'd suggested as changes in
patch 1, and eliminating the intermediate runner process is
a nice further simplification. So I'll be interested to see
your next version using pycotap.


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 :|

Re: [RFC PATCH 7/8] tests/pytest: Add a function for extracting files from an archive

[Thomas Huth]

On 12/07/2024 11.14, Daniel P. Berrangé wrote:
> On Thu, Jul 11, 2024 at 01:55:45PM +0200, Thomas Huth wrote:
>> Some Avocado-based tests use the "archive" module from avocado.utils
>> to extract files from an archive. To be able to use these tests
>> without Avocado, we have to provide our own function for extracting
>> files. Fortunately, there is already the tarfile module that will
>> provide us with this functionality, so let's just add a nice wrapper
>> function around that.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   tests/pytest/qemu_pytest/utils.py | 21 +++++++++++++++++++++
>>   1 file changed, 21 insertions(+)
>>   create mode 100644 tests/pytest/qemu_pytest/utils.py
>>
>> diff --git a/tests/pytest/qemu_pytest/utils.py b/tests/pytest/qemu_pytest/utils.py
>> new file mode 100644
>> index 0000000000..4eb5e5d5e5
>> --- /dev/null
>> +++ b/tests/pytest/qemu_pytest/utils.py
>> @@ -0,0 +1,21 @@
>> +# Utilities for python-based QEMU tests
>> +#
>> +# Copyright 2024 Red Hat, Inc.
>> +#
>> +# Authors:
>> +#  Thomas Huth <thuth@redhat.com>
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2 or
>> +# later.  See the COPYING file in the top-level directory.
>> +
>> +import tarfile
>> +
>> +def archive_extract(archive, dest_dir, member=None):
>> +    with tarfile.open(archive) as tf:
>> +        if hasattr(tarfile, 'data_filter'):
> 
> Not convinced this is still needed. The python docs don't say anything
> about 'data_filter' being introduced after 3.0, so can likely
> assume it always exists.

According to https://docs.python.org/3/library/tarfile.html :

"Extraction filters were added to Python 3.12, but may be backported to 
older versions as security updates. To check whether the feature is 
available, use e.g. hasattr(tarfile, 'data_filter') rather than checking the 
Python version."

And it seems to be missing in Python 3.7, indeed:

  https://docs.python.org/3.7/library/tarfile.html

So as long as we still support this old version, I think I've got to keep 
this check.

  Thomas

Re: [RFC PATCH 4/8] tests/pytest: add pytest to the meson build system

[Thomas Huth]

On 12/07/2024 12.26, Daniel P. Berrangé wrote:
> On Fri, Jul 12, 2024 at 12:14:45PM +0200, Thomas Huth wrote:
>> On 12/07/2024 11.01, Daniel P. Berrangé wrote:
>>> On Thu, Jul 11, 2024 at 01:55:42PM +0200, Thomas Huth wrote:
>>>> From: Ani Sinha <ani@anisinha.ca>
>>>>
>>>> Integrate the pytest framework with the meson build system. This
>>>> will make meson run all the pytests under the pytest directory.
>>>
>>> Lets add a note about the compelling benefit of this new approach
>>>
>>>     With this change, each functional test becomes subject
>>>     to an individual execution timeout, defaulting to 60
>>>     seconds, but overridable per-test.
>>
>> The avocado runner uses timeouts, too, so it's not really an additional
>> benefit that we get here.
> 
> At the meson level though, we can't put an overall cap on
> the execution time, as there's only 1 huge test visible,
> and thus the meson timeout multiplier also won't get
> honoured IIUC.
> 
>>
>>> For CI purposes we'll need to add 'python3-pytest' to
>>> tests/lcitool/projects/qemu.yml, and re-generate the
>>> the dockerfiles. Some of the other non-gitlab CI
>>> integrations probably need manual additions of pytest
>>> packages.
>>
>> I'm currently rather looking into getting rid of pytest and to use pycotap
>> instead: Using the TAP protocol for running the tests, you get a much nicer
>> output from the meson test runner, which can then count the subtests and
>> properly report SKIPs for tests that have not been run.
>>
>>>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>>>> index d39d5dd6a4..68151717d7 100644
>>>> --- a/tests/Makefile.include
>>>> +++ b/tests/Makefile.include
>>>> @@ -3,12 +3,14 @@
>>>>    .PHONY: check-help
>>>>    check-help:
>>>>    	@echo "Regression testing targets:"
>>>> -	@echo " $(MAKE) check                  Run block, qapi-schema, unit, softfloat, qtest and decodetree tests"
>>>> +	@echo " $(MAKE) check                  Run block, qapi-schema, unit, softfloat, qtest, pytest and decodetree tests"
>>>>    	@echo " $(MAKE) bench                  Run speed tests"
>>>>    	@echo
>>>>    	@echo "Individual test suites:"
>>>>    	@echo " $(MAKE) check-qtest-TARGET     Run qtest tests for given target"
>>>>    	@echo " $(MAKE) check-qtest            Run qtest tests"
>>>> +	@echo " $(MAKE) check-pytest           Run pytest tests"
>>>> +	@echo " $(MAKE) check-pytest-TARGET    Run pytest for a given target"
>>>
>>> Or name it after the type of test rather than harness ?
>>>
>>>    eg  check-functional / check-functional-TARGET
>>>
>>> For that matter perhaps also for the dir name ?
>>>
>>>      tests/functional/*.py
>>
>> I almost expected that discussion again ... (see
>> https://lists.gnu.org/archive/html/qemu-devel/2021-05/msg06553.html ) ...
>> last time we couldn't really agree on such a name and decided to go with the
>> name of the framework...
>>
>> I agree that "pytest" is likely not the best name here, especially if
>> switching to the pycotap test runner instead of using the "pytest" program,
>> but "functional" might trigger the same discussion again as last time ...
>> should it rather be "functional" or "validation" or "integration" etc.?
> 
> IMHO you can just make an executive decision and pick one of those
> three. None of them are terrible, any would be a valid choice.

Ok, I think I'll go with "functional" since that still seems to be the best 
fit to me.

  Thomas

Re: [RFC PATCH 7/8] tests/pytest: Add a function for extracting files from an archive

[Daniel P. Berrangé]

On Fri, Jul 12, 2024 at 01:52:03PM +0200, Thomas Huth wrote:
> On 12/07/2024 11.14, Daniel P. Berrangé wrote:
> > On Thu, Jul 11, 2024 at 01:55:45PM +0200, Thomas Huth wrote:
> > > Some Avocado-based tests use the "archive" module from avocado.utils
> > > to extract files from an archive. To be able to use these tests
> > > without Avocado, we have to provide our own function for extracting
> > > files. Fortunately, there is already the tarfile module that will
> > > provide us with this functionality, so let's just add a nice wrapper
> > > function around that.
> > > 
> > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > > ---
> > >   tests/pytest/qemu_pytest/utils.py | 21 +++++++++++++++++++++
> > >   1 file changed, 21 insertions(+)
> > >   create mode 100644 tests/pytest/qemu_pytest/utils.py
> > > 
> > > diff --git a/tests/pytest/qemu_pytest/utils.py b/tests/pytest/qemu_pytest/utils.py
> > > new file mode 100644
> > > index 0000000000..4eb5e5d5e5
> > > --- /dev/null
> > > +++ b/tests/pytest/qemu_pytest/utils.py
> > > @@ -0,0 +1,21 @@
> > > +# Utilities for python-based QEMU tests
> > > +#
> > > +# Copyright 2024 Red Hat, Inc.
> > > +#
> > > +# Authors:
> > > +#  Thomas Huth <thuth@redhat.com>
> > > +#
> > > +# This work is licensed under the terms of the GNU GPL, version 2 or
> > > +# later.  See the COPYING file in the top-level directory.
> > > +
> > > +import tarfile
> > > +
> > > +def archive_extract(archive, dest_dir, member=None):
> > > +    with tarfile.open(archive) as tf:
> > > +        if hasattr(tarfile, 'data_filter'):
> > 
> > Not convinced this is still needed. The python docs don't say anything
> > about 'data_filter' being introduced after 3.0, so can likely
> > assume it always exists.
> 
> According to https://docs.python.org/3/library/tarfile.html :
> 
> "Extraction filters were added to Python 3.12, but may be backported to
> older versions as security updates. To check whether the feature is
> available, use e.g. hasattr(tarfile, 'data_filter') rather than checking the
> Python version."
> 
> And it seems to be missing in Python 3.7, indeed:
> 
>  https://docs.python.org/3.7/library/tarfile.html
> 
> So as long as we still support this old version, I think I've got to keep
> this check.

Opps, yes, I missed the docs. The version info is against the top heading,
not repeated against each method.


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 :|

Re: [RFC PATCH 4/8] tests/pytest: add pytest to the meson build system

[Thomas Huth]

On 12/07/2024 13.47, Daniel P. Berrangé wrote:
> On Fri, Jul 12, 2024 at 12:14:45PM +0200, Thomas Huth wrote:
>> On 12/07/2024 11.01, Daniel P. Berrangé wrote:
>>> On Thu, Jul 11, 2024 at 01:55:42PM +0200, Thomas Huth wrote:
>>>> From: Ani Sinha <ani@anisinha.ca>
>>>>
>>>> Integrate the pytest framework with the meson build system. This
>>>> will make meson run all the pytests under the pytest directory.
>>>
>>> Lets add a note about the compelling benefit of this new approach
>>>
>>>     With this change, each functional test becomes subject
>>>     to an individual execution timeout, defaulting to 60
>>>     seconds, but overridable per-test.
>>
>> The avocado runner uses timeouts, too, so it's not really an additional
>> benefit that we get here.
>>
>>> For CI purposes we'll need to add 'python3-pytest' to
>>> tests/lcitool/projects/qemu.yml, and re-generate the
>>> the dockerfiles. Some of the other non-gitlab CI
>>> integrations probably need manual additions of pytest
>>> packages.
>>
>> I'm currently rather looking into getting rid of pytest and to use pycotap
>> instead: Using the TAP protocol for running the tests, you get a much nicer
>> output from the meson test runner, which can then count the subtests and
>> properly report SKIPs for tests that have not been run.
> 
> I've just looked at pycotap and IIUC, there's no command line
> tool equivalent to '/usr/bin/pytest' at all. Each test case
> is expected to provide a stub for "__main__" to invoke the
> tests.

With pycotap 1.3.0 you could run tests through "python -m pycotap ...", too. 
But for playing nicely with meson, I'd like to set message_log and 
test_output_log to LogMode.LogToError instead of using their default value, 
so I need my own wrapper code anyway.

> As such each individual test is directly executable.
> This meshes nicely with what I'd suggested as changes in
> patch 1, and eliminating the intermediate runner process is
> a nice further simplification. So I'll be interested to see
> your next version using pycotap.

I'll try to post it as soon as I have something at least in a somewhat more 
proper shape than the RFC ;-)

  Thomas

Re: [RFC PATCH 0/8] Convert avocado tests to normal Python unittests

[Alex Bennée]

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

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [RFC PATCH 0/8] Convert avocado tests to normal Python unittests

[Daniel P. Berrangé]

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
-- 
|: 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 :|

Re: [RFC PATCH 0/8] Convert avocado tests to normal Python unittests

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 7344 bytes --]

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
>
>

[-- Attachment #2: Type: text/html, Size: 8954 bytes --]

Re: [RFC PATCH 0/8] Convert avocado tests to normal Python unittests

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 3845 bytes --]

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
>>
>>
>>

[-- Attachment #2: Type: text/html, Size: 5738 bytes --]

Re: [RFC PATCH 0/8] Convert avocado tests to normal Python unittests

[Daniel P. Berrangé]

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
-- 
|: 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 :|

Re: [RFC PATCH 0/8] Convert avocado tests to normal Python unittests

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1428 bytes --]

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 :|
>
>

[-- Attachment #2: Type: text/html, Size: 3272 bytes --]

Re: [RFC PATCH 0/8] Convert avocado tests to normal Python unittests

[Daniel P. Berrangé]

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
-- 
|: 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 :|

Re: [RFC PATCH 0/8] Convert avocado tests to normal Python unittests

[Thomas Huth]

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

Re: [RFC PATCH 0/8] Convert avocado tests to normal Python unittests

[Thomas Huth]

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

Re: [RFC PATCH 0/8] Convert avocado tests to normal Python unittests

[Paolo Bonzini]

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