[1/1] oeqa/concurrencytest: fix for locating meta-selftest

Submitted by Robert Yang on Dec. 20, 2018, 7:43 a.m. | Patch ID: 157353

Details

Message ID 29e6785a8b38d49961aa7f1138d664336052dca8.1545291716.git.liezhi.yang@windriver.com
State New
Headers show

Commit Message

Robert Yang Dec. 20, 2018, 7:43 a.m.
The previous code assumed builddir and meta-selftest are in the same dir, but
this isn't always true, builddir can be anywhere, use bitbake to locate
meta-selftest can fix the problem.

The bb.fatal() doesn't work (no error message is print), so I use raise
Exception to print error messages.

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 meta/lib/oeqa/core/utils/concurrencytest.py | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/meta/lib/oeqa/core/utils/concurrencytest.py b/meta/lib/oeqa/core/utils/concurrencytest.py
index f050289..ef1698c 100644
--- a/meta/lib/oeqa/core/utils/concurrencytest.py
+++ b/meta/lib/oeqa/core/utils/concurrencytest.py
@@ -139,6 +139,20 @@  def removebuilddir(d):
         delay = delay - 1
     bb.utils.prunedir(d)
 
+def get_selftestdir():
+    """
+    Use 'which bitbake' to locate meta-selftest dir
+    """
+    cmd = 'which bitbake'
+    retval, bitbake_path = subprocess.getstatusoutput(cmd)
+    if retval != 0:
+        raise Exception('Failed to run %s' % cmd)
+    topdir = os.path.realpath('%s/../../' % os.path.dirname(bitbake_path))
+    selftestdir = os.path.join(topdir, 'meta-selftest')
+    if not os.path.exists(selftestdir):
+        raise Exception('Failed to run %s' % cmd)
+    return selftestdir
+
 def fork_for_tests(concurrency_num, suite):
     result = []
     test_blocks = partition_tests(suite, concurrency_num)
@@ -166,7 +180,7 @@  def fork_for_tests(concurrency_num, suite):
                 if 'BUILDDIR' in os.environ:
                     builddir = os.environ['BUILDDIR']
                     newbuilddir = builddir + "-st-" + str(ourpid)
-                    selftestdir = os.path.abspath(builddir + "/../meta-selftest")
+                    selftestdir = get_selftestdir()
                     newselftestdir = newbuilddir + "/meta-selftest"
 
                     bb.utils.mkdirhier(newbuilddir)

Comments

Richard Purdie Dec. 20, 2018, 10:14 a.m.
On Wed, 2018-12-19 at 23:43 -0800, Robert Yang wrote:
> The previous code assumed builddir and meta-selftest are in the same
> dir, but this isn't always true, builddir can be anywhere, use
> bitbake to locate meta-selftest can fix the problem.
> 
> The bb.fatal() doesn't work (no error message is print), so I use
> raise Exception to print error messages.
> 
> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> ---
>  meta/lib/oeqa/core/utils/concurrencytest.py | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/lib/oeqa/core/utils/concurrencytest.py
> b/meta/lib/oeqa/core/utils/concurrencytest.py
> index f050289..ef1698c 100644
> --- a/meta/lib/oeqa/core/utils/concurrencytest.py
> +++ b/meta/lib/oeqa/core/utils/concurrencytest.py
> @@ -139,6 +139,20 @@ def removebuilddir(d):
>          delay = delay - 1
>      bb.utils.prunedir(d)
>  
> +def get_selftestdir():
> +    """
> +    Use 'which bitbake' to locate meta-selftest dir
> +    """
> +    cmd = 'which bitbake'
> +    retval, bitbake_path = subprocess.getstatusoutput(cmd)
> +    if retval != 0:
> +        raise Exception('Failed to run %s' % cmd)
> +    topdir = os.path.realpath('%s/../../' %
> os.path.dirname(bitbake_path))
> +    selftestdir = os.path.join(topdir, 'meta-selftest')
> +    if not os.path.exists(selftestdir):
> +        raise Exception('Failed to run %s' % cmd)
> +    return selftestdir
> +
>  def fork_for_tests(concurrency_num, suite):
>      result = []
>      test_blocks = partition_tests(suite, concurrency_num)
> @@ -166,7 +180,7 @@ def fork_for_tests(concurrency_num, suite):
>                  if 'BUILDDIR' in os.environ:
>                      builddir = os.environ['BUILDDIR']
>                      newbuilddir = builddir + "-st-" + str(ourpid)
> -                    selftestdir = os.path.abspath(builddir +
> "/../meta-selftest")
> +                    selftestdir = get_selftestdir()
>                      newselftestdir = newbuilddir + "/meta-selftest"

Sorry, but this change just swaps one problem for another. meta-
selftest is part of OE-Core so its position relative to bitbake isn't
fixed.

How about we use 

from oeqa.utils.commands import get_test_layer

?

Cheers,

Richard
Robert Yang Dec. 20, 2018, 10:43 a.m.
On 12/20/18 6:14 PM, Richard Purdie wrote:
> On Wed, 2018-12-19 at 23:43 -0800, Robert Yang wrote:
>> The previous code assumed builddir and meta-selftest are in the same
>> dir, but this isn't always true, builddir can be anywhere, use
>> bitbake to locate meta-selftest can fix the problem.
>>
>> The bb.fatal() doesn't work (no error message is print), so I use
>> raise Exception to print error messages.
>>
>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
>> ---
>>   meta/lib/oeqa/core/utils/concurrencytest.py | 16 +++++++++++++++-
>>   1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/meta/lib/oeqa/core/utils/concurrencytest.py
>> b/meta/lib/oeqa/core/utils/concurrencytest.py
>> index f050289..ef1698c 100644
>> --- a/meta/lib/oeqa/core/utils/concurrencytest.py
>> +++ b/meta/lib/oeqa/core/utils/concurrencytest.py
>> @@ -139,6 +139,20 @@ def removebuilddir(d):
>>           delay = delay - 1
>>       bb.utils.prunedir(d)
>>   
>> +def get_selftestdir():
>> +    """
>> +    Use 'which bitbake' to locate meta-selftest dir
>> +    """
>> +    cmd = 'which bitbake'
>> +    retval, bitbake_path = subprocess.getstatusoutput(cmd)
>> +    if retval != 0:
>> +        raise Exception('Failed to run %s' % cmd)
>> +    topdir = os.path.realpath('%s/../../' %
>> os.path.dirname(bitbake_path))
>> +    selftestdir = os.path.join(topdir, 'meta-selftest')
>> +    if not os.path.exists(selftestdir):
>> +        raise Exception('Failed to run %s' % cmd)
>> +    return selftestdir
>> +
>>   def fork_for_tests(concurrency_num, suite):
>>       result = []
>>       test_blocks = partition_tests(suite, concurrency_num)
>> @@ -166,7 +180,7 @@ def fork_for_tests(concurrency_num, suite):
>>                   if 'BUILDDIR' in os.environ:
>>                       builddir = os.environ['BUILDDIR']
>>                       newbuilddir = builddir + "-st-" + str(ourpid)
>> -                    selftestdir = os.path.abspath(builddir +
>> "/../meta-selftest")
>> +                    selftestdir = get_selftestdir()
>>                       newselftestdir = newbuilddir + "/meta-selftest"
> 
> Sorry, but this change just swaps one problem for another. meta-
> selftest is part of OE-Core so its position relative to bitbake isn't
> fixed.

I always thought that bitbake must be in oe-core, otherwise it didn't work,
but seemed that it was incorrect?

> 
> How about we use
> 
> from oeqa.utils.commands import get_test_layer

Thanks, that is much better, Updated in the repo:

commit 9d03cf9e726232a9df544435cadfe520020ef631
Author: Robert Yang <liezhi.yang@windriver.com>
Date:   Tue Dec 18 18:38:00 2018 -0800

     oeqa/concurrencytest: fix for locating meta-selftest

     The previous code assumed builddir and meta-selftest are in the same dir, but
     this isn't always true, builddir can be anywhere, use get_test_layer() to
     locate meta-selftest can fix the problem.

     Signed-off-by: Robert Yang <liezhi.yang@windriver.com>


// Robert

> 
> ?
> 
> Cheers,
> 
> Richard
> 
>
Richard Purdie Dec. 20, 2018, 12:19 p.m.
On Thu, 2018-12-20 at 18:43 +0800, Robert Yang wrote:
> 
> On 12/20/18 6:14 PM, Richard Purdie wrote:
> > On Wed, 2018-12-19 at 23:43 -0800, Robert Yang wrote:
> > Sorry, but this change just swaps one problem for another. meta-
> > selftest is part of OE-Core so its position relative to bitbake
> > isn'tfixed.
> 
> I always thought that bitbake must be in oe-core, otherwise it didn't
> work, but seemed that it was incorrect?

bitbake doesn't have a fixed location relative to oe-core. It may
expose bugs but people do put it in different locations.

> > How about we use
> > 
> > from oeqa.utils.commands import get_test_layer
> 
> Thanks, that is much better, Updated in the repo:
> 
> commit 9d03cf9e726232a9df544435cadfe520020ef631
> Author: Robert Yang <liezhi.yang@windriver.com>
> Date:   Tue Dec 18 18:38:00 2018 -0800
> 
>      oeqa/concurrencytest: fix for locating meta-selftest
> 
>      The previous code assumed builddir and meta-selftest are in the
> same dir, but
>      this isn't always true, builddir can be anywhere, use
> get_test_layer() to
>      locate meta-selftest can fix the problem.
> 
>      Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> 
> diff --git a/meta/lib/oeqa/core/utils/concurrencytest.py 
> b/meta/lib/oeqa/core/utils/concurrencytest.py
> index f050289..6c403ac 100644
> --- a/meta/lib/oeqa/core/utils/concurrencytest.py
> +++ b/meta/lib/oeqa/core/utils/concurrencytest.py
> @@ -25,6 +25,7 @@ from itertools import cycle
>   from subunit import ProtocolTestCase, TestProtocolClient
>   from subunit.test_results import AutoTimingTestResultDecorator
>   from testtools import ThreadsafeForwardingResult, iterate_tests
> +from oeqa.utils.commands import get_test_layer
> 
>   import bb.utils
>   import oe.path
> @@ -166,7 +167,7 @@ def fork_for_tests(concurrency_num, suite):
>                   if 'BUILDDIR' in os.environ:
>                       builddir = os.environ['BUILDDIR']
>                       newbuilddir = builddir + "-st-" + str(ourpid)
> -                    selftestdir = os.path.abspath(builddir +
> "/../meta-selftest")
> +                    selftestdir = get_test_layer()
>                       newselftestdir = newbuilddir + "/meta-selftest"
> 
>                       bb.utils.mkdirhier(newbuilddir)

This broke on our autobuilder:

https://autobuilder.yoctoproject.org/typhoon/#/builders/56/builds/61

It basically creates a "stampeding herd" of bitbake startups as if you
specify -j 40, it would do this 40 times nearly all at once and that
causes retry problems.

We need to move the call outside the for loop as the value doesn't
change.

Cheers,

Richard
Richard Purdie Dec. 20, 2018, 3:06 p.m.
On Thu, 2018-12-20 at 12:19 +0000, richard.purdie@linuxfoundation.org
wrote:
> On Thu, 2018-12-20 at 18:43 +0800, Robert Yang
> wrote:                    bb.utils.mkdirhier(newbuilddir)
> 
> This broke on our autobuilder:
> 
> https://autobuilder.yoctoproject.org/typhoon/#/builders/56/builds/61
> 
> It basically creates a "stampeding herd" of bitbake startups as if
> you
> specify -j 40, it would do this 40 times nearly all at once and that
> causes retry problems.
> 
> We need to move the call outside the for loop as the value doesn't
> change.

I fixed the patch but it raises some questions about the way bitbake is
behaving. I wrote a test script:

#!/usr/bin/env python3

import os
import sys
import subprocess

for i in range(50):
    sys.stdout.flush()
    sys.stderr.flush()
    pid = os.fork()
    if pid == 0:
        ourpid = os.getpid()
        print("Starting %s" % ourpid)
        sys.stdout.flush()
        newsi = os.open(os.devnull, os.O_RDWR)
        os.dup2(newsi, sys.stdin.fileno())
        try:
            output = subprocess.check_output("bitbake -e bash | grep PN=", shell=True, stderr=subprocess.STDOUT).decode("utf-8")
        except subprocess.CalledProcessError as e:
            print("output: %s" % e.output.decode("utf-8"))
            os._exit(e.returncode)
        print("output: %s" % output)
        os._exit(0)

which is a bit crazy shows some errors both on the console and in the
bitbake-cookerdaemon log and shows that closing the UI connections when
we have a client is not the right thing to be doing.

This may give some insight into some of the oe-selftest failures and
I'm looking at how to improve this behaviour...

(added Chen Qi+Randy to cc as well)

Cheers,

Richard
Robert Yang Dec. 21, 2018, 6:46 a.m.
Hi RP,

Sorry, I am on vacation today, will do more investigations next week.

// Robert

Sent from mobile phone

> 在 2018年12月20日,23:06,Richard Purdie <richard.purdie@linuxfoundation.org> 写道:
> 
> On Thu, 2018-12-20 at 12:19 +0000, richard.purdie@linuxfoundation.org
> wrote:
>> On Thu, 2018-12-20 at 18:43 +0800, Robert Yang
>> wrote:                    bb.utils.mkdirhier(newbuilddir)
>> 
>> This broke on our autobuilder:
>> 
>> https://autobuilder.yoctoproject.org/typhoon/#/builders/56/builds/61
>> 
>> It basically creates a "stampeding herd" of bitbake startups as if
>> you
>> specify -j 40, it would do this 40 times nearly all at once and that
>> causes retry problems.
>> 
>> We need to move the call outside the for loop as the value doesn't
>> change.
> 
> I fixed the patch but it raises some questions about the way bitbake is
> behaving. I wrote a test script:
> 
> #!/usr/bin/env python3
> 
> import os
> import sys
> import subprocess
> 
> for i in range(50):
>    sys.stdout.flush()
>    sys.stderr.flush()
>    pid = os.fork()
>    if pid == 0:
>        ourpid = os.getpid()
>        print("Starting %s" % ourpid)
>        sys.stdout.flush()
>        newsi = os.open(os.devnull, os.O_RDWR)
>        os.dup2(newsi, sys.stdin.fileno())
>        try:
>            output = subprocess.check_output("bitbake -e bash | grep PN=", shell=True, stderr=subprocess.STDOUT).decode("utf-8")
>        except subprocess.CalledProcessError as e:
>            print("output: %s" % e.output.decode("utf-8"))
>            os._exit(e.returncode)
>        print("output: %s" % output)
>        os._exit(0)
> 
> which is a bit crazy shows some errors both on the console and in the
> bitbake-cookerdaemon log and shows that closing the UI connections when
> we have a client is not the right thing to be doing.
> 
> This may give some insight into some of the oe-selftest failures and
> I'm looking at how to improve this behaviour...
> 
> (added Chen Qi+Randy to cc as well)
> 
> Cheers,
> 
> Richard
> 
> 
> 
>