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

[Michael S. Tsirkin]

On Tue, Jun 28, 2022 at 12:56:52PM +0530, Ani Sinha wrote:
> 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.

Same is true for e.g. ./tests/qtest/bios-tables-test.c ...


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