Nir Soffer has posted comments on this change.

Change subject: tests: v2v: add test for commit f8127d8
......................................................................


Patch Set 5:

(2 comments)

https://gerrit.ovirt.org/#/c/47367/5/tests/v2vTests.py
File tests/v2vTests.py:

Line 177:             self._assertVmDisksMatchSpec(vm, spec)
Line 178: 
Line 179:     def testGetExternalVMsWithXMLDescFailure(self):
Line 180:         if not v2v.supported():
Line 181:             raise SkipTest('v2v is not supported current os version')
Do we really need to test this code only on systems where v2v is supported? 
This should test our code, which should work anywhere.
Line 182: 
Line 183:         def _connect(uri, username, passwd):
Line 184:             vms = [VmMock(*spec) for spec in self._VM_SPECS]
Line 185: 


Line 197:         self.assertEqual(len(vms), 2)
Line 198: 
Line 199:         for vm, spec in zip(vms, self._VM_SPECS):
Line 200:             if spec.name == "RHEL_1":
Line 201:                 continue
This works but it is not very clear. We monkeypatch vms[1] so we don't get xml 
for it. The test should validate that we don't get vm[1] in the results, not vm 
with name "RHEL_1".

How about removing the spec in the same place we monkey patch the vm?

    specs = self._VM_SPECS[:]

    # Cause vm 1 to fail, so it would not appear in the results
    vms[1].XMLDesc = internal_error
    del specs[1]
Line 202:             self._assertVmMatchesSpec(vm, spec)
Line 203:             self._assertVmDisksMatchSpec(vm, spec)
Line 204: 
Line 205:     def testOutputParser(self):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I38bd3c06df263bc208e1a8c8aa6c0081ebdc218d
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Shahar Havivi <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to