From cb39f79417dce294c654435c1a783065b4983adb Mon Sep 17 00:00:00 2001 From: Matthias Bolte <matthias.bolte(a)googlemail.com&gt; Date: Sat, 9 Jul 2011 01:20:05 +0200 Subject: [PATCH] tests: Fix compressed test output padding logic The current logic tries to count from 1 to 40 and ignores paddings of 0 and 1 to 40. This doesn't work for counter + 1 mod 40 == 0 like here for counter value 159 TEST: virsh-all ........................................ 40 ........................................ 80 ........................................ 120 ....................................... 159 OK PASS: virsh-all Instead count from 0 to 39 to fix this. --- tests/test-lib.sh | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test-lib.sh b/tests/test-lib.sh index 768f96b..9eb6864 100644 --- a/tests/test-lib.sh +++ b/tests/test-lib.sh @@ -54,9 +54,9 @@ test_final() status=$2 if test "$verbose" = "0" ; then - mod=`expr \( $counter + 1 \) % 40` - if test "$mod" != "0" && test "$mod" != "1" ; then - for i in `seq $mod 40` + mod=`expr $counter % 40` + if test "$mod" != "0" ; then + for i in `seq $mod 39`

On 07/08/2011 07:40 PM, Eric Blake wrote: >> >> Instead count from 0 to 39 to fix this. >> --- >>  tests/test-lib.sh |    6 +++--- >>  1 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/tests/test-lib.sh b/tests/test-lib.sh >> index 768f96b..9eb6864 100644 >> --- a/tests/test-lib.sh >> +++ b/tests/test-lib.sh >> @@ -54,9 +54,9 @@ test_final() >>    status=$2 >> >>    if test "$verbose" = "0" ; then >> -    mod=`expr \( $counter + 1 \) % 40` >> -    if test "$mod" != "0" && test "$mod" != "1" ; then >> -      for i in `seq $mod 40` >> +    mod=`expr $counter % 40` >> +    if test "$mod" != "0" ; then >> +      for i in `seq $mod 39` > > seq is a GNU-ism, but this is no less portable than what it was before. >  (To be portable to platforms that lack seq, this should really be > written as: > > for i in 0 1 2 3 4 ... 39 Why iterate in the first place? +    if test "$mod" != "0" ; then +      for i in `seq $mod 39`       do         printf " " can portably be replaced by: printf "%${len}s" "" with len computed via expr.

On 07/08/2011 05:28 PM, Matthias Bolte wrote: > > From cb39f79417dce294c654435c1a783065b4983adb Mon Sep 17 00:00:00 2001 > From: Matthias Bolte <matthias.bolte(a)googlemail.com&gt; > Date: Sat, 9 Jul 2011 01:20:05 +0200 > Subject: [PATCH] tests: Fix compressed test output padding logic > > The current logic tries to count from 1 to 40 and ignores paddings > of 0 and 1 to 40. This doesn't work for counter + 1 mod 40 == 0 like > here for counter value 159 > > TEST: virsh-all >       ........................................ 40 >       ........................................ 80 >       ........................................ 120 >       ....................................... 159 OK > PASS: virsh-all > > Instead count from 0 to 39 to fix this. > --- >  tests/test-lib.sh |    6 +++--- >  1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tests/test-lib.sh b/tests/test-lib.sh > index 768f96b..9eb6864 100644 > --- a/tests/test-lib.sh > +++ b/tests/test-lib.sh > @@ -54,9 +54,9 @@ test_final() >    status=$2 > >    if test "$verbose" = "0" ; then > -    mod=`expr \( $counter + 1 \) % 40` > -    if test "$mod" != "0" && test "$mod" != "1" ; then > -      for i in `seq $mod 40` > +    mod=`expr $counter % 40` > +    if test "$mod" != "0" ; then > +      for i in `seq $mod 39` seq is a GNU-ism, but this is no less portable than what it was before.  (To be portable to platforms that lack seq, this should really be written as: for i in 0 1 2 3 4 ... 39 but I'm okay waiting until someone complains before we make that change). ACK.

On 07/08/2011 05:28 PM, Matthias Bolte wrote: > > From a1508239af921289cd6e357e8521ff42faf535bd Mon Sep 17 00:00:00 2001 > From: Matthias Bolte <matthias.bolte(a)googlemail.com&gt; > Date: Sat, 9 Jul 2011 01:24:16 +0200 > Subject: [PATCH] tests: Add the logic to skip the statstest to the right place > > --- >  tests/statstest.c |   31 ++++++++++++++++++++----------- >  1 files changed, 20 insertions(+), 11 deletions(-) > > > -VIRT_TEST_MAIN(mymain) > +/* Skipping the test in mymain is too late, it results in broken output. > + * Therefore, expand VIRT_TEST_MAIN here manually to be able to skip at > + * the right place. */ > +int main(int argc, char **argv) This seems fishy.  Why did tests/reconnect.c not need the same treatment?  Oh, because it didn't use VIRT_TEST_MAIN. Wouldn't it be better to teach VIRT_TEST_MAIN to behave better on a skip? diff --git i/tests/testutils.c w/tests/testutils.c index b433204..f732fdd 100644 --- i/tests/testutils.c +++ w/tests/testutils.c @@ -688,7 +688,7 @@ cleanup:     if (abs_srcdir_cleanup)         VIR_FREE(abs_srcdir);     virResetLastError(); -    if (!virTestGetVerbose()) { +    if (!virTestGetVerbose() && ret != EXIT_AM_SKIP) {         int i;         for (i = (testCounter % 40) ; i > 0 && i < 40 ; i++)             fprintf(stderr, " ");

On 07/08/2011 05:28 PM, Matthias Bolte wrote: >> TEST: virsh-all >>      ........................................ 40 >>      ........................................ 80 >>      ........................................ 120 >>      ....................................... 159 OK >> PASS: virsh-all >> >> We're obviously getting the logic wrong when there are 0 or when >> tests%40 == 39. > > Here are two patches for this, plus one to use EXIT_AM_SKIP more. An even better patch for the SKIP logic.  Why do a for loop of one space at a time, when printf can do it for us? From fbaee4f7df0bd04520948228de125ce0c6112130 Mon Sep 17 00:00:00 2001 From: Eric Blake <eblake(a)redhat.com&gt; Date: Fri, 8 Jul 2011 19:55:02 -0600 Subject: [PATCH] tests: handle skipped tests better Without this, running statstest withtout xen wasn't formatted well: TEST: statstest       0   FAIL SKIP: statstest * tests/testutils.c (virtTestMain): Special case skipped test. ---  tests/testutils.c |    8 ++++----  1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/testutils.c b/tests/testutils.c index b433204..fe04caa 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -689,10 +689,10 @@ cleanup:         VIR_FREE(abs_srcdir);     virResetLastError();     if (!virTestGetVerbose()) {

-        int i; -        for (i = (testCounter % 40) ; i > 0 && i < 40 ; i++) -            fprintf(stderr, " "); -        fprintf(stderr, " %-3d %s\n", testCounter, ret == 0 ? "OK" : "FAIL"); +        if (testCounter == 0 || testCounter % 40) +            fprintf(stderr, "%*s", 40 - (testCounter % 40), ""); +        fprintf(stderr, " %-3d %s\n", testCounter, +                ret == 0 ? "OK" : ret == EXIT_AM_SKIP ? "SKIP" : "FAIL");

Currently, the xen statstest and reconnect tests are only compiled if xend is running. Compile them unconditionally if xen headers are present, but skip the tests at runtime if xend is not running. This is in response to Eric's suggestion here https://www.redhat.com/archives/libvir-list/2011-July/msg00367.html --- configure.ac | 24 ------------------------ tests/Makefile.am | 12 ++++-------- tests/reconnect.c | 11 +++++++++++ tests/statstest.c | 12 ++++++++++++ 4 files changed, 27 insertions(+), 32 deletions(-) diff --git a/configure.ac b/configure.ac index aa589d6..ae747fb 100644 --- a/configure.ac +++ b/configure.ac @@ -1982,30 +1982,6 @@ AM_CONDITIONAL([WITH_PYTHON], [test "$with_python" = "yes"]) AC_SUBST([PYTHON_VERSION]) AC_SUBST([PYTHON_INCLUDES]) - - -AC_MSG_CHECKING([whether this host is running a Xen kernel]) -RUNNING_XEN= -if test -d /proc/sys/xen -then - RUNNING_XEN=yes -else - RUNNING_XEN=no -fi -AC_MSG_RESULT($RUNNING_XEN) - -AC_MSG_CHECKING([If XenD UNIX socket /var/run/xend/xmlrpc.sock is accessible]) -RUNNING_XEND= -if test -S /var/run/xend/xmlrpc.sock -then - RUNNING_XEND=yes -else - RUNNING_XEND=no -fi -AC_MSG_RESULT($RUNNING_XEND) - -AM_CONDITIONAL([ENABLE_XEN_TESTS], [test "$RUNNING_XEN" != "no" && test "$RUNNING_XEND" != "no"]) - AC_ARG_ENABLE([test-coverage], AC_HELP_STRING([--enable-test-coverage], [turn on code coverage instrumentation @<:@default=no@:>@]), [case "${enableval}" in diff --git a/tests/Makefile.am b/tests/Makefile.am index 48f9faa..528b88e 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -93,10 +93,7 @@ ssh_LDADD = $(COVERAGE_LDFLAGS) if WITH_XEN check_PROGRAMS += xml2sexprtest sexpr2xmltest \ - xmconfigtest xencapstest -if ENABLE_XEN_TESTS -check_PROGRAMS += statstest reconnect -endif + xmconfigtest xencapstest statstest reconnect endif if WITH_QEMU check_PROGRAMS += qemuxml2argvtest qemuxml2xmltest qemuargv2xmltest qemuhelptest @@ -216,10 +213,9 @@ if WITH_XEN TESTS += xml2sexprtest \ sexpr2xmltest \ xmconfigtest \ - xencapstest -if ENABLE_XEN_TESTS -TESTS += reconnect statstest -endif + xencapstest \ + reconnect \ + statstest endif if WITH_QEMU diff --git a/tests/reconnect.c b/tests/reconnect.c index 63877fc..4bc092f 100644 --- a/tests/reconnect.c +++ b/tests/reconnect.c @@ -4,6 +4,7 @@ #include <stdlib.h> #include "internal.h" +#include "command.h" static void errorHandler(void *userData ATTRIBUTE_UNUSED, virErrorPtr error ATTRIBUTE_UNUSED) { @@ -14,6 +15,16 @@ int main(void) { int ro = 0; virConnectPtr conn; virDomainPtr dom; + int status; + virCommandPtr cmd; + + /* skip test if xend is not running */ + cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL); + if (virCommandRun(cmd, &status) == 0 && status != 0) { + virCommandFree(cmd); + return 77; + } + virCommandFree(cmd); virSetErrorFunc(NULL, errorHandler); diff --git a/tests/statstest.c b/tests/statstest.c index c989992..7074914 100644 --- a/tests/statstest.c +++ b/tests/statstest.c @@ -8,6 +8,7 @@ #include "internal.h" #include "xen/block_stats.h" #include "testutils.h" +#include "command.h" static void testQuietError(void *userData ATTRIBUTE_UNUSED, virErrorPtr error ATTRIBUTE_UNUSED) @@ -44,6 +45,17 @@ static int mymain(void) { int ret = 0; + int status; + virCommandPtr cmd; + + /* skip test if xend is not running */ + cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL); + if (virCommandRun(cmd, &status) == 0 && status != 0) { + virCommandFree(cmd); + return 77; + } + virCommandFree(cmd); + /* Some of our tests delibrately test failure cases, so * register a handler to stop error messages cluttering * up display