Yoav Kleinberger has posted comments on this change.
Change subject: tests: Add a live merge functional test
......................................................................
Patch Set 1: Code-Review-1
(3 comments)
http://gerrit.ovirt.org/#/c/29824/1/tests/functional/utils.py
File tests/functional/utils.py:
Line 237: def getVolumeInfo(self, sdId, spId, imgId, volId):
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:
quite a bit of code duplication. Usually I don't like metaprogramming, but perhaps
introduce something like this:
class VDSMMethod:
_vdsm = VDSCli()
def __init__(self, returnKey):
self._returnKey = returnKey
def __call__(self, func):
this = self
def _decorated(self, *args, **kwargs):
methodName = func.__name__
method = getattr(this._vdsm, methodName)
result = method(*args, **kwargs)
return result['status']['code'],
result['status']['message'], result[this._returnKey]
return _decorated
Now use this in VDSProxy
class VDSProxy(object):
@VDSMMethod(returnKey='uuid')
def createVolume(self): pass
@VDSMMethod(returnKey='uuid')
def deleteVolume(self): pass
@VDSMMethod(returnKey='vmList')
def list(self): pass
Line 242: def createVolume(self, sdId, spId, imgId, size, volFormat, preallocate,
Line 243: diskType, volId, desc, baseImgId, baseVolId):
Line 244: result = self.vdscli.createVolume(sdId, spId, imgId, size, volFormat,
Line 245: preallocate, diskType, volId, desc,
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()
why is a StorageTest instance called a "disk"?
looks to me quite convoluted, using a test-case from a different test as a library for
another test.
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)
Looks like you assert that VDSM reports the result you expect. If this is true, this test
does not actually check that anything happened - only that VDSM says it did.
In principle, I could shutdown VDSM, and install a fake server that tells this test what
it wants to hear - and the test will pass, with no VDSM even running. Am I wrong?
--
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: 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