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

[Cole Robinson]

On 12/09/2014 07:07 AM, Cole Robinson wrote:
> On 12/09/2014 12:39 AM, Chen, Hanxiao wrote:
>>
>>
>>> -----Original Message-----
>>> From: Cole Robinson [mailto:crobinso at redhat.com]
>>> Sent: Friday, December 05, 2014 10:26 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/03/2014 12:20 AM, Chen, Hanxiao wrote:
>>>>
>> [snip]
>>> # 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?
>>
>> I re-test this and found that:
>> a) If run that test by root:
>>      testCLIvirtclone0283 and testCLIvirtclone0284 failed.
>>     root's privileges will override the RO scenario.
>>     But works find w/o --force.
>>
>> b) If run as non-root user:
>>      works fine with/without --force options for 283 and 284.
>>
>> Do we need to emphasis that test suite should be run as non-root user?
>
> Running the test suite as root is definitely not a good idea, just as running
> any random build system as root as scary, there could be a bug that hoses your
> system. I don't think we need to explicitly document it.
>
> I'll just drop this test, the RO handling was originally to catch some
> explicit validation we had in virtinst that is now long gone, so I don't think
> it really adds anything.
>

Pushed now

- Cole