Re: [PATCH v4 26/35] tests/functional: Convert the ppc_hv avocado test into a standalone test

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 21/8/24 10:27, Thomas Huth wrote:
> A straight forward conversion, we just also have to remove the decorator
> @skipUnless(os.getenv('SPEED')) since all non-trivial functional tests
> are running in SPEED=thorough mode now. Also make sure that the extracted
> assets are writable, so that the test does not fail if it gets re-run
> and there are stale read-only files already around.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   MAINTAINERS                                   |  2 +-
>   tests/functional/meson.build                  |  2 +
>   .../test_ppc64_hv.py}                         | 53 ++++++++-----------
>   3 files changed, 25 insertions(+), 32 deletions(-)
>   rename tests/{avocado/ppc_hv_tests.py => functional/test_ppc64_hv.py} (85%)
>   mode change 100644 => 100755
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f01e865025..cca48683ce 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1532,7 +1532,7 @@ F: tests/qtest/libqos/*spapr*
>   F: tests/qtest/rtas*
>   F: tests/qtest/libqos/rtas*
>   F: tests/functional/test_ppc64_pseries.py
> -F: tests/avocado/ppc_hv_tests.py
> +F: tests/functional/test_ppc64_hv.py
>   
>   PowerNV (Non-Virtualized)
>   M: Cédric Le Goater <clg@kaod.org>
> diff --git a/tests/functional/meson.build b/tests/functional/meson.build
> index fba3891e16..f90fb08d6a 100644
> --- a/tests/functional/meson.build
> +++ b/tests/functional/meson.build
> @@ -13,6 +13,7 @@ endif
>   test_timeouts = {
>     'netdev_ethtool' : 180,
>     'ppc_40p' : 240,
> +  'ppc64_hv' : 1000,
>     'ppc64_powernv' : 120,
>     'ppc64_pseries' : 120,
>     's390x_ccw_virtio' : 180,
> @@ -63,6 +64,7 @@ tests_ppc_thorough = [
>   ]
>   
>   tests_ppc64_thorough = [
> +  'ppc64_hv',
>     'ppc64_powernv',
>     'ppc64_pseries',
>   ]
> diff --git a/tests/avocado/ppc_hv_tests.py b/tests/functional/test_ppc64_hv.py
> old mode 100644
> new mode 100755
> similarity index 85%
> rename from tests/avocado/ppc_hv_tests.py
> rename to tests/functional/test_ppc64_hv.py
> index 0e83bbac71..df59667e07
> --- a/tests/avocado/ppc_hv_tests.py
> +++ b/tests/functional/test_ppc64_hv.py
> @@ -1,3 +1,5 @@
> +#!/usr/bin/env python3
> +#
>   # Tests that specifically try to exercise hypervisor features of the
>   # target machines. powernv supports the Power hypervisor ISA, and
>   # pseries supports the nested-HV hypervisor spec.
> @@ -7,10 +9,10 @@
>   # 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 import skipIf, skipUnless
> -from avocado.utils import archive
> -from avocado_qemu import QemuSystemTest
> -from avocado_qemu import wait_for_console_pattern, exec_command
> +from unittest import skipIf, skipUnless
> +from qemu_test.utils import archive_extract
> +from qemu_test import QemuSystemTest, Asset
> +from qemu_test import wait_for_console_pattern, exec_command
>   import os
>   import time
>   import subprocess
> @@ -45,8 +47,7 @@ def missing_deps():
>   # QEMU already installed and use that.
>   # XXX: The order of these tests seems to matter, see git blame.
>   @skipIf(missing_deps(), 'dependencies (%s) not installed' % ','.join(deps))
> -@skipUnless(os.getenv('AVOCADO_ALLOW_LARGE_STORAGE'), 'storage limited')
> -@skipUnless(os.getenv('SPEED') == 'slow', 'runtime limited')
> +@skipUnless(os.getenv('QEMU_TEST_ALLOW_LARGE_STORAGE'), 'storage limited')
>   class HypervisorTest(QemuSystemTest):
>   
>       timeout = 1000
> @@ -54,6 +55,11 @@ class HypervisorTest(QemuSystemTest):
>       panic_message = 'Kernel panic - not syncing'
>       good_message = 'VFS: Cannot open root device'
>   
> +    ASSET_ISO = Asset(
> +        ('https://dl-cdn.alpinelinux.org/alpine/v3.18/'
> +         'releases/ppc64le/alpine-standard-3.18.4-ppc64le.iso'),
> +        'c26b8d3e17c2f3f0fed02b4b1296589c2390e6d5548610099af75300edd7b3ff')
> +
>       def extract_from_iso(self, iso, path):
>           """
>           Extracts a file from an iso file into the test workdir
> @@ -72,6 +78,7 @@ def extract_from_iso(self, iso, path):
>               subprocess.run(cmd.split(),
>                              stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
>   
> +        os.chmod(filename, 0o600)
>           os.chdir(cwd)
>   
>           # Return complete path to extracted file.  Because callers to
> @@ -83,16 +90,9 @@ def extract_from_iso(self, iso, path):
>       def setUp(self):
>           super().setUp()
>   
> -        iso_url = ('https://dl-cdn.alpinelinux.org/alpine/v3.18/releases/ppc64le/alpine-standard-3.18.4-ppc64le.iso')
> -
> -        # Alpine use sha256 so I recalculated this myself
> -        iso_sha256 = 'c26b8d3e17c2f3f0fed02b4b1296589c2390e6d5548610099af75300edd7b3ff'
> -        iso_path = self.fetch_asset(iso_url, asset_hash=iso_sha256,
> -                                    algorithm = "sha256")
> -
> -        self.iso_path = iso_path
> -        self.vmlinuz = self.extract_from_iso(iso_path, '/boot/vmlinuz-lts')
> -        self.initramfs = self.extract_from_iso(iso_path, '/boot/initramfs-lts')
> +        self.iso_path = self.ASSET_ISO.fetch()
> +        self.vmlinuz = self.extract_from_iso(self.iso_path, '/boot/vmlinuz-lts')
> +        self.initramfs = self.extract_from_iso(self.iso_path, '/boot/initramfs-lts')
>   
>       def do_start_alpine(self):
>           self.vm.set_console()

Expanding context:

            kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE
            self.vm.add_args("-kernel", self.vmlinuz)
            self.vm.add_args("-initrd", self.initramfs)
            self.vm.add_args("-smp", "4", "-m", "2g")
            self.vm.add_args("-drive", 
f"file={self.iso_path},format=raw,if=none,id=drive0")

Should we use ,media=cdrom here too?