Re: [PATCH v2] tests/functional/x86_64/test_virtio_gpu: Fix various issues reported by pylint

[Thomas Huth <thuth@redhat.com>]

On 13/11/2025 12.30, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> Use the recommended order for import statements, specify the kind of
> exceptions that we try to catch, use f-strings where it makes sense,
> rewrite the vug_log_file part with a proper "with" statement and
> fix some FIXMEs by checking for the availability of the devices, etc.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   v2: Use a "with" block also for the subprocess.Popen() part
> 
>   tests/functional/x86_64/test_virtio_gpu.py | 57 +++++++++++-----------
>   1 file changed, 29 insertions(+), 28 deletions(-)
> 
> diff --git a/tests/functional/x86_64/test_virtio_gpu.py b/tests/functional/x86_64/test_virtio_gpu.py
> index be96de24da2..8c9f2350e69 100755
> --- a/tests/functional/x86_64/test_virtio_gpu.py
> +++ b/tests/functional/x86_64/test_virtio_gpu.py
> @@ -5,22 +5,23 @@
>   # 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 os
> +import socket
> +import subprocess
>   
>   from qemu_test import QemuSystemTest, Asset
>   from qemu_test import wait_for_console_pattern
>   from qemu_test import exec_command_and_wait_for_pattern
>   from qemu_test import is_readable_executable_file
>   
> -
> -import os
> -import socket
> -import subprocess
> +from qemu.machine.machine import VMLaunchFailure
>   
>   
>   def pick_default_vug_bin(test):
>       bld_dir_path = test.build_file("contrib", "vhost-user-gpu", "vhost-user-gpu")
>       if is_readable_executable_file(bld_dir_path):
>           return bld_dir_path
> +    return None
>   
>   
>   class VirtioGPUx86(QemuSystemTest):
> @@ -46,8 +47,8 @@ def wait_for_console_pattern(self, success_message, vm=None):
>           )
>   
>       def test_virtio_vga_virgl(self):
> -        # FIXME: should check presence of virtio, virgl etc
>           self.require_accelerator('kvm')
> +        self.require_device('virtio-vga-gl')
>   
>           kernel_path = self.ASSET_KERNEL.fetch()
>           initrd_path = self.ASSET_INITRD.fetch()
> @@ -68,7 +69,7 @@ def test_virtio_vga_virgl(self):
>           )
>           try:
>               self.vm.launch()
> -        except:
> +        except VMLaunchFailure:
>               # TODO: probably fails because we are missing the VirGL features
>               self.skipTest("VirGL not enabled?")
>   
> @@ -78,8 +79,8 @@ def test_virtio_vga_virgl(self):
>           )
>   
>       def test_vhost_user_vga_virgl(self):
> -        # FIXME: should check presence of vhost-user-gpu, virgl, memfd etc
>           self.require_accelerator('kvm')
> +        self.require_device('vhost-user-vga')
>   
>           vug = pick_default_vug_bin(self)
>           if not vug:
> @@ -95,27 +96,30 @@ def test_vhost_user_vga_virgl(self):
>           os.set_inheritable(qemu_sock.fileno(), True)
>           os.set_inheritable(vug_sock.fileno(), True)
>   
> -        self._vug_log_path = self.log_file("vhost-user-gpu.log")
> -        self._vug_log_file = open(self._vug_log_path, "wb")
> -        self.log.info('Complete vhost-user-gpu.log file can be '
> -                      'found at %s', self._vug_log_path)
> -
> -        vugp = subprocess.Popen(
> -            [vug, "--virgl", "--fd=%d" % vug_sock.fileno()],
> -            stdin=subprocess.DEVNULL,
> -            stdout=self._vug_log_file,
> -            stderr=subprocess.STDOUT,
> -            shell=False,
> -            close_fds=False,
> -        )
> -        self._vug_log_file.close()
> -
> +        vug_log_path = self.log_file("vhost-user-gpu.log")
> +        self.log.info('Complete vhost-user-gpu.log file can be found at %s',
> +                      vug_log_path)
> +        with open(vug_log_path, "wb") as vug_log_file:
> +            with subprocess.Popen([vug, "--virgl", f"--fd={vug_sock.fileno()}"],
> +                                  stdin=subprocess.DEVNULL,
> +                                  stdout=vug_log_file,
> +                                  stderr=subprocess.STDOUT,
> +                                  shell=False,
> +                                  close_fds=False) as vugp:
> +                 self._test_vhost_user_vga_virgl(vug, qemu_sock,
> +                                                kernel_path, initrd_path)
> +                 qemu_sock.close()
> +                 vug_sock.close()
> +                 vugp.terminate()
> +
> +    def _test_vhost_user_vga_virgl(self, vug, qemu_sock,
> +                                                kernel_path, initrd_path):

Never mind, this causes another new pylint warning about an unused parameter 
... I'll send a v3... sorry for the fuzz.

  Thomas