* [PATCH v2 01/18] tests/functional: fix mips64el test to honour workdir
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 02/18] tests/functional: automatically clean up scratch files after tests Daniel P. Berrangé
` (17 subsequent siblings)
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
The missing directory separator resulted in the kernel file being
created 1 level higher than expected.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/functional/test_mips64el_malta.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/functional/test_mips64el_malta.py b/tests/functional/test_mips64el_malta.py
index 6c6355b131..24ebcdb9c1 100755
--- a/tests/functional/test_mips64el_malta.py
+++ b/tests/functional/test_mips64el_malta.py
@@ -129,7 +129,7 @@ def do_test_i6400_framebuffer_logo(self, cpu_cores_count):
screendump_path = os.path.join(self.workdir, 'screendump.pbm')
kernel_path_gz = self.ASSET_KERNEL_4_7_0.fetch()
- kernel_path = self.workdir + "vmlinux"
+ kernel_path = self.workdir + "/vmlinux"
gzip_uncompress(kernel_path_gz, kernel_path)
tuxlogo_path = self.ASSET_TUXLOGO.fetch()
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 02/18] tests/functional: automatically clean up scratch files after tests
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 01/18] tests/functional: fix mips64el test to honour workdir Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 03/18] tests/functional: remove "AVOCADO" from env variable name Daniel P. Berrangé
` (16 subsequent siblings)
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
The build/tests/functional subdirectories are consuming huge amounts
of disk space.
Split the location for scratch files into a 'scratch' sub-directory,
separate from log files, and delete it upon completion of each test.
The new env variable QEMU_TEST_KEEP_SCRATCH can be set to preserve
this scratch dir for debugging access if required.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
docs/devel/testing/functional.rst | 6 ++++++
tests/functional/qemu_test/testcase.py | 14 +++++++++-----
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/docs/devel/testing/functional.rst b/docs/devel/testing/functional.rst
index bf6f1bb81e..6b5d0c5b98 100644
--- a/docs/devel/testing/functional.rst
+++ b/docs/devel/testing/functional.rst
@@ -65,6 +65,12 @@ to the QEMU binary that should be used for the test, for example::
$ export QEMU_TEST_QEMU_BINARY=$PWD/qemu-system-x86_64
$ python3 ../tests/functional/test_file.py
+The test framework will automatically purge any scratch files created during
+the tests. If needing to debug a failed test, it is possible to keep these
+files around on disk by setting ```QEMU_TEST_KEEP_SCRATCH=1``` as an env
+variable. Any preserved files will be deleted the next time the test is run
+without this variable set.
+
Overview
--------
diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
index 411978b5ef..b9418e2ac0 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -13,8 +13,9 @@
import logging
import os
-import subprocess
import pycotap
+import shutil
+import subprocess
import sys
import unittest
import uuid
@@ -40,11 +41,12 @@ def setUp(self, bin_prefix):
self.assertIsNotNone(self.qemu_bin, 'QEMU_TEST_QEMU_BINARY must be set')
self.arch = self.qemu_bin.split('-')[-1]
- self.workdir = os.path.join(BUILD_DIR, 'tests/functional', self.arch,
- self.id())
+ self.outputdir = os.path.join(BUILD_DIR, 'tests', 'functional',
+ self.arch, self.id())
+ self.workdir = os.path.join(self.outputdir, 'scratch')
os.makedirs(self.workdir, exist_ok=True)
- self.logdir = self.workdir
+ self.logdir = self.outputdir
self.log_filename = os.path.join(self.logdir, 'base.log')
self.log = logging.getLogger('qemu-test')
self.log.setLevel(logging.DEBUG)
@@ -56,6 +58,8 @@ def setUp(self, bin_prefix):
self.log.addHandler(self._log_fh)
def tearDown(self):
+ if "QEMU_TEST_KEEP_SCRATCH" not in os.environ:
+ shutil.rmtree(self.workdir)
self.log.removeHandler(self._log_fh)
def main():
@@ -108,7 +112,7 @@ def setUp(self):
console_log = logging.getLogger('console')
console_log.setLevel(logging.DEBUG)
- self.console_log_name = os.path.join(self.workdir, 'console.log')
+ self.console_log_name = os.path.join(self.logdir, 'console.log')
self._console_log_fh = logging.FileHandler(self.console_log_name,
mode='w')
self._console_log_fh.setLevel(logging.DEBUG)
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 03/18] tests/functional: remove "AVOCADO" from env variable name
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 01/18] tests/functional: fix mips64el test to honour workdir Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 02/18] tests/functional: automatically clean up scratch files after tests Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 04/18] tests/functional: remove todo wrt avocado.utils.wait_for Daniel P. Berrangé
` (15 subsequent siblings)
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
This env variable is a debugging flag to save screendumps in the
mips64el malta tests.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/functional/test_mips64el_malta.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/functional/test_mips64el_malta.py b/tests/functional/test_mips64el_malta.py
index 24ebcdb9c1..6d1195d362 100755
--- a/tests/functional/test_mips64el_malta.py
+++ b/tests/functional/test_mips64el_malta.py
@@ -159,7 +159,7 @@ def do_test_i6400_framebuffer_logo(self, cpu_cores_count):
loc = np.where(result >= match_threshold)
tuxlogo_count = 0
h, w = tuxlogo_bgr.shape[:2]
- debug_png = os.getenv('AVOCADO_CV2_SCREENDUMP_PNG_PATH')
+ debug_png = os.getenv('QEMU_TEST_CV2_SCREENDUMP_PNG_PATH')
for tuxlogo_count, pt in enumerate(zip(*loc[::-1]), start=1):
logger.debug('found Tux at position (x, y) = %s', pt)
cv2.rectangle(screendump_bgr, pt,
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 04/18] tests/functional: remove todo wrt avocado.utils.wait_for
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
` (2 preceding siblings ...)
2024-11-21 15:42 ` [PATCH v2 03/18] tests/functional: remove "AVOCADO" from env variable name Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 05/18] tests/functional: remove leftover :avocado: tags Daniel P. Berrangé
` (14 subsequent siblings)
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
We're not using avocado anymore, so while the TODO item is still
relevant, suggesting use of avocado.utils is not.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/functional/test_m68k_nextcube.py | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/functional/test_m68k_nextcube.py b/tests/functional/test_m68k_nextcube.py
index 89385a134a..0124622c40 100755
--- a/tests/functional/test_m68k_nextcube.py
+++ b/tests/functional/test_m68k_nextcube.py
@@ -37,8 +37,7 @@ def check_bootrom_framebuffer(self, screenshot_path):
self.vm.launch()
self.log.info('VM launched, waiting for display')
- # TODO: Use avocado.utils.wait.wait_for to catch the
- # 'displaysurface_create 1120x832' trace-event.
+ # TODO: wait for the 'displaysurface_create 1120x832' trace-event.
time.sleep(2)
self.vm.cmd('human-monitor-command',
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 05/18] tests/functional: remove leftover :avocado: tags
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
` (3 preceding siblings ...)
2024-11-21 15:42 ` [PATCH v2 04/18] tests/functional: remove todo wrt avocado.utils.wait_for Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 06/18] tests/functional: remove obsolete reference to avocado bug Daniel P. Berrangé
` (13 subsequent siblings)
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
These tags are not honoured under the new functional test harness.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/functional/test_arm_bpim2u.py | 20 --------------------
tests/functional/test_arm_orangepi.py | 27 ---------------------------
2 files changed, 47 deletions(-)
diff --git a/tests/functional/test_arm_bpim2u.py b/tests/functional/test_arm_bpim2u.py
index 2f9fa145e3..35ea58d46c 100755
--- a/tests/functional/test_arm_bpim2u.py
+++ b/tests/functional/test_arm_bpim2u.py
@@ -37,11 +37,6 @@ class BananaPiMachine(LinuxKernelTest):
'5b41b4e11423e562c6011640f9a7cd3bdd0a3d42b83430f7caa70a432e6cd82c')
def test_arm_bpim2u(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=machine:bpim2u
- :avocado: tags=accel:tcg
- """
self.set_machine('bpim2u')
deb_path = self.ASSET_DEB.fetch()
kernel_path = self.extract_from_deb(deb_path,
@@ -64,11 +59,6 @@ def test_arm_bpim2u(self):
os.remove(dtb_path)
def test_arm_bpim2u_initrd(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=accel:tcg
- :avocado: tags=machine:bpim2u
- """
self.set_machine('bpim2u')
deb_path = self.ASSET_DEB.fetch()
kernel_path = self.extract_from_deb(deb_path,
@@ -105,11 +95,6 @@ def test_arm_bpim2u_initrd(self):
os.remove(initrd_path)
def test_arm_bpim2u_gmac(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=machine:bpim2u
- :avocado: tags=device:sd
- """
self.set_machine('bpim2u')
self.require_netdev('user')
@@ -160,11 +145,6 @@ def test_arm_bpim2u_gmac(self):
@skipUnless(os.getenv('QEMU_TEST_ALLOW_LARGE_STORAGE'), 'storage limited')
def test_arm_bpim2u_openwrt_22_03_3(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=machine:bpim2u
- :avocado: tags=device:sd
- """
self.set_machine('bpim2u')
# This test download a 8.9 MiB compressed image and expand it
# to 127 MiB.
diff --git a/tests/functional/test_arm_orangepi.py b/tests/functional/test_arm_orangepi.py
index d2ed5fcc82..6d57223a03 100755
--- a/tests/functional/test_arm_orangepi.py
+++ b/tests/functional/test_arm_orangepi.py
@@ -49,11 +49,6 @@ class BananaPiMachine(LinuxKernelTest):
'20d3e07dc057e15c12452620e90ecab2047f0f7940d9cba8182ebc795927177f')
def test_arm_orangepi(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=machine:orangepi-pc
- :avocado: tags=accel:tcg
- """
self.set_machine('orangepi-pc')
deb_path = self.ASSET_DEB.fetch()
kernel_path = self.extract_from_deb(deb_path,
@@ -75,11 +70,6 @@ def test_arm_orangepi(self):
os.remove(dtb_path)
def test_arm_orangepi_initrd(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=accel:tcg
- :avocado: tags=machine:orangepi-pc
- """
self.set_machine('orangepi-pc')
deb_path = self.ASSET_DEB.fetch()
kernel_path = self.extract_from_deb(deb_path,
@@ -115,12 +105,6 @@ def test_arm_orangepi_initrd(self):
os.remove(initrd_path)
def test_arm_orangepi_sd(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=accel:tcg
- :avocado: tags=machine:orangepi-pc
- :avocado: tags=device:sd
- """
self.set_machine('orangepi-pc')
self.require_netdev('user')
deb_path = self.ASSET_DEB.fetch()
@@ -167,11 +151,6 @@ def test_arm_orangepi_sd(self):
@skipUnless(os.getenv('QEMU_TEST_ALLOW_LARGE_STORAGE'), 'storage limited')
def test_arm_orangepi_armbian(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=machine:orangepi-pc
- :avocado: tags=device:sd
- """
self.set_machine('orangepi-pc')
# This test download a 275 MiB compressed image and expand it
# to 1036 MiB, but the underlying filesystem is 1552 MiB...
@@ -208,12 +187,6 @@ def test_arm_orangepi_armbian(self):
@skipUnless(os.getenv('QEMU_TEST_ALLOW_LARGE_STORAGE'), 'storage limited')
def test_arm_orangepi_uboot_netbsd9(self):
- """
- :avocado: tags=arch:arm
- :avocado: tags=machine:orangepi-pc
- :avocado: tags=device:sd
- :avocado: tags=os:netbsd
- """
self.set_machine('orangepi-pc')
# This test download a 304MB compressed image and expand it to 2GB
deb_path = self.ASSET_UBOOT.fetch()
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 06/18] tests/functional: remove obsolete reference to avocado bug
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
` (4 preceding siblings ...)
2024-11-21 15:42 ` [PATCH v2 05/18] tests/functional: remove leftover :avocado: tags Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 07/18] tests/functional: remove comments talking about avocado Daniel P. Berrangé
` (12 subsequent siblings)
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
Historical bugs in avocado related to zstd support are not relevant to
the code now that it uses QEMU's native test harness.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/functional/qemu_test/tuxruntest.py | 1 -
1 file changed, 1 deletion(-)
diff --git a/tests/functional/qemu_test/tuxruntest.py b/tests/functional/qemu_test/tuxruntest.py
index f05aa96ad7..ed2b238c92 100644
--- a/tests/functional/qemu_test/tuxruntest.py
+++ b/tests/functional/qemu_test/tuxruntest.py
@@ -39,7 +39,6 @@ def setUp(self):
super().setUp()
# We need zstd for all the tuxrun tests
- # See https://github.com/avocado-framework/avocado/issues/5609
(has_zstd, msg) = has_cmd('zstd')
if has_zstd is False:
self.skipTest(msg)
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 07/18] tests/functional: remove comments talking about avocado
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
` (5 preceding siblings ...)
2024-11-21 15:42 ` [PATCH v2 06/18] tests/functional: remove obsolete reference to avocado bug Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 08/18] tests/functional: honour self.workdir in ACPI bits tests Daniel P. Berrangé
` (11 subsequent siblings)
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
The first comment is still relevant but should talk about our own test
harness instead. The second comment adds no value over reading the code
and can be removed.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/functional/test_acpi_bits.py | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/tests/functional/test_acpi_bits.py b/tests/functional/test_acpi_bits.py
index ee40647d5b..4c192d95cc 100755
--- a/tests/functional/test_acpi_bits.py
+++ b/tests/functional/test_acpi_bits.py
@@ -196,11 +196,12 @@ def copy_test_scripts(self):
for filename in os.listdir(bits_test_dir):
if os.path.isfile(os.path.join(bits_test_dir, filename)) and \
filename.endswith('.py2'):
- # all test scripts are named with extension .py2 so that
- # avocado does not try to load them. These scripts are
- # written for python 2.7 not python 3 and hence if avocado
- # loaded them, it would complain about python 3 specific
- # syntaxes.
+ # All test scripts are named with extension .py2 so that
+ # they are not run by accident.
+ #
+ # These scripts are intended to run inside the test VM
+ # and are written for python 2.7 not python 3, hence
+ # would cause syntax errors if loaded ouside the VM.
newfilename = os.path.splitext(filename)[0] + '.py'
shutil.copy2(os.path.join(bits_test_dir, filename),
os.path.join(target_test_dir, newfilename))
@@ -399,8 +400,6 @@ def test_acpi_smbios_bits(self):
# biosbits has been configured to run all the specified test suites
# in batch mode and then automatically initiate a vm shutdown.
- # Set timeout to BITS_TIMEOUT for SHUTDOWN event from bits VM at par
- # with the avocado test timeout.
self._vm.event_wait('SHUTDOWN', timeout=BITS_TIMEOUT)
self._vm.wait(timeout=None)
self.logger.debug("Checking console output ...")
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 08/18] tests/functional: honour self.workdir in ACPI bits tests
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
` (6 preceding siblings ...)
2024-11-21 15:42 ` [PATCH v2 07/18] tests/functional: remove comments talking about avocado Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 09/18] tests/functional: put QEMUMachine logs in testcase log directory Daniel P. Berrangé
` (10 subsequent siblings)
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
The ACPI bits test sets up its own private temporary directory into it
creates scratch files. This is justified by a suggestion that we need
to be able to preserve the scratch files. We have the ability to
preserve the scratch dir with our functional harness, so there's no
reason to diverge from standard practice in file placement.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/functional/test_acpi_bits.py | 44 +++++++++---------------------
1 file changed, 13 insertions(+), 31 deletions(-)
diff --git a/tests/functional/test_acpi_bits.py b/tests/functional/test_acpi_bits.py
index 4c192d95cc..3498b96787 100755
--- a/tests/functional/test_acpi_bits.py
+++ b/tests/functional/test_acpi_bits.py
@@ -150,7 +150,6 @@ class AcpiBitsTest(QemuBaseTest): #pylint: disable=too-many-instance-attributes
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self._vm = None
- self._workDir = None
self._baseDir = None
self._debugcon_addr = '0x403'
@@ -169,7 +168,7 @@ def copy_bits_config(self):
config_file = 'bits-cfg.txt'
bits_config_dir = os.path.join(self._baseDir, 'acpi-bits',
'bits-config')
- target_config_dir = os.path.join(self._workDir,
+ target_config_dir = os.path.join(self.workdir,
'bits-%d' %self.BITS_INTERNAL_VER,
'boot')
self.assertTrue(os.path.exists(bits_config_dir))
@@ -186,7 +185,7 @@ def copy_test_scripts(self):
bits_test_dir = os.path.join(self._baseDir, 'acpi-bits',
'bits-tests')
- target_test_dir = os.path.join(self._workDir,
+ target_test_dir = os.path.join(self.workdir,
'bits-%d' %self.BITS_INTERNAL_VER,
'boot', 'python')
@@ -225,8 +224,8 @@ def fix_mkrescue(self, mkrescue):
the directory where we have extracted our pre-built bits grub
tarball.
"""
- grub_x86_64_mods = os.path.join(self._workDir, 'grub-inst-x86_64-efi')
- grub_i386_mods = os.path.join(self._workDir, 'grub-inst')
+ grub_x86_64_mods = os.path.join(self.workdir, 'grub-inst-x86_64-efi')
+ grub_i386_mods = os.path.join(self.workdir, 'grub-inst')
self.assertTrue(os.path.exists(grub_x86_64_mods))
self.assertTrue(os.path.exists(grub_i386_mods))
@@ -247,11 +246,11 @@ def generate_bits_iso(self):
""" Uses grub-mkrescue to generate a fresh bits iso with the python
test scripts
"""
- bits_dir = os.path.join(self._workDir,
+ bits_dir = os.path.join(self.workdir,
'bits-%d' %self.BITS_INTERNAL_VER)
- iso_file = os.path.join(self._workDir,
+ iso_file = os.path.join(self.workdir,
'bits-%d.iso' %self.BITS_INTERNAL_VER)
- mkrescue_script = os.path.join(self._workDir,
+ mkrescue_script = os.path.join(self.workdir,
'grub-inst-x86_64-efi', 'bin',
'grub-mkrescue')
@@ -290,17 +289,7 @@ def setUp(self): # pylint: disable=arguments-differ
self._baseDir = Path(__file__).parent
- # workdir could also be avocado's own workdir in self.workdir.
- # At present, I prefer to maintain my own temporary working
- # directory. It gives us more control over the generated bits
- # log files and also for debugging, we may chose not to remove
- # this working directory so that the logs and iso can be
- # inspected manually and archived if needed.
- self._workDir = tempfile.mkdtemp(prefix='acpi-bits-',
- suffix='.tmp')
- self.logger.info('working dir: %s', self._workDir)
-
- prebuiltDir = os.path.join(self._workDir, 'prebuilt')
+ prebuiltDir = os.path.join(self.workdir, 'prebuilt')
if not os.path.isdir(prebuiltDir):
os.mkdir(prebuiltDir, mode=0o775)
@@ -321,10 +310,10 @@ def setUp(self): # pylint: disable=arguments-differ
# extract the bits software in the temp working directory
with zipfile.ZipFile(bits_zip_file, 'r') as zref:
- zref.extractall(self._workDir)
+ zref.extractall(self.workdir)
with tarfile.open(grub_tar_file, 'r', encoding='utf-8') as tarball:
- tarball.extractall(self._workDir)
+ tarball.extractall(self.workdir)
self.copy_test_scripts()
self.copy_bits_config()
@@ -334,7 +323,7 @@ def parse_log(self):
"""parse the log generated by running bits tests and
check for failures.
"""
- debugconf = os.path.join(self._workDir, self._debugcon_log)
+ debugconf = os.path.join(self.workdir, self._debugcon_log)
log = ""
with open(debugconf, 'r', encoding='utf-8') as filehandle:
log = filehandle.read()
@@ -360,25 +349,18 @@ def tearDown(self):
"""
if self._vm:
self.assertFalse(not self._vm.is_running)
- if not os.getenv('BITS_DEBUG') and self._workDir:
- self.logger.info('removing the work directory %s', self._workDir)
- shutil.rmtree(self._workDir)
- else:
- self.logger.info('not removing the work directory %s ' \
- 'as BITS_DEBUG is ' \
- 'passed in the environment', self._workDir)
super().tearDown()
def test_acpi_smbios_bits(self):
"""The main test case implementation."""
- iso_file = os.path.join(self._workDir,
+ iso_file = os.path.join(self.workdir,
'bits-%d.iso' %self.BITS_INTERNAL_VER)
self.assertTrue(os.access(iso_file, os.R_OK))
self._vm = QEMUBitsMachine(binary=self.qemu_bin,
- base_temp_dir=self._workDir,
+ base_temp_dir=self.workdir,
debugcon_log=self._debugcon_log,
debugcon_addr=self._debugcon_addr)
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 09/18] tests/functional: put QEMUMachine logs in testcase log directory
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
` (7 preceding siblings ...)
2024-11-21 15:42 ` [PATCH v2 08/18] tests/functional: honour self.workdir in ACPI bits tests Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 10/18] tests/functional: honour requested test VM name in QEMUMachine Daniel P. Berrangé
` (9 subsequent siblings)
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
We are not passing the 'log_dir' parameter to QEMUMachine, so the
QEMU stdout/err logs are being placed in a temp directory and thus
deleted after execution. This makes them inaccessible as gitlab
CI artifacts.
Pass the testcase log directory path into QEMUMachine to make the
logs persistent.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/functional/qemu_test/testcase.py | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
index b9418e2ac0..ca13af244b 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -163,10 +163,11 @@ def require_device(self, devicename):
self.skipTest('no support for device ' + devicename)
def _new_vm(self, name, *args):
- vm = QEMUMachine(self.qemu_bin, base_temp_dir=self.workdir)
+ vm = QEMUMachine(self.qemu_bin,
+ base_temp_dir=self.workdir,
+ log_dir=self.logdir)
self.log.debug('QEMUMachine "%s" created', name)
self.log.debug('QEMUMachine "%s" temp_dir: %s', name, vm.temp_dir)
- self.log.debug('QEMUMachine "%s" log_dir: %s', name, vm.log_dir)
if args:
vm.add_args(*args)
return vm
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 10/18] tests/functional: honour requested test VM name in QEMUMachine
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
` (8 preceding siblings ...)
2024-11-21 15:42 ` [PATCH v2 09/18] tests/functional: put QEMUMachine logs in testcase log directory Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 11/18] tests/functional: enable debug logging for QEMUMachine Daniel P. Berrangé
` (8 subsequent siblings)
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
The functional test case class is going to the trouble of passing
around a machine name, but then fails to give this QEMUMachine. As
a result, QEMUMachine will create a completely random name. Since
log file names match the machine name, this results in log files
accumulating over time.
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/functional/qemu_test/testcase.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
index ca13af244b..f9c9de1166 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -164,6 +164,7 @@ def require_device(self, devicename):
def _new_vm(self, name, *args):
vm = QEMUMachine(self.qemu_bin,
+ name=name,
base_temp_dir=self.workdir,
log_dir=self.logdir)
self.log.debug('QEMUMachine "%s" created', name)
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 11/18] tests/functional: enable debug logging for QEMUMachine
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
` (9 preceding siblings ...)
2024-11-21 15:42 ` [PATCH v2 10/18] tests/functional: honour requested test VM name in QEMUMachine Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 19:01 ` Thomas Huth
2024-11-21 15:42 ` [PATCH v2 12/18] tests/functional: logs details of console interaction operations Daniel P. Berrangé
` (7 subsequent siblings)
18 siblings, 1 reply; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
Set the 'qemu.machine' logger to 'DEBUG' level, to ensure we see log
messages related to the QEMUMachine class. Most importantly this
ensures we capture the full QEMU command line args for instances we
spawn.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/functional/qemu_test/testcase.py | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
index f9c9de1166..e2a329c3e5 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -57,9 +57,15 @@ def setUp(self, bin_prefix):
self._log_fh.setFormatter(fileFormatter)
self.log.addHandler(self._log_fh)
+ # Capture QEMUMachine logging
+ self.machinelog = logging.getLogger('qemu.machine')
+ self.machinelog.setLevel(logging.DEBUG)
+ self.machinelog.addHandler(self._log_fh)
+
def tearDown(self):
if "QEMU_TEST_KEEP_SCRATCH" not in os.environ:
shutil.rmtree(self.workdir)
+ self.machinelog.removeHandler(self._log_fh)
self.log.removeHandler(self._log_fh)
def main():
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 11/18] tests/functional: enable debug logging for QEMUMachine
2024-11-21 15:42 ` [PATCH v2 11/18] tests/functional: enable debug logging for QEMUMachine Daniel P. Berrangé
@ 2024-11-21 19:01 ` Thomas Huth
0 siblings, 0 replies; 21+ messages in thread
From: Thomas Huth @ 2024-11-21 19:01 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé
On 21/11/2024 16.42, Daniel P. Berrangé wrote:
> Set the 'qemu.machine' logger to 'DEBUG' level, to ensure we see log
> messages related to the QEMUMachine class. Most importantly this
> ensures we capture the full QEMU command line args for instances we
> spawn.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/functional/qemu_test/testcase.py | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
> index f9c9de1166..e2a329c3e5 100644
> --- a/tests/functional/qemu_test/testcase.py
> +++ b/tests/functional/qemu_test/testcase.py
> @@ -57,9 +57,15 @@ def setUp(self, bin_prefix):
> self._log_fh.setFormatter(fileFormatter)
> self.log.addHandler(self._log_fh)
>
> + # Capture QEMUMachine logging
> + self.machinelog = logging.getLogger('qemu.machine')
> + self.machinelog.setLevel(logging.DEBUG)
> + self.machinelog.addHandler(self._log_fh)
> +
> def tearDown(self):
> if "QEMU_TEST_KEEP_SCRATCH" not in os.environ:
> shutil.rmtree(self.workdir)
> + self.machinelog.removeHandler(self._log_fh)
> self.log.removeHandler(self._log_fh)
>
> def main():
Tested-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 12/18] tests/functional: logs details of console interaction operations
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
` (10 preceding siblings ...)
2024-11-21 15:42 ` [PATCH v2 11/18] tests/functional: enable debug logging for QEMUMachine Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 13/18] tests/functional: don't try to wait for the empty string Daniel P. Berrangé
` (6 subsequent siblings)
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
When functional tests go wrong, it will often be related to the console
interaction wait state. By logging the messages that we're looking for,
and data we're about to be sending, it'll be easier to diagnose where
tests are getting stuck.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/functional/qemu_test/cmd.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/functional/qemu_test/cmd.py b/tests/functional/qemu_test/cmd.py
index cbabb1ceed..98722a9cf6 100644
--- a/tests/functional/qemu_test/cmd.py
+++ b/tests/functional/qemu_test/cmd.py
@@ -85,6 +85,9 @@ def _console_interaction(test, success_message, failure_message,
vm = test.vm
console = vm.console_file
console_logger = logging.getLogger('console')
+ test.log.debug(
+ f"Console interaction: success_msg='{success_message}' " +
+ f"failure_msg='{failure_message}' send_string='{send_string}'")
while True:
if send_string:
vm.console_socket.sendall(send_string.encode())
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 13/18] tests/functional: don't try to wait for the empty string
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
` (11 preceding siblings ...)
2024-11-21 15:42 ` [PATCH v2 12/18] tests/functional: logs details of console interaction operations Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 14/18] tests/functional: require non-NULL success_message for console wait Daniel P. Berrangé
` (5 subsequent siblings)
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
Telling exec_command_wand_wait_for_pattern to wait for the empty
string does not make any conceptual sense, as a check for empty
string will always succeed. It makes even less sense when followed
by a call to wait_for_console_pattern() with a real match.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/functional/test_virtio_gpu.py | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/functional/test_virtio_gpu.py b/tests/functional/test_virtio_gpu.py
index 441cbdcf2d..d5027487ac 100755
--- a/tests/functional/test_virtio_gpu.py
+++ b/tests/functional/test_virtio_gpu.py
@@ -80,9 +80,8 @@ def test_virtio_vga_virgl(self):
self.wait_for_console_pattern("as init process")
exec_command_and_wait_for_pattern(
- self, "/usr/sbin/modprobe virtio_gpu", ""
+ self, "/usr/sbin/modprobe virtio_gpu", "features: +virgl +edid"
)
- self.wait_for_console_pattern("features: +virgl +edid")
def test_vhost_user_vga_virgl(self):
# FIXME: should check presence of vhost-user-gpu, virgl, memfd etc
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 14/18] tests/functional: require non-NULL success_message for console wait
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
` (12 preceding siblings ...)
2024-11-21 15:42 ` [PATCH v2 13/18] tests/functional: don't try to wait for the empty string Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 15/18] tests/functional: rewrite console handling to be bytewise Daniel P. Berrangé
` (4 subsequent siblings)
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
When waiting for expected output, the 'success_message' is a mandatory
parameter, with 'failure_message' defaulting to None.
The code has logic which indicates it was trying to cope with
'success_message' being None and 'failure_message' being non-None but
it does not appear able to actually do anything useful. The check for
'success_message is None' will break out of the loop before any check
for 'failure_message' has been performed.
IOW, for practcal purposes 'success_message' must be non-None unless
'send_string' is set. Assert this expectation and simplify the loop
logic.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/functional/qemu_test/cmd.py | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tests/functional/qemu_test/cmd.py b/tests/functional/qemu_test/cmd.py
index 98722a9cf6..f6c4e4dda1 100644
--- a/tests/functional/qemu_test/cmd.py
+++ b/tests/functional/qemu_test/cmd.py
@@ -81,6 +81,8 @@ def is_readable_executable_file(path):
def _console_interaction(test, success_message, failure_message,
send_string, keep_sending=False, vm=None):
assert not keep_sending or send_string
+ assert success_message or send_string
+
if vm is None:
vm = test.vm
console = vm.console_file
@@ -95,7 +97,7 @@ def _console_interaction(test, success_message, failure_message,
send_string = None # send only once
# Only consume console output if waiting for something
- if success_message is None and failure_message is None:
+ if success_message is None:
if send_string is None:
break
continue
@@ -107,7 +109,7 @@ def _console_interaction(test, success_message, failure_message,
if not msg:
continue
console_logger.debug(msg)
- if success_message is None or success_message in msg:
+ if success_message in msg:
break
if failure_message and failure_message in msg:
console.close()
@@ -138,6 +140,7 @@ def interrupt_interactive_console_until_pattern(test, success_message,
:param interrupt_string: a string to send to the console before trying
to read a new line
"""
+ assert success_message
_console_interaction(test, success_message, failure_message,
interrupt_string, True)
@@ -152,6 +155,7 @@ def wait_for_console_pattern(test, success_message, failure_message=None,
:param success_message: if this message appears, test succeeds
:param failure_message: if this message appears, test fails
"""
+ assert success_message
_console_interaction(test, success_message, failure_message, None, vm=vm)
def exec_command(test, command):
@@ -180,6 +184,7 @@ def exec_command_and_wait_for_pattern(test, command,
:param success_message: if this message appears, test succeeds
:param failure_message: if this message appears, test fails
"""
+ assert success_message
_console_interaction(test, success_message, failure_message, command + '\r')
def get_qemu_img(test):
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 15/18] tests/functional: rewrite console handling to be bytewise
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
` (13 preceding siblings ...)
2024-11-21 15:42 ` [PATCH v2 14/18] tests/functional: require non-NULL success_message for console wait Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 16/18] tests/functional: remove time.sleep usage from tuxrun tests Daniel P. Berrangé
` (3 subsequent siblings)
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé,
Cédric Le Goater
The console interaction that waits for predicted strings uses
readline(), and thus is only capable of waiting for strings
that are followed by a newline.
This is inconvenient when needing to match on some things,
particularly login prompts, or shell prompts, causing tests
to use time.sleep(...) instead, which is unreliable.
Switch to reading the console 1 byte at a time, comparing
against the success/failure messages until we see a match,
regardless of whether a newline is encountered.
The success/failure comparisons are done with the python bytes
type, rather than strings, to avoid the problem of needing to
decode partially received multibyte utf8 characters.
Heavily inspired by a patch proposed by Cédric, but written
again to work in bytes, rather than strings.
Co-developed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/functional/qemu_test/cmd.py | 79 +++++++++++++++++++++++++------
1 file changed, 64 insertions(+), 15 deletions(-)
diff --git a/tests/functional/qemu_test/cmd.py b/tests/functional/qemu_test/cmd.py
index f6c4e4dda1..11c8334a7c 100644
--- a/tests/functional/qemu_test/cmd.py
+++ b/tests/functional/qemu_test/cmd.py
@@ -78,6 +78,54 @@ def run_cmd(args):
def is_readable_executable_file(path):
return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
+# @test: functional test to fail if @failure is seen
+# @vm: the VM whose console to process
+# @success: a non-None string to look for
+# @failure: a string to look for that triggers test failure, or None
+#
+# Read up to 1 line of text from @vm, looking for @success
+# and optionally @failure.
+#
+# If @success or @failure are seen, immediately return True,
+# even if end of line is not yet seen. ie remainder of the
+# line is left unread.
+#
+# If end of line is seen, with neither @success or @failure
+# return False
+#
+# If @failure is seen, then mark @test as failed
+def _console_read_line_until_match(test, vm, success, failure):
+ msg = bytes([])
+ done = False
+ while True:
+ c = vm.console_socket.recv(1)
+ if c is None:
+ done = True
+ test.fail(
+ f"EOF in console, expected '{success}'")
+ break
+ msg += c
+
+ if success in msg:
+ done = True
+ break
+ if failure and failure in msg:
+ done = True
+ vm.console_socket.close()
+ test.fail(
+ f"'{failure}' found in console, expected '{success}'")
+
+ if c == b'\n':
+ break
+
+ console_logger = logging.getLogger('console')
+ try:
+ console_logger.debug(msg.decode().strip())
+ except:
+ console_logger.debug(msg)
+
+ return done
+
def _console_interaction(test, success_message, failure_message,
send_string, keep_sending=False, vm=None):
assert not keep_sending or send_string
@@ -85,11 +133,22 @@ def _console_interaction(test, success_message, failure_message,
if vm is None:
vm = test.vm
- console = vm.console_file
- console_logger = logging.getLogger('console')
+
test.log.debug(
f"Console interaction: success_msg='{success_message}' " +
f"failure_msg='{failure_message}' send_string='{send_string}'")
+
+ # We'll process console in bytes, to avoid having to
+ # deal with unicode decode errors from receiving
+ # partial utf8 byte sequences
+ success_message_b = None
+ if success_message is not None:
+ success_message_b = success_message.encode()
+
+ failure_message_b = None
+ if failure_message is not None:
+ failure_message_b = failure_message.encode()
+
while True:
if send_string:
vm.console_socket.sendall(send_string.encode())
@@ -102,20 +161,10 @@ def _console_interaction(test, success_message, failure_message,
break
continue
- try:
- msg = console.readline().decode().strip()
- except UnicodeDecodeError:
- msg = None
- if not msg:
- continue
- console_logger.debug(msg)
- if success_message in msg:
+ if _console_read_line_until_match(test, vm,
+ success_message_b,
+ failure_message_b):
break
- if failure_message and failure_message in msg:
- console.close()
- fail = 'Failure message found in console: "%s". Expected: "%s"' % \
- (failure_message, success_message)
- test.fail(fail)
def interrupt_interactive_console_until_pattern(test, success_message,
failure_message=None,
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 16/18] tests/functional: remove time.sleep usage from tuxrun tests
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
` (14 preceding siblings ...)
2024-11-21 15:42 ` [PATCH v2 15/18] tests/functional: rewrite console handling to be bytewise Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 17/18] tests/functional: add a QMP backdoor for debugging stalled tests Daniel P. Berrangé
` (2 subsequent siblings)
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
The tuxrun tests send a series of strings to the guest to login
and then run commands. Since we have been unable to match on
console output that isn't followed by a newline, the test used
many time.sleep() statements to pretend to synchronize with
the guest.
This has proved to be unreliable for the aarch64be instance of
the tuxrun tests, with the test often hanging. The hang is a
very subtle timing problem, and it is suspected that some
(otherwise apparently harmless) I/O error messages could be
resulting in full FIFO buffers, stalling interaction with
the guest.
With the newly rewritten console interaction able to match
strings that don't have a following newline, the tux run
tests can now match directly on the login prompt, and/or
shell PS1 prompt.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2689
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/functional/qemu_test/tuxruntest.py | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/tests/functional/qemu_test/tuxruntest.py b/tests/functional/qemu_test/tuxruntest.py
index ed2b238c92..ab3b27da43 100644
--- a/tests/functional/qemu_test/tuxruntest.py
+++ b/tests/functional/qemu_test/tuxruntest.py
@@ -124,16 +124,12 @@ def run_tuxtest_tests(self, haltmsg):
then do a few things on the console. Trigger a shutdown and
wait to exit cleanly.
"""
- self.wait_for_console_pattern("Welcome to TuxTest")
- time.sleep(0.2)
- exec_command(self, 'root')
- time.sleep(0.2)
- exec_command(self, 'cat /proc/interrupts')
- time.sleep(0.1)
- exec_command(self, 'cat /proc/self/maps')
- time.sleep(0.1)
- exec_command(self, 'uname -a')
- time.sleep(0.1)
+ ps1='root@tuxtest:~#'
+ self.wait_for_console_pattern('tuxtest login:')
+ exec_command_and_wait_for_pattern(self, 'root', ps1)
+ exec_command_and_wait_for_pattern(self, 'cat /proc/interrupts', ps1)
+ exec_command_and_wait_for_pattern(self, 'cat /proc/self/maps', ps1)
+ exec_command_and_wait_for_pattern(self, 'uname -a', ps1)
exec_command_and_wait_for_pattern(self, 'halt', haltmsg)
# Wait for VM to shut down gracefully if it can
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 17/18] tests/functional: add a QMP backdoor for debugging stalled tests
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
` (15 preceding siblings ...)
2024-11-21 15:42 ` [PATCH v2 16/18] tests/functional: remove time.sleep usage from tuxrun tests Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 15:42 ` [PATCH v2 18/18] tests/functional: avoid accessing log_filename on earlier failures Daniel P. Berrangé
2024-11-21 16:41 ` [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Alex Bennée
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
Support the QEMU_TEST_QMP_BACKDOOR=backdoor.sock env variable as a
way to get a QMP backdoor for debugging a stalled QEMU test. Most
typically this would be used if running the tests directly:
$ QEMU_TEST_QMP_BACKDOOR=backdoor.sock \
QEMU_TEST_QEMU_BINARY=./build/qemu-system-arm \
PYTHONPATH=./python \
./tests/functional/test_arm_tuxrun.py
And then, when the test stalls, in a second shell run:
$ ./scripts/qmp/qmp-shell backdoor.sock
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
docs/devel/testing/functional.rst | 10 ++++++++++
tests/functional/qemu_test/testcase.py | 7 +++++++
2 files changed, 17 insertions(+)
diff --git a/docs/devel/testing/functional.rst b/docs/devel/testing/functional.rst
index 6b5d0c5b98..b8ad7b0bf7 100644
--- a/docs/devel/testing/functional.rst
+++ b/docs/devel/testing/functional.rst
@@ -176,6 +176,16 @@ primarily depend on the value of the ``qemu_bin`` class attribute.
If it is not explicitly set by the test code, its default value will
be the result the QEMU_TEST_QEMU_BINARY environment variable.
+Debugging hung QEMU
+^^^^^^^^^^^^^^^^^^^
+
+When test cases go wrong it may be helpful to debug a stalled QEMU
+process. While the QEMUMachine class owns the primary QMP monitor
+socket, it is possible to request a second QMP monitor be created
+by setting the ``QEMU_TEST_QMP_BACKDOOR`` env variable to refer
+to a UNIX socket name. The ``qmp-shell`` command can then be
+attached to the stalled QEMU to examine its live state.
+
Attribute reference
-------------------
diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
index e2a329c3e5..fceafb32b0 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -175,6 +175,13 @@ def _new_vm(self, name, *args):
log_dir=self.logdir)
self.log.debug('QEMUMachine "%s" created', name)
self.log.debug('QEMUMachine "%s" temp_dir: %s', name, vm.temp_dir)
+
+ sockpath = os.environ.get("QEMU_TEST_QMP_BACKDOOR", None)
+ if sockpath is not None:
+ vm.add_args("-chardev",
+ f"socket,id=backdoor,path={sockpath},server=on,wait=off",
+ "-mon", "chardev=backdoor,mode=control")
+
if args:
vm.add_args(*args)
return vm
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 18/18] tests/functional: avoid accessing log_filename on earlier failures
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
` (16 preceding siblings ...)
2024-11-21 15:42 ` [PATCH v2 17/18] tests/functional: add a QMP backdoor for debugging stalled tests Daniel P. Berrangé
@ 2024-11-21 15:42 ` Daniel P. Berrangé
2024-11-21 16:41 ` [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Alex Bennée
18 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2024-11-21 15:42 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Ani Sinha, Alex Bennée, Peter Maydell,
Philippe Mathieu-Daudé, Daniel P. Berrangé
If a failure occurs early in the QemuBaseTest constructor, the
'log_filename' object atttribute may not exist yet. This happens
most notably if the QEMU_TEST_QEMU_BINARY is not set. We can't
initialize 'log_filename' earlier as we use the binary to identify
the architecture which is then used to build the path in which the
logs are stored.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/functional/qemu_test/testcase.py | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
index fceafb32b0..90ae59eb54 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -81,10 +81,12 @@ def main():
res = unittest.main(module = None, testRunner = tr, exit = False,
argv=["__dummy__", path])
for (test, message) in res.result.errors + res.result.failures:
- print('More information on ' + test.id() + ' could be found here:'
- '\n %s' % test.log_filename, file=sys.stderr)
- if hasattr(test, 'console_log_name'):
- print(' %s' % test.console_log_name, file=sys.stderr)
+
+ if hasattr(test, "log_filename"):
+ print('More information on ' + test.id() + ' could be found here:'
+ '\n %s' % test.log_filename, file=sys.stderr)
+ if hasattr(test, 'console_log_name'):
+ print(' %s' % test.console_log_name, file=sys.stderr)
sys.exit(not res.result.wasSuccessful())
--
2.46.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun
2024-11-21 15:42 [PATCH v2 00/18] test/functional: improve functional test debugging & fix tuxrun Daniel P. Berrangé
` (17 preceding siblings ...)
2024-11-21 15:42 ` [PATCH v2 18/18] tests/functional: avoid accessing log_filename on earlier failures Daniel P. Berrangé
@ 2024-11-21 16:41 ` Alex Bennée
18 siblings, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2024-11-21 16:41 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Thomas Huth, Ani Sinha, Peter Maydell,
Philippe Mathieu-Daudé
Daniel P. Berrangé <berrange@redhat.com> writes:
> This started out as a series to get rid of the many GBs of temp
> files the functional tests leave behind. Then it expanded into
> improving the functional test debugging by ensuring we preserve
> the QEMU stdout/stderr log file created by the QEMUMachine class.
> In the course of doing that I encountered some other minor points
> worth fixing, and then got side tracked into looking at the tuxrun
> hangs with aarch64be. Investigating the latter exposed some further
> holes in the debugging story prompting yet more patches, as well as
> a final solution for tuxrun. So this series does:
>
> * Purge all scratch files created by tests
> * Preserve the stdout/stderr log file
> * Capture debug log messages on QEMUMachine
> * Provide a QMP backdoor for debugging stuck QEMUs
> * Enhance console handling for partial line matches
> * Fix the tuxrun tests by eliminating sleeps
>
> There's quite alot of code here, but at the same time it feels like
> the kind of stuff that'll be valuable either in the 9.2 release, or
> in the soon to exist 9.2 stable branch.
>
> NB, with this series applied Thomas' tuxrun conversion to functional
> testing survives 200 iterations on my machine, whereas it would
> reliably hang in < 20, and often in < 10, before.
Queued to testing/next, thanks.
I'll combine with plugins/next and some other misc fixes and post a
pre-PR for next week.
>
> Changed in v2:
>
> - Changed console interaction to forbid 'failure_message'
> without 'success_message'
> - Reword console interaction log messages
> - Avoid stack trace when seeing early failure
> - Rewrote comment in acpi bits test
> - Avoid duplicate os.environ access
>
> Daniel P. Berrangé (18):
> tests/functional: fix mips64el test to honour workdir
> tests/functional: automatically clean up scratch files after tests
> tests/functional: remove "AVOCADO" from env variable name
> tests/functional: remove todo wrt avocado.utils.wait_for
> tests/functional: remove leftover :avocado: tags
> tests/functional: remove obsolete reference to avocado bug
> tests/functional: remove comments talking about avocado
> tests/functional: honour self.workdir in ACPI bits tests
> tests/functional: put QEMUMachine logs in testcase log directory
> tests/functional: honour requested test VM name in QEMUMachine
> tests/functional: enable debug logging for QEMUMachine
> tests/functional: logs details of console interaction operations
> tests/functional: don't try to wait for the empty string
> tests/functional: require non-NULL success_message for console wait
> tests/functional: rewrite console handling to be bytewise
> tests/functional: remove time.sleep usage from tuxrun tests
> tests/functional: add a QMP backdoor for debugging stalled tests
> tests/functional: avoid accessing log_filename on earlier failures
>
> docs/devel/testing/functional.rst | 16 +++++
> tests/functional/qemu_test/cmd.py | 89 +++++++++++++++++++-----
> tests/functional/qemu_test/testcase.py | 43 +++++++++---
> tests/functional/qemu_test/tuxruntest.py | 17 ++---
> tests/functional/test_acpi_bits.py | 57 +++++----------
> tests/functional/test_arm_bpim2u.py | 20 ------
> tests/functional/test_arm_orangepi.py | 27 -------
> tests/functional/test_m68k_nextcube.py | 3 +-
> tests/functional/test_mips64el_malta.py | 4 +-
> tests/functional/test_virtio_gpu.py | 3 +-
> 10 files changed, 150 insertions(+), 129 deletions(-)
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 21+ messages in thread