Dan Kenigsberg has posted comments on this change.
Change subject: Added unit test libvirtvmTests.py:TestLibvirtvm.testBuildCmdLine
......................................................................
Patch Set 5: I would prefer that you didn't submit this
(6 inline comments)
(Partial review only). Thanks for your patch!
....................................................
File tests/libvirtvmTests.py
Line 21: import xml.etree.ElementTree as ET
Line 22: import libvirtvm
Line 23: from vdsm import constants
Line 24: from testrunner import VdsmTestCase as TestCaseBase
Line 25: import testValidation
Please order imports properly: st lib, site-packages, then vdsm-internal
Line 26: import copy
Line 27: import caps
Line 28: from vdsm import utils
Line 29: import ast
Line 387: # Patch Drive.blockDev to skip the block device checking.
Line 388: drive._blockDev = blockDev
Line 389: self.assertXML(drive.getXML(), xml % SERIAL)
Line 390:
Line 391: @testValidation.ValidateRunningAsRoot
Why do you need root? We should create XML as any user.
Line 392: def testBuildCmdLine(self):
Line 393: #Copy the data that needs to be restored for other tests
Line 394: tempConstantVdsmRun = copy.deepcopy(constants.P_VDSM_RUN)
Line 395:
Line 393: #Copy the data that needs to be restored for other tests
Line 394: tempConstantVdsmRun = copy.deepcopy(constants.P_VDSM_RUN)
Line 395:
Line 396: #Data that is collected by _BuildCmdLine
Line 397: osd = caps.osversion()
I would prefer to monkey-patch caps.osversion to return an expected output.
Line 398: SystemVersion = osd.get('version', '') + '-' +
osd.get('release', '')
Line 399: inputDict = {'hostUUID': utils.getHostUUID(),
Line 400: 'SystemVersion': SystemVersion}
Line 401: inputFileScenarios = './testBuildCmdLine.scenarios.in'
Line 396: #Data that is collected by _BuildCmdLine
Line 397: osd = caps.osversion()
Line 398: SystemVersion = osd.get('version', '') + '-' +
osd.get('release', '')
Line 399: inputDict = {'hostUUID': utils.getHostUUID(),
Line 400: 'SystemVersion': SystemVersion}
Why are you using ast eval instead of importing a python module?
Line 401: inputFileScenarios = './testBuildCmdLine.scenarios.in'
Line 402: inputFileOutputXml = './testBuildCmdLine.xml.in'
Line 403:
Line 404: #Fill the test data
Line 419:
Line 420: self.assertEqual(len(selfConfigsScenarios),
len(expectedXMLs),
Line 421: "TestScenario and expected XML arrays differ
in size")
Line 422:
Line 423: constants.P_VDSM_RUN = "./"
Please use the monkey patch module for such changes - it's safer.
Line 424:
Line 425: #Perform test
Line 426: for testNum in range(len(selfConfigsScenarios)):
Line 427: self.assertEqual(type(expectedXMLs[testNum]) is str,
Line 422:
Line 423: constants.P_VDSM_RUN = "./"
Line 424:
Line 425: #Perform test
Line 426: for testNum in range(len(selfConfigsScenarios)):
for x, y in zip(xs, ys)
is more pythonic.
Line 427: self.assertEqual(type(expectedXMLs[testNum]) is str,
Line 428: True, "Element at " + str(testNum) + "
from " +
Line 429: inputFileOutputXml + "is not a base
string")
Line 430:
--
To view, visit http://gerrit.ovirt.org/14111
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I74b898a6398a72608d7933009644703aa3f8d831
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maciej Lichon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Maciej Lichon <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
