[PATCH 10/22] tests/functional: switch over to using self.build_file(...)

Daniel P. Berrangé posted 22 patches 3 weeks, 5 days ago
There is a newer version of this series
[PATCH 10/22] tests/functional: switch over to using self.build_file(...)
Posted by Daniel P. Berrangé 3 weeks, 5 days ago
This removes direct access of the 'BUILD_DIR' variable.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/functional/qemu_test/testcase.py | 4 ++--
 tests/functional/test_aarch64_virt.py  | 6 +++---
 tests/functional/test_virtio_gpu.py    | 9 ++++-----
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
index 5b1e6ba04f..2f32742387 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -127,8 +127,8 @@ def setUp(self, bin_prefix):
         self.arch = self.qemu_bin.split('-')[-1]
         self.socketdir = None
 
-        self.outputdir = os.path.join(BUILD_DIR, 'tests', 'functional',
-                                      self.arch, self.id())
+        self.outputdir = self.build_file('tests', 'functional',
+                                         self.arch, self.id())
         self.workdir = os.path.join(self.outputdir, 'scratch')
         os.makedirs(self.workdir, exist_ok=True)
 
diff --git a/tests/functional/test_aarch64_virt.py b/tests/functional/test_aarch64_virt.py
index 07b78f6a84..29eeb8e32d 100755
--- a/tests/functional/test_aarch64_virt.py
+++ b/tests/functional/test_aarch64_virt.py
@@ -14,7 +14,7 @@
 import os
 import logging
 
-from qemu_test import (BUILD_DIR, QemuSystemTest, Asset, exec_command,
+from qemu_test import (QemuSystemTest, Asset, exec_command,
                        wait_for_console_pattern, get_qemu_img, run_cmd)
 
 
@@ -52,8 +52,8 @@ def test_alpine_virt_tcg_gic_max(self):
                          "mte=on,"
                          "gic-version=max,iommu=smmuv3")
         self.vm.add_args("-smp", "2", "-m", "1024")
-        self.vm.add_args('-bios', os.path.join(BUILD_DIR, 'pc-bios',
-                                               'edk2-aarch64-code.fd'))
+        self.vm.add_args('-bios', self.build_file('pc-bios',
+                                                  'edk2-aarch64-code.fd'))
         self.vm.add_args("-drive", f"file={iso_path},media=cdrom,format=raw")
         self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
         self.vm.add_args('-object', 'rng-random,id=rng0,filename=/dev/urandom')
diff --git a/tests/functional/test_virtio_gpu.py b/tests/functional/test_virtio_gpu.py
index c4562618d9..39dcf376dd 100755
--- a/tests/functional/test_virtio_gpu.py
+++ b/tests/functional/test_virtio_gpu.py
@@ -6,8 +6,7 @@
 # later.  See the COPYING file in the top-level directory.
 
 
-from qemu_test import (BUILD_DIR, QemuSystemTest, Asset,
-                       wait_for_console_pattern,
+from qemu_test import (QemuSystemTest, Asset, wait_for_console_pattern,
                        exec_command_and_wait_for_pattern,
                        is_readable_executable_file)
 from qemu.utils import kvm_available
@@ -17,12 +16,12 @@
 import subprocess
 
 
-def pick_default_vug_bin():
+def pick_default_vug_bin(test):
     relative_path = "./contrib/vhost-user-gpu/vhost-user-gpu"
     if is_readable_executable_file(relative_path):
         return relative_path
 
-    bld_dir_path = os.path.join(BUILD_DIR, relative_path)
+    bld_dir_path = test.build_file(relative_path)
     if is_readable_executable_file(bld_dir_path):
         return bld_dir_path
 
@@ -85,7 +84,7 @@ def test_vhost_user_vga_virgl(self):
         # FIXME: should check presence of vhost-user-gpu, virgl, memfd etc
         self.require_accelerator('kvm')
 
-        vug = pick_default_vug_bin()
+        vug = pick_default_vug_bin(self)
         if not vug:
             self.skipTest("Could not find vhost-user-gpu")
 
-- 
2.46.0


Re: [PATCH 10/22] tests/functional: switch over to using self.build_file(...)
Posted by Thomas Huth 3 weeks, 3 days ago
On 29/11/2024 18.31, Daniel P. Berrangé wrote:
> This removes direct access of the 'BUILD_DIR' variable.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/functional/qemu_test/testcase.py | 4 ++--
>   tests/functional/test_aarch64_virt.py  | 6 +++---
>   tests/functional/test_virtio_gpu.py    | 9 ++++-----
>   3 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
> index 5b1e6ba04f..2f32742387 100644
> --- a/tests/functional/qemu_test/testcase.py
> +++ b/tests/functional/qemu_test/testcase.py
> @@ -127,8 +127,8 @@ def setUp(self, bin_prefix):
>           self.arch = self.qemu_bin.split('-')[-1]
>           self.socketdir = None
>   
> -        self.outputdir = os.path.join(BUILD_DIR, 'tests', 'functional',
> -                                      self.arch, self.id())
> +        self.outputdir = self.build_file('tests', 'functional',
> +                                         self.arch, self.id())
>           self.workdir = os.path.join(self.outputdir, 'scratch')
>           os.makedirs(self.workdir, exist_ok=True)
>   
> diff --git a/tests/functional/test_aarch64_virt.py b/tests/functional/test_aarch64_virt.py
> index 07b78f6a84..29eeb8e32d 100755
> --- a/tests/functional/test_aarch64_virt.py
> +++ b/tests/functional/test_aarch64_virt.py
> @@ -14,7 +14,7 @@
>   import os
>   import logging
>   
> -from qemu_test import (BUILD_DIR, QemuSystemTest, Asset, exec_command,
> +from qemu_test import (QemuSystemTest, Asset, exec_command,
>                          wait_for_console_pattern, get_qemu_img, run_cmd)
>   
>   
> @@ -52,8 +52,8 @@ def test_alpine_virt_tcg_gic_max(self):
>                            "mte=on,"
>                            "gic-version=max,iommu=smmuv3")
>           self.vm.add_args("-smp", "2", "-m", "1024")
> -        self.vm.add_args('-bios', os.path.join(BUILD_DIR, 'pc-bios',
> -                                               'edk2-aarch64-code.fd'))
> +        self.vm.add_args('-bios', self.build_file('pc-bios',
> +                                                  'edk2-aarch64-code.fd'))
>           self.vm.add_args("-drive", f"file={iso_path},media=cdrom,format=raw")
>           self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
>           self.vm.add_args('-object', 'rng-random,id=rng0,filename=/dev/urandom')
> diff --git a/tests/functional/test_virtio_gpu.py b/tests/functional/test_virtio_gpu.py
> index c4562618d9..39dcf376dd 100755
> --- a/tests/functional/test_virtio_gpu.py
> +++ b/tests/functional/test_virtio_gpu.py
> @@ -6,8 +6,7 @@
>   # later.  See the COPYING file in the top-level directory.
>   
>   
> -from qemu_test import (BUILD_DIR, QemuSystemTest, Asset,
> -                       wait_for_console_pattern,
> +from qemu_test import (QemuSystemTest, Asset, wait_for_console_pattern,
>                          exec_command_and_wait_for_pattern,
>                          is_readable_executable_file)
>   from qemu.utils import kvm_available
> @@ -17,12 +16,12 @@
>   import subprocess
>   
>   
> -def pick_default_vug_bin():
> +def pick_default_vug_bin(test):
>       relative_path = "./contrib/vhost-user-gpu/vhost-user-gpu"
>       if is_readable_executable_file(relative_path):
>           return relative_path

I wonder whether we should drop the above two lines of special casing and 
always use the code below instead?

> -    bld_dir_path = os.path.join(BUILD_DIR, relative_path)
> +    bld_dir_path = test.build_file(relative_path)

... then you could also get rid of the hard-coded slashes in relative_path?

>       if is_readable_executable_file(bld_dir_path):
>           return bld_dir_path
>   
> @@ -85,7 +84,7 @@ def test_vhost_user_vga_virgl(self):
>           # FIXME: should check presence of vhost-user-gpu, virgl, memfd etc
>           self.require_accelerator('kvm')
>   
> -        vug = pick_default_vug_bin()
> +        vug = pick_default_vug_bin(self)
>           if not vug:
>               self.skipTest("Could not find vhost-user-gpu")
>   

Anyway:
Reviewed-by: Thomas Huth <thuth@redhat.com>


Re: [PATCH 10/22] tests/functional: switch over to using self.build_file(...)
Posted by Daniel P. Berrangé 3 weeks, 3 days ago
On Mon, Dec 02, 2024 at 10:26:59AM +0100, Thomas Huth wrote:
> On 29/11/2024 18.31, Daniel P. Berrangé wrote:
> > This removes direct access of the 'BUILD_DIR' variable.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >   tests/functional/qemu_test/testcase.py | 4 ++--
> >   tests/functional/test_aarch64_virt.py  | 6 +++---
> >   tests/functional/test_virtio_gpu.py    | 9 ++++-----
> >   3 files changed, 9 insertions(+), 10 deletions(-)
> > 
> > diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
> > index 5b1e6ba04f..2f32742387 100644
> > --- a/tests/functional/qemu_test/testcase.py
> > +++ b/tests/functional/qemu_test/testcase.py
> > @@ -127,8 +127,8 @@ def setUp(self, bin_prefix):
> >           self.arch = self.qemu_bin.split('-')[-1]
> >           self.socketdir = None
> > -        self.outputdir = os.path.join(BUILD_DIR, 'tests', 'functional',
> > -                                      self.arch, self.id())
> > +        self.outputdir = self.build_file('tests', 'functional',
> > +                                         self.arch, self.id())
> >           self.workdir = os.path.join(self.outputdir, 'scratch')
> >           os.makedirs(self.workdir, exist_ok=True)
> > diff --git a/tests/functional/test_aarch64_virt.py b/tests/functional/test_aarch64_virt.py
> > index 07b78f6a84..29eeb8e32d 100755
> > --- a/tests/functional/test_aarch64_virt.py
> > +++ b/tests/functional/test_aarch64_virt.py
> > @@ -14,7 +14,7 @@
> >   import os
> >   import logging
> > -from qemu_test import (BUILD_DIR, QemuSystemTest, Asset, exec_command,
> > +from qemu_test import (QemuSystemTest, Asset, exec_command,
> >                          wait_for_console_pattern, get_qemu_img, run_cmd)
> > @@ -52,8 +52,8 @@ def test_alpine_virt_tcg_gic_max(self):
> >                            "mte=on,"
> >                            "gic-version=max,iommu=smmuv3")
> >           self.vm.add_args("-smp", "2", "-m", "1024")
> > -        self.vm.add_args('-bios', os.path.join(BUILD_DIR, 'pc-bios',
> > -                                               'edk2-aarch64-code.fd'))
> > +        self.vm.add_args('-bios', self.build_file('pc-bios',
> > +                                                  'edk2-aarch64-code.fd'))
> >           self.vm.add_args("-drive", f"file={iso_path},media=cdrom,format=raw")
> >           self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
> >           self.vm.add_args('-object', 'rng-random,id=rng0,filename=/dev/urandom')
> > diff --git a/tests/functional/test_virtio_gpu.py b/tests/functional/test_virtio_gpu.py
> > index c4562618d9..39dcf376dd 100755
> > --- a/tests/functional/test_virtio_gpu.py
> > +++ b/tests/functional/test_virtio_gpu.py
> > @@ -6,8 +6,7 @@
> >   # later.  See the COPYING file in the top-level directory.
> > -from qemu_test import (BUILD_DIR, QemuSystemTest, Asset,
> > -                       wait_for_console_pattern,
> > +from qemu_test import (QemuSystemTest, Asset, wait_for_console_pattern,
> >                          exec_command_and_wait_for_pattern,
> >                          is_readable_executable_file)
> >   from qemu.utils import kvm_available
> > @@ -17,12 +16,12 @@
> >   import subprocess
> > -def pick_default_vug_bin():
> > +def pick_default_vug_bin(test):
> >       relative_path = "./contrib/vhost-user-gpu/vhost-user-gpu"
> >       if is_readable_executable_file(relative_path):
> >           return relative_path
> 
> I wonder whether we should drop the above two lines of special casing and
> always use the code below instead?
> 
> > -    bld_dir_path = os.path.join(BUILD_DIR, relative_path)
> > +    bld_dir_path = test.build_file(relative_path)
> 
> ... then you could also get rid of the hard-coded slashes in relative_path?

Hmmm, yes, the first check is rather redundant, will cull it.

> 
> >       if is_readable_executable_file(bld_dir_path):
> >           return bld_dir_path
> > @@ -85,7 +84,7 @@ def test_vhost_user_vga_virgl(self):
> >           # FIXME: should check presence of vhost-user-gpu, virgl, memfd etc
> >           self.require_accelerator('kvm')
> > -        vug = pick_default_vug_bin()
> > +        vug = pick_default_vug_bin(self)
> >           if not vug:
> >               self.skipTest("Could not find vhost-user-gpu")
> 
> Anyway:
> Reviewed-by: Thomas Huth <thuth@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 :|