Hi Stefano, On 13/08/2024 04:21, 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. > > 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. > > 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 think this comment is no longer true and needs to be removed.
> > Signed-off-by: Stefano Stabellini <stefano.stabell...@amd.com> > > --- > Changes in v2: > - add empty lines for readability I can only see it being done for one script. > - changes qemu-key file extension to exp > - drop xen_msg that is unused > - use capital letters for exported variables > - check for both dom0 and domU message on dom0less tests > > https://gitlab.com/xen-project/people/sstabellini/xen/-/pipelines/1410820286 > --- > automation/scripts/qemu-alpine-x86_64.sh | 16 +++--- > automation/scripts/qemu-key.exp | 51 +++++++++++++++++++ > automation/scripts/qemu-smoke-dom0-arm32.sh | 15 +++--- > automation/scripts/qemu-smoke-dom0-arm64.sh | 15 +++--- > .../scripts/qemu-smoke-dom0less-arm32.sh | 17 +++---- > .../scripts/qemu-smoke-dom0less-arm64.sh | 15 +++--- > 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, 110 insertions(+), 71 deletions(-) > create mode 100755 automation/scripts/qemu-key.exp > > diff --git a/automation/scripts/qemu-alpine-x86_64.sh > b/automation/scripts/qemu-alpine-x86_64.sh > index 8e398dcea3..5359e0820b 100755 > --- a/automation/scripts/qemu-alpine-x86_64.sh > +++ b/automation/scripts/qemu-alpine-x86_64.sh > @@ -77,18 +77,16 @@ 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 \ > -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" > + > +./automation/scripts/qemu-key.exp > diff --git a/automation/scripts/qemu-key.exp b/automation/scripts/qemu-key.exp > new file mode 100755 > index 0000000000..9f3a3364fa > --- /dev/null > +++ b/automation/scripts/qemu-key.exp > @@ -0,0 +1,51 @@ > +#!/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} It should be send_error to send the string to stderr > + 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)] && [info exists env(PASSED)]} { > + expect { > + "$env(PASSED)" { > + expect "$env(LOG_MSG)" > + exit 0 > + } > + "$env(LOG_MSG)" { > + expect "$env(PASSED)" > + exit 0 > + } > + } > +} > + > +if {[info exists env(LOG_MSG)]} { > + expect "$env(LOG_MSG)" > +} > + > +if {[info exists env(PASSED)]} { > + expect { > + "$env(PASSED)" { > + exit 0 > + } > + } > +} Given that this script treats both LOG_MSG and PASSED as optional, if a script does not define PASSED, it will just continue until timeout. Might be worth making PASSED mandatory. Something like: if {[info exists env(LOG_MSG)]} { expect { "$env(PASSED)" { expect "$env(LOG_MSG)" exit 0 } "$env(LOG_MSG)" { expect "$env(PASSED)" exit 0 } } } expect { "$env(PASSED)" { exit 0 } } In any case, you can do the changes on commit: Reviewed-by: Michal Orzel <michal.or...@amd.com> ~Michal
