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

2015-12-18 Thread Radim Krčmář
2015-12-17 13:09-0600, Andrew Jones:
> 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.

And parts of run() would need to be rewritten as well ... I am not sure
it's worth to invest the time.

>> (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.

No, but its quite likely that the host has bash when something as big as
QEMU is already there, or at least that it shouldn't be a huge problem
to install 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.

Ok, stderr from qemu needs to be redirected too then.
I will add an extra param to set file for __*log helpers or helpers that
redirect to stderr.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-12-17 Thread Andrew Jones
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ář 
> ---
>  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 < $standalone
> -#!/bin/sh
> +#!/bin/bash
>  
> -EOF
> -if [ "$arch" ]; then
> - cat <> $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 <> $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 <> $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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html