Change in vdsm[master]: virt: add test to allow custom XML parsing test

fromani at redhat.com fromani at redhat.com
Thu Apr 16 13:14:06 UTC 2015


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:


-- 
To view, visit https://gerrit.ovirt.org/39909
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3bb24e2854e6f7b93c5108eca8b6f79f17353e85
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list