Re: [PATCH 3/3] CI: Drop custom handling of tools/tests

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 28/05/2025 1:16 am, Stefano Stabellini wrote:
> On Mon, 26 May 2025, Andrew Cooper wrote:
>> On 26/05/2025 6:22 pm, Anthony PERARD wrote:
>>> On Mon, May 26, 2025 at 05:45:29PM +0100, Andrew Cooper wrote:
>>>> On 20/05/2025 9:52 pm, Andrew Cooper wrote:
>>>>> diff --git a/automation/scripts/run-tools-tests 
>>>>> b/automation/scripts/run-tools-tests
>>>>> index 770e97c3e943..8d7aa8fa5140 100755
>>>>> --- a/automation/scripts/run-tools-tests
>>>>> +++ b/automation/scripts/run-tools-tests
>>>>> @@ -12,30 +12,25 @@ printf '<?xml version="1.0" encoding="UTF-8"?>\n' > 
>>>>> "$xml_out"
>>>>>  printf '<testsuites name="tools.tests">\n' >> "$xml_out"
>>>>>  printf ' <testsuite name="tools.tests">\n' >> "$xml_out"
>>>>>  failed=
>>>>> -for dir in "$1"/*; do
>>>>> -    [ -d "$dir" ] || continue
>>>>> -    echo "Running test in $dir"
>>>>> -    printf '  <testcase name="%s">\n' "$dir" >> "$xml_out"
>>>>> -    ret=
>>>>> -    for f in "$dir"/*; do
>>>>> -        [ -f "$f" ] || continue
>>>>> -        [ -x "$f" ] || continue
>>>>> -        "$f" 2>&1 | tee /tmp/out
>>>>> -        ret=$?
>>>>> -        if [ "$ret" -ne 0 ]; then
>>>>> -            echo "FAILED: $ret"
>>>>> -            failed+=" $dir"
>>>>> -            printf '   <failure type="failure" message="binary %s exited 
>>>>> with code %d">\n' "$f" "$ret" >> "$xml_out"
>>>>> -            # TODO: could use xml escaping... but current tests seems to
>>>>> -            # produce sane output
>>>>> -            cat /tmp/out >> "$xml_out"
>>>>> -            printf '   </failure>\n' >> "$xml_out"
>>>>> -        else
>>>>> -            echo "PASSED"
>>>>> -        fi
>>>>> -    done
>>>>> -    if [ -z "$ret" ]; then
>>>>> -        printf '   <skipped type="skipped" message="no executable test 
>>>>> found in %s"/>\n' "$dir" >> "$xml_out"
>>>>> +for f in "$1"/*; do
>>>>> +    if [ -x "$f" ]; then
>>>>> +        echo "SKIP: $f not executable"
>>>>> +        continue
>>>> This should be ! -x
>>>>
>>>> I had that hunk in the wrong patch when posting this series.
>>> With that fixed:
>>> Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

Thanks.

>
>
>> Thanks.
>>
>>> But I think there's an issue with the script...
>>>
>>>>> +    "$f" 2>&1 | tee /tmp/out
>>>>> +    ret=$?
>>>>> +    if [ "$ret" -ne 0 ]; then
>>> Is this checking the correct exit value? It seems that without `set -o
>>> pipefail`, we only have the exit value of `tee` which should never fail.
>>> But I think we should grab the value of ${PIPESTATUS[0]} to actually
>>> read the exit value of $f.
>> Hmm yes, I think this needs adjusting.
>>
>> It turns out there are multiple problems with junit, including the fact
>> that putting failures in here doesn't cause the outer job to fail. 
>>
>> The internet suggest having a 'script: grep "<failure" junit.xml' step
>> to work around this.
>>
>> I think that wants to be a separate series.  The question is whether to
>> do this series first or second.  I expect I'm going to need to backport
>> all of this work to eventually get XTF back onto the older trees.
>  
> I'll leave the choice to you

In which case I'll get this committed now, because it's one useful step
forward and reduces my queue a bit.

~Andrew