[virt-tools-list] [virt-manager PATCH] test: fix a virt-clone test cases

[Cole Robinson]

On 12/03/2014 12:20 AM, Chen, Hanxiao wrote:
>
>
>> -----Original Message-----
>> From: Cole Robinson [mailto:crobinso at redhat.com]
>> Sent: Tuesday, December 02, 2014 11:00 PM
>> To: Chen, Hanxiao/陈 晗霄; virt-tools-list at redhat.com
>> Subject: Re: [virt-tools-list] [virt-manager PATCH] test: fix a virt-clone test
>> cases
>>
>> On 12/01/2014 10:39 PM, Chen Hanxiao wrote:
>>> remove '--force option' in test case virtclone0283
>>>
>>> Signed-off-by: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
>>> ---
>>>    tests/clitest.py | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tests/clitest.py b/tests/clitest.py
>>> index 6b3e45d..5c9d635 100644
>>> --- a/tests/clitest.py
>>> +++ b/tests/clitest.py
>>> @@ -916,7 +916,7 @@ c.add_invalid("--original-xml %(CLONE_DISK_XML)s
>> --file %(NEWCLONEIMG1)s --file
>>>    c.add_invalid("--original-xml %(CLONE_STORAGE_XML)s --file /tmp/clonevol")
>> # XML w/ managed storage, specify unmanaged path (should fail)
>>>    c.add_invalid("--original-xml %(CLONE_NOEXIST_XML)s --file %(EXISTIMG1)s")
>> # XML w/ non-existent storage, WITHOUT --preserve
>>>    c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file %(ROIMG)s
>> --file %(ROIMG)s --force")  # XML w/ managed storage, specify RO image without
>> preserve
>>> -c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file %(ROIMG)s
>> --file %(ROIMGNOEXIST)s --force")  # XML w/ managed storage, specify RO non
>> existent
>>> +c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file %(ROIMG)s
>> --file %(ROIMGNOEXIST)s")  # XML w/ managed storage, specify RO non existent
>>>
>>>
>>>
>>>
>>
>> Can you explain exactly what's wrong here?
>>
>
> testCLIvirtclone0283:
>
> When I run test suites, I got a NG case:
> ./virt-clone --debug --connect __virtinst_test__test:////home/virt-manager/tests/testdriver.xml,predictable -n clonetest --original-xml tests/cli-test-xml/clone-disk.xml --file /tmp/__virtinst_cli_clitest_rodir/cli_exist3ro.img --file /tmp/__virtinst_cli_clitest_rodir/idontexist.img --force
>
> In clone-disk.xml, we got 2 disk device and a CD-ROM to clone.
> For output, we specified two files, one of them not existed.
>
> --force will set force option.
>
> then we could see:
> DEBUG (diskbackend:149) Path '/tmp/__virtinst_cli_clitest_rodir' is target for pool '__virtinst_cli_clitest_rodir-3'. Creating volume 'cli_exist3ro.img'.
> [Wed, 03 Dec 2014 11:57:29 virt-clone 21637] DEBUG (cli:321) --force skipping error condition 'This will overwrite the existing path '/tmp/__virtinst_cli_clitest_rodir/cli_exist3ro.img''
> [Wed, 03 Dec 2014 11:57:29 virt-clone 21637] WARNING (cli:322) This will overwrite the existing path '/tmp/__virtinst_cli_clitest_rodir/cli_exist3ro.img'
> [Wed, 03 Dec 2014 11:57:29 virt-clone 21637] DEBUG (cli:321) --force skipping error condition 'This will overwrite the existing path '/tmp/__virtinst_cli_clitest_rodir/cli_exist3ro.img''
> [Wed, 03 Dec 2014 11:57:29 virt-clone 21637] WARNING (cli:322) This will overwrite the existing path '/tmp/__virtinst_cli_clitest_rodir/cli_exist3ro.img'
>
> without --force, this case will fail, as add_invalid expected.
>
> Does this case be designed to test this?
>
> As Giuseppe's comments, I think we should remove both '--force' option.
>
> diff --git a/tests/clitest.py b/tests/clitest.py
> index 6b3e45d..759b472 100644
> --- a/tests/clitest.py
> +++ b/tests/clitest.py
> @@ -915,8 +915,8 @@ c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file virt-install --file %(EX
>   c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file %(NEWCLONEIMG1)s --file %(NEWCLONEIMG2)s --fo
>   c.add_invalid("--original-xml %(CLONE_STORAGE_XML)s --file /tmp/clonevol")  # XML w/ managed storage,
>   c.add_invalid("--original-xml %(CLONE_NOEXIST_XML)s --file %(EXISTIMG1)s")  # XML w/ non-existent sto
> -c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file %(ROIMG)s --file %(ROIMG)s --force")  # XML w
> -c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file %(ROIMG)s --file %(ROIMGNOEXIST)s --force")
> +c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file %(ROIMG)s --file %(ROIMG)s")  # XML w/ manage
> +c.add_invalid("--original-xml %(CLONE_DISK_XML)s --file %(ROIMG)s --file %(ROIMGNOEXIST)s")  # XML w/
>

Change the second instance to c.add_valid, the test suite gives me:

$ python setup.py test --only testCLIvirtclone0283
...
RuntimeError: Error cloning diskimage /tmp/__virtinst_cli_exist1.img to 
/tmp/__virtinst_cli_clitest_rodir/cli_exist3ro.img: [Errno 13] Permission 
denied: '/tmp/__virtinst_cli_clitest_rodir/cli_exist3ro.img'

Which is the error condition we are trying to test for (clone choking on 
trying to copy to a read-only image)

If I then remove --force, it then errors with:

$ python setup.py test --only testCLIvirtclone0283
...
[Fri, 05 Dec 2014 09:24:46 virt-clone 13894] ERROR (cli:263) This will 
overwrite the existing path 
'/tmp/__virtinst_cli_clitest_rodir/cli_exist3ro.img' (Use --force to override)

Which shows that --force is required to test the read-only scenario. So I'm 
still not sure what the point of your patch is, the test suite is running fine 
for me at the moment. Are you seeing failures without your patch?

- Cole