Hi Stefano,
On 10/08/2024 08:59, Stefano Stabellini wrote:
> Use expect to invoke QEMU so that we can terminate the test as soon as
> we get the right string in the output instead of waiting until the
> final timeout.
This is a great improvement.
>
> For timeout, instead of an hardcoding the value, use a Gitlab CI
> variable "QEMU_TIMEOUT" that can be changed depending on the latest
> status of the Gitlab CI runners.
What is the current value of QEMU_TIMEOUT? I don't see it being mentioned
anywhere.
Could tests define this env var if needed? What would be the precedence then?
I'm thinking that some tests don't need a very big timeout that otherwise is
necessary
for longer tests (e.g. xtf test does not need to wait 720s).
>
> Note that for simplicity in the case of dom0less test, the script only
> checks for the $passed string. That is because the dom0 prompt and the
> $passed string could happen in any order making it difficult to check
> with expect which checks for strings in a specific order.
I use expect in my local testing too and I search for both strings. More below.
>
> Signed-off-by: Stefano Stabellini <stefano.stabell...@amd.com>
> ---
> automation/scripts/qemu-alpine-x86_64.sh | 15 +++----
> automation/scripts/qemu-key.ex | 42 +++++++++++++++++++
> automation/scripts/qemu-smoke-dom0-arm32.sh | 15 ++++---
> automation/scripts/qemu-smoke-dom0-arm64.sh | 15 ++++---
> .../scripts/qemu-smoke-dom0less-arm32.sh | 14 +++----
> .../scripts/qemu-smoke-dom0less-arm64.sh | 14 +++----
> automation/scripts/qemu-smoke-ppc64le.sh | 12 +++---
> automation/scripts/qemu-smoke-riscv64.sh | 12 +++---
> automation/scripts/qemu-smoke-x86-64.sh | 14 +++----
> automation/scripts/qemu-xtf-dom0less-arm64.sh | 14 +++----
> 10 files changed, 97 insertions(+), 70 deletions(-)
> create mode 100755 automation/scripts/qemu-key.ex
>
> diff --git a/automation/scripts/qemu-alpine-x86_64.sh
> b/automation/scripts/qemu-alpine-x86_64.sh
> index 8e398dcea3..24b23a573c 100755
> --- a/automation/scripts/qemu-alpine-x86_64.sh
> +++ b/automation/scripts/qemu-alpine-x86_64.sh
> @@ -77,18 +77,15 @@ EOF
> # Run the test
> rm -f smoke.serial
> set +e
> -timeout -k 1 720 \
> -qemu-system-x86_64 \
> +export qemu_cmd="qemu-system-x86_64 \
Usually (even respected by analyze.sh) exported env variables are written in
capital letters
> -cpu qemu64,+svm \
> -m 2G -smp 2 \
> -monitor none -serial stdio \
> -nographic \
> -device virtio-net-pci,netdev=n0 \
> - -netdev user,id=n0,tftp=binaries,bootfile=/pxelinux.0 |& \
> - # Remove carriage returns from the stdout output, as gitlab
> - # interface chokes on them
> - tee smoke.serial | sed 's/\r//'
> + -netdev user,id=n0,tftp=binaries,bootfile=/pxelinux.0"
>
> -set -e
> -(grep -q "Domain-0" smoke.serial && grep -q "BusyBox" smoke.serial) || exit 1
> -exit 0
> +export qemu_log="smoke.serial"
> +export log_msg="Domain-0"
> +export passed="BusyBox"
NIT: one empty line here for readability
> +./automation/scripts/qemu-key.ex
> diff --git a/automation/scripts/qemu-key.ex b/automation/scripts/qemu-key.ex
NIT: usually expect script have '.exp' extension.
> new file mode 100755
> index 0000000000..569ef2781f
> --- /dev/null
> +++ b/automation/scripts/qemu-key.ex
> @@ -0,0 +1,42 @@
> +#!/usr/bin/expect -f
> +
> +set timeout $env(QEMU_TIMEOUT)
> +
> +log_file -a $env(qemu_log)
> +
> +match_max 10000
> +
> +eval spawn $env(qemu_cmd)
> +
> +expect_after {
> + -re "(.*)\r" {
> + exp_continue
> + }
> + timeout {send_user "ERROR-Timeout!\n"; exit 1}
> + eof {send_user "ERROR-EOF!\n"; exit 1}
> +}
> +
> +if {[info exists env(uboot_cmd)]} {
> + expect "=>"
> +
> + send "$env(uboot_cmd)\r"
> +}
> +
> +if {[info exists env(log_msg)]} {
> + expect "$env(log_msg)"
> +}
> +
> +if {[info exists env(xen_cmd)]} {
> + send "$env(xen_cmd)\r"
> +}
Does not seem to be used at all. For now, I'd suggest to drop it.
> +
> +if {[info exists env(passed)]} {
> + expect {
> + "$env(passed)" {
> + exit 0
> + }
> + }
> +}
As said above, in my local testing, I search for both strings (after all we
should test that dom0 works too) as follows:
if {[info exists env(dom0_passed)] && [info exists env(domu_passed)]} {
expect {
"$env(dom0_passed)" {
expect "$env(domu_passed)"
exit 0
}
"$env(domu_passed)" {
expect "$env(dom0_passed)"
exit 0
}
}
}
Apart from that:
Reviewed-by: Michal Orzel <michal.or...@amd.com>
~Michal
