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

[Andrew Jones <drjones@redhat.com>]

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