Change in vdsm[master]: tests: add more tests for custom error responses
fromani at redhat.com
fromani at redhat.com
Thu Mar 19 14:32:00 UTC 2015
Francesco Romani has posted comments on this change.
Change subject: tests: add more tests for custom error responses
......................................................................
Patch Set 4:
(4 comments)
https://gerrit.ovirt.org/#/c/38665/4/tests/vmTests.py
File tests/vmTests.py:
Line 897: {'type': 'graphics', 'device': 'vnc', 'port': '-1'}]
Line 898:
Line 899: TICKET_PARAMS = {
Line 900: 'userName': 'admin',
Line 901: 'userId': 'fdfc627c-d875-11e0-90f0-83df133b58cc'}
> Have you considered moving this to top of the module? Did it look better/wo
No good reason to be "public", I can rename it.
A mildly valid reason to keep them here in this class and not in the module is to reduce the noise, but it is not a compelling reason.
Line 902:
Line 903: @MonkeyPatch(libvirtconnection, 'get', lambda x: fake.Connection())
Line 904: @permutations([[define.NORMAL], [define.ERROR]])
Line 905: def testTimeOffsetNotPresentByDefault(self, exitCode):
Line 984: 'graphicsType': graphicsType,
Line 985: 'password': '***',
Line 986: 'ttl': 0,
Line 987: 'existingConnAction': 'disconnect',
Line 988: 'params': self.TICKET_PARAMS}
> It would be nicer to have pep8 name and have this as a property imho (in a
good point.
Line 989:
Line 990: def _verifyDeviceUpdate(self, device, allDevices, domXml, devXml):
Line 991: def _check_ticket_params(domXML, conf, params):
Line 992: self.assertEqual(params, self.TICKET_PARAMS)
Line 1050:
Line 1051: @permutations([[libvirt.VIR_ERR_OPERATION_DENIED, 'setNumberOfCpusErr',
Line 1052: 'Failed to set the number of cpus'],
Line 1053: [libvirt.VIR_ERR_NO_DOMAIN, 'noVM', None]])
Line 1054: def testSetNumberOfVcpusFailed(self, virtError, vdsmError, errorMessage):
> Lots of non-pep8 naming :(
Yes, and only because of sloppiness. Will fix.
Line 1055: with MonkeyPatchScope([(hooks, 'before_set_num_of_cpus',
Line 1056: lambda: None)]):
Line 1057: with fake.VM() as testvm:
Line 1058: testvm._dom = fake.Domain(virtError=virtError,
Line 1062:
Line 1063: self.assertEqual(res, response.error(vdsmError))
Line 1064:
Line 1065: def testUpdateDeviceGraphicsFailed(self):
Line 1066: def _check_ticket_params(domXML, conf, params):
> This is repeeating approach in _verifyDeviceUpdate and here, I'd say either
Will factor out.
Line 1067: pass
Line 1068:
Line 1069: with MonkeyPatchScope([(hooks, 'before_vm_set_ticket',
Line 1070: _check_ticket_params)]):
--
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: 4
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