Re: [PATCH v1 2/3] automation: add jobs running tests from tools/tests/*

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Wed, Feb 12, 2025 at 06:07:37PM -0800, Stefano Stabellini wrote:
> On Thu, 13 Feb 2025, Marek Marczykowski-Górecki wrote:
> > There are a bunch of tests in tools/tests/, let them run in CI.
> > For each subdirectory expect "make run" will run the test, and observe
> > its exit code. This way, adding new tests is easy, and they will be
> > automatically picked up.
> > 
> > For better visibility, log test output to junit xml format, and let
> > gitlab ingest it. Set SUT_ADDR variable with name/address of the system
> > under test, so a network can be used to extract the file. The actual
> > address is set using DHCP. And for the test internal network, still add
> > the 192.168.0.1 IP (but don't replace the DHCP-provided one).
> > 
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> 
> Very nice!!
> 
> Only one comment below
> 
> 
> > ---
> >  automation/gitlab-ci/test.yaml     | 23 +++++++++++++++-
> >  automation/scripts/build           |  1 +-
> >  automation/scripts/qubes-x86-64.sh | 27 +++++++++++++++++-
> >  automation/scripts/run-tools-tests | 47 +++++++++++++++++++++++++++++++-
> >  4 files changed, 97 insertions(+), 1 deletion(-)
> >  create mode 100755 automation/scripts/run-tools-tests
> > 
> > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> > index 1822e3ea5fd7..c21a37933881 100644
> > --- a/automation/gitlab-ci/test.yaml
> > +++ b/automation/gitlab-ci/test.yaml
> > @@ -130,6 +130,7 @@
> >      PCIDEV: "03:00.0"
> >      PCIDEV_INTR: "MSI-X"
> >      CONSOLE_OPTS: "console=com1 com1=115200,8n1"
> > +    SUT_ADDR: test-2.testnet
> >    artifacts:
> >      paths:
> >        - smoke.serial
> > @@ -263,6 +264,28 @@ adl-pvshim-x86-64-gcc-debug:
> >      - *x86-64-test-needs
> >      - alpine-3.18-gcc-debug
> >  
> > +adl-tools-tests-pv-x86-64-gcc-debug:
> > +  extends: .adl-x86-64
> > +  script:
> > +    - ./automation/scripts/qubes-x86-64.sh tools-tests-pv 2>&1 | tee 
> > ${LOGFILE}
> > +  artifacts:
> > +    reports:
> > +      junit: tests-junit.xml
> > +  needs:
> > +    - *x86-64-test-needs
> > +    - alpine-3.18-gcc-debug
> > +
> > +adl-tools-tests-pvh-x86-64-gcc-debug:
> > +  extends: .adl-x86-64
> > +  script:
> > +    - ./automation/scripts/qubes-x86-64.sh tools-tests-pvh 2>&1 | tee 
> > ${LOGFILE}
> > +  artifacts:
> > +    reports:
> > +      junit: tests-junit.xml
> > +  needs:
> > +    - *x86-64-test-needs
> > +    - alpine-3.18-gcc-debug
> > +
> >  zen3p-smoke-x86-64-gcc-debug:
> >    extends: .zen3p-x86-64
> >    script:
> > diff --git a/automation/scripts/build b/automation/scripts/build
> > index 952599cc25c2..522efe774ef3 100755
> > --- a/automation/scripts/build
> > +++ b/automation/scripts/build
> > @@ -109,5 +109,6 @@ else
> >      # even though dist/ contains everything, while some containers don't 
> > even
> >      # build Xen
> >      cp -r dist binaries/
> > +    cp -r tools/tests binaries/
> >      collect_xen_artefacts
> >  fi
> > diff --git a/automation/scripts/qubes-x86-64.sh 
> > b/automation/scripts/qubes-x86-64.sh
> > index 7eb3ce1bf703..81d239cc8b75 100755
> > --- a/automation/scripts/qubes-x86-64.sh
> > +++ b/automation/scripts/qubes-x86-64.sh
> > @@ -10,6 +10,8 @@ set -ex
> >  #  - pci-pv         PV dom0,  PV domU + PCI Passthrough
> >  #  - pvshim         PV dom0,  PVSHIM domU
> >  #  - s3             PV dom0,  S3 suspend/resume
> > +#  - tools-tests-pv PV dom0, run tests from tools/tests/*
> > +#  - tools-tests-pvh PVH dom0, run tests from tools/tests/*
> >  test_variant=$1
> >  
> >  ### defaults
> > @@ -19,6 +21,7 @@ timeout=120
> >  domU_type="pvh"
> >  domU_vif="'bridge=xenbr0',"
> >  domU_extra_config=
> > +retrieve_xml=
> >  
> >  case "${test_variant}" in
> >      ### test: smoke test & smoke test PVH & smoke test HVM & smoke test 
> > PVSHIM
> > @@ -126,6 +129,21 @@ done
> >  "
> >          ;;
> >  
> > +    ### tests: tools-tests-pv, tools-tests-pvh
> > +    "tools-tests-pv"|"tools-tests-pvh")
> > +        retrieve_xml=1
> > +        passed="test passed"
> > +        domU_check=""
> > +        dom0_check="
> > +/tests/run-tools-tests /tests /tmp/tests-junit.xml && echo \"${passed}\"
> > +nc -l -p 8080 < /tmp/tests-junit.xml >/dev/null &
> > +"
> > +        if [ "${test_variant}" = "tools-tests-pvh" ]; then
> > +            extra_xen_opts="dom0=pvh"
> > +        fi
> > +
> > +        ;;
> > +
> >      *)
> >          echo "Unrecognised test_variant '${test_variant}'" >&2
> >          exit 1
> > @@ -178,6 +196,8 @@ mkdir srv
> >  mkdir sys
> >  rm var/run
> >  cp -ar ../binaries/dist/install/* .
> > +cp -ar ../binaries/tests .
> > +cp -a ../automation/scripts/run-tools-tests tests/
> >  
> >  echo "#!/bin/bash
> >  
> > @@ -188,7 +208,8 @@ brctl addbr xenbr0
> >  brctl addif xenbr0 eth0
> >  ifconfig eth0 up
> >  ifconfig xenbr0 up
> > -ifconfig xenbr0 192.168.0.1
> > +timeout 30s udhcpc -i xenbr0

This is actually wrong with tests doing passthrough. I'll send v2 that
limits it.

> > +ip addr add dev xenbr0 192.168.0.1/24
> >  
> >  " > etc/local.d/xen.start
> >  
> > @@ -272,6 +293,10 @@ if [ $timeout -le 0 ]; then
> >      exit 1
> >  fi
> >  
> > +if [ -n "$retrieve_xml" ]; then
> > +    nc -w 10 "$SUT_ADDR" 8080 > tests-junit.xml </dev/null
> > +fi
> > +
> >  sleep 1
> >  
> >  (grep -q "^Welcome to Alpine Linux" smoke.serial && grep -q "${passed}" 
> > smoke.serial) || exit 1
> > diff --git a/automation/scripts/run-tools-tests 
> > b/automation/scripts/run-tools-tests
> > new file mode 100755
> > index 000000000000..242a9edad941
> > --- /dev/null
> > +++ b/automation/scripts/run-tools-tests
> > @@ -0,0 +1,47 @@
> > +#!/bin/sh
> 
> It should be /bin/bash

That script is running inside SUT (started from initramfs) which is
rather minimal. I think it currently has bash, but with the initramfs at
over 200MB (compressed) I can see trimming it in the future...

> You could also consider -e and maybe -x

That is a good idea (but also failures need to not break the XML
structure, so it will clutter the script a bit).

> > +usage() {
> > +    echo "Usage: $0 tests-dir xml-out"
> > +}
> > +
> > +xml_out=$2
> > +if [ -z "$xml_out" ]; then
> > +  xml_out=/dev/null
> > +fi
> > +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"
> > +            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' "$f" "$ret" >> "$xml_out"
> > +        else
> > +            echo "PASSED"
> > +        fi
> > +    done
> > +    if [ -z "$ret" ]; then
> > +        printf '   <skipped type="skipped" message="test not found"/>\n' 
> > >> "$xml_out"
> > +    fi
> > +    printf '  </testcase>\n' "$dir" >> "$xml_out"
> > +done
> > +printf ' </testsuite>\n' >> "$xml_out"
> > +printf '</testsuites>\n' >> "$xml_out"
> > +
> > +if [ -n "$failed" ]; then
> > +    exit 1
> > +fi
> > -- 
> > git-series 0.9.1
> > 


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

Attachment: signature.asc
Description: PGP signature