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 <alitke(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Yoav Kleinberger <ykleinbe(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes