| Message ID | 1450374823-7648-4-git-send-email-rkrcmar@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- 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
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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. (We already depend on QEMU ...) Apart from that, there are changes in output and exit codes. - summary doesn't go to stderr - PASS/FAIL is colourful - FAILed scripts return 1 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(-)