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.per...@vates.tech>
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.
Thanks,
--
Anthony PERARD
