All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] selftest/context: Avoid tracebacks from tests using multiprocessing
@ 2020-02-19 18:38 Richard Purdie
  2020-02-19 18:38 ` [PATCH 2/3] oeqa/selftest: Startardise seperate builddir for concurrent and non-concurrent selftest Richard Purdie
  2020-02-19 18:38 ` [PATCH 3/3] oeqa/selftest: Drop 'backup' code and SIGTERM handler Richard Purdie
  0 siblings, 2 replies; 5+ messages in thread
From: Richard Purdie @ 2020-02-19 18:38 UTC (permalink / raw)
  To: openembedded-core

We can see tracebacks where the SIGTERM handler catches things
it shouldn't. Avoid exit(1) unless we're the process that
it was intended for.

[YOCTO #13664]

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/lib/oeqa/selftest/context.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/meta/lib/oeqa/selftest/context.py b/meta/lib/oeqa/selftest/context.py
index c4eb5d614eb..3d3b19c6e80 100644
--- a/meta/lib/oeqa/selftest/context.py
+++ b/meta/lib/oeqa/selftest/context.py
@@ -280,11 +280,15 @@ class OESelftestTestContextExecutor(OETestContextExecutor):
         return rc
 
     def _signal_clean_handler(self, signum, frame):
-        sys.exit(1)
+        if self.ourpid == os.getpid():
+            sys.exit(1)
     
     def run(self, logger, args):
         self._process_args(logger, args)
 
+        # Setup a SIGTERM handler to allow restoration of files like local.conf and bblayers.conf
+        # but don't interfer with other processes
+        self.ourpid = os.getpid()
         signal.signal(signal.SIGTERM, self._signal_clean_handler)
 
         rc = None
-- 
2.25.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] oeqa/selftest: Startardise seperate builddir for concurrent and non-concurrent selftest
  2020-02-19 18:38 [PATCH 1/3] selftest/context: Avoid tracebacks from tests using multiprocessing Richard Purdie
@ 2020-02-19 18:38 ` Richard Purdie
  2020-02-19 18:38 ` [PATCH 3/3] oeqa/selftest: Drop 'backup' code and SIGTERM handler Richard Purdie
  1 sibling, 0 replies; 5+ messages in thread
From: Richard Purdie @ 2020-02-19 18:38 UTC (permalink / raw)
  To: openembedded-core

Currently oe-selftest reuses the current build directory and the concurrent
version run with -j does not.

Standardise and use a separate new build directory in both cases. This will lead
to simpler code and more reliable user run tests.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/lib/oeqa/core/context.py               | 12 +++--
 meta/lib/oeqa/core/utils/concurrencytest.py | 35 ++-------------
 meta/lib/oeqa/selftest/context.py           | 50 +++++++++++++++++++++
 3 files changed, 58 insertions(+), 39 deletions(-)

diff --git a/meta/lib/oeqa/core/context.py b/meta/lib/oeqa/core/context.py
index 14fc6a54f4b..16320af1157 100644
--- a/meta/lib/oeqa/core/context.py
+++ b/meta/lib/oeqa/core/context.py
@@ -72,6 +72,9 @@ class OETestContext(object):
                 modules_required, **kwargs)
         self.suites = self.loader.discover()
 
+    def prepareSuite(self, suites, processes):
+        return suites
+
     def runTests(self, processes=None, skips=[]):
         self.runner = self.runnerClass(self, descriptions=False, verbosity=2)
 
@@ -79,14 +82,9 @@ class OETestContext(object):
         self.skipTests(skips)
 
         self._run_start_time = time.time()
-        if processes:
-            from oeqa.core.utils.concurrencytest import ConcurrentTestSuite
-
-            concurrent_suite = ConcurrentTestSuite(self.suites, processes)
-            result = self.runner.run(concurrent_suite)
-        else:
+        if not processes:
             self.runner.buffer = True
-            result = self.runner.run(self.suites)
+        result = self.runner.run(self.prepareSuite(self.suites, processes))
         self._run_end_time = time.time()
 
         return result
diff --git a/meta/lib/oeqa/core/utils/concurrencytest.py b/meta/lib/oeqa/core/utils/concurrencytest.py
index 0d9c01e6d45..fac59f765ac 100644
--- a/meta/lib/oeqa/core/utils/concurrencytest.py
+++ b/meta/lib/oeqa/core/utils/concurrencytest.py
@@ -177,9 +177,10 @@ class dummybuf(object):
 #
 class ConcurrentTestSuite(unittest.TestSuite):
 
-    def __init__(self, suite, processes):
+    def __init__(self, suite, processes, setupfunc):
         super(ConcurrentTestSuite, self).__init__([suite])
         self.processes = processes
+        self.setupfunc = setupfunc
 
     def run(self, result):
         tests, totaltests = fork_for_tests(self.processes, self)
@@ -272,37 +273,7 @@ def fork_for_tests(concurrency_num, suite):
                 stream = os.fdopen(c2pwrite, 'wb', 1)
                 os.close(c2pread)
 
-                # Create a new separate BUILDDIR for each group of tests
-                if 'BUILDDIR' in os.environ:
-                    builddir = os.environ['BUILDDIR']
-                    newbuilddir = builddir + "-st-" + str(ourpid)
-                    newselftestdir = newbuilddir + "/meta-selftest"
-
-                    bb.utils.mkdirhier(newbuilddir)
-                    oe.path.copytree(builddir + "/conf", newbuilddir + "/conf")
-                    oe.path.copytree(builddir + "/cache", newbuilddir + "/cache")
-                    oe.path.copytree(selftestdir, newselftestdir)
-
-                    for e in os.environ:
-                        if builddir in os.environ[e]:
-                            os.environ[e] = os.environ[e].replace(builddir, newbuilddir)
-
-                    subprocess.check_output("git init; git add *; git commit -a -m 'initial'", cwd=newselftestdir, shell=True)
-
-                    # Tried to used bitbake-layers add/remove but it requires recipe parsing and hence is too slow
-                    subprocess.check_output("sed %s/conf/bblayers.conf -i -e 's#%s#%s#g'" % (newbuilddir, selftestdir, newselftestdir), cwd=newbuilddir, shell=True)
-
-                    os.chdir(newbuilddir)
-
-                    for t in process_suite:
-                        if not hasattr(t, "tc"):
-                            continue
-                        cp = t.tc.config_paths
-                        for p in cp:
-                            if selftestdir in cp[p] and newselftestdir not in cp[p]:
-                                cp[p] = cp[p].replace(selftestdir, newselftestdir)
-                            if builddir in cp[p] and newbuilddir not in cp[p]:
-                                cp[p] = cp[p].replace(builddir, newbuilddir)
+                (builddir, newbuilddir) = suite.setupfunc("-st-" + str(ourpid), selftestdir, process_suite)
 
                 # Leave stderr and stdout open so we can see test noise
                 # Close stdin so that the child goes away if it decides to
diff --git a/meta/lib/oeqa/selftest/context.py b/meta/lib/oeqa/selftest/context.py
index 3d3b19c6e80..2cb6c763852 100644
--- a/meta/lib/oeqa/selftest/context.py
+++ b/meta/lib/oeqa/selftest/context.py
@@ -10,11 +10,13 @@ import glob
 import sys
 import importlib
 import signal
+import subprocess
 from shutil import copyfile
 from random import choice
 
 import oeqa
 import oe
+import bb.utils
 
 from oeqa.core.context import OETestContext, OETestContextExecutor
 from oeqa.core.exception import OEQAPreRun, OEQATestNotFound
@@ -29,6 +31,54 @@ class OESelftestTestContext(OETestContext):
         self.custommachine = None
         self.config_paths = config_paths
 
+    def setup_builddir(self, suffix, selftestdir, suite):
+        builddir = os.environ['BUILDDIR']
+        if not selftestdir:
+            selftestdir = get_test_layer()
+        newbuilddir = builddir + suffix
+        newselftestdir = newbuilddir + "/meta-selftest"
+
+        if os.path.exists(newbuilddir):
+            self.logger.error("Build directory %s already exists, aborting")
+            sys.exit(1)
+
+        bb.utils.mkdirhier(newbuilddir)
+        oe.path.copytree(builddir + "/conf", newbuilddir + "/conf")
+        oe.path.copytree(builddir + "/cache", newbuilddir + "/cache")
+        oe.path.copytree(selftestdir, newselftestdir)
+
+        for e in os.environ:
+            if builddir in os.environ[e]:
+                os.environ[e] = os.environ[e].replace(builddir, newbuilddir)
+
+        subprocess.check_output("git init; git add *; git commit -a -m 'initial'", cwd=newselftestdir, shell=True)
+
+        # Tried to used bitbake-layers add/remove but it requires recipe parsing and hence is too slow
+        subprocess.check_output("sed %s/conf/bblayers.conf -i -e 's#%s#%s#g'" % (newbuilddir, selftestdir, newselftestdir), cwd=newbuilddir, shell=True)
+
+        os.chdir(newbuilddir)
+
+        for t in suite:
+            if not hasattr(t, "tc"):
+                continue
+            cp = t.tc.config_paths
+            for p in cp:
+                if selftestdir in cp[p] and newselftestdir not in cp[p]:
+                    cp[p] = cp[p].replace(selftestdir, newselftestdir)
+                if builddir in cp[p] and newbuilddir not in cp[p]:
+                    cp[p] = cp[p].replace(builddir, newbuilddir)
+
+        return (builddir, newbuilddir)
+
+    def prepareSuite(self, suites, processes):
+        if processes:
+            from oeqa.core.utils.concurrencytest import ConcurrentTestSuite
+
+            return ConcurrentTestSuite(suites, processes, self.setup_builddir)
+        else:
+            self.setup_builddir("-st", None, suites)
+            return suites
+
     def runTests(self, processes=None, machine=None, skips=[]):
         if machine:
             self.custommachine = machine
-- 
2.25.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] oeqa/selftest: Drop 'backup' code and SIGTERM handler
  2020-02-19 18:38 [PATCH 1/3] selftest/context: Avoid tracebacks from tests using multiprocessing Richard Purdie
  2020-02-19 18:38 ` [PATCH 2/3] oeqa/selftest: Startardise seperate builddir for concurrent and non-concurrent selftest Richard Purdie
@ 2020-02-19 18:38 ` Richard Purdie
  2020-02-19 22:26   ` Peter Kjellerstedt
  1 sibling, 1 reply; 5+ messages in thread
From: Richard Purdie @ 2020-02-19 18:38 UTC (permalink / raw)
  To: openembedded-core

Now selftest is using its own copied build directory, we can stop worrying about
copying files around as backup, and drop the SIGTERM handler to try and restore
them, simplifying the code.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/lib/oeqa/selftest/case.py    | 30 +-------------------
 meta/lib/oeqa/selftest/context.py | 47 ++-----------------------------
 2 files changed, 4 insertions(+), 73 deletions(-)

diff --git a/meta/lib/oeqa/selftest/case.py b/meta/lib/oeqa/selftest/case.py
index ac3308d8a44..dcad4f76ecd 100644
--- a/meta/lib/oeqa/selftest/case.py
+++ b/meta/lib/oeqa/selftest/case.py
@@ -6,7 +6,6 @@
 
 import sys
 import os
-import shutil
 import glob
 import errno
 from unittest.util import safe_repr
@@ -30,9 +29,7 @@ class OESelftestTestCase(OETestCase):
         cls.builddir = cls.tc.config_paths['builddir']
 
         cls.localconf_path = cls.tc.config_paths['localconf']
-        cls.localconf_backup = cls.tc.config_paths['localconf_class_backup']
         cls.local_bblayers_path = cls.tc.config_paths['bblayers']
-        cls.local_bblayers_backup = cls.tc.config_paths['bblayers_class_backup']
 
         cls.testinc_path = os.path.join(cls.tc.config_paths['builddir'],
                 "conf/selftest.inc")
@@ -43,8 +40,7 @@ class OESelftestTestCase(OETestCase):
 
         cls._track_for_cleanup = [
             cls.testinc_path, cls.testinc_bblayers_path,
-            cls.machineinc_path, cls.localconf_backup,
-            cls.local_bblayers_backup]
+            cls.machineinc_path]
 
         cls.add_include()
 
@@ -102,30 +98,6 @@ class OESelftestTestCase(OETestCase):
     def setUp(self):
         super(OESelftestTestCase, self).setUp()
         os.chdir(self.builddir)
-        # Check if local.conf or bblayers.conf files backup exists
-        # from a previous failed test and restore them
-        if os.path.isfile(self.localconf_backup) or os.path.isfile(
-                self.local_bblayers_backup):
-            self.logger.debug("\
-Found a local.conf and/or bblayers.conf backup from a previously aborted test.\
-Restoring these files now, but tests should be re-executed from a clean environment\
-to ensure accurate results.")
-            try:
-                shutil.copyfile(self.localconf_backup, self.localconf_path)
-            except OSError as e:
-                if e.errno != errno.ENOENT:
-                    raise
-            try:
-                shutil.copyfile(self.local_bblayers_backup,
-                                self.local_bblayers_path)
-            except OSError as e:
-                if e.errno != errno.ENOENT:
-                    raise
-        else:
-            # backup local.conf and bblayers.conf
-            shutil.copyfile(self.localconf_path, self.localconf_backup)
-            shutil.copyfile(self.local_bblayers_path, self.local_bblayers_backup)
-            self.logger.debug("Creating local.conf and bblayers.conf backups.")
         # we don't know what the previous test left around in config or inc files
         # if it failed so we need a fresh start
         try:
diff --git a/meta/lib/oeqa/selftest/context.py b/meta/lib/oeqa/selftest/context.py
index 2cb6c763852..0ab6462bbf8 100644
--- a/meta/lib/oeqa/selftest/context.py
+++ b/meta/lib/oeqa/selftest/context.py
@@ -9,9 +9,7 @@ import time
 import glob
 import sys
 import importlib
-import signal
 import subprocess
-from shutil import copyfile
 from random import choice
 
 import oeqa
@@ -185,26 +183,10 @@ class OESelftestTestContextExecutor(OETestContextExecutor):
 
         builddir = os.environ.get("BUILDDIR")
         self.tc_kwargs['init']['config_paths'] = {}
-        self.tc_kwargs['init']['config_paths']['testlayer_path'] = \
-                get_test_layer()
+        self.tc_kwargs['init']['config_paths']['testlayer_path'] = get_test_layer()
         self.tc_kwargs['init']['config_paths']['builddir'] = builddir
-        self.tc_kwargs['init']['config_paths']['localconf'] = \
-                os.path.join(builddir, "conf/local.conf")
-        self.tc_kwargs['init']['config_paths']['localconf_backup'] = \
-                os.path.join(builddir, "conf/local.conf.orig")
-        self.tc_kwargs['init']['config_paths']['localconf_class_backup'] = \
-                os.path.join(builddir, "conf/local.conf.bk")
-        self.tc_kwargs['init']['config_paths']['bblayers'] = \
-                os.path.join(builddir, "conf/bblayers.conf")
-        self.tc_kwargs['init']['config_paths']['bblayers_backup'] = \
-                os.path.join(builddir, "conf/bblayers.conf.orig")
-        self.tc_kwargs['init']['config_paths']['bblayers_class_backup'] = \
-                os.path.join(builddir, "conf/bblayers.conf.bk")
-
-        copyfile(self.tc_kwargs['init']['config_paths']['localconf'],
-                self.tc_kwargs['init']['config_paths']['localconf_backup'])
-        copyfile(self.tc_kwargs['init']['config_paths']['bblayers'], 
-                self.tc_kwargs['init']['config_paths']['bblayers_backup'])
+        self.tc_kwargs['init']['config_paths']['localconf'] = os.path.join(builddir, "conf/local.conf")
+        self.tc_kwargs['init']['config_paths']['bblayers'] = os.path.join(builddir, "conf/bblayers.conf")
 
         def tag_filter(tags):
             if args.exclude_tags:
@@ -329,18 +311,9 @@ class OESelftestTestContextExecutor(OETestContextExecutor):
 
         return rc
 
-    def _signal_clean_handler(self, signum, frame):
-        if self.ourpid == os.getpid():
-            sys.exit(1)
-    
     def run(self, logger, args):
         self._process_args(logger, args)
 
-        # Setup a SIGTERM handler to allow restoration of files like local.conf and bblayers.conf
-        # but don't interfer with other processes
-        self.ourpid = os.getpid()
-        signal.signal(signal.SIGTERM, self._signal_clean_handler)
-
         rc = None
         try:
             if args.machine:
@@ -369,20 +342,6 @@ class OESelftestTestContextExecutor(OETestContextExecutor):
                 rc = self._internal_run(logger, args)
         finally:
             config_paths = self.tc_kwargs['init']['config_paths']
-            if os.path.exists(config_paths['localconf_backup']):
-                copyfile(config_paths['localconf_backup'],
-                        config_paths['localconf'])
-                os.remove(config_paths['localconf_backup'])
-
-            if os.path.exists(config_paths['bblayers_backup']):
-                copyfile(config_paths['bblayers_backup'], 
-                        config_paths['bblayers'])
-                os.remove(config_paths['bblayers_backup'])
-
-            if os.path.exists(config_paths['localconf_class_backup']):
-                os.remove(config_paths['localconf_class_backup'])
-            if os.path.exists(config_paths['bblayers_class_backup']):
-                os.remove(config_paths['bblayers_class_backup'])
 
             output_link = os.path.join(os.path.dirname(args.output_log),
                     "%s-results.log" % self.name)
-- 
2.25.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 3/3] oeqa/selftest: Drop 'backup' code and SIGTERM handler
  2020-02-19 18:38 ` [PATCH 3/3] oeqa/selftest: Drop 'backup' code and SIGTERM handler Richard Purdie
@ 2020-02-19 22:26   ` Peter Kjellerstedt
  2020-02-19 22:28     ` Richard Purdie
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Kjellerstedt @ 2020-02-19 22:26 UTC (permalink / raw)
  To: Richard Purdie, openembedded-core

> -----Original Message-----
> From: openembedded-core-bounces@lists.openembedded.org <openembedded-core-bounces@lists.openembedded.org> On Behalf Of Richard Purdie
> Sent: den 19 februari 2020 19:38
> To: openembedded-core@lists.openembedded.org
> Subject: [OE-core] [PATCH 3/3] oeqa/selftest: Drop 'backup' code and SIGTERM handler
> 
> Now selftest is using its own copied build directory, we can stop worrying about
> copying files around as backup, and drop the SIGTERM handler to try and restore
> them, simplifying the code.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  meta/lib/oeqa/selftest/case.py    | 30 +-------------------
>  meta/lib/oeqa/selftest/context.py | 47 ++-----------------------------
>  2 files changed, 4 insertions(+), 73 deletions(-)

[cut]

> @@ -329,18 +311,9 @@ class
> OESelftestTestContextExecutor(OETestContextExecutor):
> 
>          return rc
> 
> -    def _signal_clean_handler(self, signum, frame):
> -        if self.ourpid == os.getpid():
> -            sys.exit(1)
> -
>      def run(self, logger, args):
>          self._process_args(logger, args)
> 
> -        # Setup a SIGTERM handler to allow restoration of files like local.conf and bblayers.conf
> -        # but don't interfer with other processes
> -        self.ourpid = os.getpid()
> -        signal.signal(signal.SIGTERM, self._signal_clean_handler)
> -

What's the point of the first patch in this series, when you just 
remove it all again here in the third patch?

>          rc = None
>          try:
>              if args.machine:

//Peter


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 3/3] oeqa/selftest: Drop 'backup' code and SIGTERM handler
  2020-02-19 22:26   ` Peter Kjellerstedt
@ 2020-02-19 22:28     ` Richard Purdie
  0 siblings, 0 replies; 5+ messages in thread
From: Richard Purdie @ 2020-02-19 22:28 UTC (permalink / raw)
  To: Peter Kjellerstedt, openembedded-core

On Wed, 2020-02-19 at 22:26 +0000, Peter Kjellerstedt wrote:
> > -----Original Message-----
> > From: openembedded-core-bounces@lists.openembedded.org <
> > openembedded-core-bounces@lists.openembedded.org> On Behalf Of
> > Richard Purdie
> > Sent: den 19 februari 2020 19:38
> > To: openembedded-core@lists.openembedded.org
> > Subject: [OE-core] [PATCH 3/3] oeqa/selftest: Drop 'backup' code
> > and SIGTERM handler
> > 
> > Now selftest is using its own copied build directory, we can stop
> > worrying about
> > copying files around as backup, and drop the SIGTERM handler to try
> > and restore
> > them, simplifying the code.
> > 
> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> > ---
> >  meta/lib/oeqa/selftest/case.py    | 30 +-------------------
> >  meta/lib/oeqa/selftest/context.py | 47 ++-------------------------
> > ----
> >  2 files changed, 4 insertions(+), 73 deletions(-)
> 
> [cut]
> 
> > @@ -329,18 +311,9 @@ class
> > OESelftestTestContextExecutor(OETestContextExecutor):
> > 
> >          return rc
> > 
> > -    def _signal_clean_handler(self, signum, frame):
> > -        if self.ourpid == os.getpid():
> > -            sys.exit(1)
> > -
> >      def run(self, logger, args):
> >          self._process_args(logger, args)
> > 
> > -        # Setup a SIGTERM handler to allow restoration of files
> > like local.conf and bblayers.conf
> > -        # but don't interfer with other processes
> > -        self.ourpid = os.getpid()
> > -        signal.signal(signal.SIGTERM, self._signal_clean_handler)
> > -
> 
> What's the point of the first patch in this series, when you just 
> remove it all again here in the third patch?

The bug affects stable series builds as well as master. We're probably
unlikely to accept 2/3 or 3/3 into stable builds? I therefore wrote it
as a series, just in case the stable maintainers do pick up 1/3 which
I'd figured out before writing 2/3 and 3/3.

Cheers,

Richard




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-02-19 22:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-19 18:38 [PATCH 1/3] selftest/context: Avoid tracebacks from tests using multiprocessing Richard Purdie
2020-02-19 18:38 ` [PATCH 2/3] oeqa/selftest: Startardise seperate builddir for concurrent and non-concurrent selftest Richard Purdie
2020-02-19 18:38 ` [PATCH 3/3] oeqa/selftest: Drop 'backup' code and SIGTERM handler Richard Purdie
2020-02-19 22:26   ` Peter Kjellerstedt
2020-02-19 22:28     ` Richard Purdie

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.