Dan Kenigsberg has posted comments on this change. Change subject: Adding unittests for caps.py ......................................................................
Patch Set 4: (5 comments) https://gerrit.ovirt.org/#/c/41077/4/tests/capsTests.py File tests/capsTests.py: Line 198: def testGetLiveSnapshotSupport(self): Line 199: capsData = self._readCaps("caps_libvirt_intel_i73770_nosnap.out") Line 200: Line 201: result = caps._getLiveSnapshotSupport('i686', capsData) Line 202: self.assertEqual(result, None) assertIsNone is a bit nicer Line 203: Line 204: result = caps._getLiveSnapshotSupport(caps.Architecture.X86_64, Line 205: capsData) Line 206: self.assertEqual(result, False) Line 202: self.assertEqual(result, None) Line 203: Line 204: result = caps._getLiveSnapshotSupport(caps.Architecture.X86_64, Line 205: capsData) Line 206: self.assertEqual(result, False) and here, assertFalse give a nicer output on failure. Line 207: Line 208: capsData = self._readCaps("caps_libvirt_intel_i73770.out") Line 209: Line 210: result = caps._getLiveSnapshotSupport('i686', capsData) Line 216: Line 217: def testGetAllCpuModels(self): Line 218: fileName = '/usr/share/libvirt/cpu_map.xml' Line 219: result = caps._getAllCpuModels(capfile=fileName, Line 220: arch='x86') x86 is a bit confusing as it is not one of caps.Architecture. can you explain why? Line 221: Line 222: expected = {'qemu32': None, 'Haswell': 'Intel', 'cpu64-rhel6': None, Line 223: 'cpu64-rhel5': None, 'Broadwell': 'Intel', 'pentium2': Line 224: None, 'pentiumpro': None, 'athlon': 'AMD', 'Nehalem': Line 228: 'core2duo': 'Intel', 'Penryn': 'Intel', 'qemu64': None, Line 229: 'phenom': 'AMD', 'Opteron_G2': 'AMD', '486': None, Line 230: 'Westmere': 'Intel', 'pentium3': None, 'n270': 'Intel', Line 231: 'SandyBridge': 'Intel', 'kvm64': None} Line 232: self.assertEqual(expected, result) this test is bound to break when libvirt adds more CPU to their map. why do you use the absolute path to libvirt, and not the file that you've added to the tests? Line 233: Line 234: def testGetAllCpuModels_noArch(self): Line 235: fileName = '/usr/share/libvirt/cpu_map.xml' Line 236: result = caps._getAllCpuModels(capfile=fileName, https://gerrit.ovirt.org/#/c/41077/4/tests/cpu_map.xml File tests/cpu_map.xml: Line 1: <cpus> please add an xml comment telling from where this files was taken. Line 2: <arch name='x86'> Line 3: <!-- vendor definitions --> Line 4: <vendor name='Intel' string='GenuineIntel'/> Line 5: <vendor name='AMD' string='AuthenticAMD'/> -- To view, visit https://gerrit.ovirt.org/41077 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I86c2203bf245ba209e10b0905ae1090835408201 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Marcin Mirecki <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
