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