Francesco Romani has posted comments on this change.
Change subject: virt: add test to allow custom XML parsing test ......................................................................
Patch Set 2:
(3 comments)
nice, my concerns are mostly about the commit message which seems stale, and one very minor nit inside about variable naming.
IMHO, it would be overly awesome if we can just drop an (uniquely named) xml file under, let's say, tests/devices/data, and have some tests automatically pick it up, maybe using glob* or something to build permutations.
But I don't know if: - other developers share this vision - it is OK or even feasible given out current infrastructure.
https://gerrit.ovirt.org/#/c/39909/2//COMMIT_MSG Commit Message:
Line 6: Line 7: virt: add test to allow custom XML parsing test Line 8: Line 9: Tests should run against explicit device specification, but it may be Line 10: useful to have a means of simply testing that parse of given XML "of a given XML"? Can you please reword the sentence, anyway? I'm not a native english speaker, but I believe it could be made easier to parse (:)). Line 11: succeeds. This patch implements such functionality, allowing user to Line 12: supply XML in tests/devices/data and modify Line 13: tests/parsing/custom_vm_tests.py to see if the parsing conforms general Line 14: device specification.
Line 10: useful to have a means of simply testing that parse of given XML Line 11: succeeds. This patch implements such functionality, allowing user to Line 12: supply XML in tests/devices/data and modify Line 13: tests/parsing/custom_vm_tests.py to see if the parsing conforms general Line 14: device specification. but in the patch you added xml_tests.py.
Perhaps is the commit message out of date? Line 15: Line 16: Change-Id: I3bb24e2854e6f7b93c5108eca8b6f79f17353e85
https://gerrit.ovirt.org/#/c/39909/2/tests/devices/parsing/xml_tests.py File tests/devices/parsing/xml_tests.py:
Line 20: devices = [{'device': 'spice', 'type': 'graphics'}] Line 21: Line 22: test_path = os.path.realpath(__file__) Line 23: dir_name = os.path.split(test_path)[0] Line 24: api_path = os.path.join( why "api"? Line 25: dir_name, '..', 'data', domain_xml) Line 26: Line 27: domain = None Line 28: with open(api_path, 'r') as domxml: