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

[Erik Skultety]

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 at 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
>