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.per...@vates.tech>
Reviewed-by: Stefano Stabellini <sstabell...@kernel.org>
> 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
