Re: [PATCH kvm-unit-tests v2 03/12] scripts/mkstandalone: use common run function

Andrew Jones
2015-12-17T19:09:48Z

On Thu, Dec 17, 2015 at 06:53:34PM +0100, Radim Krčmář wrote:
> The biggest change is dependency on bash. An alternative would be to
> rewrite `run` in POSIX shell, but I think it's ok to presume that KVM
> unit tests will run on a system where installing bash isn't a problem.

Hmm... as hard as I worked to avoid the dependency on bash for the
standalone tests, then I'm reluctant to give up on that. I do agree
that having the dependency for the printf-%q trick helps a ton in
making the code more maintainable though.

> (We already depend on QEMU ...)

Dependency on qemu doesn't imply a dependency on bash. The idea behind
the standalone version of kvm-unit-tests tests is that you can receive
one in your email and launch it.

>
> Apart from that, there are changes in output and exit codes.
> - summary doesn't go to stderr

I wanted the summary on stderr so when you redirect the output of the
test to a file the output would directly diff with the corresponding
output in test.log from a system where run_tests.sh was used.

> - PASS/FAIL is colourful
> - FAILed scripts return 1

I'm not sure why I did exit 0 instead of exit $ret. I guess that was
just a thinko on my part. exit $ret makes more sense.

>
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
> v2: new (I can fix the stderr in v3)
>
> scripts/mkstandalone.sh | 59 +++++++++++++++++++++----------------------------
> 1 file changed, 25 insertions(+), 34 deletions(-)
>
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index 3ce244aff67b..cf2182dbd936 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -20,6 +20,13 @@ fi
> unittests=$TEST_DIR/unittests.cfg
> mkdir -p tests
>
> +escape ()
> +{
> + for arg in "${@}"; do
> + printf "%q " "$arg"; # XXX: trailing whitespace
> + done
> +}
> +
> function mkstandalone()
> {
> local testname="$1"
> @@ -49,33 +56,14 @@ function mkstandalone()
> cmdline=$(cut -d' ' -f2- <<< "$cmdline")
>
> cat <<EOF > $standalone
> -#!/bin/sh
> +#!/bin/bash
>
> -EOF
> -if [ "$arch" ]; then
> - cat <<EOF >> $standalone
> ARCH=\`uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/'\`
> -[ "\$ARCH" = "aarch64" ] && ARCH="arm64"
> -if [ "\$ARCH" != "$arch" ]; then
> - echo "skip $testname ($arch only)" 1>&2
> - exit 1
> -fi
>
> EOF
> -fi
> -if [ "$check" ]; then
> - cat <<EOF >> $standalone
> -for param in $check; do
> - path=\`echo \$param | cut -d= -f1\`
> - value=\`echo \$param | cut -d= -f2\`
> - if [ -f "\$path" ] && [ "\`cat \$path\`" != "\$value" ]; then
> - echo "skip $testname (\$path not equal to \$value)" 1>&2
> - exit 1
> - fi
> -done
>
> -EOF
> -fi
> +cat scripts/run.bash >> $standalone
> +
> if [ ! -f $kernel ]; then
> cat <<EOF >> $standalone
> echo "skip $testname (test kernel not present)" 1>&2
> @@ -100,9 +88,13 @@ MAX_SMP="MAX_SMP"
> echo \$qemu $cmdline -smp $smp $opts
>
> cmdline="\`echo '$cmdline' | sed s%$kernel%_NO_FILE_4Uhere_%\`"
> -if \$qemu \$cmdline 2>&1 | grep 'No accelerator found'; then
> - ret=2
> -else
> +if \$qemu \$cmdline 2>&1 | grep 'No accelerator found' >/dev/null; then
> + echo "skip $testname (QEMU doesn't support KVM)"
> + exit 1
> +fi
> +
> +__run()
> +{
> MAX_SMP=\`getconf _NPROCESSORS_CONF\`
> while \$qemu \$cmdline -smp \$MAX_SMP 2>&1 | grep 'exceeds max cpus' > /dev/null; do
> MAX_SMP=\`expr \$MAX_SMP - 1\`
> @@ -110,16 +102,15 @@ else
>
> cmdline="\`echo '$cmdline' | sed s%$kernel%\$bin%\`"
> \$qemu \$cmdline -smp $smp $opts
> - ret=\$?
> -fi
> -echo Return value from qemu: \$ret
> -if [ \$ret -le 1 ]; then
> - echo PASS $testname 1>&2
> -else
> - echo FAIL $testname 1>&2
> -fi
> +}
> +
> +__eval_log() { eval "\${@}"; }
> +
> +run `escape "${@}"`
> +ret=$?
> +
> rm -f \$bin
> -exit 0
> +exit \$ret
> EOF
> fi
> chmod +x $standalone
> --
> 2.6.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html