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

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

[Thomas Huth]

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>
---
 v3: Fix stupid issue with unused parameter

 tests/functional/x86_64/test_virtio_gpu.py | 56 +++++++++++-----------
 1 file changed, 28 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..58b0f72ba42 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,29 @@ 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(qemu_sock,
+                                                kernel_path, initrd_path)
+                qemu_sock.close()
+                vug_sock.close()
+                vugp.terminate()
+
+    def _test_vhost_user_vga_virgl(self, qemu_sock, kernel_path, initrd_path):
         self.vm.set_console()
         self.vm.add_args("-cpu", "host")
         self.vm.add_args("-m", "2G")
         self.vm.add_args("-object", "memory-backend-memfd,id=mem,size=2G")
         self.vm.add_args("-machine", "pc,memory-backend=mem,accel=kvm")
-        self.vm.add_args("-chardev", "socket,id=vug,fd=%d" % qemu_sock.fileno())
+        self.vm.add_args("-chardev", f"socket,id=vug,fd={qemu_sock.fileno()}")
         self.vm.add_args("-device", "vhost-user-vga,chardev=vug")
         self.vm.add_args("-display", "egl-headless")
         self.vm.add_args(
@@ -128,17 +131,14 @@ def test_vhost_user_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?")
         self.wait_for_console_pattern("as init process")
         exec_command_and_wait_for_pattern(self, "/usr/sbin/modprobe virtio_gpu",
                                           "features: +virgl +edid")
         self.vm.shutdown()
-        qemu_sock.close()
-        vug_sock.close()
-        vugp.terminate()
-        vugp.wait()
+
 
 if __name__ == '__main__':
     QemuSystemTest.main()
-- 
2.51.1

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

[Akihiko Odaki]

On 2025/11/13 20:40, 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>

Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

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

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

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

Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro