From: Anton Mikanovich <amikan@ilbers.de>
To: Adriaan Schmidt <adriaan.schmidt@siemens.com>,
isar-users@googlegroups.com
Subject: Re: [RFC PATCH v2] testsuite: refactor
Date: Fri, 10 Dec 2021 18:05:03 +0300 [thread overview]
Message-ID: <e650b7b5-afb3-3700-4940-38e9a3e21931@ilbers.de> (raw)
In-Reply-To: <20211210081526.2944853-1-adriaan.schmidt@siemens.com>
Thanks for v2, it looks much better now.
But there are still some comments bellow.
10.12.2021 11:15, Adriaan Schmidt wrote:
> - make test initialization more explicit.
> tests now call init() to initialize build_dir and environment,
> and configure() to generate the config file and bitbake_args
> - only configure() touches the config file. if config needs to change
> during one test case, it is created from scratch (no appending
> by the test case itself).
> - build_dir is not passed via avocado parameter. each test
> can set it in the call to init(). this makes dependencies
> between tests explicit and permits parallelization.
> - in the main invocation script, rename --build to --base.
> what we're setting here is not the bitbake build_dir but
> the base dir for avocado test output.
>
> Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
> ---
> scripts/ci_build.sh | 22 +++---
> testsuite/build_test/build_test.py | 35 +++++-----
> testsuite/build_test/cibase.py | 103 +++++++++--------------------
> testsuite/build_test/cibuilder.py | 98 +++++++++++++++++++--------
> 4 files changed, 128 insertions(+), 130 deletions(-)
>
> diff --git a/scripts/ci_build.sh b/scripts/ci_build.sh
> index e9ba034..339ebca 100755
> --- a/scripts/ci_build.sh
> +++ b/scripts/ci_build.sh
> @@ -27,8 +27,8 @@ fi
> # Get Avocado build tests path
> BUILD_TEST_DIR="$(pwd)/testsuite/build_test"
>
> -# Start build in Isar tree by default
> -BUILD_DIR=./build
> +# Start tests in current path by default
> +BASE_DIR=./build
>
> # Check dependencies
> DEPENDENCIES="umoci skopeo"
> @@ -45,8 +45,8 @@ show_help() {
> echo " $0 [params]"
> echo
> echo "Parameters:"
> - echo " -b, --build BUILD_DIR set path to build directory. If not set,"
> - echo " the build will be started in current path."
> + echo " -b, --base BASE_DIR set path to base directory. If not set,"
> + echo " the tests will be started in current path."
> echo " -c, --cross enable cross-compilation."
> echo " -d, --debug enable debug bitbake output."
> echo " -f, --fast cross build reduced set of configurations."
> @@ -73,8 +73,8 @@ do
> show_help
> exit 0
> ;;
> - -b|--build)
> - BUILD_DIR="$2"
> + -b|--base)
> + BASE_DIR="$2"
> shift
> ;;
> -c|--cross)
> @@ -117,10 +117,10 @@ fi
> mkdir -p .config/avocado
> cat <<EOF > .config/avocado/avocado.conf
> [datadir.paths]
> -base_dir = $(realpath $BUILD_DIR)/
> -test_dir = $(realpath $BUILD_DIR)/tests
> -data_dir = $(realpath $BUILD_DIR)/data
> -logs_dir = $(realpath $BUILD_DIR)/job-results
> +base_dir = $(realpath $BASE_DIR)/
> +test_dir = $(realpath $BASE_DIR)/tests
> +data_dir = $(realpath $BASE_DIR)/data
> +logs_dir = $(realpath $BASE_DIR)/job-results
> EOF
> export VIRTUAL_ENV="./"
>
> @@ -129,4 +129,4 @@ set -x
>
> avocado $VERBOSE run "$BUILD_TEST_DIR/build_test.py" \
> -t $TAGS --test-runner=runner --disable-sysinfo \
> - -p build_dir="$BUILD_DIR" -p quiet=$QUIET -p cross=$CROSS_BUILD
> + -p quiet=$QUIET -p cross=$CROSS_BUILD
> diff --git a/testsuite/build_test/build_test.py b/testsuite/build_test/build_test.py
> index de9e3fc..7359295 100644
> --- a/testsuite/build_test/build_test.py
> +++ b/testsuite/build_test/build_test.py
> @@ -3,7 +3,7 @@
> import os
>
> from avocado import skipUnless
> -from avocado.utils import path
> +from avocado.utils import path, process
Does it really needed here?
> from cibase import CIBaseTest
>
> UMOCI_AVAILABLE = True
> @@ -30,7 +30,7 @@ class ReproTest(CIBaseTest):
> 'mc:qemuarm64-stretch:isar-image-base'
> ]
>
> - self.perform_repro_test(targets, 1)
> + self.perform_repro_test(targets, signed=True)
>
> def test_repro_unsigned(self):
> targets = [
> @@ -38,7 +38,7 @@ class ReproTest(CIBaseTest):
> 'mc:qemuarm-stretch:isar-image-base'
> ]
>
> - self.perform_repro_test(targets, 0)
> + self.perform_repro_test(targets)
>
> class CcacheTest(CIBaseTest):
>
> @@ -69,7 +69,7 @@ class CrossTest(CIBaseTest):
> 'mc:rpi-stretch:isar-image-base'
> ]
>
> - self.perform_build_test(targets, 1, None)
> + self.perform_build_test(targets, cross=True)
>
> def test_cross_ubuntu(self):
> targets = [
> @@ -77,7 +77,7 @@ class CrossTest(CIBaseTest):
> ]
>
> try:
> - self.perform_build_test(targets, 1, None)
> + self.perform_build_test(targets, cross=True)
> except:
> self.cancel('KFAIL')
>
> @@ -87,7 +87,7 @@ class CrossTest(CIBaseTest):
> ]
>
> try:
> - self.perform_build_test(targets, 1, None)
> + self.perform_build_test(targets, cross=True)
> except:
> self.cancel('KFAIL')
>
> @@ -101,7 +101,7 @@ class SdkTest(CIBaseTest):
> def test_sdk(self):
> targets = ['mc:qemuarm-stretch:isar-image-base']
>
> - self.perform_build_test(targets, 1, 'do_populate_sdk')
> + self.perform_build_test(targets, bitbake_cmd='do_populate_sdk')
>
> class NoCrossTest(CIBaseTest):
>
> @@ -131,10 +131,9 @@ class NoCrossTest(CIBaseTest):
> ]
>
> # Cleanup after cross build
> - self.deletetmp(self.params.get('build_dir',
> - default=os.path.dirname(__file__) + '/../../build'))
> -
> - self.perform_build_test(targets, 0, None)
> + self.init()
Is that the second init? Why we need it here?
> + self.delete_from_build_dir('tmp')
> + self.perform_build_test(targets, cross=False)
>
> def test_nocross_bullseye(self):
> targets = [
> @@ -146,7 +145,7 @@ class NoCrossTest(CIBaseTest):
> ]
>
> try:
> - self.perform_build_test(targets, 0, None)
> + self.perform_build_test(targets, cross=False)
> except:
> self.cancel('KFAIL')
>
> @@ -158,19 +157,16 @@ class RebuildTest(CIBaseTest):
> :avocado: tags=rebuild,fast,full
> """
> def test_rebuild(self):
> - is_cross_build = int(self.params.get('cross', default=0))
> -
> + self.init()
The same question here. Init is already done inside perform_build_test.
> layerdir_core = self.getlayerdir('core')
>
> dpkgbase_file = layerdir_core + '/classes/dpkg-base.bbclass'
> -
Please remove changes that just remove empty lines. It can be confusing
for the history.
> self.backupfile(dpkgbase_file)
> with open(dpkgbase_file, 'a') as file:
> file.write('do_fetch_append() {\n\n}')
>
> try:
> - self.perform_build_test('mc:qemuamd64-stretch:isar-image-base',
> - is_cross_build, None)
> + self.perform_build_test('mc:qemuamd64-stretch:isar-image-base')
> finally:
> self.restorefile(dpkgbase_file)
>
> @@ -189,6 +185,7 @@ class WicTest(CIBaseTest):
> self.perform_wic_test('mc:qemuamd64-stretch:isar-image-base',
> wks_path, wic_path)
>
> +
The same with adding newlines.
> class ContainerImageTest(CIBaseTest):
>
> """
> @@ -204,7 +201,7 @@ class ContainerImageTest(CIBaseTest):
> 'mc:container-amd64-bullseye:isar-image-base'
> ]
>
> - self.perform_container_test(targets, None)
> + self.perform_build_test(targets, container=True)
>
> class ContainerSdkTest(CIBaseTest):
>
> @@ -217,4 +214,4 @@ class ContainerSdkTest(CIBaseTest):
> def test_container_sdk(self):
> targets = ['mc:container-amd64-stretch:isar-image-base']
>
> - self.perform_container_test(targets, 'do_populate_sdk')
> + self.perform_build_test(targets, bitbake_cmd='do_populate_sdk', container=True)
> diff --git a/testsuite/build_test/cibase.py b/testsuite/build_test/cibase.py
> index 0b053aa..9c06c94 100644
> --- a/testsuite/build_test/cibase.py
> +++ b/testsuite/build_test/cibase.py
> @@ -8,50 +8,24 @@ import time
> from cibuilder import CIBuilder
> from avocado.utils import process
>
> -isar_root = os.path.dirname(__file__) + '/../..'
>
> class CIBaseTest(CIBuilder):
> -
> - def prep(self, testname, targets, cross, debsrc_cache):
> - build_dir = self.params.get('build_dir', default=isar_root + '/build')
> - build_dir = os.path.realpath(build_dir)
> - quiet = int(self.params.get('quiet', default=0))
> - bitbake_args = '-v'
> -
> - if quiet:
> - bitbake_args = ''
> -
> - self.log.info('===================================================')
> - self.log.info('Running ' + testname + ' test for:')
> - self.log.info(targets)
> - self.log.info('Isar build folder is: ' + build_dir)
> - self.log.info('===================================================')
> -
> - self.init(build_dir)
> - self.confprepare(build_dir, 1, cross, debsrc_cache)
> -
> - return build_dir, bitbake_args;
> -
> - def perform_build_test(self, targets, cross, bitbake_cmd):
> - build_dir, bb_args = self.prep('Isar build', targets, cross, 1)
> + def perform_build_test(self, targets, **kwargs):
> + self.init(**kwargs)
> + self.configure(**kwargs)
>
> self.log.info('Starting build...')
>
> - self.bitbake(build_dir, targets, bitbake_cmd, bb_args)
> -
> - def perform_repro_test(self, targets, signed):
> - cross = int(self.params.get('cross', default=0))
> - build_dir, bb_args = self.prep('repro Isar build', targets, cross, 0)
> + self.bitbake(targets, **kwargs)
>
> + def perform_repro_test(self, targets, signed=False, **kwargs):
> gpg_pub_key = os.path.dirname(__file__) + '/../base-apt/test_pub.key'
> gpg_priv_key = os.path.dirname(__file__) + '/../base-apt/test_priv.key'
>
> - if signed:
> - with open(build_dir + '/conf/ci_build.conf', 'a') as file:
> - # Enable use of signed cached base repository
> - file.write('BASE_REPO_KEY="file://' + gpg_pub_key + '"\n')
> + self.init(**kwargs)
> + self.configure(gpg_pub_key=gpg_pub_key if signed else None, **kwargs)
>
> - os.chdir(build_dir)
> + os.chdir(self.build_dir)
>
> os.environ['GNUPGHOME'] = tempfile.mkdtemp()
> result = process.run('gpg --import %s %s' % (gpg_pub_key, gpg_priv_key))
> @@ -59,33 +33,31 @@ class CIBaseTest(CIBuilder):
> if result.exit_status:
> self.fail('GPG import failed')
>
> - self.bitbake(build_dir, targets, None, bb_args)
> + self.bitbake(targets, **kwargs)
>
> - self.deletetmp(build_dir)
> - with open(build_dir + '/conf/ci_build.conf', 'a') as file:
> - file.write('ISAR_USE_CACHED_BASE_REPO = "1"\n')
> - file.write('BB_NO_NETWORK = "1"\n')
> + self.delete_from_build_dir('tmp')
> + self.configure(gpg_pub_key=gpg_pub_key if signed else None, offline=True, **kwargs)
>
> - self.bitbake(build_dir, targets, None, bb_args)
> + self.bitbake(targets, **kwargs)
>
> # Disable use of cached base repository
> - self.confcleanup(build_dir)
> + self.unconfigure()
>
> if not signed:
> # Try to build with changed configuration with no cleanup
> - self.bitbake(build_dir, targets, None, bb_args)
> + self.bitbake(targets, **kwargs)
>
> # Cleanup
> - self.deletetmp(build_dir)
> + self.delete_from_build_dir('tmp')
>
> - def perform_wic_test(self, targets, wks_path, wic_path):
> - cross = int(self.params.get('cross', default=0))
> - build_dir, bb_args = self.prep('WIC exclude build', targets, cross, 1)
> + def perform_wic_test(self, targets, wks_path, wic_path, **kwargs):
> + self.init(**kwargs)
> + self.configure(**kwargs)
>
> layerdir_isar = self.getlayerdir('isar')
>
> wks_file = layerdir_isar + wks_path
> - wic_img = build_dir + wic_path
> + wic_img = self.build_dir + wic_path
>
> if not os.path.isfile(wic_img):
> self.fail('No build started before: ' + wic_img + ' not exist')
> @@ -99,44 +71,30 @@ class CIBaseTest(CIBuilder):
> for line in lines:
> file.write(re.sub(r'part \/ ', 'part \/ --exclude-path usr ',
> line))
> -
> try:
> - self.bitbake(build_dir, targets, None, bb_args)
> + self.bitbake(targets, **kwargs)
> finally:
> self.restorefile(wks_file)
> -
> self.restorefile(wic_img)
>
> - def perform_container_test(self, targets, bitbake_cmd):
> - cross = int(self.params.get('cross', default=0))
> - build_dir, bb_args = self.prep('Isar Container', targets, cross, 1)
> -
> - self.containerprep(build_dir)
> -
> - self.bitbake(build_dir, targets, bitbake_cmd, bb_args)
> -
> + def perform_ccache_test(self, targets, **kwargs):
> + self.init(**kwargs)
> + self.configure(ccache=True, **kwargs)
>
> - def perform_ccache_test(self, targets):
> - build_dir, bb_args = self.prep('Isar ccache build', targets, 0, 0)
> -
> - self.deletetmp(build_dir)
> - process.run('rm -rf ' + build_dir + '/ccache', sudo=True)
> -
> - with open(build_dir + '/conf/ci_build.conf', 'a') as file:
> - file.write('USE_CCACHE = "1"\n')
> - file.write('CCACHE_TOP_DIR = "${TOPDIR}/ccache"')
> + self.delete_from_build_dir('tmp')
> + self.delete_from_build_dir('ccache')
>
> self.log.info('Starting build and filling ccache dir...')
> start = time.time()
> - self.bitbake(build_dir, targets, None, bb_args)
> + self.bitbake(targets, **kwargs)
> first_time = time.time() - start
> self.log.info('Non-cached build: ' + str(round(first_time)) + 's')
>
> - self.deletetmp(build_dir)
> + self.delete_from_build_dir('tmp')
>
> self.log.info('Starting build and using ccache dir...')
> start = time.time()
> - self.bitbake(build_dir, targets, None, bb_args)
> + self.bitbake(targets, **kwargs)
> second_time = time.time() - start
> self.log.info('Cached build: ' + str(round(second_time)) + 's')
>
> @@ -145,5 +103,6 @@ class CIBaseTest(CIBuilder):
> self.fail('No speedup after rebuild with ccache')
>
> # Cleanup
> - self.deletetmp(build_dir)
> - process.run('rm -rf ' + build_dir + '/ccache', sudo=True)
> + self.delete_from_build_dir('tmp')
> + self.delete_from_build_dir('ccache')
> +
> diff --git a/testsuite/build_test/cibuilder.py b/testsuite/build_test/cibuilder.py
> index 94786c7..6a9f6a0 100644
> --- a/testsuite/build_test/cibuilder.py
> +++ b/testsuite/build_test/cibuilder.py
> @@ -2,7 +2,6 @@
>
> import logging
> import os
> -import re
> import select
> import shutil
> import subprocess
> @@ -11,9 +10,8 @@ from avocado import Test
> from avocado.utils import path
> from avocado.utils import process
>
> -isar_root = os.path.dirname(__file__) + '/../..'
> +isar_root = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..'))
> backup_prefix = '.ci-backup'
> -
> app_log = logging.getLogger("avocado.app")
>
> class CIBuilder(Test):
> @@ -28,17 +26,55 @@ class CIBuilder(Test):
> self._file_handler.setFormatter(formatter)
> app_log.addHandler(self._file_handler)
>
> - def init(self, build_dir):
> +
> + def init(self, build_dir='build', **kwargs):
> + # initialize build_dir and setup environment
> + # needs to run once (per test case)
> + self.build_dir = os.path.join(isar_root, build_dir)
> os.chdir(isar_root)
> - path.usable_rw_dir(build_dir)
> + path.usable_rw_dir(self.build_dir)
> output = process.getoutput('/bin/bash -c "source isar-init-build-env \
> - %s 2>&1 >/dev/null; env"' % build_dir)
> + %s 2>&1 >/dev/null; env"' % self.build_dir)
> env = dict(((x.split('=', 1) + [''])[:2] \
> for x in output.splitlines() if x != ''))
> os.environ.update(env)
>
> - def confprepare(self, build_dir, compat_arch, cross, debsrc_cache):
> - with open(build_dir + '/conf/ci_build.conf', 'w') as f:
> +
> + def configure(self, compat_arch=True, cross=None, debsrc_cache=True,
> + container=False, ccache=False, sstate=False, offline=False,
> + gpg_pub_key=None, **kwargs):
> + # write configuration file and set bitbake_args
> + # can run multiple times per test case
> +
> + # get parameters from avocado cmdline
> + quiet = bool(int(self.params.get('quiet', default=0)))
> + if cross is None:
> + cross = bool(int(self.params.get('cross', default=0)))
> +
> + # get parameters from environment
> + distro_apt_premir = os.getenv('DISTRO_APT_PREMIRRORS')
> +
> + self.log.info(f'===================================================\n'
> + f'Configuring build_dir {self.build_dir}\n'
> + f' compat_arch = {compat_arch}\n'
> + f' cross = {cross}\n'
> + f' debsrc_cache = {debsrc_cache}\n'
> + f' offline = {offline}\n'
> + f' container = {container}\n'
> + f' ccache = {ccache}\n'
> + f' sstate = {sstate}\n'
> + f' gpg_pub_key = {gpg_pub_key}\n'
> + f'===================================================')
> +
> + # determine bitbake_args
> + self.bitbake_args = []
> + if not quiet:
> + self.bitbake_args.append('-v')
> + if not sstate:
> + self.bitbake_args.append('--no-setscene')
> +
> + # write ci_build.conf
> + with open(self.build_dir + '/conf/ci_build.conf', 'w') as f:
> if compat_arch:
> f.write('ISAR_ENABLE_COMPAT_ARCH_amd64 = "1"\n')
> f.write('ISAR_ENABLE_COMPAT_ARCH_arm64 = "1"\n')
> @@ -48,36 +84,42 @@ class CIBuilder(Test):
> f.write('ISAR_CROSS_COMPILE = "1"\n')
> if debsrc_cache:
> f.write('BASE_REPO_FEATURES = "cache-deb-src"\n')
> - distro_apt_premir = os.getenv('DISTRO_APT_PREMIRRORS')
> + if offline:
> + f.write('ISAR_USE_CACHED_BASE_REPO = "1"\n')
> + f.write('BB_NO_NETWORK = "1"\n')
> + if container:
> + f.write('SDK_FORMATS = "docker-archive"\n')
> + f.write('IMAGE_INSTALL_remove = "example-module-${KERNEL_NAME} enable-fsck"\n')
> + if gpg_pub_key:
> + f.write('BASE_REPO_KEY="file://' + gpg_pub_key + '"\n')
> if distro_apt_premir:
> f.write('DISTRO_APT_PREMIRRORS = "%s"\n' % distro_apt_premir)
>
> - with open(build_dir + '/conf/local.conf', 'r+') as f:
> + # include ci_build.conf in local.conf
> + with open(self.build_dir + '/conf/local.conf', 'r+') as f:
> for line in f:
> if 'include ci_build.conf' in line:
> break
> else:
> f.write('\ninclude ci_build.conf')
>
> - def containerprep(self, build_dir):
> - with open(build_dir + '/conf/ci_build.conf', 'a') as f:
> - f.write('SDK_FORMATS = "docker-archive"\n')
> - f.write('IMAGE_INSTALL_remove = "example-module-${KERNEL_NAME} enable-fsck"\n')
> -
> - def confcleanup(self, build_dir):
> - open(build_dir + '/conf/ci_build.conf', 'w').close()
> + def unconfigure(self):
> + open(self.build_dir + '/conf/ci_build.conf', 'w').close()
>
> - def deletetmp(self, build_dir):
> - process.run('rm -rf ' + build_dir + '/tmp', sudo=True)
> + def delete_from_build_dir(self, path):
> + process.run('rm -rf ' + self.build_dir + '/' + path, sudo=True)
>
> - def bitbake(self, build_dir, target, cmd, args):
> - os.chdir(build_dir)
> + def bitbake(self, target, bitbake_cmd=None, **kwargs):
> + self.log.info('===================================================')
> + self.log.info('Building ' + str(target))
> + self.log.info('===================================================')
> + os.chdir(self.build_dir)
> cmdline = ['bitbake']
> - if args:
> - cmdline.append(args)
> - if cmd:
> + if self.bitbake_args:
> + cmdline.extend(self.bitbake_args)
> + if bitbake_cmd:
> cmdline.append('-c')
> - cmdline.append(cmd)
> + cmdline.append(bitbake_cmd)
> if isinstance(target, list):
> cmdline.extend(target)
> else:
> @@ -124,9 +166,9 @@ class CIBuilder(Test):
> try:
> path.find_command('bitbake')
> except path.CmdNotFoundError:
> - build_dir = self.params.get('build_dir',
> - default=isar_root + '/build')
> - self.init(build_dir)
> + self.log.error("Broken test implementation: must call init() before "
> + "using getlayerdir()!")
> + raise
> output = process.getoutput('bitbake -e | grep "^LAYERDIR_.*="')
> env = dict(((x.split('=', 1) + [''])[:2] \
> for x in output.splitlines() if x != ''))
--
Anton Mikanovich
Promwad Ltd.
External service provider of ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn, Germany
+49 (89) 122 67 24-0
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov
next prev parent reply other threads:[~2021-12-10 15:05 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-10 8:15 Adriaan Schmidt
2021-12-10 15:05 ` Anton Mikanovich [this message]
2021-12-10 16:12 ` Schmidt, Adriaan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e650b7b5-afb3-3700-4940-38e9a3e21931@ilbers.de \
--to=amikan@ilbers.de \
--cc=adriaan.schmidt@siemens.com \
--cc=isar-users@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox