Re: [PATCH v6 13/13] sstate: add test case

[Henning Schild <henning.schild@siemens.com>]

Am Tue, 16 Nov 2021 19:46:45 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> 11.11.2021 09:47, Adriaan Schmidt wrote:
> > This also adds a parameter `sstate` to the build function, which is
> > only passed as `1` for the actual sstate test. None of the other
> > tests uses sstate caching.
> >
> > Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
> > ---
> >   testsuite/build_test/build_test.py | 101
> > ++++++++++++++++++++++++++--- testsuite/build_test/cibase.py     |
> > 14 ++-- 2 files changed, 101 insertions(+), 14 deletions(-)
> >
> > diff --git a/testsuite/build_test/build_test.py
> > b/testsuite/build_test/build_test.py index d39c10c..817f4ce 100644
> > --- a/testsuite/build_test/build_test.py
> > +++ b/testsuite/build_test/build_test.py
> > @@ -1,9 +1,10 @@
> >   #!/usr/bin/env python3
> >   
> > +import glob
> >   import os
> >   
> >   from avocado import skipUnless
> > -from avocado.utils import path
> > +from avocado.utils import path, process
> >   from cibase import CIBaseTest
> >   
> >   UMOCI_AVAILABLE = True
> > @@ -58,7 +59,7 @@ class CrossTest(CIBaseTest):
> >               'mc:rpi-stretch:isar-image-base'
> >                     ]
> >   
> > -        self.perform_build_test(targets, 1, None)
> > +        self.perform_build_test(targets, 1, 0, None)
> >   
> >       def test_cross_ubuntu(self):
> >           targets = [
> > @@ -66,7 +67,7 @@ class CrossTest(CIBaseTest):
> >                     ]
> >   
> >           try:
> > -            self.perform_build_test(targets, 1, None)
> > +            self.perform_build_test(targets, 1, 0, None)
> >           except:
> >               self.cancel('KFAIL')
> >   
> > @@ -76,7 +77,7 @@ class CrossTest(CIBaseTest):
> >                     ]
> >   
> >           try:
> > -            self.perform_build_test(targets, 1, None)
> > +            self.perform_build_test(targets, 1, 0, None)
> >           except:
> >               self.cancel('KFAIL')
> >   
> > @@ -90,7 +91,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, 1, 0, 'do_populate_sdk')
> >   
> >   class NoCrossTest(CIBaseTest):
> >   
> > @@ -123,7 +124,7 @@ class NoCrossTest(CIBaseTest):
> >           self.deletetmp(self.params.get('build_dir',
> >                          default=os.path.dirname(__file__) +
> > '/../../build')) 
> > -        self.perform_build_test(targets, 0, None)
> > +        self.perform_build_test(targets, 0, 0, None)
> >   
> >       def test_nocross_bullseye(self):
> >           targets = [
> > @@ -134,7 +135,7 @@ class NoCrossTest(CIBaseTest):
> >                     ]
> >   
> >           try:
> > -            self.perform_build_test(targets, 0, None)
> > +            self.perform_build_test(targets, 0, 0, None)
> >           except:
> >               self.cancel('KFAIL')
> >   
> > @@ -158,7 +159,7 @@ class RebuildTest(CIBaseTest):
> >   
> >           try:
> >               self.perform_build_test('mc:qemuamd64-stretch:isar-image-base',
> > -                                    is_cross_build, None)
> > +                                    is_cross_build, 0, None)
> >           finally:
> >               self.restorefile(dpkgbase_file)
> >   
> > @@ -206,3 +207,87 @@ class ContainerSdkTest(CIBaseTest):
> >           targets = ['mc:container-amd64-stretch:isar-image-base']
> >   
> >           self.perform_container_test(targets, 'do_populate_sdk')
> > +
> > +class SstateTest(CIBaseTest):
> > +
> > +    """
> > +    Test builds with artifacts taken from sstate cache
> > +
> > +    :avocado: tags=sstate,fast,full
> > +    """
> > +    def check_executed_tasks(self, build_dir, target, expected):
> > +        taskorder_file =
> > glob.glob(f'{build_dir}/tmp/work/{self.distro_distroarch}/{target}/*/temp/log.task_order')
> > +        try:
> > +            with open(taskorder_file[0], 'r') as f:
> > +                tasks = [l.split()[0] for l in f.readlines()]
> > +        except (FileNotFoundError, IndexError):
> > +            tasks = []
> > +        if expected is None:
> > +            # require that no tasks were executed
> > +            return len(tasks) == 0
> > +        for e in expected:
> > +            should_run = True
> > +            if e.startswith('!'):
> > +                should_run = False
> > +                e = e[1:]
> > +            if should_run != (e in tasks):
> > +                self.log.error(f"{target}: executed tasks
> > {str(tasks)} did not match expected {str(expected)}")
> > +                return False
> > +        return True
> > +
> > +    def test_sstate(self):
> > +        build_dir, bb_args = self.prep('Sstate', '', 0, 1, 1)
> > +        image_target = 'mc:qemuamd64-bullseye:isar-image-base'
> > +        package_target = 'mc:qemuamd64-bullseye:hello'
> > +        self.distro_distroarch = 'debian-bullseye-amd64'
> > +
> > +        # Cleanup sstate and tmp before test
> > +        process.run(f'rm -rf {build_dir}/sstate-cache', sudo=True)
> > +        self.deletetmp(build_dir)
> > +
> > +        # Populate cache
> > +        self.perform_build_test(image_target, 0, 1, None)
> > +
> > +        # Rebuild image
> > +        self.deletetmp(build_dir)
> > +        self.perform_build_test(image_target, 0, 1, None)
> > +        if not all([
> > +                self.check_executed_tasks(build_dir,
> > 'isar-bootstrap-target',
> > +                    ['do_bootstrap_setscene', '!do_bootstrap']),
> > +                self.check_executed_tasks(build_dir,
> > 'buildchroot-target',
> > +                    ['do_rootfs_setscene', '!do_rootfs']),
> > +                self.check_executed_tasks(build_dir,
> > 'isar-image-base-*-wic-img',
> > +                    ['do_rootfs_setscene', '!do_rootfs'])
> > +            ]):
> > +            self.fail("Failed rebuild image")
> > +
> > +        # Rebuild single package
> > +        self.deletetmp(build_dir)
> > +        self.perform_build_test(package_target, 0, 1, None)
> > +        if not all([
> > +                self.check_executed_tasks(build_dir,
> > 'isar-bootstrap-target',
> > +                    ['do_bootstrap_setscene']),
> > +                self.check_executed_tasks(build_dir,
> > 'buildchroot-target',
> > +                    ['!do_buildchroot_deploy']),
> > +                self.check_executed_tasks(build_dir, 'hello',
> > +                    ['do_dpkg_build_setscene', 'do_deploy_deb',
> > '!do_dpkg_build'])
> > +            ]):
> > +            self.fail("Failed rebuild single package")
> > +
> > +        # Rebuild package and image
> > +        self.deletetmp(build_dir)
> > +        process.run(f'find {build_dir}/sstate-cache/ -name
> > sstate:hello:* -delete')
> > +        self.perform_build_test(image_target, 0, 1, None)
> > +        if not all([
> > +                self.check_executed_tasks(build_dir,
> > 'isar-bootstrap-target',
> > +                    ['do_bootstrap_setscene', '!do_bootstrap']),
> > +                self.check_executed_tasks(build_dir,
> > 'buildchroot-target',
> > +                    ['do_rootfs_setscene', '!do_rootfs']),
> > +                self.check_executed_tasks(build_dir, 'hello',
> > +                    ['do_fetch', 'do_dpkg_build']),
> > +                # TODO: if we actually make a change to hello,
> > then we could test that do_rootfs is executed.
> > +                #       currently, hello is rebuilt, but its
> > sstate sig/hash does not change.
> > +                self.check_executed_tasks(build_dir,
> > 'isar-image-base-*-wic-img',
> > +                    ['do_rootfs_setscene', '!do_rootfs'])
> > +            ]):
> > +            self.fail("Failed rebuild package and image")
> > diff --git a/testsuite/build_test/cibase.py
> > b/testsuite/build_test/cibase.py index 0bff7e4..3cba412 100644
> > --- a/testsuite/build_test/cibase.py
> > +++ b/testsuite/build_test/cibase.py
> > @@ -11,7 +11,7 @@ isar_root = os.path.dirname(__file__) + '/../..'
> >   
> >   class CIBaseTest(CIBuilder):
> >   
> > -    def prep(self, testname, targets, cross, debsrc_cache):
> > +    def prep(self, testname, targets, cross, debsrc_cache, sstate):
> >           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)) @@ -19,6 +19,8 @@ class CIBaseTest(CIBuilder):
> >   
> >           if not quiet:
> >               bitbake_args.append('-v')
> > +        if not sstate:
> > +            bitbake_args.append('--no-setscene')
> >   
> >           self.log.info('===================================================')
> >           self.log.info('Running ' + testname + ' test for:')
> > @@ -31,8 +33,8 @@ class CIBaseTest(CIBuilder):
> >   
> >           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, cross, sstate,
> > bitbake_cmd):
> > +        build_dir, bb_args = self.prep('Isar build', targets,
> > cross, 1, sstate) 
> >           self.log.info('Starting build...')
> >   
> > @@ -40,7 +42,7 @@ class CIBaseTest(CIBuilder):
> >   
> >       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)
> > +        build_dir, bb_args = self.prep('repro Isar build',
> > targets, cross, 0, 0) 
> >           gpg_pub_key = os.path.dirname(__file__) +
> > '/../base-apt/test_pub.key' gpg_priv_key =
> > os.path.dirname(__file__) + '/../base-apt/test_priv.key' @@ -79,7
> > +81,7 @@ class CIBaseTest(CIBuilder): 
> >       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)
> > +        build_dir, bb_args = self.prep('WIC exclude build',
> > targets, cross, 1, 0) 
> >           layerdir_isar = self.getlayerdir('isar')
> >   
> > @@ -108,7 +110,7 @@ class CIBaseTest(CIBuilder):
> >   
> >       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)
> > +        build_dir, bb_args = self.prep('Isar Container', targets,
> > cross, 1, 0) 
> >           self.containerprep(build_dir)
> >     
> 
> Thanks for adding test case, but if you need to add it to fast/full
> in should be moved to the beginning of the build_test.py (after repro 
> maybe) to not cleanup images needed for start_vm testing. Executing
> it at the end will also remove all the logs of common tests.
> Maybe we can not remove tmp but just set any empty SSTATE_DIR?
> 
> Changing testsuite API should probably be moved to the separate patch.
> 

While i am always _for_ adding test-cases for anything. In this case i
already voted against it. The sstate patches are simply too valuable to
delay them any further! They will significantly speed up anything ...
and maybe cause some issues as well.
But dealing with such potential issues will also become significantly
faster after a merge! I have like 5+ open MRs that first got delayed
with avocado and which i am shy to rebase because that sstate will
hopefully speed us up by orders of magnitude!

I personally am all for merging this ASAP and getting nitpicky
afterwards. That is what we did with avocado ... which in fact is in a
pretty horrible state but was just something to be done.

Not sure how much more to stress this. We will gain soo much speed ...
which we already see in our downstream layers that are sstate-enabled.

Henning