Change in vdsm[master]: tests: negative tests for updateDevice - graphics

mpolednik at redhat.com mpolednik at redhat.com
Mon Apr 27 13:10:21 UTC 2015


Martin Polednik has posted comments on this change.

Change subject: tests: negative tests for updateDevice - graphics
......................................................................


Patch Set 9: Code-Review+1

(3 comments)

Please reconsider pep8 attribute/variable naming - I would really like to move this patch to the package bootstrapped in https://gerrit.ovirt.org/#/c/39364/ (possibly going with tests/devices/graphics/current.py and tests/devices/graphics/legacy.py).

Everything noted is very minor and not a blocker, only stylistic upgrade in my opnion.

https://gerrit.ovirt.org/#/c/38665/9/tests/vmTests.py
File tests/vmTests.py:

Line 998: 
Line 999:         with MonkeyPatchScope([(hooks, 'before_vm_set_ticket',
Line 1000:                                 _check_ticket_params)]):
Line 1001:             params = {'graphicsType': device_type}
Line 1002:             params.update(_GRAPHICS_DEVICE_PARAMS)
Very minor: updating the device specification looks a bit nicer and verbose, you could possibly swap it to use be

params = _GRAPHICS_DEVICE_PARAMS
params.update({'graphicsType': device_type})
Line 1003:             return testvm.updateDevice(params)
Line 1004: 
Line 1005:     def _verifyDeviceUpdate(self, device, allDevices, domXml, devXml):
Line 1006:         with fake.VM(devices=allDevices) as testvm:


Line 1112: 
Line 1113:             def _fail(*args):
Line 1114:                 raise vm.TimeoutError(defmsg=message)
Line 1115: 
Line 1116:             dom = fake.Domain(domXml)
I would really like to go with domain in future (sorry for noticing it that late, it is just a bit more clear and nicer)
Line 1117:             dom.updateDeviceFlags = _fail
Line 1118:             testvm._dom = dom
Line 1119: 
Line 1120:             res = self._updateGraphicsDevice(testvm, device)


Line 1113:             def _fail(*args):
Line 1114:                 raise vm.TimeoutError(defmsg=message)
Line 1115: 
Line 1116:             dom = fake.Domain(domXml)
Line 1117:             dom.updateDeviceFlags = _fail
Lambda would look nicer here
Line 1118:             testvm._dom = dom
Line 1119: 
Line 1120:             res = self._updateGraphicsDevice(testvm, device)
Line 1121: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf3d03f125f9c53339d9ad7fef8c31482bab0bfb
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list