From: Anthony PERARD <anthony.perard@vates.tech>

The main script expect to find the string "$passed" or it just timeout
and doesn't try to download the junit file in this case. So we ignore
the return value of run-tools-test to always print "$passed" and
instead look for failure in the generated junit file. If the junit
report is incomplete, this will also result in a failure of the job.

Signed-off-by: Anthony PERARD <anthony.perard@vates.tech>
---

Notes:
    Changes in v2:
    - This squash both patch "CI: Ignore run-tools-test return value" and
      "CI: Have the gitlab job fail on tools/tests failure"
    - grep for '<failure type="failure"' instead of '</failure>'

 automation/scripts/qubes-x86-64.sh | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
index 2750d24eba..21dcd9b063 100755
--- a/automation/scripts/qubes-x86-64.sh
+++ b/automation/scripts/qubes-x86-64.sh
@@ -135,10 +135,11 @@ done
     ### tests: tools-tests-pv, tools-tests-pvh
     "tools-tests-pv"|"tools-tests-pvh")
         retrieve_xml=1
-        passed="test passed"
+        passed="run-tools-test over"
         domU_check=""
         dom0_check="
-/root/run-tools-tests /usr/lib/xen/tests /tmp/tests-junit.xml && echo \"${passed}\"
+/root/run-tools-tests /usr/lib/xen/tests /tmp/tests-junit.xml ||:
+echo \"${passed}\"
 nc -l -p 8080 < /tmp/tests-junit.xml >/dev/null &
 "
         if [ "${test_variant}" = "tools-tests-pvh" ]; then
@@ -297,6 +298,14 @@ TEST_RESULT=$?
 
 if [ -n "$retrieve_xml" ]; then
     nc -w 10 "$SUT_ADDR" 8080 > tests-junit.xml </dev/null
+    # Findout if one of the test failed
+    if ! grep -q '</testsuites>' tests-junit.xml; then
+        echo "ERROR: tests-junit.xml is incomplete or missing."
+        TEST_RESULT=1
+    # Only match "type=failure" to allow to "tolerable" for example.
+    elif grep -q '<failure type="failure"' tests-junit.xml; then
+        TEST_RESULT=1
+    fi
 fi
 
 exit "$TEST_RESULT"
-- 
Anthony PERARD

On Wed, Jul 30, 2025 at 05:26:00PM +0200, Anthony PERARD wrote:
> From: Anthony PERARD <anthony.perard@vates.tech>
> 
> The main script expect to find the string "$passed" or it just timeout
> and doesn't try to download the junit file in this case. So we ignore
> the return value of run-tools-test to always print "$passed" and
> instead look for failure in the generated junit file. If the junit
> report is incomplete, this will also result in a failure of the job.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@vates.tech>
> ---
> 
> Notes:
>     Changes in v2:
>     - This squash both patch "CI: Ignore run-tools-test return value" and
>       "CI: Have the gitlab job fail on tools/tests failure"
>     - grep for '<failure type="failure"' instead of '</failure>'
> 
>  automation/scripts/qubes-x86-64.sh | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
> index 2750d24eba..21dcd9b063 100755
> --- a/automation/scripts/qubes-x86-64.sh
> +++ b/automation/scripts/qubes-x86-64.sh
> @@ -135,10 +135,11 @@ done
>      ### tests: tools-tests-pv, tools-tests-pvh
>      "tools-tests-pv"|"tools-tests-pvh")
>          retrieve_xml=1
> -        passed="test passed"
> +        passed="run-tools-test over"
>          domU_check=""
>          dom0_check="
> -/root/run-tools-tests /usr/lib/xen/tests /tmp/tests-junit.xml && echo \"${passed}\"
> +/root/run-tools-tests /usr/lib/xen/tests /tmp/tests-junit.xml ||:
> +echo \"${passed}\"
>  nc -l -p 8080 < /tmp/tests-junit.xml >/dev/null &
>  "
>          if [ "${test_variant}" = "tools-tests-pvh" ]; then
> @@ -297,6 +298,14 @@ TEST_RESULT=$?
>  
>  if [ -n "$retrieve_xml" ]; then
>      nc -w 10 "$SUT_ADDR" 8080 > tests-junit.xml </dev/null
> +    # Findout if one of the test failed
> +    if ! grep -q '</testsuites>' tests-junit.xml; then
> +        echo "ERROR: tests-junit.xml is incomplete or missing."
> +        TEST_RESULT=1
> +    # Only match "type=failure" to allow to "tolerable" for example.
> +    elif grep -q '<failure type="failure"' tests-junit.xml; then

Maybe drop -q here, or add some message? Otherwise it's not obvious why it failed. See for example:
https://gitlab.com/xen-project/people/marmarek/xen/-/jobs/10968674458

Yes, you can click "Test summary" link, but it isn't obvious by looking
at the final part of the log. Alternatively, this can be combined with
my "CI: list failed tests at the end of tools job" patch (already
reviewed by Andrew), but I had an impression this series is supposed to
be included instead of that one.

> +        TEST_RESULT=1
> +    fi
>  fi
>  
>  exit "$TEST_RESULT"
> -- 
> Anthony PERARD
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

On Sat, Aug 09, 2025 at 06:22:33PM +0200, Marek Marczykowski-Górecki wrote:
> On Wed, Jul 30, 2025 at 05:26:00PM +0200, Anthony PERARD wrote:
> > From: Anthony PERARD <anthony.perard@vates.tech>
> > 
> > The main script expect to find the string "$passed" or it just timeout
> > and doesn't try to download the junit file in this case. So we ignore
> > the return value of run-tools-test to always print "$passed" and
> > instead look for failure in the generated junit file. If the junit
> > report is incomplete, this will also result in a failure of the job.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@vates.tech>
> > ---
> > 
> > Notes:
> >     Changes in v2:
> >     - This squash both patch "CI: Ignore run-tools-test return value" and
> >       "CI: Have the gitlab job fail on tools/tests failure"
> >     - grep for '<failure type="failure"' instead of '</failure>'
> > 
> >  automation/scripts/qubes-x86-64.sh | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
> > index 2750d24eba..21dcd9b063 100755
> > --- a/automation/scripts/qubes-x86-64.sh
> > +++ b/automation/scripts/qubes-x86-64.sh
> > @@ -135,10 +135,11 @@ done
> >      ### tests: tools-tests-pv, tools-tests-pvh
> >      "tools-tests-pv"|"tools-tests-pvh")
> >          retrieve_xml=1
> > -        passed="test passed"
> > +        passed="run-tools-test over"
> >          domU_check=""
> >          dom0_check="
> > -/root/run-tools-tests /usr/lib/xen/tests /tmp/tests-junit.xml && echo \"${passed}\"
> > +/root/run-tools-tests /usr/lib/xen/tests /tmp/tests-junit.xml ||:
> > +echo \"${passed}\"
> >  nc -l -p 8080 < /tmp/tests-junit.xml >/dev/null &
> >  "
> >          if [ "${test_variant}" = "tools-tests-pvh" ]; then
> > @@ -297,6 +298,14 @@ TEST_RESULT=$?
> >  
> >  if [ -n "$retrieve_xml" ]; then
> >      nc -w 10 "$SUT_ADDR" 8080 > tests-junit.xml </dev/null
> > +    # Findout if one of the test failed
> > +    if ! grep -q '</testsuites>' tests-junit.xml; then
> > +        echo "ERROR: tests-junit.xml is incomplete or missing."
> > +        TEST_RESULT=1
> > +    # Only match "type=failure" to allow to "tolerable" for example.
> > +    elif grep -q '<failure type="failure"' tests-junit.xml; then
> 
> Maybe drop -q here, or add some message? Otherwise it's not obvious why it failed. See for example:
> https://gitlab.com/xen-project/people/marmarek/xen/-/jobs/10968674458
> 
> Yes, you can click "Test summary" link, but it isn't obvious by looking
> at the final part of the log. Alternatively, this can be combined with
> my "CI: list failed tests at the end of tools job" patch (already
> reviewed by Andrew), but I had an impression this series is supposed to
> be included instead of that one.

Dropping "-q" sounds good here, it mean in your pipeline example we
would have the following in the logs:
   <failure type="failure" message="binary /usr/lib/xen/tests/test_vpci exited with code 134">
Which should be could enough for now.

Thanks,

-- 
Anthony PERARD