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

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

On Thu, Jul 17, 2025 at 11:42 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Manos Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
>
> > Add argument parsing to functional tests to improve developer experience
> > when running individual tests. All logs are printed to stdout
> > interspersed with TAP output.
> >
> >   ./pyvenv/bin/python3 ../tests/functional/test_aarch64_virt.py --help
> >   usage: test_aarch64_virt [-h] [-d]
>
> Am I holding it wrong?
>
>   ➜  ./pyvenv/bin/python ../../tests/functional/test_aarch64_virt.py --help
>   Traceback (most recent call last):
>     File "/home/alex/lsrc/qemu.git/builds/all/../../tests/functional/test_aarch64_virt.py", line 16, in <module>
>       from qemu_test import QemuSystemTest, Asset, exec_command_and_wait_for_pattern
>     File "/home/alex/lsrc/qemu.git/tests/functional/qemu_test/__init__.py", line 14, in <module>
>       from .testcase import QemuBaseTest, QemuUserTest, QemuSystemTest
>     File "/home/alex/lsrc/qemu.git/tests/functional/qemu_test/testcase.py", line 26, in <module>
>       from qemu.machine import QEMUMachine
>   ModuleNotFoundError: No module named 'qemu'
>
> I thought the point of the venv is we had all the modules we need
> automatically available to the PYTHONPATH?

Is PYTHONPATH exported? Check that you've done the instructions
detailed here: https://www.qemu.org/docs/master/devel/testing/functional.html#running-tests

>
> >
> >   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>
> > ---
> >  docs/devel/testing/functional.rst      |  2 ++
> >  tests/functional/qemu_test/testcase.py | 51 ++++++++++++++++++++++++++++++++--
> >  2 files changed, 50 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..fad7a946c6677e9ef5c42b8f77187ba836c11aeb 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,9 @@ def assets_available(self):
> >          return True
> >
> >      def setUp(self):
> > +        path = os.path.basename(sys.argv[0])[:-3]
> > +        args = parse_args(path)
> > +        self.debug_output = args.debug
> >          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]
> > @@ -221,6 +239,16 @@ def setUp(self):
> >          self.machinelog.setLevel(logging.DEBUG)
> >          self.machinelog.addHandler(self._log_fh)
> >
> > +        if self.debug_output:
> > +            handler = logging.StreamHandler(sys.stdout)
> > +            handler.setLevel(logging.DEBUG)
> > +            formatter = logging.Formatter(
> > +                "%(asctime)s - %(name)s - %(levelname)s - %(message)s"
> > +            )
> > +            handler.setFormatter(formatter)
> > +            self.log.addHandler(handler)
> > +            self.machinelog.addHandler(handler)
> > +
> >          if not self.assets_available():
> >              self.skipTest('One or more assets is not available')
> >
> > @@ -230,11 +258,16 @@ 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, logging.StreamHandler(sys.stdout)]:
> > +            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 +325,14 @@ 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.debug_output:
> > +            handler = logging.StreamHandler(sys.stdout)
> > +            handler.setLevel(logging.DEBUG)
> > +            formatter = logging.Formatter(
> > +                "%(asctime)s - %(name)s - %(levelname)s - %(message)s"
> > +            )
> > +            handler.setFormatter(formatter)
> > +            console_log.addHandler(handler)
> >
> >      def set_machine(self, machinename):
> >          # TODO: We should use QMP to get the list of available machines
> > @@ -398,5 +439,9 @@ 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)
> > +        for handler in [
> > +            self._console_log_fh,
> > +            logging.StreamHandler(sys.stdout),
> > +        ]:
> > +            logging.getLogger("console").removeHandler(handler)
> >          super().tearDown()
> >
> > ---
> > base-commit: c079d3a31e45093286c65f8ca5350beb3a4404a9
> > change-id: 20250716-functional_tests_debug_arg-aa0a5f6b9375
>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro