|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v1 1/4] CI: unify x86 XTF test runner
On Wed, Apr 16, 2025 at 05:16:49PM -0700, Stefano Stabellini wrote:
> On Wed, 16 Apr 2025, dmkhn@xxxxxxxxx wrote:
> > From: Denis Mukhin <dmukhin@xxxxxxxx>
> >
> > Add test runner script qemu-xtf.sh which is allows any XTF x86 test to be
> > easily executed. Test runner is invoked from the qemu-smoke* jobs with the
> > hardcoded parameters.
> >
> > Each x86 XTF job lead time is reduced a bit since only the test-related code
> > is built, not the entire XTF project.
> >
> > Add .gitignore to avoid committing test artifacts by mistake.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
> > ---
> > automation/gitlab-ci/test.yaml | 8 +-
> > automation/scripts/.gitignore | 6 +
> > .../scripts/include/configs/xtf-x86-64-config | 0
> > automation/scripts/include/xtf-runner | 134 ++++++++++++++++++
> > automation/scripts/include/xtf-x86-64 | 31 ++++
> > automation/scripts/qemu-smoke-x86-64.sh | 26 ----
> > automation/scripts/qemu-xtf.sh | 26 ++++
> > 7 files changed, 201 insertions(+), 30 deletions(-)
> > create mode 100644 automation/scripts/.gitignore
> > create mode 100644 automation/scripts/include/configs/xtf-x86-64-config
> > create mode 100644 automation/scripts/include/xtf-runner
> > create mode 100644 automation/scripts/include/xtf-x86-64
> > delete mode 100755 automation/scripts/qemu-smoke-x86-64.sh
> > create mode 100755 automation/scripts/qemu-xtf.sh
> >
> > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> > index 5ce445b78f..3adc841335 100644
> > --- a/automation/gitlab-ci/test.yaml
> > +++ b/automation/gitlab-ci/test.yaml
> > @@ -659,28 +659,28 @@ qemu-alpine-x86_64-gcc:
> > qemu-smoke-x86-64-gcc:
> > extends: .qemu-smoke-x86-64
> > script:
> > - - ./automation/scripts/qemu-smoke-x86-64.sh pv 2>&1 | tee ${LOGFILE}
> > + - ./automation/scripts/qemu-xtf.sh x86-64 pv64 example 2>&1 | tee
> > ${LOGFILE}
> > needs:
> > - debian-12-x86_64-gcc-debug
> >
> > qemu-smoke-x86-64-clang:
> > extends: .qemu-smoke-x86-64
> > script:
> > - - ./automation/scripts/qemu-smoke-x86-64.sh pv 2>&1 | tee ${LOGFILE}
> > + - ./automation/scripts/qemu-xtf.sh x86-64 pv64 example 2>&1 | tee
> > ${LOGFILE}
> > needs:
> > - debian-12-x86_64-clang-debug
> >
> > qemu-smoke-x86-64-gcc-pvh:
> > extends: .qemu-smoke-x86-64
> > script:
> > - - ./automation/scripts/qemu-smoke-x86-64.sh pvh 2>&1 | tee ${LOGFILE}
> > + - ./automation/scripts/qemu-xtf.sh x86-64 hvm64 example 2>&1 | tee
> > ${LOGFILE}
> > needs:
> > - debian-12-x86_64-gcc-debug
> >
> > qemu-smoke-x86-64-clang-pvh:
> > extends: .qemu-smoke-x86-64
> > script:
> > - - ./automation/scripts/qemu-smoke-x86-64.sh pvh 2>&1 | tee ${LOGFILE}
> > + - ./automation/scripts/qemu-xtf.sh x86-64 hvm64 example 2>&1 | tee
> > ${LOGFILE}
> > needs:
> > - debian-12-x86_64-clang-debug
> >
> > diff --git a/automation/scripts/.gitignore b/automation/scripts/.gitignore
> > new file mode 100644
> > index 0000000000..2f2d6e1ebd
> > --- /dev/null
> > +++ b/automation/scripts/.gitignore
> > @@ -0,0 +1,6 @@
> > +!include
> > +
> > +binaries
> > +smoke.serial
> > +xen
> > +xtf*/
>
> I am not sure this works as intended: I thought the paths are relative
> to the location of the .gitignore file, but I could be wrong.
git allows .gitignore at any level in the source tree so that lenghty
paths could be avoided.
>
>
> > diff --git a/automation/scripts/include/configs/xtf-x86-64-config
> > b/automation/scripts/include/configs/xtf-x86-64-config
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/automation/scripts/include/xtf-runner
> > b/automation/scripts/include/xtf-runner
> > new file mode 100644
> > index 0000000000..55b7b34b89
> > --- /dev/null
> > +++ b/automation/scripts/include/xtf-runner
> > @@ -0,0 +1,134 @@
> > +#!/bin/bash
> > +#
> > +# XTF test utilities.
> > +#
> > +# Environment variables:
> > +# BOOT_MSG: Expected boot message
> > +# FW_PREFIX: Firmware images path including '/' at the end
> > +# PASSED: XTF test printout in case of a pass
> > +# QEMU_PREFIX: QEMU path including '/' at the end
> > +# TEST_LOG: Output log file
> > +# UBOOT_CMD: U-Boot command line
> > +# WORKDIR: Test working directory
> > +# XEN_BINARY: Xen binary location
> > +# XEN_CONSOLE: Xen console device name
> > +# XTF_SRC_CONFIG: XTF config file
> > +# XTF_SRC_BRANCH: XTF branch
> > +# XTF_SRC_URI: XTF source code URI
> > +
> > +function die()
> > +{
> > + set +x
> > + echo "FATAL: $*" >&2
> > + exit 1
> > +}
> > +
> > +[ -z "$TOP" ] && die "\$TOP is not set"
> > +
> > +# Output log file
> > +TEST_LOG="${TEST_LOG:-smoke.serial}"
> > +# XTF test printout in case of a pass
> > +PASSED="${PASSED:-Test result: SUCCESS}"
> > +# Expected boot message
> > +BOOT_MSG="${BOOT_MSG:-Latest ChangeSet: }"
> > +# Test working directory
> > +WORKDIR="${WORKDIR:-binaries}"
> > +# XTF source code
> > +XTF_SRC_CONFIG="${XTF_CONFIG:-${TOP}/include/configs/xtf-${ARCH}-config}"
>
> Should this be XTF_SRC_CONFIG="${XTF_SRC_CONFIG:- ?
Thanks for the catch!
>
>
> > +# Build an XTF test binary.
> > +# $1 Test variant.
> > +# $2 Test name.
> > +function xtf_build_binary()
> > +{
> > + local xtf_variant=$1
> > + local xtf_name=$2
> > + local xtf_dir="xtf-${ARCH}"
> > +
> > + # Crude check for local testing
> > + if [ ! -d ${xtf_dir}/.git ]; then
> > + git clone ${XTF_SRC_URI} ${xtf_dir} -b ${XTF_SRC_BRANCH}
> > + fi
> > +
> > + make \
> > + -C ${xtf_dir} \
> > + -j$(nproc) \
> > + $(tr '\n' ' ' < ${XTF_SRC_CONFIG}) \
> > + TESTS=tests/${xtf_name}
> > +
> > + export XTF_NAME="${xtf_name}"
> > + export XTF_VARIANT="${xtf_variant}"
> > + export XTF_WORKDIR="$(readlink -f ${xtf_dir})"
> > + export
> > XTF_BINARY="${XTF_WORKDIR}/tests/${xtf_name}/test-${xtf_variant}-${xtf_name}"
> > +}
> > +
> > +# Build Xen command line for running an XTF test.
> > +# $1 Test variant.
> > +# $2 Test name.
> > +function xtf_build_cmdline()
> > +{
> > + local xtf_variant=$1
> > + local xtf_name=$2
> > + declare -a cmdline=()
> > +
> > + cmdline+=("loglvl=all noreboot console_timestamps=boot")
> > + cmdline+=("console=${XEN_CONSOLE}")
> > +
> > + # NB: OK to have hvm64, which is x86-only variant
> > + if [[ $xtf_variant == "hvm64" ]]; then
> > + cmdline+=("dom0-iommu=none dom0=pvh")
> > + fi
> > +
> > + export XEN_CMDLINE="${cmdline[@]}"
> > +}
> > +
> > +# Build an XTF test environment.
> > +# $1 Test variant.
> > +# $2 Test name.
> > +function xtf_build_test()
> > +{
> > + local v=$1
> > + local xtf_name=$2
> > + local xtf_variant=""
> > +
> > + for x in ${XTF_SRC_VARIANTS}; do
> > + if [[ "${x}" == "${v}" ]]; then
> > + xtf_variant=${v}
> > + break
> > + fi
> > + done
> > + if [[ -z $xtf_variant ]]; then
> > + die "unsupported test variant '$1', supported variants:
> > ${XTF_SRC_VARIANTS}"
> > + fi
> > +
> > + xtf_build_binary ${xtf_variant} ${xtf_name}
> > + xtf_build_cmdline ${xtf_variant} ${xtf_name}
> > +}
> > +
> > +# Execute an XTF test.
> > +function xtf_run_test()
> > +{
> > + rm -f ${TEST_LOG}
> > + export BOOT_MSG PASSED TEST_CMD TEST_LOG UBOOT_CMD
> > + ${TOP}/console.exp | sed 's/\r\+$//'
> > +}
> > +
> > +# Setup environment and run an XTF test.
> > +# $1 Test variant.
> > +# $2 Test name.
> > +function xtf_test()
> > +{
> > + # Out: FW_*, QEMU_*, XEN_{BINARY,CONSOLE}, XTF_SRC_*
> > + xtf_arch_prepare
> > +
> > + # In: XTF_SRC_*
> > + # OUt: XTF_{BINARY,NAME,VARIANT,WORKDIR} and XEN_CMDLINE
> > + xtf_build_test $@
> > +
> > + # In: FW_*, QEMU_*, XTF_*, XEN_*
> > + # Out: BOOT_MSG, PASSED, TEST_{CMD,LOG}, UBOOT_CMD
> > + xtf_arch_setup
> > +
> > + # In: BOOT_MSG, PASSED, TEST_{CMD,LOG}, UBOOT_CMD
> > + xtf_run_test
> > +}
> > diff --git a/automation/scripts/include/xtf-x86-64
> > b/automation/scripts/include/xtf-x86-64
> > new file mode 100644
> > index 0000000000..edddf18b38
> > --- /dev/null
> > +++ b/automation/scripts/include/xtf-x86-64
> > @@ -0,0 +1,31 @@
> > +#!/bin/bash
> > +#
> > +# XTF test utilities (x86_64).
> > +#
> > +
> > +# Arch-specific environment overrides.
> > +function xtf_arch_prepare()
> > +{
> > + export FW_PREFIX="${FW_PREFIX:-}"
> > + export QEMU_PREFIX="${QEMU_PREFIX:-}"
>
> It looks like QEMU_PREFIX is only used within this file (and other
> similar files in later patches). If so, maybe for the sake of reducing
> global variables and variables in general we could get rid of
> QEMU_PREFIX.
Turned out that Arm and x86 CI jobs use different paths to QEMU binaries.
dev host may have some other location, that's why I parameterized QEMU
location.
>
>
> > + export XEN_BINARY="${XEN_BINARY:-${WORKDIR}/xen}"
> > + export XEN_CONSOLE="${XEN_CONSOLE:-com1}"
>
> Instead of XEN_CONSOLE, I'd suggest to have an arch-specific variable
> for Xen command line options where we can put the console as well as
> other things. However, it is also totally fine as is.
I initially did that, but then there's a big portion of command line parameters
which is shared across all supported architectures, with console= pointing to
arch-specific console device.
I will add arch-specific variable and drop XEN_CONSOLE.
>
> In fact, I think you went above and beyond in terms of generic
> abstractions, and I think it would have been OK to go with fewer
> variables, and more ad-hoc custom code.
>
>
> > + export XTF_SRC_BRANCH="${XTF_SRC_BRANCH:-master}"
> > + export
> > XTF_SRC_URI="${XTF_SRC_URI:-https://xenbits.xen.org/git-http/xtf.git}";
> > + export XTF_SRC_VARIANTS="hvm64 pv64"
> > +}
> > +
> > +# Perform arch-specific XTF environment setup.
> > +function xtf_arch_setup()
> > +{
> > + export TEST_CMD="${QEMU_PREFIX}qemu-system-x86_64 \
> > + -no-reboot \
> > + -nographic \
> > + -monitor none \
> > + -serial stdio \
> > + -m 512 \
> > + -kernel ${XEN_BINARY} \
> > + -initrd ${XTF_BINARY} \
> > + -append \"${XEN_CMDLINE}\" \
> > + "
> > +}
> > diff --git a/automation/scripts/qemu-smoke-x86-64.sh
> > b/automation/scripts/qemu-smoke-x86-64.sh
> > deleted file mode 100755
> > index da0c26cc2f..0000000000
> > --- a/automation/scripts/qemu-smoke-x86-64.sh
> > +++ /dev/null
> > @@ -1,26 +0,0 @@
> > -#!/bin/bash
> > -
> > -set -ex -o pipefail
> > -
> > -# variant should be either pv or pvh
> > -variant=$1
> > -
> > -# Clone and build XTF
> > -git clone https://xenbits.xen.org/git-http/xtf.git
> > -cd xtf && make -j$(nproc) && cd -
> > -
> > -case $variant in
> > - pvh) k=test-hvm64-example extra="dom0-iommu=none dom0=pvh" ;;
> > - *) k=test-pv64-example extra= ;;
> > -esac
> > -
> > -rm -f smoke.serial
> > -export TEST_CMD="qemu-system-x86_64 -nographic -kernel binaries/xen \
> > - -initrd xtf/tests/example/$k \
> > - -append \"loglvl=all console=com1 noreboot console_timestamps=boot
> > $extra\" \
> > - -m 512 -monitor none -serial stdio"
> > -
> > -export TEST_LOG="smoke.serial"
> > -export PASSED="Test result: SUCCESS"
> > -
> > -./automation/scripts/console.exp | sed 's/\r\+$//'
> > diff --git a/automation/scripts/qemu-xtf.sh b/automation/scripts/qemu-xtf.sh
> > new file mode 100755
> > index 0000000000..2e16d4aece
> > --- /dev/null
> > +++ b/automation/scripts/qemu-xtf.sh
> > @@ -0,0 +1,26 @@
> > +#!/bin/bash
> > +#
> > +# XTF test runner (QEMU).
> > +#
> > +
> > +set -e -o pipefail
> > +
> > +if [ $# -lt 3 ]; then
> > + echo "Usage: $(basename $0) ARCH XTF-VARIANT XTF-NAME"
> > + exit 1
> > +fi
> > +
> > +export ARCH="$1"
> > +shift
> > +
> > +export TOP="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)"
> > +if [ ! -f "${TOP}/include/xtf-${ARCH}" ]; then
> > + echo "unsupported architecture '${ARCH}'" >&2
> > + exit 1
> > +fi
>
> This seems a bit complex.. maybe we can assume that we are in xen.git/ ?
Will do that.
>
>
> > +set -x
> > +source ${TOP}/include/xtf-runner
> > +source ${TOP}/include/xtf-${ARCH}
>
> So here we could do:
>
> source automation/scripts/include/xtf-runner
>
> ? Or is it important to be able to detect and calculate the directory?
> If so, why not use TOP=$(pwd) or TOP=${PWD} or TOP=$(dirname "$0")?
> By the way, if we need a variable for the top directory I would call it
> XEN_ROOT instead of TOP.
>
> I see the patches are using both TOP and WORKDIR, maybe we can get rid
> of one of them? This is only a matter of taste but I think it would be
> simpler with fewer variables and most of the others look unavoidable.
I will try to eliminate TOP uses.
>
> Everything else looks OK to me.
>
>
> > +xtf_test $@
> > --
> > 2.34.1
> >
> >
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |