Francesco Romani has posted comments on this change.

Change subject: caps: tests: add (more) unittests for caps.py
......................................................................


Patch Set 13:

(3 comments)

I don't care that much on list vs tuple in test, and I reluctantly agree to 
ship a copy of cpu_map.xml

A few nits inside, but it's already good enough.

https://gerrit.ovirt.org/#/c/41077/13/tests/capsTests.py
File tests/capsTests.py:

Line 41: 
Line 42: 
Line 43: class TestCaps(TestCaseBase):
Line 44: 
Line 45:     cpu_map_file = 
os.path.join(os.path.dirname(os.path.realpath(__file__)),
If this is a 'constant',
LETS_NAME_IT_AS_SUCH
Line 46:                                 'cpu_map.xml')
Line 47: 
Line 48:     def tearDown(self):
Line 49:         for name in dir(caps):


Line 222:     def test_getAllCpuModels(self):
Line 223:         result = caps._getAllCpuModels(capfile=self.cpu_map_file,
Line 224:                                        arch=caps.Architecture.X86_64)
Line 225: 
Line 226:         expected = {'qemu32': None,
thanks for reformatting.
Let's walk the extra mile:

  expected = {
    'qemu32: None,

    ...

    'kvm64': None
  }
Line 227:                     'Haswell': 'Intel',
Line 228:                     'cpu64-rhel6': None,
Line 229:                     'cpu64-rhel5': None,
Line 230:                     'Broadwell': 'Intel',


https://gerrit.ovirt.org/#/c/41077/13/vdsm/caps.py
File vdsm/caps.py:

Line 391:                     if m.nodeName == 'machine']
Line 392:     return []
Line 393: 
Line 394: 
Line 395: def _getAllCpuModels(capfile='/usr/share/libvirt/cpu_map.xml', 
arch=None):
please use a module constant
Line 396:     with open(capfile) as xml:
Line 397:         cpu_map = minidom.parseString(xml.read())
Line 398: 
Line 399:     if arch is None:


-- 
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: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Marcin Mirecki <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to