[PATCH v2] tests/functional: add --debug CLI arg

[PATCH v2] tests/functional: add --debug CLI arg

[Manos Pitsidianakis]

Add argument parsing to functional tests to improve developer experience
when running individual tests. All logs are printed to stdout
interspersed with TAP output.

Example usage, assuming current build directory with qemu source code in
the parent directory (see docs/devel/testing/functional.rst for details):

  $ export PYTHONPATH=../python:../tests/functional
  $ export QEMU_TEST_QEMU_BINARY="$(pwd)/qemu-system-aarch64"
  $ ./pyvenv/bin/python3 ../tests/functional/test_aarch64_virt.py --help
  usage: test_aarch64_virt [-h] [-d]

  QEMU Functional test

  options:
    -h, --help   show this help message and exit
    -d, --debug  Also print test and console logs on stdout. This will
                 make the TAP output invalid and is meant for debugging
                 only.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
Changes in v2:
- Store stdout handler in `self` object (thanks Daniel)
- Deduplicate handler removal code (Daniel)
- Amend commit description to mention PYTHONPATH (thanks Alex)
- Link to v1: https://lore.kernel.org/qemu-devel/20250716-functional_tests_debug_arg-v1-1-6a9cd68318bb@linaro.org
---
 docs/devel/testing/functional.rst      |  2 ++
 tests/functional/qemu_test/testcase.py | 48 +++++++++++++++++++++++++++++++---
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/docs/devel/testing/functional.rst b/docs/devel/testing/functional.rst
index 9e56dd1b1189216b9b4aede00174c15203f38b41..9d08abe2848277d635befb0296f578cfaa4bd66d 100644
--- a/docs/devel/testing/functional.rst
+++ b/docs/devel/testing/functional.rst
@@ -63,6 +63,8 @@ directory should be your build folder. For example::
   $ export QEMU_TEST_QEMU_BINARY=$PWD/qemu-system-x86_64
   $ pyvenv/bin/python3 ../tests/functional/test_file.py
 
+By default, functional tests redirect informational logs and console output to
+log files. Specify the ``--debug`` flag to also print those to standard output.
 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
diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
index 2082c6fce43b0544d4e4258cd4155f555ed30cd4..3ecaaeffd4df2945fb4c44b4ddef6911527099b9 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -11,6 +11,7 @@
 # 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 argparse
 import logging
 import os
 from pathlib import Path
@@ -31,6 +32,20 @@
 from .uncompress import uncompress
 
 
+def parse_args(test_name: str) -> argparse.Namespace:
+    parser = argparse.ArgumentParser(
+        prog=test_name, description="QEMU Functional test"
+    )
+    parser.add_argument(
+        "-d",
+        "--debug",
+        action="store_true",
+        help="Also print test and console logs on stdout. This will make the"
+        " TAP output invalid and is meant for debugging only.",
+    )
+    return parser.parse_args()
+
+
 class QemuBaseTest(unittest.TestCase):
 
     '''
@@ -196,6 +211,16 @@ def assets_available(self):
         return True
 
     def setUp(self):
+        path = os.path.basename(sys.argv[0])[:-3]
+        args = parse_args(path)
+        self.stdout_handler = None
+        if args.debug:
+            self.stdout_handler = logging.StreamHandler(sys.stdout)
+            self.stdout_handler.setLevel(logging.DEBUG)
+            formatter = logging.Formatter(
+                "%(asctime)s - %(name)s - %(levelname)s - %(message)s"
+            )
+            self.stdout_handler.setFormatter(formatter)
         self.qemu_bin = os.getenv('QEMU_TEST_QEMU_BINARY')
         self.assertIsNotNone(self.qemu_bin, 'QEMU_TEST_QEMU_BINARY must be set')
         self.arch = self.qemu_bin.split('-')[-1]
@@ -215,12 +240,17 @@ def setUp(self):
             '%(asctime)s - %(levelname)s: %(message)s')
         self._log_fh.setFormatter(fileFormatter)
         self.log.addHandler(self._log_fh)
+        if self.stdout_handler:
+            self.log.addHandler(self.stdout_handler)
 
         # Capture QEMUMachine logging
         self.machinelog = logging.getLogger('qemu.machine')
         self.machinelog.setLevel(logging.DEBUG)
         self.machinelog.addHandler(self._log_fh)
 
+        if self.stdout_handler:
+            self.machinelog.addHandler(self.stdout_handler)
+
         if not self.assets_available():
             self.skipTest('One or more assets is not available')
 
@@ -230,11 +260,18 @@ def tearDown(self):
         if self.socketdir is not None:
             shutil.rmtree(self.socketdir.name)
             self.socketdir = None
-        self.machinelog.removeHandler(self._log_fh)
-        self.log.removeHandler(self._log_fh)
+        for handler in [self._log_fh, self.stdout_handler]:
+            if handler is None:
+                continue
+            self.machinelog.removeHandler(handler)
+            self.log.removeHandler(handler)
 
     def main():
         path = os.path.basename(sys.argv[0])[:-3]
+        # If argparse receives --help or an unknown argument, it will raise a
+        # SystemExit which will get caught by the test runner. Parse the
+        # arguments here too to handle that case.
+        parse_args(path)
 
         cache = os.environ.get("QEMU_TEST_PRECACHE", None)
         if cache is not None:
@@ -292,6 +329,8 @@ def setUp(self):
         fileFormatter = logging.Formatter('%(asctime)s: %(message)s')
         self._console_log_fh.setFormatter(fileFormatter)
         console_log.addHandler(self._console_log_fh)
+        if self.stdout_handler:
+            console_log.addHandler(self.stdout_handler)
 
     def set_machine(self, machinename):
         # TODO: We should use QMP to get the list of available machines
@@ -398,5 +437,8 @@ def set_vm_arg(self, arg, value):
     def tearDown(self):
         for vm in self._vms.values():
             vm.shutdown()
-        logging.getLogger('console').removeHandler(self._console_log_fh)
+        console_log = logging.getLogger("console")
+        console_log.removeHandler(self._console_log_fh)
+        if self.stdout_handler:
+            console_log.removeHandler(self.stdout_handler)
         super().tearDown()

---
base-commit: f96b157ebb93f94cd56ebbc99bc20982b8fd86ef
change-id: 20250716-functional_tests_debug_arg-aa0a5f6b9375

--
γαῖα πυρί μιχθήτω

Re: [PATCH v2] tests/functional: add --debug CLI arg

[Daniel P. Berrangé]

On Thu, Jul 17, 2025 at 01:34:13PM +0300, Manos Pitsidianakis wrote:
> Add argument parsing to functional tests to improve developer experience
> when running individual tests. All logs are printed to stdout
> interspersed with TAP output.
> 
> Example usage, assuming current build directory with qemu source code in
> the parent directory (see docs/devel/testing/functional.rst for details):
> 
>   $ export PYTHONPATH=../python:../tests/functional
>   $ export QEMU_TEST_QEMU_BINARY="$(pwd)/qemu-system-aarch64"
>   $ ./pyvenv/bin/python3 ../tests/functional/test_aarch64_virt.py --help
>   usage: test_aarch64_virt [-h] [-d]
> 
>   QEMU Functional test
> 
>   options:
>     -h, --help   show this help message and exit
>     -d, --debug  Also print test and console logs on stdout. This will
>                  make the TAP output invalid and is meant for debugging
>                  only.
> 
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
> Changes in v2:
> - Store stdout handler in `self` object (thanks Daniel)
> - Deduplicate handler removal code (Daniel)
> - Amend commit description to mention PYTHONPATH (thanks Alex)
> - Link to v1: https://lore.kernel.org/qemu-devel/20250716-functional_tests_debug_arg-v1-1-6a9cd68318bb@linaro.org

You've ignored my v1 review requests that the code for creating
log handlers should be moved into a helper method in util.py, and
likewise that the argparse code moved into util.py, and thus not
called in both main & setUp.


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

Re: [PATCH v2] tests/functional: add --debug CLI arg

[Manos Pitsidianakis]

On Fri, Jul 18, 2025 at 12:20 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Thu, Jul 17, 2025 at 01:34:13PM +0300, Manos Pitsidianakis wrote:
> > Add argument parsing to functional tests to improve developer experience
> > when running individual tests. All logs are printed to stdout
> > interspersed with TAP output.
> >
> > Example usage, assuming current build directory with qemu source code in
> > the parent directory (see docs/devel/testing/functional.rst for details):
> >
> >   $ export PYTHONPATH=../python:../tests/functional
> >   $ export QEMU_TEST_QEMU_BINARY="$(pwd)/qemu-system-aarch64"
> >   $ ./pyvenv/bin/python3 ../tests/functional/test_aarch64_virt.py --help
> >   usage: test_aarch64_virt [-h] [-d]
> >
> >   QEMU Functional test
> >
> >   options:
> >     -h, --help   show this help message and exit
> >     -d, --debug  Also print test and console logs on stdout. This will
> >                  make the TAP output invalid and is meant for debugging
> >                  only.
> >
> > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > ---
> > Changes in v2:
> > - Store stdout handler in `self` object (thanks Daniel)
> > - Deduplicate handler removal code (Daniel)
> > - Amend commit description to mention PYTHONPATH (thanks Alex)
> > - Link to v1: https://lore.kernel.org/qemu-devel/20250716-functional_tests_debug_arg-v1-1-6a9cd68318bb@linaro.org
>
> You've ignored my v1 review requests that the code for creating
> log handlers should be moved into a helper method in util.py, and
> likewise that the argparse code moved into util.py, and thus not
> called in both main & setUp.

Hi Daniel,

I forgot to reply to those comments. The arg parsing logic needs to be
called from both, as explained in the comment. Plus, there's no reason
to put it in the util module, it's only used in one file. It doesn't
really help to split stuff out of testcase.py if they are only called
from that file, plus it's not a big file in the first place.

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

-- 
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd

Re: [PATCH v2] tests/functional: add --debug CLI arg

[Thomas Huth]

On 17/07/2025 12.34, Manos Pitsidianakis wrote:
> Add argument parsing to functional tests to improve developer experience
> when running individual tests. All logs are printed to stdout
> interspersed with TAP output.
> 
> Example usage, assuming current build directory with qemu source code in
> the parent directory (see docs/devel/testing/functional.rst for details):
> 
>    $ export PYTHONPATH=../python:../tests/functional
>    $ export QEMU_TEST_QEMU_BINARY="$(pwd)/qemu-system-aarch64"
>    $ ./pyvenv/bin/python3 ../tests/functional/test_aarch64_virt.py --help
>    usage: test_aarch64_virt [-h] [-d]
> 
>    QEMU Functional test
> 
>    options:
>      -h, --help   show this help message and exit
>      -d, --debug  Also print test and console logs on stdout. This will
>                   make the TAP output invalid and is meant for debugging
>                   only.
> 
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
> Changes in v2:
> - Store stdout handler in `self` object (thanks Daniel)
> - Deduplicate handler removal code (Daniel)
> - Amend commit description to mention PYTHONPATH (thanks Alex)
> - Link to v1: https://lore.kernel.org/qemu-devel/20250716-functional_tests_debug_arg-v1-1-6a9cd68318bb@linaro.org
> ---
>   docs/devel/testing/functional.rst      |  2 ++
>   tests/functional/qemu_test/testcase.py | 48 +++++++++++++++++++++++++++++++---
>   2 files changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/devel/testing/functional.rst b/docs/devel/testing/functional.rst
> index 9e56dd1b1189216b9b4aede00174c15203f38b41..9d08abe2848277d635befb0296f578cfaa4bd66d 100644
> --- a/docs/devel/testing/functional.rst
> +++ b/docs/devel/testing/functional.rst
> @@ -63,6 +63,8 @@ directory should be your build folder. For example::
>     $ export QEMU_TEST_QEMU_BINARY=$PWD/qemu-system-x86_64
>     $ pyvenv/bin/python3 ../tests/functional/test_file.py
>   
> +By default, functional tests redirect informational logs and console output to
> +log files. Specify the ``--debug`` flag to also print those to standard output.
>   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
> diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
> index 2082c6fce43b0544d4e4258cd4155f555ed30cd4..3ecaaeffd4df2945fb4c44b4ddef6911527099b9 100644
> --- a/tests/functional/qemu_test/testcase.py
> +++ b/tests/functional/qemu_test/testcase.py
> @@ -11,6 +11,7 @@
>   # 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 argparse
>   import logging
>   import os
>   from pathlib import Path
> @@ -31,6 +32,20 @@
>   from .uncompress import uncompress
>   
>   
> +def parse_args(test_name: str) -> argparse.Namespace:
> +    parser = argparse.ArgumentParser(
> +        prog=test_name, description="QEMU Functional test"
> +    )
> +    parser.add_argument(
> +        "-d",
> +        "--debug",
> +        action="store_true",
> +        help="Also print test and console logs on stdout. This will make the"
> +        " TAP output invalid and is meant for debugging only.",
> +    )
> +    return parser.parse_args()
> +
> +
>   class QemuBaseTest(unittest.TestCase):
>   
>       '''
> @@ -196,6 +211,16 @@ def assets_available(self):
>           return True
>   
>       def setUp(self):
> +        path = os.path.basename(sys.argv[0])[:-3]
> +        args = parse_args(path)
> +        self.stdout_handler = None
> +        if args.debug:
> +            self.stdout_handler = logging.StreamHandler(sys.stdout)
> +            self.stdout_handler.setLevel(logging.DEBUG)
> +            formatter = logging.Formatter(
> +                "%(asctime)s - %(name)s - %(levelname)s - %(message)s"
> +            )
> +            self.stdout_handler.setFormatter(formatter)
>           self.qemu_bin = os.getenv('QEMU_TEST_QEMU_BINARY')
>           self.assertIsNotNone(self.qemu_bin, 'QEMU_TEST_QEMU_BINARY must be set')
>           self.arch = self.qemu_bin.split('-')[-1]
> @@ -215,12 +240,17 @@ def setUp(self):
>               '%(asctime)s - %(levelname)s: %(message)s')
>           self._log_fh.setFormatter(fileFormatter)
>           self.log.addHandler(self._log_fh)
> +        if self.stdout_handler:
> +            self.log.addHandler(self.stdout_handler)
>   
>           # Capture QEMUMachine logging
>           self.machinelog = logging.getLogger('qemu.machine')
>           self.machinelog.setLevel(logging.DEBUG)
>           self.machinelog.addHandler(self._log_fh)
>   
> +        if self.stdout_handler:
> +            self.machinelog.addHandler(self.stdout_handler)
> +
>           if not self.assets_available():
>               self.skipTest('One or more assets is not available')
>   
> @@ -230,11 +260,18 @@ def tearDown(self):
>           if self.socketdir is not None:
>               shutil.rmtree(self.socketdir.name)
>               self.socketdir = None
> -        self.machinelog.removeHandler(self._log_fh)
> -        self.log.removeHandler(self._log_fh)
> +        for handler in [self._log_fh, self.stdout_handler]:
> +            if handler is None:
> +                continue
> +            self.machinelog.removeHandler(handler)
> +            self.log.removeHandler(handler)
>   
>       def main():
>           path = os.path.basename(sys.argv[0])[:-3]
> +        # If argparse receives --help or an unknown argument, it will raise a
> +        # SystemExit which will get caught by the test runner. Parse the
> +        # arguments here too to handle that case.
> +        parse_args(path)

Parsing the args twice, in setUp and main, is somewhat ugly. What about only 
parsing them in main, and then set a global variable if debug mode should be 
enabled? Then it's enough to check that variable in the setUp function.

  Thomas

Re: [PATCH v2] tests/functional: add --debug CLI arg

[Daniel P. Berrangé]

On Fri, Jul 25, 2025 at 06:28:31AM +0200, Thomas Huth wrote:
> On 17/07/2025 12.34, Manos Pitsidianakis wrote:
> > Add argument parsing to functional tests to improve developer experience
> > when running individual tests. All logs are printed to stdout
> > interspersed with TAP output.
> > 
> > Example usage, assuming current build directory with qemu source code in
> > the parent directory (see docs/devel/testing/functional.rst for details):
> > 
> >    $ export PYTHONPATH=../python:../tests/functional
> >    $ export QEMU_TEST_QEMU_BINARY="$(pwd)/qemu-system-aarch64"
> >    $ ./pyvenv/bin/python3 ../tests/functional/test_aarch64_virt.py --help
> >    usage: test_aarch64_virt [-h] [-d]
> > 
> >    QEMU Functional test
> > 
> >    options:
> >      -h, --help   show this help message and exit
> >      -d, --debug  Also print test and console logs on stdout. This will
> >                   make the TAP output invalid and is meant for debugging
> >                   only.
> > 
> > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > ---
> > Changes in v2:
> > - Store stdout handler in `self` object (thanks Daniel)
> > - Deduplicate handler removal code (Daniel)
> > - Amend commit description to mention PYTHONPATH (thanks Alex)
> > - Link to v1: https://lore.kernel.org/qemu-devel/20250716-functional_tests_debug_arg-v1-1-6a9cd68318bb@linaro.org
> > ---
> >   docs/devel/testing/functional.rst      |  2 ++
> >   tests/functional/qemu_test/testcase.py | 48 +++++++++++++++++++++++++++++++---
> >   2 files changed, 47 insertions(+), 3 deletions(-)
> > 
> > diff --git a/docs/devel/testing/functional.rst b/docs/devel/testing/functional.rst
> > index 9e56dd1b1189216b9b4aede00174c15203f38b41..9d08abe2848277d635befb0296f578cfaa4bd66d 100644
> > --- a/docs/devel/testing/functional.rst
> > +++ b/docs/devel/testing/functional.rst
> > @@ -63,6 +63,8 @@ directory should be your build folder. For example::
> >     $ export QEMU_TEST_QEMU_BINARY=$PWD/qemu-system-x86_64
> >     $ pyvenv/bin/python3 ../tests/functional/test_file.py
> > +By default, functional tests redirect informational logs and console output to
> > +log files. Specify the ``--debug`` flag to also print those to standard output.
> >   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
> > diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
> > index 2082c6fce43b0544d4e4258cd4155f555ed30cd4..3ecaaeffd4df2945fb4c44b4ddef6911527099b9 100644
> > --- a/tests/functional/qemu_test/testcase.py
> > +++ b/tests/functional/qemu_test/testcase.py
> > @@ -11,6 +11,7 @@
> >   # 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 argparse
> >   import logging
> >   import os
> >   from pathlib import Path
> > @@ -31,6 +32,20 @@
> >   from .uncompress import uncompress
> > +def parse_args(test_name: str) -> argparse.Namespace:
> > +    parser = argparse.ArgumentParser(
> > +        prog=test_name, description="QEMU Functional test"
> > +    )
> > +    parser.add_argument(
> > +        "-d",
> > +        "--debug",
> > +        action="store_true",
> > +        help="Also print test and console logs on stdout. This will make the"
> > +        " TAP output invalid and is meant for debugging only.",
> > +    )
> > +    return parser.parse_args()
> > +
> > +
> >   class QemuBaseTest(unittest.TestCase):
> >       '''
> > @@ -196,6 +211,16 @@ def assets_available(self):
> >           return True
> >       def setUp(self):
> > +        path = os.path.basename(sys.argv[0])[:-3]
> > +        args = parse_args(path)
> > +        self.stdout_handler = None
> > +        if args.debug:
> > +            self.stdout_handler = logging.StreamHandler(sys.stdout)
> > +            self.stdout_handler.setLevel(logging.DEBUG)
> > +            formatter = logging.Formatter(
> > +                "%(asctime)s - %(name)s - %(levelname)s - %(message)s"
> > +            )
> > +            self.stdout_handler.setFormatter(formatter)
> >           self.qemu_bin = os.getenv('QEMU_TEST_QEMU_BINARY')
> >           self.assertIsNotNone(self.qemu_bin, 'QEMU_TEST_QEMU_BINARY must be set')
> >           self.arch = self.qemu_bin.split('-')[-1]
> > @@ -215,12 +240,17 @@ def setUp(self):
> >               '%(asctime)s - %(levelname)s: %(message)s')
> >           self._log_fh.setFormatter(fileFormatter)
> >           self.log.addHandler(self._log_fh)
> > +        if self.stdout_handler:
> > +            self.log.addHandler(self.stdout_handler)
> >           # Capture QEMUMachine logging
> >           self.machinelog = logging.getLogger('qemu.machine')
> >           self.machinelog.setLevel(logging.DEBUG)
> >           self.machinelog.addHandler(self._log_fh)
> > +        if self.stdout_handler:
> > +            self.machinelog.addHandler(self.stdout_handler)
> > +
> >           if not self.assets_available():
> >               self.skipTest('One or more assets is not available')
> > @@ -230,11 +260,18 @@ def tearDown(self):
> >           if self.socketdir is not None:
> >               shutil.rmtree(self.socketdir.name)
> >               self.socketdir = None
> > -        self.machinelog.removeHandler(self._log_fh)
> > -        self.log.removeHandler(self._log_fh)
> > +        for handler in [self._log_fh, self.stdout_handler]:
> > +            if handler is None:
> > +                continue
> > +            self.machinelog.removeHandler(handler)
> > +            self.log.removeHandler(handler)
> >       def main():
> >           path = os.path.basename(sys.argv[0])[:-3]
> > +        # If argparse receives --help or an unknown argument, it will raise a
> > +        # SystemExit which will get caught by the test runner. Parse the
> > +        # arguments here too to handle that case.
> > +        parse_args(path)
> 
> Parsing the args twice, in setUp and main, is somewhat ugly. What about only
> parsing them in main, and then set a global variable if debug mode should be
> enabled? Then it's enough to check that variable in the setUp function.

This is what I requested in the v1 too, with the CLI parsing all moved
into the util.py file too.

This would keep the QemuBaseTest class cleanly separated from command
line argument handling.

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