Re: [PATCH 10/11] tests/functional: Convert the s390x avocado tests into standalone tests

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Jul 16, 2024 at 01:26:13PM +0200, Thomas Huth wrote:
> These tests use archive.lzma_uncompress() from the Avocado utils,
> so provide a small helper function for this, based on the
> standard lzma module from Python instead.
> 
> And while we're at it, replace the MD5 hashes in the topology test
> with proper SHA256 hashes, since MD5 should not be used anymore
> nowadays.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  MAINTAINERS                                   |  4 +-
>  tests/functional/meson.build                  |  6 ++
>  tests/functional/qemu_test/utils.py           |  7 ++
>  .../test_s390x_ccw_virtio.py}                 | 32 ++++-----
>  .../test_s390x_topology.py}                   | 70 +++++++------------
>  5 files changed, 52 insertions(+), 67 deletions(-)
>  rename tests/{avocado/machine_s390_ccw_virtio.py => functional/test_s390x_ccw_virtio.py} (95%)
>  mode change 100644 => 100755
>  rename tests/{avocado/s390_topology.py => functional/test_s390x_topology.py} (90%)
>  mode change 100644 => 100755
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 025227954c..cbefb6fb81 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1739,7 +1739,7 @@ S: Supported
>  F: hw/s390x/
>  F: include/hw/s390x/
>  F: configs/devices/s390x-softmmu/default.mak
> -F: tests/avocado/machine_s390_ccw_virtio.py
> +F: tests/functional/test_s390x_ccw_virtio.py
>  T: git https://github.com/borntraeger/qemu.git s390-next
>  L: qemu-s390x@nongnu.org
>  
> @@ -1802,7 +1802,7 @@ F: hw/s390x/cpu-topology.c
>  F: target/s390x/kvm/stsi-topology.c
>  F: docs/devel/s390-cpu-topology.rst
>  F: docs/system/s390x/cpu-topology.rst
> -F: tests/avocado/s390_topology.py
> +F: tests/functional/test_s390x_topology.py
>  
>  X86 Machines
>  ------------
> diff --git a/tests/functional/meson.build b/tests/functional/meson.build
> index c8fc9f6c07..f6de9af8a2 100644
> --- a/tests/functional/meson.build
> +++ b/tests/functional/meson.build
> @@ -13,6 +13,7 @@ endif
>  test_timeouts = {
>    'netdev_ethtool' : 180,
>    'ppc_74xx' : 90,
> +  's390x_ccw_virtio' : 180,
>  }
>  
>  tests_generic = [
> @@ -47,6 +48,11 @@ tests_ppc_thorough = [
>    'ppc_bamboo',
>  ]
>  
> +tests_s390x_thorough = [
> +  's390x_ccw_virtio',
> +  's390x_topology',
> +]
> +
>  tests_sparc64_thorough = [
>    'sparc64_sun4u',
>  ]
> diff --git a/tests/functional/qemu_test/utils.py b/tests/functional/qemu_test/utils.py
> index 4eb5e5d5e5..8115d9d1da 100644
> --- a/tests/functional/qemu_test/utils.py
> +++ b/tests/functional/qemu_test/utils.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 lzma
> +import shutil
>  import tarfile
>  
>  def archive_extract(archive, dest_dir, member=None):
> @@ -19,3 +21,8 @@ def archive_extract(archive, dest_dir, member=None):
>              tf.extract(member=member, path=dest_dir)
>          else:
>              tf.extractall(path=dest_dir)
> +
> +def lzma_uncompress(xz_path, output_path):
> +    with lzma.open(xz_path, 'rb') as lzma_in:
> +        with open(output_path, 'wb') as raw_out:
> +            shutil.copyfileobj(lzma_in, raw_out)

Avocado short-circuited if output_path already existed
for speed. Worth doing the same.

The inner 'with' should be surrounded by a try/except
that does os.remove(output_path) on error. Avocado
didn't have this safety net either, but we should add
it.

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