[libvirt] [PATCH 0/2] Introduce basic virt-admin-self-test
I've realized we do no testing of virt-admin yet while reviewing Erik's patches. Michal Privoznik (2): virsh: Move cmdSelfTest to vsh tests: Self test virt-admin .gitignore | 1 + tests/Makefile.am | 1 + tests/virsh-self-test | 21 ++++++++++++++++----- tests/virt-admin-self-test | 1 + tools/virsh.c | 45 +-------------------------------------------- tools/virt-admin.c | 1 + tools/vsh.c | 38 ++++++++++++++++++++++++++++++++++++++ tools/vsh.h | 11 +++++++++++ 8 files changed, 70 insertions(+), 49 deletions(-) create mode 120000 tests/virt-admin-self-test -- 2.8.4
This command should be exposed to other shells of ours. They are gonna need it as soon as we want to test them too. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh.c | 45 +-------------------------------------------- tools/vsh.c | 38 ++++++++++++++++++++++++++++++++++++++ tools/vsh.h | 11 +++++++++++ 3 files changed, 50 insertions(+), 44 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index cb60edc..1068447 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -346,44 +346,6 @@ 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_("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; - - vshPrint(ctl, "Do not use the following output:\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 @@ -894,18 +856,13 @@ static const vshCmdDef virshCmds[] = { VSH_CMD_HELP, VSH_CMD_PWD, VSH_CMD_QUIT, + VSH_CMD_SELF_TEST, {.name = "connect", .handler = cmdConnect, .opts = opts_connect, .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} }; diff --git a/tools/vsh.c b/tools/vsh.c index be6a073..c51bdd7 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -3328,3 +3328,41 @@ cmdQuit(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) ctl->imode = false; return true; } + +/* ----------------- + * Command self-test + * ----------------- */ + +const vshCmdInfo info_selftest[] = { + {.name = "help", + .data = N_("internal command for testing virsh") + }, + {.name = "desc", + .data = N_("internal use only") + }, + {.name = NULL} +}; + +/* Prints help for every command. + * That runs vshCmddefOptParse which validates + * the per-command options structure. */ +bool +cmdSelfTest(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + const vshCmdGrp *grp; + const vshCmdDef *def; + + vshPrint(ctl, "Do not use the following output:\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; +} diff --git a/tools/vsh.h b/tools/vsh.h index e53910f..7f08191 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -376,12 +376,14 @@ extern const vshCmdOptDef opts_echo[]; extern const vshCmdInfo info_echo[]; extern const vshCmdInfo info_pwd[]; extern const vshCmdInfo info_quit[]; +extern const vshCmdInfo info_selftest[]; bool cmdHelp(vshControl *ctl, const vshCmd *cmd); bool cmdCd(vshControl *ctl, const vshCmd *cmd); bool cmdEcho(vshControl *ctl, const vshCmd *cmd); bool cmdPwd(vshControl *ctl, const vshCmd *cmd); bool cmdQuit(vshControl *ctl, const vshCmd *cmd); +bool cmdSelfTest(vshControl *ctl, const vshCmd *cmd); # define VSH_CMD_CD \ { \ @@ -437,6 +439,15 @@ bool cmdQuit(vshControl *ctl, const vshCmd *cmd); .flags = VSH_CMD_FLAG_NOCONNECT \ } +# define VSH_CMD_SELF_TEST \ + { \ + .name = "self-test", \ + .handler = cmdSelfTest, \ + .opts = NULL, \ + .info = info_selftest, \ + .flags = VSH_CMD_FLAG_NOCONNECT | VSH_CMD_FLAG_ALIAS \ + } + /* readline */ char * vshReadline(vshControl *ctl, const char *prompt); -- 2.8.4
On 14/09/16 10:17, Michal Privoznik wrote:
This command should be exposed to other shells of ours. They are gonna need it as soon as we want to test them too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh.c | 45 +-------------------------------------------- tools/vsh.c | 38 ++++++++++++++++++++++++++++++++++++++ tools/vsh.h | 11 +++++++++++ 3 files changed, 50 insertions(+), 44 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index cb60edc..1068447 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -346,44 +346,6 @@ 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_("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; - - vshPrint(ctl, "Do not use the following output:\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 @@ -894,18 +856,13 @@ static const vshCmdDef virshCmds[] = { VSH_CMD_HELP, VSH_CMD_PWD, VSH_CMD_QUIT, + VSH_CMD_SELF_TEST, {.name = "connect", .handler = cmdConnect, .opts = opts_connect, .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} };
diff --git a/tools/vsh.c b/tools/vsh.c index be6a073..c51bdd7 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -3328,3 +3328,41 @@ cmdQuit(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) ctl->imode = false; return true; } + +/* ----------------- + * Command self-test + * ----------------- */ + +const vshCmdInfo info_selftest[] = { + {.name = "help", + .data = N_("internal command for testing virsh") + },
s/virsh/virt shells/ or something alike you can come up with :) ACK Erik
Just like we are running 'virsh self-test' from within our test suite, we should run 'virt-admin self-test' too. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- .gitignore | 1 + tests/Makefile.am | 1 + tests/virsh-self-test | 21 ++++++++++++++++----- tests/virt-admin-self-test | 1 + tools/virt-admin.c | 1 + 5 files changed, 20 insertions(+), 5 deletions(-) create mode 120000 tests/virt-admin-self-test diff --git a/.gitignore b/.gitignore index e87c085..879ec24 100644 --- a/.gitignore +++ b/.gitignore @@ -169,6 +169,7 @@ /tests/qemucapsprobe !/tests/virsh-self-test !/tests/virt-aa-helper-test +!/tests/virt-admin-self-test /tests/objectlocking /tests/objectlocking-files.txt /tests/objectlocking.cm[ix] diff --git a/tests/Makefile.am b/tests/Makefile.am index 0cd8391..924029a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -369,6 +369,7 @@ libvirtd_test_scripts = \ virsh-read-non-seekable \ virsh-schedinfo \ virsh-self-test \ + virt-admin-self-test \ virsh-start \ virsh-undefine \ virsh-uriprecedence \ diff --git a/tests/virsh-self-test b/tests/virsh-self-test index 641810f..22396bc 100755 --- a/tests/virsh-self-test +++ b/tests/virsh-self-test @@ -21,14 +21,25 @@ fail=0 -test_url=test:///default +basename=$(basename $0) -test_intro "virsh-self-test" -$abs_top_builddir/tools/virsh -c $test_url self-test > /dev/null +if test "x$basename" = "xvirsh-self-test" ; then + binary=virsh + extra_args="-c test:///default" +elif test "x$basename" = "xvirt-admin-self-test" ; then + binary=virt-admin + extra_args="" +else + echo "Unknown binary: $basename"; + exit 1 +fi + +test_intro "$0" +$abs_top_builddir/tools/${binary} ${extra_args} self-test > /dev/null status=$? -test_result 1 "virsh-self-test" $status +test_result 1 "$0" ${status} -if test "$status" != "0" ; then +if test "${status}" != "0" ; then fail=1 fi diff --git a/tests/virt-admin-self-test b/tests/virt-admin-self-test new file mode 120000 index 0000000..d4ad624 --- /dev/null +++ b/tests/virt-admin-self-test @@ -0,0 +1 @@ +./virsh-self-test \ No newline at end of file diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 12ec057..36c92f5 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -1242,6 +1242,7 @@ static const vshCmdDef vshAdmCmds[] = { VSH_CMD_HELP, VSH_CMD_PWD, VSH_CMD_QUIT, + VSH_CMD_SELF_TEST, {.name = "uri", .handler = cmdURI, .opts = NULL, -- 2.8.4
On 14/09/16 10:17, Michal Privoznik wrote:
Just like we are running 'virsh self-test' from within our test suite, we should run 'virt-admin self-test' too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- .gitignore | 1 + tests/Makefile.am | 1 + tests/virsh-self-test | 21 ++++++++++++++++----- tests/virt-admin-self-test | 1 + tools/virt-admin.c | 1 + 5 files changed, 20 insertions(+), 5 deletions(-) create mode 120000 tests/virt-admin-self-test
diff --git a/.gitignore b/.gitignore index e87c085..879ec24 100644 --- a/.gitignore +++ b/.gitignore @@ -169,6 +169,7 @@ /tests/qemucapsprobe !/tests/virsh-self-test !/tests/virt-aa-helper-test +!/tests/virt-admin-self-test /tests/objectlocking /tests/objectlocking-files.txt /tests/objectlocking.cm[ix] diff --git a/tests/Makefile.am b/tests/Makefile.am index 0cd8391..924029a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -369,6 +369,7 @@ libvirtd_test_scripts = \ virsh-read-non-seekable \ virsh-schedinfo \ virsh-self-test \ + virt-admin-self-test \ virsh-start \ virsh-undefine \ virsh-uriprecedence \ diff --git a/tests/virsh-self-test b/tests/virsh-self-test index 641810f..22396bc 100755 --- a/tests/virsh-self-test +++ b/tests/virsh-self-test @@ -21,14 +21,25 @@
fail=0
-test_url=test:///default +basename=$(basename $0)
-test_intro "virsh-self-test" -$abs_top_builddir/tools/virsh -c $test_url self-test > /dev/null +if test "x$basename" = "xvirsh-self-test" ; then + binary=virsh + extra_args="-c test:///default" +elif test "x$basename" = "xvirt-admin-self-test" ; then + binary=virt-admin + extra_args="" +else + echo "Unknown binary: $basename"; + exit 1 +fi + +test_intro "$0" +$abs_top_builddir/tools/${binary} ${extra_args} self-test > /dev/null status=$? -test_result 1 "virsh-self-test" $status +test_result 1 "$0" ${status}
-if test "$status" != "0" ; then +if test "${status}" != "0" ; then fail=1 fi
diff --git a/tests/virt-admin-self-test b/tests/virt-admin-self-test new file mode 120000
Although a working solution, we would end up with two identical scripts. How about extracting the generic bit (since you've already refactored the hunk above, thus putting some effort into making it nicely generic), and enclosing it into a separate function and a separate module, e.g. self_test() and let's say module virt-shell-test (or whatever) and then just source it the same way as we do it with test-lib.sh from within virsh-self-test and virt-admin-self-test. Erik
index 0000000..d4ad624 --- /dev/null +++ b/tests/virt-admin-self-test @@ -0,0 +1 @@ +./virsh-self-test \ No newline at end of file diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 12ec057..36c92f5 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -1242,6 +1242,7 @@ static const vshCmdDef vshAdmCmds[] = { VSH_CMD_HELP, VSH_CMD_PWD, VSH_CMD_QUIT, + VSH_CMD_SELF_TEST, {.name = "uri", .handler = cmdURI, .opts = NULL,
On 14.09.2016 12:39, Erik Skultety wrote:
On 14/09/16 10:17, Michal Privoznik wrote:
Just like we are running 'virsh self-test' from within our test suite, we should run 'virt-admin self-test' too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- .gitignore | 1 + tests/Makefile.am | 1 + tests/virsh-self-test | 21 ++++++++++++++++----- tests/virt-admin-self-test | 1 + tools/virt-admin.c | 1 + 5 files changed, 20 insertions(+), 5 deletions(-) create mode 120000 tests/virt-admin-self-test
diff --git a/.gitignore b/.gitignore index e87c085..879ec24 100644 --- a/.gitignore +++ b/.gitignore @@ -169,6 +169,7 @@ /tests/qemucapsprobe !/tests/virsh-self-test !/tests/virt-aa-helper-test +!/tests/virt-admin-self-test /tests/objectlocking /tests/objectlocking-files.txt /tests/objectlocking.cm[ix] diff --git a/tests/Makefile.am b/tests/Makefile.am index 0cd8391..924029a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -369,6 +369,7 @@ libvirtd_test_scripts = \ virsh-read-non-seekable \ virsh-schedinfo \ virsh-self-test \ + virt-admin-self-test \ virsh-start \ virsh-undefine \ virsh-uriprecedence \ diff --git a/tests/virsh-self-test b/tests/virsh-self-test index 641810f..22396bc 100755 --- a/tests/virsh-self-test +++ b/tests/virsh-self-test @@ -21,14 +21,25 @@
fail=0
-test_url=test:///default +basename=$(basename $0)
-test_intro "virsh-self-test" -$abs_top_builddir/tools/virsh -c $test_url self-test > /dev/null +if test "x$basename" = "xvirsh-self-test" ; then + binary=virsh + extra_args="-c test:///default" +elif test "x$basename" = "xvirt-admin-self-test" ; then + binary=virt-admin + extra_args="" +else + echo "Unknown binary: $basename"; + exit 1 +fi + +test_intro "$0" +$abs_top_builddir/tools/${binary} ${extra_args} self-test > /dev/null status=$? -test_result 1 "virsh-self-test" $status +test_result 1 "$0" ${status}
-if test "$status" != "0" ; then +if test "${status}" != "0" ; then fail=1 fi
diff --git a/tests/virt-admin-self-test b/tests/virt-admin-self-test new file mode 120000
Although a working solution, we would end up with two identical scripts.
Well, the virt-admin-self-test is really just a symlink to virsh-self-test (which can handle the case if run under different name).
How about extracting the generic bit (since you've already refactored the hunk above, thus putting some effort into making it nicely generic), and enclosing it into a separate function and a separate module, e.g. self_test() and let's say module virt-shell-test (or whatever) and then just source it the same way as we do it with test-lib.sh from within virsh-self-test and virt-admin-self-test.
Well, I like the symlink approach better. What you described is just more work IMO ;-) Michal
On 14/09/16 12:50, Michal Privoznik wrote:
On 14.09.2016 12:39, Erik Skultety wrote:
On 14/09/16 10:17, Michal Privoznik wrote:
Just like we are running 'virsh self-test' from within our test suite, we should run 'virt-admin self-test' too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- .gitignore | 1 + tests/Makefile.am | 1 + tests/virsh-self-test | 21 ++++++++++++++++----- tests/virt-admin-self-test | 1 + tools/virt-admin.c | 1 + 5 files changed, 20 insertions(+), 5 deletions(-) create mode 120000 tests/virt-admin-self-test
diff --git a/.gitignore b/.gitignore index e87c085..879ec24 100644 --- a/.gitignore +++ b/.gitignore @@ -169,6 +169,7 @@ /tests/qemucapsprobe !/tests/virsh-self-test !/tests/virt-aa-helper-test +!/tests/virt-admin-self-test /tests/objectlocking /tests/objectlocking-files.txt /tests/objectlocking.cm[ix] diff --git a/tests/Makefile.am b/tests/Makefile.am index 0cd8391..924029a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -369,6 +369,7 @@ libvirtd_test_scripts = \ virsh-read-non-seekable \ virsh-schedinfo \ virsh-self-test \ + virt-admin-self-test \ virsh-start \ virsh-undefine \ virsh-uriprecedence \ diff --git a/tests/virsh-self-test b/tests/virsh-self-test index 641810f..22396bc 100755 --- a/tests/virsh-self-test +++ b/tests/virsh-self-test @@ -21,14 +21,25 @@
fail=0
-test_url=test:///default +basename=$(basename $0)
-test_intro "virsh-self-test" -$abs_top_builddir/tools/virsh -c $test_url self-test > /dev/null +if test "x$basename" = "xvirsh-self-test" ; then + binary=virsh + extra_args="-c test:///default" +elif test "x$basename" = "xvirt-admin-self-test" ; then + binary=virt-admin + extra_args="" +else + echo "Unknown binary: $basename"; + exit 1 +fi + +test_intro "$0" +$abs_top_builddir/tools/${binary} ${extra_args} self-test > /dev/null status=$? -test_result 1 "virsh-self-test" $status +test_result 1 "$0" ${status}
-if test "$status" != "0" ; then +if test "${status}" != "0" ; then fail=1 fi
diff --git a/tests/virt-admin-self-test b/tests/virt-admin-self-test new file mode 120000
Although a working solution, we would end up with two identical scripts.
Well, the virt-admin-self-test is really just a symlink to virsh-self-test (which can handle the case if run under different name).
Sigh... never mind, ACK. Erik
How about extracting the generic bit (since you've already refactored the hunk above, thus putting some effort into making it nicely generic), and enclosing it into a separate function and a separate module, e.g. self_test() and let's say module virt-shell-test (or whatever) and then just source it the same way as we do it with test-lib.sh from within virsh-self-test and virt-admin-self-test.
Well, I like the symlink approach better. What you described is just more work IMO ;-)
Michal
On 14.09.2016 12:58, Erik Skultety wrote:
On 14/09/16 12:50, Michal Privoznik wrote:
On 14.09.2016 12:39, Erik Skultety wrote:
On 14/09/16 10:17, Michal Privoznik wrote:
Just like we are running 'virsh self-test' from within our test suite, we should run 'virt-admin self-test' too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- .gitignore | 1 + tests/Makefile.am | 1 + tests/virsh-self-test | 21 ++++++++++++++++----- tests/virt-admin-self-test | 1 + tools/virt-admin.c | 1 + 5 files changed, 20 insertions(+), 5 deletions(-) create mode 120000 tests/virt-admin-self-test
diff --git a/.gitignore b/.gitignore index e87c085..879ec24 100644 --- a/.gitignore +++ b/.gitignore @@ -169,6 +169,7 @@ /tests/qemucapsprobe !/tests/virsh-self-test !/tests/virt-aa-helper-test +!/tests/virt-admin-self-test /tests/objectlocking /tests/objectlocking-files.txt /tests/objectlocking.cm[ix] diff --git a/tests/Makefile.am b/tests/Makefile.am index 0cd8391..924029a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -369,6 +369,7 @@ libvirtd_test_scripts = \ virsh-read-non-seekable \ virsh-schedinfo \ virsh-self-test \ + virt-admin-self-test \ virsh-start \ virsh-undefine \ virsh-uriprecedence \ diff --git a/tests/virsh-self-test b/tests/virsh-self-test index 641810f..22396bc 100755 --- a/tests/virsh-self-test +++ b/tests/virsh-self-test @@ -21,14 +21,25 @@
fail=0
-test_url=test:///default +basename=$(basename $0)
-test_intro "virsh-self-test" -$abs_top_builddir/tools/virsh -c $test_url self-test > /dev/null +if test "x$basename" = "xvirsh-self-test" ; then + binary=virsh + extra_args="-c test:///default" +elif test "x$basename" = "xvirt-admin-self-test" ; then + binary=virt-admin + extra_args="" +else + echo "Unknown binary: $basename"; + exit 1 +fi + +test_intro "$0" +$abs_top_builddir/tools/${binary} ${extra_args} self-test > /dev/null status=$? -test_result 1 "virsh-self-test" $status +test_result 1 "$0" ${status}
-if test "$status" != "0" ; then +if test "${status}" != "0" ; then fail=1 fi
diff --git a/tests/virt-admin-self-test b/tests/virt-admin-self-test new file mode 120000
Although a working solution, we would end up with two identical scripts.
Well, the virt-admin-self-test is really just a symlink to virsh-self-test (which can handle the case if run under different name).
Sigh... never mind, ACK.
Thank you, I've pushed these. Michal
participants (2)
-
Erik Skultety -
Michal Privoznik