Change in vdsm[master]: v2v: add fake virt-v2v process for testing without virt-v2v

nsoffer at redhat.com nsoffer at redhat.com
Sun Nov 8 09:43:10 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: v2v: add fake virt-v2v process for testing without virt-v2v
......................................................................


Patch Set 4: Code-Review-1

(5 comments)

https://gerrit.ovirt.org/#/c/47738/4/tests/v2vTests.py
File tests/v2vTests.py:

Line 118: 
Line 119: 
Line 120: class FakeV2VCommand(object):
Line 121:     def __init__(self):
Line 122:         self.cmd = os.path.join(os.path.abspath('.'), 'fake-virt-v2v.py')
Use CommandPath class for this, specifying the path to fake-virt-v2v.py
Line 123: 
Line 124: 
Line 125: def hypervisorConnect(uri, username, passwd):
Line 126:     return LibvirtMock()


Line 293:                 'imageID': disk['imageID'],
Line 294:                 'path': '/rhev/data-center/mydomain/images',
Line 295:                 'volumeID': disk['volumeID']
Line 296:             }
Line 297:             self._prepared_volumes.append(drive)
You are duplicating ImportVm code here, touching internal implementation details. This make this test fragile and will make it hard to modify the code under test.

Better use fake irs object that does nothing in prepareImage and teardownImage:

    class FakeIRS(object):

        @recorded
        def prepareImage(self, domainID, poolID, imageID, volumeID):
            pass

        @recorded
        def teardownImage(self, domainID, poolID, imageID):
            pass

Now we can add 2 tests that for preparing and tearing down images during the import.
Line 298: 
Line 299:     @recorded
Line 300:     def teardown_volumes(self):
Line 301:         return 0


Line 314:                              'volumeID':
Line 315:                              '00000000-0000-0000-0000-000000000004'}]}
Line 316:         url = 'vpx://adminr%40vsphere@0.0.0.0/ovirt/0.0.0.0?no_verify=1'
Line 317:         ivm = v2v.ImportVm.from_libvirt(url, 'root', 'mypassword', vminfo,
Line 318:                                         self._id, None)
Pass FakeIRS instance here instead of None.


Line 315:                              '00000000-0000-0000-0000-000000000004'}]}
Line 316:         url = 'vpx://adminr%40vsphere@0.0.0.0/ovirt/0.0.0.0?no_verify=1'
Line 317:         ivm = v2v.ImportVm.from_libvirt(url, 'root', 'mypassword', vminfo,
Line 318:                                         self._id, None)
Line 319:         ivm._import()
> is that a test only for fake-virt-v2v?
Right, to be sure it is producing the expected output.

When we know it is good, we can use it to test v2v code.


Line 315:                              '00000000-0000-0000-0000-000000000004'}]}
Line 316:         url = 'vpx://adminr%40vsphere@0.0.0.0/ovirt/0.0.0.0?no_verify=1'
Line 317:         ivm = v2v.ImportVm.from_libvirt(url, 'root', 'mypassword', vminfo,
Line 318:                                         self._id, None)
Line 319:         ivm._import()
> As long as the v2v code is working this code will pass without exception.
OK, run import, check that the import was successful.  Would you name this testSuccessfulImport?


-- 
To view, visit https://gerrit.ovirt.org/47738
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If062a1136921af19a2ffcb4d147611bbc9cf5464
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list