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.pitsidiana...@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-6a9cd6831...@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 :|
