Adam Litke has posted comments on this change. Change subject: tests: Add a live merge functional test ......................................................................
Patch Set 1: (3 comments) http://gerrit.ovirt.org/#/c/29824/1/tests/functional/utils.py File tests/functional/utils.py: Line 238: result = self.vdscli.getVolumeInfo(sdId, spId, imgId, volId) Line 239: return result['status']['code'], result['status']['message'],\ Line 240: result['info'] Line 241: Line 242: def createVolume(self, sdId, spId, imgId, size, volFormat, preallocate, > OK Nir, I agree. Yeah, personally I do not like this proxy at all. I don't think it adds any value at all. But I am using it because someone did like it this way and apparently some reviewers did at the time too. I'd like to see it refactored and simplified once the cli switches to jsonRPC where we can also start verifying that the data returned actually conforms to the documented API schema. Another battle for another day. Line 243: diskType, volId, desc, baseImgId, baseVolId): Line 244: result = self.vdscli.createVolume(sdId, spId, imgId, size, volFormat, Line 245: preallocate, diskType, volId, desc, Line 246: baseImgId, baseVolId) http://gerrit.ovirt.org/#/c/29824/1/tests/functional/virtTests.py File tests/functional/virtTests.py: Line 240: Line 241: @requireKVM Line 242: @permutations([['localfs'], ['iscsi'], ['nfs']]) Line 243: def testVmWithStorage(self, backendType): Line 244: disk = storageTests.StorageTest() > Yea, this is a bit extreme. The common part should move to utils instead of Agreed. I just built on what other tests in this file are currently doing. In a future revision I will try to factor it out a bit better. Line 245: disk.setUp() Line 246: conf = storageTests.storageLayouts[backendType] Line 247: drives = disk.generateDriveConf(conf) Line 248: customization = {'vmId': '88888888-eeee-ffff-aaaa-111111111111', Line 552: jobId) Line 553: jobIds.append(jobId) Line 554: self._waitBlockJobs(vmId, jobIds) Line 555: actual = self._getVolumeChains(vmId) Line 556: self.assertEquals(chains, actual) > No one is going to mock VDSM. My point is that this doesn't really check wh I agree that we should actually confirm that the operations have done what we expect by using qemu-io. I just didn't get that far during the hackathon when I created this test. -- To view, visit http://gerrit.ovirt.org/29824 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idd5a2f7eedaef9e90981256de66fc3ed21658e89 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Yoav Kleinberger <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
