[PATCH 0/4] tests/functional: misc fixes for reliability / debuggability

[PATCH 0/4] tests/functional: misc fixes for reliability / debuggability

[Daniel P. Berrangé]

Daniel P. Berrangé (4):
  tests/functional: fix infinite loop on console EOF
  tests/functional: avoid duplicate messages on failures
  tests/functional: avoid tearDown failure when QEMU dies
  tests/functional: purge scratch dir on test startup

 tests/functional/qemu_test/cmd.py      |  2 +-
 tests/functional/qemu_test/testcase.py | 14 +++++++++++---
 2 files changed, 12 insertions(+), 4 deletions(-)

-- 
2.50.1

[PATCH 1/4] tests/functional: fix infinite loop on console EOF

[Daniel P. Berrangé]

The 'recv' method will return an empty byte array, not None, when
the socket has EOF.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/functional/qemu_test/cmd.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/functional/qemu_test/cmd.py b/tests/functional/qemu_test/cmd.py
index dc5f422b77..2fb731ca24 100644
--- a/tests/functional/qemu_test/cmd.py
+++ b/tests/functional/qemu_test/cmd.py
@@ -51,7 +51,7 @@ def _console_read_line_until_match(test, vm, success, failure):
     done = False
     while True:
         c = vm.console_socket.recv(1)
-        if c is None:
+        if not c:
             done = True
             test.fail(
                 f"EOF in console, expected '{success}'")
-- 
2.50.1

Re: [PATCH 1/4] tests/functional: fix infinite loop on console EOF

[Thomas Huth]

On 08/09/2025 15.57, Daniel P. Berrangé wrote:
> The 'recv' method will return an empty byte array, not None, when
> the socket has EOF.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/functional/qemu_test/cmd.py | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/functional/qemu_test/cmd.py b/tests/functional/qemu_test/cmd.py
> index dc5f422b77..2fb731ca24 100644
> --- a/tests/functional/qemu_test/cmd.py
> +++ b/tests/functional/qemu_test/cmd.py
> @@ -51,7 +51,7 @@ def _console_read_line_until_match(test, vm, success, failure):
>       done = False
>       while True:
>           c = vm.console_socket.recv(1)
> -        if c is None:
> +        if not c:
>               done = True
>               test.fail(
>                   f"EOF in console, expected '{success}'")

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

Re: [PATCH 1/4] tests/functional: fix infinite loop on console EOF

[Alex Bennée]

Daniel P. Berrangé <berrange@redhat.com> writes:

> The 'recv' method will return an empty byte array, not None, when
> the socket has EOF.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

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

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH 2/4] tests/functional: avoid duplicate messages on failures

[Daniel P. Berrangé]

In some scenarios the same tests is mentioned in both the
'res.results.errors' and 'res.results.failures' array returned
by unittest.main(). This was seen when the 'tearDown' method
raised an exception.

In such a case, we printed out the same information about where
to find a log file twice for each test. Track which tests we
have already reported on, to avoid the duplication.

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 fbeb171058..82a7724404 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -251,13 +251,14 @@ def main():
                                    test_output_log = pycotap.LogMode.LogToError)
         res = unittest.main(module = None, testRunner = tr, exit = False,
                             argv=[sys.argv[0], path] + sys.argv[1:])
+        failed = {}
         for (test, message) in res.result.errors + res.result.failures:
-
-            if hasattr(test, "log_filename"):
+            if hasattr(test, "log_filename") and not test.id() in failed:
                 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)
+                failed[test.id()] = True
         sys.exit(not res.result.wasSuccessful())
 
 
-- 
2.50.1

Re: [PATCH 2/4] tests/functional: avoid duplicate messages on failures

[Thomas Huth]

On 08/09/2025 15.57, Daniel P. Berrangé wrote:
> In some scenarios the same tests is mentioned in both the
> 'res.results.errors' and 'res.results.failures' array returned
> by unittest.main(). This was seen when the 'tearDown' method
> raised an exception.
> 
> In such a case, we printed out the same information about where
> to find a log file twice for each test. Track which tests we
> have already reported on, to avoid the duplication.
> 
> 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 fbeb171058..82a7724404 100644
> --- a/tests/functional/qemu_test/testcase.py
> +++ b/tests/functional/qemu_test/testcase.py
> @@ -251,13 +251,14 @@ def main():
>                                      test_output_log = pycotap.LogMode.LogToError)
>           res = unittest.main(module = None, testRunner = tr, exit = False,
>                               argv=[sys.argv[0], path] + sys.argv[1:])
> +        failed = {}
>           for (test, message) in res.result.errors + res.result.failures:
> -
> -            if hasattr(test, "log_filename"):
> +            if hasattr(test, "log_filename") and not test.id() in failed:
>                   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)
> +                failed[test.id()] = True
>           sys.exit(not res.result.wasSuccessful())

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

[PATCH 3/4] tests/functional: avoid tearDown failure when QEMU dies

[Daniel P. Berrangé]

In a QEMU process under test dies unexpectedly, the 'shutdown'
method may well raise an exeception. This causes the tearDown
method to fail, which means any later cleanup code fails to
get run. Most notably the log handlers don't get removed so
the base.log file from an earlier test will get polluted with
messages from any subsequent tests. The tearDown failure also
results in pages of exceptions printed on the console, which
obscures the real failure message / trace printed by the test.

Ignore any shutdown failures in the tearDown method, since any
test which cares about clean shutdown should have already
cleaned up any running VMs. The tearDown method is just there
as a safety net to cleanup resources. The base.log file will
still containing log messages from the failed 'vm.shutdown'
call too.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/functional/qemu_test/testcase.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
index 82a7724404..faa0a4f0db 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -404,7 +404,10 @@ def set_vm_arg(self, arg, value):
 
     def tearDown(self):
         for vm in self._vms.values():
-            vm.shutdown()
+            try:
+                vm.shutdown()
+            except Exception as ex:
+                self.log.error("Failed to teardown VM: %s" % ex)
         logging.getLogger('console').removeHandler(self._console_log_fh)
         self._console_log_fh.close()
         super().tearDown()
-- 
2.50.1

Re: [PATCH 3/4] tests/functional: avoid tearDown failure when QEMU dies

[Thomas Huth]

On 08/09/2025 15.57, Daniel P. Berrangé wrote:
> In a QEMU process under test dies unexpectedly, the 'shutdown'
> method may well raise an exeception. This causes the tearDown
> method to fail, which means any later cleanup code fails to
> get run. Most notably the log handlers don't get removed so
> the base.log file from an earlier test will get polluted with
> messages from any subsequent tests. The tearDown failure also
> results in pages of exceptions printed on the console, which
> obscures the real failure message / trace printed by the test.
> 
> Ignore any shutdown failures in the tearDown method, since any
> test which cares about clean shutdown should have already
> cleaned up any running VMs. The tearDown method is just there
> as a safety net to cleanup resources. The base.log file will
> still containing log messages from the failed 'vm.shutdown'
> call too.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/functional/qemu_test/testcase.py | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
> index 82a7724404..faa0a4f0db 100644
> --- a/tests/functional/qemu_test/testcase.py
> +++ b/tests/functional/qemu_test/testcase.py
> @@ -404,7 +404,10 @@ def set_vm_arg(self, arg, value):
>   
>       def tearDown(self):
>           for vm in self._vms.values():
> -            vm.shutdown()
> +            try:
> +                vm.shutdown()
> +            except Exception as ex:
> +                self.log.error("Failed to teardown VM: %s" % ex)
>           logging.getLogger('console').removeHandler(self._console_log_fh)
>           self._console_log_fh.close()
>           super().tearDown()

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

[PATCH 4/4] tests/functional: purge scratch dir on test startup

[Daniel P. Berrangé]

The test suite purges the scratch dir in the tearDown method, but
if python crashes (or is non-gracefully killed) this won't get run.
Also the user can set QEMU_TEST_KEEP_SCRATCH to disable cleanup.

Purging the scratch dir on startup ensures that tests always run
from a clean state.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/functional/qemu_test/testcase.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
index faa0a4f0db..2c0abde395 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -205,6 +205,10 @@ def setUp(self):
         self.outputdir = self.build_file('tests', 'functional',
                                          self.arch, self.id())
         self.workdir = os.path.join(self.outputdir, 'scratch')
+        if os.path.exists(self.workdir):
+            # Purge as safety net in case of unclean termination of
+            # previous test, or use of QEMU_TEST_KEEP_SCRATCH
+            shutil.rmtree(self.workdir)
         os.makedirs(self.workdir, exist_ok=True)
 
         self.log_filename = self.log_file('base.log')
-- 
2.50.1

Re: [PATCH 4/4] tests/functional: purge scratch dir on test startup

[Alex Bennée]

Daniel P. Berrangé <berrange@redhat.com> writes:

> The test suite purges the scratch dir in the tearDown method, but
> if python crashes (or is non-gracefully killed) this won't get run.
> Also the user can set QEMU_TEST_KEEP_SCRATCH to disable cleanup.
>
> Purging the scratch dir on startup ensures that tests always run
> from a clean state.
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

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

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro