[libvirt] [PATCH] Skip some xen tests if xend is not running
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 -- 1.7.5.4
On 07/07/2011 04:00 PM, Jim Fehlig wrote:
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(-)
Nice - it removes more lines than it adds, while still improving compilation coverage!
+ + /* skip test if xend is not running */ + cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL); + if (virCommandRun(cmd, &status) == 0 && status != 0) {
If we fail to run the command for external reasons (such as no /usr/sbin/xend binary, or the status command was killed by a signal instead of a normal exit), we probably still want to skip this test. How about this (in both tests): if (virCommandRun(cmd, &status) != 0 || status != 0) {
+ virCommandFree(cmd); + return 77;
I forget we had a macro to avoid the magic number: s/77/EXIT_AM_SKIP/
+ } + virCommandFree(cmd); + /* Some of our tests delibrately test failure cases, so
Hmm, while you're touching this, how about s/delibrately/deliberately/ ACK with those changes. Oh, and our testsuite has a cosmetic bug. After applying your patch, I see this during 'make check': TEST: xencapstest .......... 10 OK PASS: xencapstest SKIP: reconnect TEST: statstest 0 FAIL SKIP: statstest Bonus points for fixing up that output to say SKIP instead of FAIL and to align it correctly (but that can be a separate patch). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Eric Blake wrote:
On 07/07/2011 04:00 PM, Jim Fehlig wrote:
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(-)
Nice - it removes more lines than it adds, while still improving compilation coverage!
+ + /* skip test if xend is not running */ + cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL); + if (virCommandRun(cmd, &status) == 0 && status != 0) {
If we fail to run the command for external reasons (such as no /usr/sbin/xend binary, or the status command was killed by a signal instead of a normal exit), we probably still want to skip this test. How about this (in both tests):
if (virCommandRun(cmd, &status) != 0 || status != 0) {
+ virCommandFree(cmd); + return 77;
I forget we had a macro to avoid the magic number: s/77/EXIT_AM_SKIP/
+ } + virCommandFree(cmd); + /* Some of our tests delibrately test failure cases, so
Hmm, while you're touching this, how about s/delibrately/deliberately/
ACK with those changes.
I've pushed this with your suggestions.
Oh, and our testsuite has a cosmetic bug. After applying your patch, I see this during 'make check':
TEST: xencapstest .......... 10 OK PASS: xencapstest SKIP: reconnect TEST: statstest 0 FAIL SKIP: statstest
Bonus points for fixing up that output to say SKIP instead of FAIL and to align it correctly (but that can be a separate patch).
I'll look into it. Thanks, Jim
Jim Fehlig wrote:
Eric Blake wrote:
[...]
Oh, and our testsuite has a cosmetic bug. After applying your patch, I see this during 'make check':
TEST: xencapstest .......... 10 OK PASS: xencapstest SKIP: reconnect TEST: statstest 0 FAIL SKIP: statstest
Bonus points for fixing up that output to say SKIP instead of FAIL and to align it correctly (but that can be a separate patch).
I'll look into it.
Already fixed I see. If only some of my downstream work was picked up by others while away for a few days... :-). Regards, Jim
On 07/07/2011 04:21 PM, Eric Blake wrote:
Oh, and our testsuite has a cosmetic bug. After applying your patch, I see this during 'make check':
TEST: xencapstest .......... 10 OK PASS: xencapstest SKIP: reconnect TEST: statstest 0 FAIL SKIP: statstest
Bonus points for fixing up that output to say SKIP instead of FAIL and to align it correctly (but that can be a separate patch).
As long as we're investigating formatting errors, this one is also annoying: 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. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2011/7/8 Eric Blake <eblake@redhat.com>:
On 07/07/2011 04:21 PM, Eric Blake wrote:
Oh, and our testsuite has a cosmetic bug. After applying your patch, I see this during 'make check':
TEST: xencapstest .......... 10 OK PASS: xencapstest SKIP: reconnect TEST: statstest 0 FAIL SKIP: statstest
Bonus points for fixing up that output to say SKIP instead of FAIL and to align it correctly (but that can be a separate patch).
As long as we're investigating formatting errors, this one is also annoying:
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. -- Matthias Bolte http://photron.blogspot.com
On 07/08/2011 05:28 PM, Matthias Bolte wrote:
From cb39f79417dce294c654435c1a783065b4983adb Mon Sep 17 00:00:00 2001 From: Matthias Bolte <matthias.bolte@googlemail.com> 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. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
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. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2011/7/9 Eric Blake <eblake@redhat.com>:
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.
Yes, that approach is nicer and also the logic it a bit simpler. Here's a v2. -- Matthias Bolte http://photron.blogspot.com
On 07/09/2011 02:44 AM, Matthias Bolte wrote:
Yes, that approach is nicer and also the logic it a bit simpler. Here's a v2.
-- Matthias Bolte http://photron.blogspot.com
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
Also seq isn't portable. Therefore, calculate the correct padding length directly and use printf to output it at once. --- tests/test-lib.sh | 9 ++------- 1 files changed, 2 insertions(+), 7 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2011/7/9 Eric Blake <eblake@redhat.com>:
On 07/09/2011 02:44 AM, Matthias Bolte wrote:
Yes, that approach is nicer and also the logic it a bit simpler. Here's a v2.
-- Matthias Bolte http://photron.blogspot.com
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
Also seq isn't portable. Therefore, calculate the correct padding length directly and use printf to output it at once. --- tests/test-lib.sh | 9 ++------- 1 files changed, 2 insertions(+), 7 deletions(-)
ACK.
Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com
On 07/09/2011 07:27 AM, Eric Blake wrote:
Also seq isn't portable. Therefore, calculate the correct padding length directly and use printf to output it at once. --- tests/test-lib.sh | 9 ++------- 1 files changed, 2 insertions(+), 7 deletions(-)
ACK.
On second thought, for the case where ($counter % 40 == 0), we want 40 spaces if $counter is 0, but 0 spaces if $counter is positive. I think we need a slight tweak. Rather than: + len=`expr 40 - \( $counter % 40 \)` how about: if test $counter = 0; then len=40 else len=`expr 39 - \( \( $counter - 1 \) % 40 \)` fi -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2011/7/9 Eric Blake <eblake@redhat.com>:
On 07/09/2011 07:27 AM, Eric Blake wrote:
Also seq isn't portable. Therefore, calculate the correct padding length directly and use printf to output it at once. --- tests/test-lib.sh | 9 ++------- 1 files changed, 2 insertions(+), 7 deletions(-)
ACK.
On second thought, for the case where ($counter % 40 == 0), we want 40 spaces if $counter is 0, but 0 spaces if $counter is positive. I think we need a slight tweak. Rather than:
+ len=`expr 40 - \( $counter % 40 \)`
how about:
if test $counter = 0; then len=40 else len=`expr 39 - \( \( $counter - 1 \) % 40 \)` fi
True, the current expression doesn't work for counter = 40. You're expression fixed this, but no reason for special casing 0 here, as modulo on negative values is perfectly fine and yields the right result here expr 39 - \( \( 0 - 1 \) % 40 \) is 40 ACK, to you're equation (but without special casing 0) as I already pushed my patch. -- Matthias Bolte http://photron.blogspot.com
On 07/09/2011 09:42 AM, Matthias Bolte wrote:
True, the current expression doesn't work for counter = 40. You're expression fixed this, but no reason for special casing 0 here, as modulo on negative values is perfectly fine and yields the right result here
expr 39 - \( \( 0 - 1 \) % 40 \) is 40
ACK, to you're equation (but without special casing 0) as I already pushed my patch.
Here's what I pushed to clean up this thread. diff --git c/tests/test-lib.sh w/tests/test-lib.sh index 527dfda..918bf73 100644 --- c/tests/test-lib.sh +++ w/tests/test-lib.sh @@ -54,7 +54,7 @@ test_final() status=$2 if test "$verbose" = "0" ; then - len=`expr 40 - \( $counter % 40 \)` + len=`expr 39 - \( \( $counter - 1 \) % 40 \)` printf "%${len}s" "" if test "$status" = "0" ; then printf " %-3d OK\n" $counter diff --git c/tests/testutils.c w/tests/testutils.c index c89f70f..ac5d298 100644 --- c/tests/testutils.c +++ w/tests/testutils.c @@ -693,9 +693,8 @@ cleanup: VIR_FREE(abs_srcdir); virResetLastError(); if (!virTestGetVerbose() && ret != EXIT_AM_SKIP) { - int i; - for (i = (testCounter % 40) ; i > 0 && i < 40 ; i++) - fprintf(stderr, " "); + if (testCounter == 0 || testCounter % 40) + fprintf(stderr, "%*s", 40 - (testCounter % 40), ""); fprintf(stderr, " %-3d %s\n", testCounter, ret == 0 ? "OK" : "FAIL"); } return ret; -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2011/7/9 Eric Blake <eblake@redhat.com>:
On 07/08/2011 05:28 PM, Matthias Bolte wrote:
From cb39f79417dce294c654435c1a783065b4983adb Mon Sep 17 00:00:00 2001 From: Matthias Bolte <matthias.bolte@googlemail.com> 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.
I withdraw this patch in favor of this one https://www.redhat.com/archives/libvir-list/2011-July/msg00520.html -- Matthias Bolte http://photron.blogspot.com
On 07/08/2011 05:28 PM, Matthias Bolte wrote:
From a1508239af921289cd6e357e8521ff42faf535bd Mon Sep 17 00:00:00 2001 From: Matthias Bolte <matthias.bolte@googlemail.com> 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, " "); -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2011/7/9 Eric Blake <eblake@redhat.com>:
On 07/08/2011 05:28 PM, Matthias Bolte wrote:
From a1508239af921289cd6e357e8521ff42faf535bd Mon Sep 17 00:00:00 2001 From: Matthias Bolte <matthias.bolte@googlemail.com> 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, " ");
This can still be improved in the look of the output. This results in TEST: statstest SKIP: statstest Other tests that decide to be skipped at compile time just result in SKIP: <testname> Problem with the runtime skip decision is that we need to setup stuff before and want to print the TEST: <testname> before doing that to get error messages probably assigned to this test in the output. So we cannot achieve SKIP: <testname> only here. In order to get rid of the indentation of SKIP: <testname> we need to defer the output of the indentation until we know that the test isn't skipped and is actually running test cases. Here's a v2 that does this. -- Matthias Bolte http://photron.blogspot.com
On 07/09/2011 03:51 AM, Matthias Bolte wrote:
Here's a v2 that does this.
-- Matthias Bolte http://photron.blogspot.com Subject: [PATCH] tests: Improve output of tests that decide to skip at runtime
Don't print OK/FAIL for test that decide to be skipped after calling virtTestMain. Delay printing of the indentation before the first test until we know that the test didn't decide to be skipped.
Also make the reconnect test use VIRT_TEST_MAIN. --- tests/Makefile.am | 2 +- tests/reconnect.c | 17 ++++++++++------- tests/testutils.c | 10 +++++++--- 3 files changed, 18 insertions(+), 11 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2011/7/9 Eric Blake <eblake@redhat.com>:
On 07/09/2011 03:51 AM, Matthias Bolte wrote:
Here's a v2 that does this.
-- Matthias Bolte http://photron.blogspot.com Subject: [PATCH] tests: Improve output of tests that decide to skip at runtime
Don't print OK/FAIL for test that decide to be skipped after calling virtTestMain. Delay printing of the indentation before the first test until we know that the test didn't decide to be skipped.
Also make the reconnect test use VIRT_TEST_MAIN. --- tests/Makefile.am | 2 +- tests/reconnect.c | 17 ++++++++++------- tests/testutils.c | 10 +++++++--- 3 files changed, 18 insertions(+), 11 deletions(-)
ACK.
Thanks, pushed too. -- Matthias Bolte http://photron.blogspot.com
On 07/08/2011 05:28 PM, Matthias Bolte wrote:
From 79e8a7e876722a67c37d930762e7a8b32701c6ca Mon Sep 17 00:00:00 2001 From: Matthias Bolte <matthias.bolte@googlemail.com> Date: Sat, 9 Jul 2011 01:24:44 +0200 Subject: [PATCH] tests: Use EXIT_AM_SKIP instead of 77 directly
--- tests/esxutilstest.c | 4 ++-- tests/qemuhelptest.c | 5 ++++- tests/qemuxml2argvtest.c | 5 ++++- tests/vmx2xmltest.c | 4 ++-- tests/xml2vmxtest.c | 4 ++-- 5 files changed, 14 insertions(+), 8 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2011/7/9 Eric Blake <eblake@redhat.com>:
On 07/08/2011 05:28 PM, Matthias Bolte wrote:
From 79e8a7e876722a67c37d930762e7a8b32701c6ca Mon Sep 17 00:00:00 2001 From: Matthias Bolte <matthias.bolte@googlemail.com> Date: Sat, 9 Jul 2011 01:24:44 +0200 Subject: [PATCH] tests: Use EXIT_AM_SKIP instead of 77 directly
--- tests/esxutilstest.c | 4 ++-- tests/qemuhelptest.c | 5 ++++- tests/qemuxml2argvtest.c | 5 ++++- tests/vmx2xmltest.c | 4 ++-- tests/xml2vmxtest.c | 4 ++-- 5 files changed, 14 insertions(+), 8 deletions(-)
ACK.
Thanks. pushed. -- Matthias Bolte http://photron.blogspot.com
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@redhat.com> 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"); } return ret; } -- 1.7.4.4 -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2011/7/9 Eric Blake <eblake@redhat.com>:
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@redhat.com> 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()) {
When we add && ret != EXIT_AM_SKIP here ...
- 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");
.. then we don't need to handle EXIT_AM_SKIP here. ACK. -- Matthias Bolte http://photron.blogspot.com
On 07/09/2011 03:53 AM, Matthias Bolte wrote:
if (!virTestGetVerbose()) {
When we add && ret != EXIT_AM_SKIP here ...
as you did in https://www.redhat.com/archives/libvir-list/2011-July/msg00522.html
- 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");
.. then we don't need to handle EXIT_AM_SKIP here.
I'll wait until your patches are in, at which point, I think the only thing left is just the fprintf optimization to avoid the loop (that is, only the first two lines out of the four + in the above hunk).
ACK.
-- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Thu, Jul 07, 2011 at 04:00:01PM -0600, Jim Fehlig wrote:
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
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Jim Fehlig -
Matthias Bolte