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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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>

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.

~Andrew