Dan Kenigsberg has posted comments on this change.
Change subject: test: Add test cases for domxml generation ......................................................................
Patch Set 1: I would prefer that you didn't submit this
(2 inline comments)
Wow, thanks for this much-awaited tests. I feel bad about these untestable appendXXX functions that you somehow managed to test...
I think that a cleaner approach would have been to have a list of [(conf, expectedXML)...] instead, and call testDomXML(conf, xml) to iterate over them.
What is your opinion? Except for the side-effects, I think this test can be taken as it is, even while it digs into the ugly implementation of libvirtvm._DomXML().
.................................................... File tests/libvirtvmTests.py Line 39: def assertXML(self, element, expectedXML, tagName=None, attrName=None): Line 40: if tagName is None: Line 41: converted = element.toprettyxml() Line 42: else: Line 43: for node in element.getElementsByTagName(tagName): just an idea - how about using xpath instead? Line 44: # Use attribute to tell a specific elements from others Line 45: # with the same tag name. Line 46: if not attrName or attrName and node.hasAttribute(attrName): Line 47: converted = node.toprettyxml()
Line 371: 'name': 'vda', 'format': 'cow', 'device': 'disk', Line 372: 'path': '/tmp/disk1.img', 'type': 'disk', 'readonly': 'False', Line 373: 'shared': 'True', 'serial': SERIAL} Line 374: Line 375: self.conf['custom'] = {'viodiskcache': 'writethrough'} I think it is bad practice to have a test case with side-effects. We should never assume a specific order in which they run. Line 376: qcow2Virtio = libvirtvm.Drive(self.conf, self.log, **dev) Line 377: # Patch Drive.blockDev to skip the block device checking. Line 378: qcow2Virtio._blockDev = False
-- To view, visit http://gerrit.ovirt.org/8544 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I1b0f9956084df8cec81f93a1a95944437e00c3e4 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com