[libvirt] [PATCH 0/6] Remove 'make check' heating
https://xkcd.com/1172/ Ján Tomko (6): Remove virsh-all tests: mock gnutls_dh_params_generate2 Introduce virsh self-test Remove virsh-synopsis Drop virrandomtest Mark virsh-optparse as expensive .gitignore | 1 + tests/Makefile.am | 8 +--- tests/virnettlscontexttest.c | 2 +- tests/virnettlssessiontest.c | 2 +- tests/virrandommock.c | 51 +++++++++++++++++++++ tests/virrandomtest.c | 86 ------------------------------------ tests/virsh-optparse | 2 + tests/{virsh-all => virsh-self-test} | 31 ++++--------- tests/virsh-synopsis | 44 ------------------ tools/virsh.c | 50 +++++++++++++++++++++ 10 files changed, 115 insertions(+), 162 deletions(-) delete mode 100644 tests/virrandomtest.c rename tests/{virsh-all => virsh-self-test} (54%) delete mode 100755 tests/virsh-synopsis -- 2.7.3
Since e8ac4a7 this test wastes some CPU cycles by blindly trying to run almost every virsh command, blindly throwing away the output and the return value and returning success if 'virsh help' successfully returned at least one command. Drop it completely. --- tests/Makefile.am | 1 - tests/virsh-all | 52 ---------------------------------------------------- 2 files changed, 53 deletions(-) delete mode 100755 tests/virsh-all diff --git a/tests/Makefile.am b/tests/Makefile.am index 4a1e0f3..b4fbcc4 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -361,7 +361,6 @@ libvirtd_test_scripts = \ libvirtd-fail \ libvirtd-pool \ virconftest.sh \ - virsh-all \ virsh-cpuset \ virsh-define-dev-segfault \ virsh-int-overflow \ diff --git a/tests/virsh-all b/tests/virsh-all deleted file mode 100755 index 4a91e4e..0000000 --- a/tests/virsh-all +++ /dev/null @@ -1,52 +0,0 @@ -#!/bin/sh -# blindly run each and every command listed by "virsh help" - -# Copyright (C) 2008, 2009 Red Hat, Inc. - -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 2 of the License, or -# (at your option) any later version. - -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. - -# You should have received a copy of the GNU General Public License -# along with this program. If not, see -# <http://www.gnu.org/licenses/>. - -. "$(dirname $0)/test-lib.sh" - -test_expensive - -fail=0 - -test_url=test:///default - -$abs_top_builddir/tools/virsh -c $test_url help | grep -v connect > cmds || framework_failure -cmds=$(sed -n 's/^ \([^ ][^ ]*\) .*/\1/p' cmds) || framework_failure -test -n "$cmds" || framework_failure - -test_intro "virsh-all" - -counter=0 -for i in $cmds; do - counter=`eval "expr $counter + 1"` - - # For now, just run the command and ignore output - $abs_top_builddir/tools/virsh -c $test_url $i < /dev/null > /dev/null 2>&1 - # Temporarily ignoring exit status - #status=$? - status=0 - test_result $counter $i $status - - if test "$status" = "1" ; then - fail=1 - fi -done - -test_final $counter $fail - -(exit $fail); exit $fail -- 2.7.3
On Fri, Jun 17, 2016 at 20:04:36 +0200, Ján Tomko wrote:
Since e8ac4a7 this test wastes some CPU cycles by blindly trying to run almost every virsh command, blindly throwing away the output and the return value and returning success if 'virsh help' successfully returned at least one command.
Drop it completely. --- tests/Makefile.am | 1 - tests/virsh-all | 52 ---------------------------------------------------- 2 files changed, 53 deletions(-) delete mode 100755 tests/virsh-all
[...]
diff --git a/tests/virsh-all b/tests/virsh-all deleted file mode 100755 index 4a91e4e..0000000 --- a/tests/virsh-all +++ /dev/null @@ -1,52 +0,0 @@
-counter=0 -for i in $cmds; do - counter=`eval "expr $counter + 1"` - - # For now, just run the command and ignore output - $abs_top_builddir/tools/virsh -c $test_url $i < /dev/null > /dev/null 2>&1 - # Temporarily ignoring exit status - #status=$? - status=0
Lol!
- test_result $counter $i $status
ACK
This function generates some big random numbers. Cache the result and supply it to any subsequent generate2 calls. --- tests/virnettlscontexttest.c | 2 +- tests/virnettlssessiontest.c | 2 +- tests/virrandommock.c | 51 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c index b062be6..9f62413 100644 --- a/tests/virnettlscontexttest.c +++ b/tests/virnettlscontexttest.c @@ -633,7 +633,7 @@ mymain(void) return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -VIRT_TEST_MAIN(mymain) +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virrandommock.so") #else diff --git a/tests/virnettlssessiontest.c b/tests/virnettlssessiontest.c index 47fbec6..0d2e106 100644 --- a/tests/virnettlssessiontest.c +++ b/tests/virnettlssessiontest.c @@ -487,7 +487,7 @@ mymain(void) return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -VIRT_TEST_MAIN(mymain) +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virrandommock.so") #else diff --git a/tests/virrandommock.c b/tests/virrandommock.c index 6df5e20..04703a1 100644 --- a/tests/virrandommock.c +++ b/tests/virrandommock.c @@ -37,3 +37,54 @@ virRandomBytes(unsigned char *buf, return 0; } + + +#ifdef WITH_GNUTLS +# include <assert.h> +# include <stdio.h> +# include <gnutls/gnutls.h> + +static int (*realgenerate2)(gnutls_dh_params_t dparams, + unsigned int bits); + +static void init_syms(void) +{ + if (realgenerate2) + return; + + realgenerate2 = dlsym(RTLD_NEXT, "gnutls_dh_params_generate2"); + if (realgenerate2) + return; + + fprintf(stderr, "Error getting symbols"); + abort(); +} + +static gnutls_dh_params_t params_cache; +unsigned int cachebits; + +int +gnutls_dh_params_generate2(gnutls_dh_params_t dparams, + unsigned int bits) +{ + int rc = 0; + + init_syms(); + + if (!params_cache) { + if (gnutls_dh_params_init(¶ms_cache) < 0) { + fprintf(stderr, "Error initializing params cache"); + abort(); + } + rc = realgenerate2(params_cache, bits); + + if (rc < 0) + return rc; + cachebits = bits; + } + + assert(cachebits == bits); + + return gnutls_dh_params_cpy(dparams, params_cache); +} +#endif -- 2.7.3
On Fri, Jun 17, 2016 at 20:04:37 +0200, Ján Tomko wrote:
This function generates some big random numbers.
Cache the result and supply it to any subsequent generate2 calls. --- tests/virnettlscontexttest.c | 2 +- tests/virnettlssessiontest.c | 2 +- tests/virrandommock.c | 51 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/tests/virrandommock.c b/tests/virrandommock.c index 6df5e20..04703a1 100644 --- a/tests/virrandommock.c +++ b/tests/virrandommock.c @@ -37,3 +37,54 @@ virRandomBytes(unsigned char *buf,
return 0; } + + +#ifdef WITH_GNUTLS +# include <assert.h> +# include <stdio.h> +# include <gnutls/gnutls.h> + +static int (*realgenerate2)(gnutls_dh_params_t dparams, + unsigned int bits); + +static void init_syms(void) +{
We have a macro to help with all the stuff below. It also has better error message. You should use it: VIR_MOCK_REAL_INIT
+ if (realgenerate2) + return; + + realgenerate2 = dlsym(RTLD_NEXT, "gnutls_dh_params_generate2"); + if (realgenerate2) + return; + + fprintf(stderr, "Error getting symbols"); + abort(); +} + +static gnutls_dh_params_t params_cache; +unsigned int cachebits;
Perhaps this should be static too.
+ +int +gnutls_dh_params_generate2(gnutls_dh_params_t dparams, + unsigned int bits) +{ + int rc = 0; + + init_syms(); + + if (!params_cache) { + if (gnutls_dh_params_init(¶ms_cache) < 0) { + fprintf(stderr, "Error initializing params cache"); + abort(); + } + rc = realgenerate2(params_cache, bits); + + if (rc < 0) + return rc; + cachebits = bits; + } + + assert(cachebits == bits);
I'd rather not use assert here. Since you already have abort available you can use it directly.
+ + return gnutls_dh_params_cpy(dparams, params_cache); +}
ACK with fixes.
A new hidden command for virsh that will iterate over all command groups and commands and print help for every single one. This involves running vshCmddefOptParse so we can get an error if one of the command's option structure is invalid. --- .gitignore | 1 + tests/Makefile.am | 1 + tests/virsh-self-test | 37 +++++++++++++++++++++++++++++++++++++ tools/virsh.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+) create mode 100755 tests/virsh-self-test diff --git a/.gitignore b/.gitignore index 39c0423..8ce7e18 100644 --- a/.gitignore +++ b/.gitignore @@ -162,6 +162,7 @@ /tests/*test /tests/commandhelper /tests/qemucapsprobe +!/tests/virsh-self-test !/tests/virt-aa-helper-test /tests/objectlocking /tests/objectlocking-files.txt diff --git a/tests/Makefile.am b/tests/Makefile.am index b4fbcc4..b09526a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -368,6 +368,7 @@ libvirtd_test_scripts = \ virsh-read-bufsiz \ virsh-read-non-seekable \ virsh-schedinfo \ + virsh-self-test \ virsh-start \ virsh-synopsis \ virsh-undefine \ diff --git a/tests/virsh-self-test b/tests/virsh-self-test new file mode 100755 index 0000000..42e8605 --- /dev/null +++ b/tests/virsh-self-test @@ -0,0 +1,37 @@ +#!/bin/sh +# run virsh self-test to make sure command option structures are valid + +# Copyright (C) 2008, 2009 Red Hat, Inc. + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 2 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see +# <http://www.gnu.org/licenses/>. + +. "$(dirname $0)/test-lib.sh" + +fail=0 + +test_url=test:///default + +test_intro "virsh-self-test" +$abs_top_builddir/tools/virsh -c $test_url self-test > /dev/null +status=$? +test_result 1 "virsh-self-test" $status + +if test "$status" = "1" ; then + fail=1 +fi + +test_final $counter $fail + +(exit $fail); exit $fail diff --git a/tools/virsh.c b/tools/virsh.c index 2a807d9..8eb05d3 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -341,6 +341,50 @@ virshConnectionHandler(vshControl *ctl) return NULL; } +/* ----------------- + * Command self-test + * ----------------- */ + +static const vshCmdInfo info_selftest[] = { + {.name = "help", + .data = N_("internal command for testing virsh") + }, + {.name = "desc", + .data = N_("This message should not be output.") + }, + {.name = NULL} +}; + +/* Prints help for every command. + * That runs vshCmddefOptParse which validates + * the per-command options structure. */ +static bool +cmdSelfTest(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + const vshCmdGrp *grp; + const vshCmdDef *def; + /* + const char *blacklist [] = { + "self-test", + "connect" + }; */ + + vshPrint(ctl, "%s", _("Grouped commands:\n\n")); + + for (grp = cmdGroups; grp->name; grp++) { + for (def = grp->commands; def->name; def++) { + if (def->flags & VSH_CMD_FLAG_ALIAS) + continue; + + if (!vshCmddefHelp(ctl, def->name)) + return false; + } + } + + return true; +} + + /* --------------- * Misc utils * --------------- @@ -857,6 +901,12 @@ static const vshCmdDef virshCmds[] = { .info = info_connect, .flags = VSH_CMD_FLAG_NOCONNECT }, + {.name = "self-test", + .handler = cmdSelfTest, + .opts = NULL, + .info = info_selftest, + .flags = VSH_CMD_FLAG_NOCONNECT | VSH_CMD_FLAG_ALIAS + }, {.name = NULL} }; -- 2.7.3
On Fri, Jun 17, 2016 at 20:04:38 +0200, Ján Tomko wrote:
A new hidden command for virsh that will iterate over all command groups and commands and print help for every single one.
This involves running vshCmddefOptParse so we can get an error if one of the command's option structure is invalid. --- .gitignore | 1 + tests/Makefile.am | 1 + tests/virsh-self-test | 37 +++++++++++++++++++++++++++++++++++++ tools/virsh.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+) create mode 100755 tests/virsh-self-test
diff --git a/tests/virsh-self-test b/tests/virsh-self-test new file mode 100755 index 0000000..42e8605 --- /dev/null +++ b/tests/virsh-self-test @@ -0,0 +1,37 @@ +#!/bin/sh +# run virsh self-test to make sure command option structures are valid + +# Copyright (C) 2008, 2009 Red Hat, Inc.
This year doesn't look correct
+ +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 2 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see +# <http://www.gnu.org/licenses/>. + +. "$(dirname $0)/test-lib.sh" + +fail=0 + +test_url=test:///default + +test_intro "virsh-self-test" +$abs_top_builddir/tools/virsh -c $test_url self-test > /dev/null +status=$? +test_result 1 "virsh-self-test" $status + +if test "$status" = "1" ; then + fail=1 +fi + +test_final $counter $fail + +(exit $fail); exit $fail diff --git a/tools/virsh.c b/tools/virsh.c index 2a807d9..8eb05d3 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -341,6 +341,50 @@ virshConnectionHandler(vshControl *ctl) return NULL; }
+/* ----------------- + * Command self-test + * ----------------- */ + +static const vshCmdInfo info_selftest[] = { + {.name = "help", + .data = N_("internal command for testing virsh") + }, + {.name = "desc", + .data = N_("This message should not be output.")
$ tools/virsh self-test --help NAME self-test - internal command for testing virsh SYNOPSIS self-test DESCRIPTION This message should not be output. I'd go with "DO NOT USE THIS COMMAND", or "internal use only".
+ }, + {.name = NULL} +}; + +/* Prints help for every command. + * That runs vshCmddefOptParse which validates + * the per-command options structure. */ +static bool +cmdSelfTest(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + const vshCmdGrp *grp; + const vshCmdDef *def; + /* + const char *blacklist [] = { + "self-test", + "connect" + }; */
You probably forgot to delete this.
+ + vshPrint(ctl, "%s", _("Grouped commands:\n\n"));
This doesn't look necessary. Perhaps you could change it to a warning that this output shouldn't be used.
+ + for (grp = cmdGroups; grp->name; grp++) { + for (def = grp->commands; def->name; def++) { + if (def->flags & VSH_CMD_FLAG_ALIAS) + continue;
ACK
This tests checks that the first word after SYNOPSIS in virsh help ${command} output is ${command}. This was only good to check that the command option structures are valid, which is now served by 'virsh self-test'. --- tests/Makefile.am | 1 - tests/virsh-synopsis | 44 -------------------------------------------- 2 files changed, 45 deletions(-) delete mode 100755 tests/virsh-synopsis diff --git a/tests/Makefile.am b/tests/Makefile.am index b09526a..3ec7e7a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -370,7 +370,6 @@ libvirtd_test_scripts = \ virsh-schedinfo \ virsh-self-test \ virsh-start \ - virsh-synopsis \ virsh-undefine \ virsh-uriprecedence \ virsh-vcpupin \ diff --git a/tests/virsh-synopsis b/tests/virsh-synopsis deleted file mode 100755 index d0d1197..0000000 --- a/tests/virsh-synopsis +++ /dev/null @@ -1,44 +0,0 @@ -#!/bin/sh -# ensure that each command's help "SYNOPSIS" line starts with the command name - -# Copyright (C) 2008 Red Hat, Inc. - -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 2 of the License, or -# (at your option) any later version. - -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. - -# You should have received a copy of the GNU General Public License -# along with this program. If not, see -# <http://www.gnu.org/licenses/>. - -. "$(dirname $0)/test-lib.sh" - -if test "$VERBOSE" = yes; then - set -x - $abs_top_builddir/tools/virsh --version -fi - -fail=0 - -test_url=test:///default - -$abs_top_builddir/tools/virsh -c $test_url help > cmds || framework_failure -cmds=$(sed -n 's/^ \([^ ][^ ]*\) .*/\1/p' cmds) || framework_failure -test -n "$cmds" || framework_failure - -for i in $cmds; do - $abs_top_builddir/tools/virsh -c $test_url help $i > help || fail=1 - grep -A1 '^ SYNOPSIS$' help > synopsis \ - || { echo 1>&2 missing or invalid help SYNOPSIS for $i; fail=1; } - sed -n 2p synopsis > s2 || framework_failure - grep -E "^ $i( |$)" s2 > /dev/null \ - || { echo 1>&2 "invalid help SYNOPSIS for $i:"; cat s2 1>&2; fail=1; } -done - -(exit $fail); exit $fail -- 2.7.3
On Fri, Jun 17, 2016 at 20:04:39 +0200, Ján Tomko wrote:
This tests checks that the first word after SYNOPSIS in virsh help ${command} output is ${command}.
This was only good to check that the command option structures are valid, which is now served by 'virsh self-test'. --- tests/Makefile.am | 1 - tests/virsh-synopsis | 44 -------------------------------------------- 2 files changed, 45 deletions(-) delete mode 100755 tests/virsh-synopsis
ACK
This test only checks if mocking of virRandomBytes works correctly. Drop it to avoid infinite recursion by testing the test suite. --- tests/Makefile.am | 5 --- tests/virrandomtest.c | 86 --------------------------------------------------- 2 files changed, 91 deletions(-) delete mode 100644 tests/virrandomtest.c diff --git a/tests/Makefile.am b/tests/Makefile.am index 3ec7e7a..d2be2fc 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -170,7 +170,6 @@ test_programs = virshtest sockettest \ virbitmaptest \ vircgrouptest \ vircryptotest \ - virrandomtest \ virpcitest \ virendiantest \ virfiletest \ @@ -1073,10 +1072,6 @@ vircryptotest_SOURCES = \ vircryptotest.c testutils.h testutils.c vircryptotest_LDADD = $(LDADDS) -virrandomtest_SOURCES = \ - virrandomtest.c testutils.h testutils.c -virrandomtest_LDADD = $(LDADDS) - virhostdevtest_SOURCES = \ virhostdevtest.c testutils.h testutils.c virhostdevtest_LDADD = $(LDADDS) diff --git a/tests/virrandomtest.c b/tests/virrandomtest.c deleted file mode 100644 index bafe608..0000000 --- a/tests/virrandomtest.c +++ /dev/null @@ -1,86 +0,0 @@ -/* - * Copyright (C) 2016 Red Hat, Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library. If not, see - * <http://www.gnu.org/licenses/>. - * - * Author: John Ferlan <jferlan@redhat.com> - */ - -#include <config.h> - -#include "internal.h" -#include "viralloc.h" -#include "virrandom.h" -#include "testutils.h" - -#ifndef WIN32 - -# define VIR_FROM_THIS VIR_FROM_NONE - -static int -testRandomBytes(const void *unused ATTRIBUTE_UNUSED) -{ - int ret = -1; - size_t i; - uint8_t *data; - size_t datalen = 32; - - if (VIR_ALLOC_N(data, datalen) < 0) - return -1; - - if (virRandomBytes(data, datalen)) { - fprintf(stderr, "Failed to generate random bytes"); - goto cleanup; - } - - for (i = 0; i < datalen; i++) { - if (data[i] != i) { - fprintf(stderr, - "virRandomBytes data[%zu]='%x' not in sequence\n", - i, data[i]); - goto cleanup; - } - } - - ret = 0; - - cleanup: - VIR_FREE(data); - return ret; -} - - -static int -mymain(void) -{ - int ret = 0; - - if (virTestRun("RandomBytes", testRandomBytes, NULL) < 0) - ret = -1; - - return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; -} - -VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virrandommock.so") - -#else - -int -main(void) -{ - return EXIT_AM_SKIP; -} - -#endif -- 2.7.3
On Fri, Jun 17, 2016 at 20:04:40 +0200, Ján Tomko wrote:
This test only checks if mocking of virRandomBytes works correctly.
Drop it to avoid infinite recursion by testing the test suite. --- tests/Makefile.am | 5 --- tests/virrandomtest.c | 86 --------------------------------------------------- 2 files changed, 91 deletions(-) delete mode 100644 tests/virrandomtest.c
ACK
--- tests/virsh-optparse | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/virsh-optparse b/tests/virsh-optparse index cbd6c30..cb0f3d4 100755 --- a/tests/virsh-optparse +++ b/tests/virsh-optparse @@ -19,6 +19,8 @@ . "$(dirname $0)/test-lib.sh" +test_expensive + # If $abs_top_builddir/tools is not early in $PATH, put it there, # so that we can safely invoke "virsh" simply with its name. case $PATH in -- 2.7.3
participants (2)
-
Ján Tomko -
Peter Krempa