Re: [libvirt] [PATCH 2/2] tests: Self test virt-admin

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(a)redhat.com&gt; >> --- >> .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