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

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

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