Nir Soffer has posted comments on this change. Change subject: tests: add more tests for custom error responses ......................................................................
Patch Set 5: Code-Review-1 (4 comments) Please separate the unrelated changes. https://gerrit.ovirt.org/#/c/38665/5/tests/vmTests.py File tests/vmTests.py: Line 69: Line 70: Line 71: _TICKET_PARAMS = { Line 72: 'userName': 'admin', Line 73: 'userId': 'fdfc627c-d875-11e0-90f0-83df133b58cc' Is this related to adding more tests for error? Line 74: } Line 75: Line 76: Line 77: class TestVm(XMLTestCase): Line 988: 'password': '***', Line 989: 'ttl': 0, Line 990: 'existingConnAction': 'disconnect', Line 991: 'params': _TICKET_PARAMS Line 992: } Why is this a property of the test class? Can be moved to module function near _TICKET_PARAMS. Line 993: Line 994: def _updateGraphicsDevice(self, testvm, device_type): Line 995: def _check_ticket_params(domXML, conf, params): Line 996: self.assertEqual(params, _TICKET_PARAMS) Line 1006: testvm._dom = fake.Domain(domXml) Line 1007: Line 1008: self._updateGraphicsDevice(testvm, device['device']) Line 1009: Line 1010: self.assertEquals(testvm._dom.devXml, devXml) How is this refactoring related to this patch? Line 1011: Line 1012: def testDomainNotRunningWithoutDomain(self): Line 1013: with fake.VM() as testvm: Line 1014: self.assertEqual(testvm._dom, None) https://gerrit.ovirt.org/#/c/38665/5/tests/vmfakelib.py File tests/vmfakelib.py: Line 109: def vcpusFlags(self, flags): Line 110: return -1 Line 111: Line 112: def setVcpusFlags(self, numberOfCpus, flags): Line 113: self._failIfRequested() A nicer way to have methods that allways fail is to do: class ... def _fail(self, *args, **kw): ... raise err setVcpusFlags = _fail Line 114: Line 115: def metadata(self, type, uri, flags): Line 116: self._failIfRequested() Line 117: -- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
