Re: [PATCH 09/12] acpi/tests/bits: add acpi and smbios python tests that uses biosbits

[Ani Sinha]

On Tue, Jun 28, 2022 at 12:50 PM Thomas Huth <thuth@redhat.com> wrote:
>
> On 27/06/2022 09.28, Ani Sinha wrote:
> > This change adds python based qtest framework that can be used to run
> > qtests from within a virtual environment. A bash script creates the virtual
> > environment and then runs the python based tests from within that 
> > environment.
> > All dependent python packages are installed in the virtual environment using
> > pip module. QEMU python test modules are also available in the environment 
> > for
> > spawning the QEMU based VMs.
> >
> > It also introduces QEMU acpi/smbios biosbits python test script which is run
> > from within the python virtual environment.
> >
> > Signed-off-by: Ani Sinha <ani@anisinha.ca>
> > ---
> >   tests/qtest/acpi-bits/acpi-bits-test-venv.sh |  59 ++++
> >   tests/qtest/acpi-bits/acpi-bits-test.py      | 327 +++++++++++++++++++
> >   tests/qtest/acpi-bits/meson.build            |  39 +++
> >   tests/qtest/acpi-bits/requirements.txt       |   1 +
> >   4 files changed, 426 insertions(+)
> >   create mode 100644 tests/qtest/acpi-bits/acpi-bits-test-venv.sh
> >   create mode 100644 tests/qtest/acpi-bits/acpi-bits-test.py
> >   create mode 100644 tests/qtest/acpi-bits/meson.build
> >   create mode 100644 tests/qtest/acpi-bits/requirements.txt
> >
> > diff --git a/tests/qtest/acpi-bits/acpi-bits-test-venv.sh 
> > b/tests/qtest/acpi-bits/acpi-bits-test-venv.sh
> > new file mode 100644
> > index 0000000000..124e03ce18
> > --- /dev/null
> > +++ b/tests/qtest/acpi-bits/acpi-bits-test-venv.sh
> > @@ -0,0 +1,59 @@
> > +#!/usr/bin/env bash
> > +# Generates a python virtual environment for the test to run.
> > +# Then runs python test scripts from within that virtual environment.
> > +#
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 2 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> > +#
> > +# Author: Ani Sinha <ani@anisinha.ca>
> > +
> > +set -e
> > +
> > +MYPATH=$(realpath ${BASH_SOURCE:-$0})
> > +MYDIR=$(dirname $MYPATH)
> > +
> > +if [ -z "$QTEST_SOURCE_ROOT" ]; then
> > +    echo -n "Please set QTEST_SOURCE_ROOT env pointing"
> > +    echo " to the root of the qemu source tree."
> > +    echo -n "This is required so that the test can find the "
> > +    echo "python modules that it needs for execution."
> > +    exit 1
> > +fi
> > +SRCDIR=$QTEST_SOURCE_ROOT
> > +TESTSCRIPTS=("acpi-bits-test.py")
> > +PIPCMD="-m pip -q --disable-pip-version-check"
> > +# we need to save the old value of PWD before we do a change-dir later
> > +QTEST_PWD=$PWD
> > +
> > +TESTS_PYTHON=/usr/bin/python3
> > +TESTS_VENV_REQ=requirements.txt
> > +
> > +# sadly for pip -e and -t options do not work together.
> > +# please see https://github.com/pypa/pip/issues/562
> > +cd $MYDIR
> > +
> > +$TESTS_PYTHON -m venv .
> > +$TESTS_PYTHON $PIPCMD install -e $SRCDIR/python/
> > +[ -f $TESTS_VENV_REQ ] && \
> > +    $TESTS_PYTHON $PIPCMD install -r $TESTS_VENV_REQ
> > +
> > +# venv is activated at this point.
> > +
> > +# run the test
> > +for testscript in ${TESTSCRIPTS[@]} ; do
> > +    export QTEST_PWD; python3 $testscript
> > +done
> > +
> > +cd $QTEST_PWD
> > +
> > +exit 0
> > diff --git a/tests/qtest/acpi-bits/acpi-bits-test.py 
> > b/tests/qtest/acpi-bits/acpi-bits-test.py
> > new file mode 100644
> > index 0000000000..673567bf8e
> > --- /dev/null
> > +++ b/tests/qtest/acpi-bits/acpi-bits-test.py
> > @@ -0,0 +1,327 @@
> > +#!/usr/bin/env python3
> > +# group: rw quick
> > +# Exercize QEMU generated ACPI/SMBIOS tables using biosbits,
> > +# https://biosbits.org/
> > +#
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 2 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> > +#
> > +# Some parts are slightly taken from qtest.py and iotests.py
> > +#
> > +# Authors:
> > +#  Ani Sinha <ani@anisinha.ca>
> > +
> > +# pylint: disable=invalid-name
> > +
> > +"""
> > +QEMU bios tests using biosbits available at
> > +https://biosbits.org/.
> > +"""
> > +
> > +import logging
> > +import os
> > +import re
> > +import shutil
> > +import subprocess
> > +import sys
> > +import tarfile
> > +import tempfile
> > +import time
> > +import unittest
> > +import zipfile
> > +from typing import (
> > +    List,
> > +    Optional,
> > +    Sequence,
> > +)
> > +from tap import TAPTestRunner
> > +from qemu.machine import QEMUMachine
> > +
> > +QTESTQEMUPROG = os.getenv('QTEST_QEMU_BINARY')
> > +QTEST_PWD = os.getenv('QTEST_PWD')
> > +
> > +def get_arch():
> > +    """finds the arch from the qemu binary name"""
> > +    match = re.search('.*qemu-system-(.*)', QTESTQEMUPROG)
> > +    if match:
> > +        return match.group(1)
> > +    return 'x86_64'
> > +
> > +ARCH = get_arch()
> > +
> > +class QEMUBitsMachine(QEMUMachine):
> > +    """
> > +    A QEMU VM, with isa-debugcon enabled and bits iso passed
> > +    using -cdrom to QEMU commandline.
> > +    """
> > +    def __init__(self,
> > +                 binary: str,
> > +                 args: Sequence[str] = (),
> > +                 wrapper: Sequence[str] = (),
> > +                 name: Optional[str] = None,
> > +                 base_temp_dir: str = "/var/tmp",
> > +                 debugcon_log: str = "debugcon-log.txt",
> > +                 debugcon_addr: str = "0x403",
> > +                 sock_dir: Optional[str] = None,
> > +                 qmp_timer: Optional[float] = None):
> > +        # pylint: disable=too-many-arguments
> > +
> > +        if name is None:
> > +            name = "qemu-bits-%d" % os.getpid()
> > +        if sock_dir is None:
> > +            sock_dir = base_temp_dir
> > +        super().__init__(binary, args, wrapper=wrapper, name=name,
> > +                         base_temp_dir=base_temp_dir,
> > +                         sock_dir=sock_dir, qmp_timer=qmp_timer)
> > +        self.debugcon_log = debugcon_log
> > +        self.debugcon_addr = debugcon_addr
> > +        self.base_temp_dir = base_temp_dir
> > +
> > +    @property
> > +    def _base_args(self) -> List[str]:
> > +        args = super()._base_args
> > +        args.extend([
> > +            '-chardev',
> > +            'file,path=%s,id=debugcon' %os.path.join(self.base_temp_dir,
> > +                                                     self.debugcon_log),
> > +            '-device',
> > +            'isa-debugcon,iobase=%s,chardev=debugcon' %self.debugcon_addr,
> > +        ])
> > +        return args
>
> So is this patch series *really* related to qtests? qtests are using a
> special "accelerator" mode of QEMU where it gets launched with the "-qtest"
> parameter. I can't see that you're using "-qtest" here or anywhere else, so
> this rather looks like another framework to me to run python-based QEMU
> tests (similar to the avocado tests).

yes you are right. This does not use or need the qtest accelerator
because we are not inspecting the guest memory for anything.

>
> Thus if this is really not related to qtests, may I suggest to move this
> into another folder instead? Maybe tests/pytests/acpi-bits or something 
> similar?

The problem I faced with this test is that it does not quite fall into
the qtest category. Nor does it fall into the integration test
category. I asked Igor and he suggested I use the qtest framework.
Should we invent a new class of tests then? How many such tests are we
going to have in the future?  Lets see what others think.

>
>   Thomas
>