Francesco Romani has uploaded a new change for review.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
tests: v2v: add test for commit fb4c72a
In commit fb4c72a we made sure to continue to extract VM information if the call to get allocation and capacity failed. This patch adds one test to make it unlikely to break this again.
Change-Id: Iad523dc7c34276c6811513220cd7230ca5fc6e45 Signed-off-by: Francesco Romani fromani@redhat.com --- M tests/v2vTests.py 1 file changed, 45 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/64/47364/1
diff --git a/tests/v2vTests.py b/tests/v2vTests.py index d793bf9..cd79427 100644 --- a/tests/v2vTests.py +++ b/tests/v2vTests.py @@ -19,6 +19,7 @@
from StringIO import StringIO
+import libvirt
import v2v from vdsm import libvirtconnection @@ -27,7 +28,9 @@
from nose.plugins.skip import SkipTest from testlib import VdsmTestCase as TestCaseBase -from monkeypatch import MonkeyPatch +from monkeypatch import MonkeyPatch, MonkeyPatchScope + +import vmfakelib as fake
class VmMock(object): @@ -73,6 +76,10 @@
# FIXME: extend vmfakelib allowing to set predefined domain in Connection class class LibvirtMock(object): + + def __init__(self, storage_lookup_fail=False): + self._storage_lookup_fail = storage_lookup_fail + def close(self): pass
@@ -80,6 +87,9 @@ return [VmMock()]
def storageVolLookupByPath(self, name): + if self._storage_lookup_fail: + e = fake.Error(libvirt.VIR_ERR_INTERNAL_ERROR) + raise e return LibvirtMock.Volume()
class Volume(object): @@ -140,9 +150,13 @@ self.assertEquals(vm['smp'], 1) self.assertEquals(len(vm['disks']), 1) self.assertEquals(len(vm['networks']), 1) + disk = vm['disks'][0] self.assertEquals(disk['dev'], 'sda') self.assertEquals(disk['alias'], '[datastore1] RHEL/RHEL.vmdk') + self.assertIn('capacity', disk) + self.assertIn('allocation', disk) + network = vm['networks'][0] self.assertEquals(network['type'], 'bridge') self.assertEquals(network['macAddr'], '00:0c:29:c6:a6:11') @@ -193,3 +207,33 @@ self.assertEquals(network['model'], 'E1000') self.assertEquals(network['type'], 'bridge') self.assertEquals(network['dev'], 'Ethernet 1') + + def testGetExternalVMsWithoutDisksInfo(self): + if not v2v.supported(): + raise SkipTest('v2v is not supported current os version') + + def _connect(uri, username, passwd): + return LibvirtMock(storage_lookup_fail=True) + + with MonkeyPatchScope([(libvirtconnection, 'open_connection', + _connect)]): + vms = v2v.get_external_vms('esx://mydomain', 'user', + ProtectedPassword('password'))['vmList'] + self.assertEquals(len(vms), 1) + vm = vms[0] + self.assertEquals(vm['vmId'], '564d7cb4-8e3d-06ec-ce82-7b2b13c6a611') + self.assertEquals(vm['memSize'], 2048) + self.assertEquals(vm['smp'], 1) + + self.assertEquals(len(vm['disks']), 1) + disk = vm['disks'][0] + self.assertEquals(disk['dev'], 'sda') + self.assertEquals(disk['alias'], '[datastore1] RHEL/RHEL.vmdk') + self.assertNotIn('capacity', disk) + self.assertNotIn('allocation', disk) + + self.assertEquals(len(vm['networks']), 1) + network = vm['networks'][0] + self.assertEquals(network['type'], 'bridge') + self.assertEquals(network['macAddr'], '00:0c:29:c6:a6:11') + self.assertEquals(network['bridge'], 'VM Network')
automation@ovirt.org has posted comments on this change.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Francesco Romani has posted comments on this change.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
Patch Set 1: Verified+1
verified running the tests.
automation@ovirt.org has posted comments on this change.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
Patch Set 2:
* Update tracker::#1265556::OK * Check Bug-Url::OK * Check Public Bug::#1265556::OK, public bug * Check Product::#1265556::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Shahar Havivi has posted comments on this change.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
Patch Set 2: Code-Review+1
Nir Soffer has posted comments on this change.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
Nice, tiny cleanup would be even nicer.
https://gerrit.ovirt.org/#/c/47364/2/tests/v2vTests.py File tests/v2vTests.py:
Line 88: Line 89: def storageVolLookupByPath(self, name): Line 90: if self._storage_lookup_fail: Line 91: e = fake.Error(libvirt.VIR_ERR_INTERNAL_ERROR) Line 92: raise e raise fakeError(...)? Line 93: return LibvirtMock.Volume() Line 94: Line 95: class Volume(object): Line 96: def info(self):
Nir Soffer has posted comments on this change.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
Patch Set 2:
(2 comments)
Suggested way to simplify the mock.
https://gerrit.ovirt.org/#/c/47364/2/tests/v2vTests.py File tests/v2vTests.py:
Line 88: Line 89: def storageVolLookupByPath(self, name): Line 90: if self._storage_lookup_fail: Line 91: e = fake.Error(libvirt.VIR_ERR_INTERNAL_ERROR) Line 92: raise e
raise fakeError(...)?
Introducing flags such as _storage_lookup_fail is more complicated then needed. We can override this method in the specific test - see bellow. Line 93: return LibvirtMock.Volume() Line 94: Line 95: class Volume(object): Line 96: def info(self):
Line 212: if not v2v.supported(): Line 213: raise SkipTest('v2v is not supported current os version') Line 214: Line 215: def _connect(uri, username, passwd): Line 216: return LibvirtMock(storage_lookup_fail=True) We can do it in a simpler way:
connection = LibvirtMock() def internal_error(name): raise fake.Error(...) connection.storageVolLookupByPath = internal_error Line 217: Line 218: with MonkeyPatchScope([(libvirtconnection, 'open_connection', Line 219: _connect)]): Line 220: vms = v2v.get_external_vms('esx://mydomain', 'user',
automation@ovirt.org has posted comments on this change.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
Patch Set 3:
* Update tracker::#1265556::OK * Check Bug-Url::OK * Check Public Bug::#1265556::OK, public bug * Check Product::#1265556::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Francesco Romani has posted comments on this change.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
Patch Set 2:
(2 comments)
https://gerrit.ovirt.org/#/c/47364/2/tests/v2vTests.py File tests/v2vTests.py:
Line 88: Line 89: def storageVolLookupByPath(self, name): Line 90: if self._storage_lookup_fail: Line 91: e = fake.Error(libvirt.VIR_ERR_INTERNAL_ERROR) Line 92: raise e
Introducing flags such as _storage_lookup_fail is more complicated then
Done Line 93: return LibvirtMock.Volume() Line 94: Line 95: class Volume(object): Line 96: def info(self):
Line 212: if not v2v.supported(): Line 213: raise SkipTest('v2v is not supported current os version') Line 214: Line 215: def _connect(uri, username, passwd): Line 216: return LibvirtMock(storage_lookup_fail=True)
We can do it in a simpler way:
yes, this is way nicer and simpler. Will switch to this idiom. Line 217: Line 218: with MonkeyPatchScope([(libvirtconnection, 'open_connection', Line 219: _connect)]): Line 220: vms = v2v.get_external_vms('esx://mydomain', 'user',
automation@ovirt.org has posted comments on this change.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
Patch Set 4:
* Update tracker::#1265556::OK * Check Bug-Url::OK * Check Public Bug::#1265556::OK, public bug * Check Product::#1265556::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
Patch Set 5:
* Update tracker::#1265556::OK * Check Bug-Url::OK * Check Public Bug::#1265556::OK, public bug * Check Product::#1265556::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Nir Soffer has posted comments on this change.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
Patch Set 5: Code-Review+1
Shahar Havivi has posted comments on this change.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
Patch Set 5: Code-Review+1
Francesco Romani has posted comments on this change.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
Patch Set 5: Verified+1
verified running the tests.
automation@ovirt.org has posted comments on this change.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
Patch Set 6:
* Update tracker::#1265556::OK * Check Bug-Url::OK * Check Public Bug::#1265556::OK, public bug * Check Product::#1265556::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Dan Kenigsberg has posted comments on this change.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
Patch Set 6: Code-Review+2
raising score
Dan Kenigsberg has submitted this change and it was merged.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
tests: v2v: add test for commit fb4c72a
In commit fb4c72a we made sure to continue to extract VM information if the call to get allocation and capacity failed. This patch adds one test to make it unlikely to break this again.
Change-Id: Iad523dc7c34276c6811513220cd7230ca5fc6e45 Bug-Url: https://bugzilla.redhat.com/1265556 Signed-off-by: Francesco Romani fromani@redhat.com Reviewed-on: https://gerrit.ovirt.org/47364 Reviewed-by: Nir Soffer nsoffer@redhat.com Reviewed-by: Shahar Havivi shavivi@redhat.com Continuous-Integration: Jenkins CI Reviewed-by: Dan Kenigsberg danken@redhat.com --- M tests/v2vTests.py 1 file changed, 45 insertions(+), 1 deletion(-)
Approvals: Nir Soffer: Looks good to me, but someone else must approve Shahar Havivi: Looks good to me, but someone else must approve Jenkins CI: Passed CI tests Dan Kenigsberg: Looks good to me, approved Francesco Romani: Verified
automation@ovirt.org has posted comments on this change.
Change subject: tests: v2v: add test for commit fb4c72a ......................................................................
Patch Set 7:
* Update tracker::#1265556::OK * Set MODIFIED::bug 1265556::::#1265556::::IGNORE, not oVirt prod but ovirt-engine
vdsm-patches@lists.fedorahosted.org