Francesco Romani has posted comments on this change. Change subject: virt: Make BIOS messages available on vmconsole ......................................................................
Patch Set 4: Code-Review+1 (3 comments) codewise looks ok, tests could be improved. https://gerrit.ovirt.org/#/c/48404/4/tests/vmTests.py File tests/vmTests.py: Line 1407: self.assertEqual(res, response.error("thawErr", Line 1408: message="fake error")) Line 1409: Line 1410: Line 1411: class UseSerialTests(TestCaseBase): I'd rename 'SerialBiosTests'. Maybe put it alongside the other xml tests on this module? Perhaps next to console device tests? Line 1412: Line 1413: def testUseSerial(self): Line 1414: test_cases = (('serial', True), Line 1415: ('virtio', False),) Line 1411: class UseSerialTests(TestCaseBase): Line 1412: Line 1413: def testUseSerial(self): Line 1414: test_cases = (('serial', True), Line 1415: ('virtio', False),) please use permutations, there are many examples under tests/* Line 1416: for console_type, use_serial in test_cases: Line 1417: devices = {'device': 'console', 'type': 'console', Line 1418: 'specParams': {'consoleType': console_type}}, Line 1419: with fake.VM(devices=devices, create_device_objects=True) as v: Line 1417: devices = {'device': 'console', 'type': 'console', Line 1418: 'specParams': {'consoleType': console_type}}, Line 1419: with fake.VM(devices=devices, create_device_objects=True) as v: Line 1420: dom_xml = v._buildDomainXML() Line 1421: self.assertEqual('<bios useserial="yes"/>' in dom_xml, please use smarter approach than string check, using ElementTree facilities. Line 1422: use_serial) Line 1423: Line 1424: Line 1425: def _load_xml(name): -- To view, visit https://gerrit.ovirt.org/48404 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifc560d2562a913b7dba9a5c01952429459595973 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
