[OE-core] [PATCH 09/10] buildhistory.bbclass: add ptest_log

[Robert Yang]

On 07/31/2017 09:30 PM, Paul Eggleton wrote:
> Hi Robert,
>
> A few minor comments below.
>
> On Monday, 31 July 2017 11:50:09 AM CEST Robert Yang wrote:
>> The ptest log will be saved to buildhistory/ptest_log, we can easily get
>> the regression result between builds by:
>> $ git show HEAD ptest_log/pass.fail.skip.*
>>
>> [YOCTO #11547]
>>
>> Signed-off-by: Robert Yang <liezhi.yang at windriver.com>
>> ---
>>  meta/classes/buildhistory.bbclass | 28 ++++++++++++++++++++++++++++
>>  1 file changed, 28 insertions(+)
>>
>> diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/
> buildhistory.bbclass
>> index a3e4c7a734a..26430a63d31 100644
>> --- a/meta/classes/buildhistory.bbclass
>> +++ b/meta/classes/buildhistory.bbclass
>> @@ -912,3 +912,31 @@ def write_latest_srcrev(d, pkghistdir):
>>      else:
>>          if os.path.exists(srcrevfile):
>>              os.remove(srcrevfile)
>> +
>> +do_testimage[postfuncs] += "write_ptest_result"
>> +do_testimage[vardepsexclude] += "write_ptest_result"
>> +
>> +python write_ptest_result() {
>> +    write_latest_ptest_result(d, d.getVar('BUILDHISTORY_DIR'))
>> +}
>> +
>> +def write_latest_ptest_result(d, histdir):
>> +    import glob
>> +    import subprocess
>> +    test_log_dir = d.getVar('TEST_LOG_DIR')
>> +    input_ptest_log = os.path.join(test_log_dir, 'ptest_log')
>> +    output_ptest_log = os.path.join(histdir, 'ptest_log')
>
> Would it be reasonable for this to just be "ptest"? To ask the question a
> different way, would you expect to save other types of ptest information into
> buildhistory, or is this likely to be it?

I'm not sure atm, so use "ptest" sounds reasonable. I will update it.

>
>> +    if os.path.exists(input_ptest_log):
>> +        # Lock it avoid race issue
>> +        lock = bb.utils.lockfile(output_ptest_log + "/ptest_log.lock")
>> +        bb.utils.mkdirhier(output_ptest_log)
>> +        oe.path.copytree(input_ptest_log, output_ptest_log)
>> +        # Sort test result
>> +        for result in glob.glob('%s/pass.fail.*' % output_ptest_log):
>> +            bb.debug(1, 'Processing %s' % result)
>> +            cmd = "sort %s -o %s" % (result, result)
>> +            bb.debug(1, 'Running %s' % cmd)
>> +            ret = subprocess.call(cmd, shell=True)
>
> As a matter of good practice and since it's easy to do here I'd suggest not
> using shell=True and passing the command as a list instead, that way there
> can't be any issues with spaces or other shell interpretations.

Thanks, I will update it.

>
>> +            if ret != 0:
>> +                bb.error('Failed to run %s!' % cmd)
>> +        bb.utils.unlockfile(lock)
>
> Shouldn't you be using try...finally here to ensure the lockfile gets unlocked
> in the case of exceptions?

Yes, good idea.

// Robert

>
> Cheers,
> Paul
>
>