On 10/03/2012 11:47 AM, Chris Evich wrote:
+ Cleaned up many pylint/style warnings
+ Added unique extension for temp files retained due to exception
+ Unified del/exit-handling behavior
+ Added XMLTreeFile method for retrieving an XPATH to an element
+ Improved unittests
Good stuff! Two minor comments:
Signed-off-by: Chris Evich <[email protected]>
---
client/shared/xml_utils.py | 83 +++++++++++++++++++++++++++--------
client/shared/xml_utils_unittest.py | 52 ++++++++++++++--------
client/tests | 2 +-
3 files changed, 98 insertions(+), 39 deletions(-)
diff --git a/client/shared/xml_utils.py b/client/shared/xml_utils.py
index 2caf0df..44c5bb7 100644
--- a/client/shared/xml_utils.py
+++ b/client/shared/xml_utils.py
@@ -48,35 +48,35 @@ import logging
from autotest.client.shared import ElementTree
# Also used by unittests
-TMPPFX='xml_utils_temp_'
-TMPSFX='.xml'
-EXSFX='_exception_retained'
+TMPPFX = 'xml_utils_temp_'
+TMPSFX = '.xml'
+EXSFX = '_exception_retained'
+ENCODING = "UTF-8"
class TempXMLFile(file):
"""
Temporary XML file auto-removed on instance del / module exit.
"""
- def __init__(self, suffix=TMPSFX, prefix=TMPPFX, mode="wb+", buffer=1):
+ def __init__(self, suffix=TMPSFX, prefix=TMPPFX, mode="wb+", buffsz=1):
"""
Initialize temporary XML file removed on instance destruction.
param: suffix: temporary file's suffix
param: prefix: temporary file's prefix
- param: mode: file access mode
- param: buffer: size of buffer in bytes, 1: line buffered
+ param: mode: second parameter to file()/open()
+ param: buffer: third parameter to file()/open()
"""
- fd,path = tempfile.mkstemp(suffix=suffix, prefix=prefix)
+ fd, path = tempfile.mkstemp(suffix=suffix, prefix=prefix)
os.close(fd)
- super(TempXMLFile, self).__init__(path, mode, buffer)
+ super(TempXMLFile, self).__init__(path, mode, buffsz)
def _info(self):
"""
Inform user that file was not auto-deleted due to exceptional exit.
"""
- logging.info("Retaining backup of %s in %s", self.sourcefilename,
- self.name + EXSFX)
+ logging.info("Retaining %s", self.name + EXSFX)
def unlink(self):
@@ -105,7 +105,7 @@ class TempXMLFile(file):
"""
unlink temporary file on instance delete.
"""
-
+ self.close()
self.unlink()
@@ -128,10 +128,19 @@ class XMLBackup(TempXMLFile):
def __del__(self):
+ # Drop reference, don't delete source!
self.sourcefilename = None
super(XMLBackup, self).__del__()
+ def _info(self):
+ """
+ Inform user that file was not auto-deleted due to exceptional exit.
+ """
+ logging.info("Retaining backup of %s in %s", self.sourcefilename,
+ self.name + EXSFX)
^ For one, the info message here is inconsistent with the definition of
_info above, so better to make them equal.
+
def backup(self):
"""
Overwrite temporary backup with contents of original source.
@@ -139,7 +148,8 @@ class XMLBackup(TempXMLFile):
self.flush()
self.seek(0)
- shutil.copyfileobj(file(self.sourcefilename, "rb"),
super(XMLBackup,self))
+ shutil.copyfileobj(file(self.sourcefilename, "rb"),
+ super(XMLBackup, self))
self.seek(0)
@@ -150,7 +160,8 @@ class XMLBackup(TempXMLFile):
self.flush()
self.seek(0)
- shutil.copyfileobj(super(XMLBackup,self), file(self.sourcefilename,
"wb+"))
+ shutil.copyfileobj(super(XMLBackup, self),
+ file(self.sourcefilename, "wb+"))
self.seek(0)
@@ -175,8 +186,9 @@ class XMLTreeFile(ElementTree.ElementTree, XMLBackup):
# to hold the original content.
try:
# Test if xml is a valid filename
- self.sourcebackupfile = open(xml, "rb")
+ self.sourcebackupfile = file(xml, "rb")
self.sourcebackupfile.close()
+ # XMLBackup init will take care of creating a copy
except (IOError, OSError):
# Assume xml is a string that needs a temporary source file
self.sourcebackupfile = TempXMLFile()
@@ -191,6 +203,11 @@ class XMLTreeFile(ElementTree.ElementTree, XMLBackup):
self.flush() # make sure it's on-disk
+ def __str__(self):
+ self.write()
+ self.flush()
+ return self.sourcebackupfile.read()
+
def backup_copy(self):
"""Return a copy of instance, including copies of files"""
return self.__class__(self.name)
@@ -219,6 +236,30 @@ class XMLTreeFile(ElementTree.ElementTree, XMLBackup):
return None
+ def get_xpath(self, element):
+ """Return the XPath string formed from first-match tag names"""
+ parent_map = self.get_parent_map()
+ root = self.getroot()
+ assert root in parent_map.values()
+ if element == root:
+ return '.'
+ # List of strings reversed at end
+ path_list = []
+ while element != root:
+ # 2.7+ ElementPath supports predicates, so:
+ # element_index = list(parent_map[element]).index(element)
+ # element_index += 1 # XPath indexes are 1 based
+ # if element_index > 1:
+ # path_list.append(u"%s[%d]" % (element.tag, element_index))
+ # else:
+ # path_list.append(u"%s" % element.tag)
+ path_list.append(u"%s" % element.tag)
+ element = parent_map[element]
We've got to at least remove the code commented above.
+ assert element == root
+ path_list.reverse()
+ return "/".join(path_list)
+
+
def remove(self, element):
"""
Removes a matching subelement.
@@ -234,16 +275,18 @@ class XMLTreeFile(ElementTree.ElementTree, XMLBackup):
@param: xpath: element name or path to remove
"""
- self.remove(self.find(xpath))
+ self.remove(self.find(xpath)) # can't remove root
- def write(self, filename=None, encoding="UTF-8"):
+ # This overrides the file.write() method
+ def write(self, filename=None, encoding=ENCODING):
"""
Write current XML tree to filename, or self.name if None.
"""
if filename is None:
filename = self.name
+ # Avoid calling file.write() by mistake
ElementTree.ElementTree.write(self, filename, encoding)
@@ -310,15 +353,17 @@ class TemplateXML(XMLTreeFile):
# XMLBase.__init__ calls self.write() after super init
- def parse(self, source):
+ def parse(self, source, parser=None):
"""
Parse source XML file or filename using TemplateXMLTreeBuilder
@param: source: XML file or filename
@param: parser: ignored
"""
-
- return super(TemplateXML, self).parse(source, self.parser)
+ if parser is None:
+ return super(TemplateXML, self).parse(source, self.parser)
+ else:
+ return super(TemplateXML, self).parse(source, parser)
def restore(self):
diff --git a/client/shared/xml_utils_unittest.py
b/client/shared/xml_utils_unittest.py
index 05ffd97..ebaa141 100755
--- a/client/shared/xml_utils_unittest.py
+++ b/client/shared/xml_utils_unittest.py
@@ -60,6 +60,7 @@ class xml_test_data(unittest.TestCase):
if len(leftovers) > 0:
self.fail('Leftover files: %s' % str(leftovers))
+
def canonicalize_test_xml(self):
et = ElementTree.parse(self.XMLFILE)
et.write(self.XMLFILE, encoding="UTF-8")
@@ -68,6 +69,22 @@ class xml_test_data(unittest.TestCase):
f.close()
+ def is_same_contents(self, filename, other=None):
+ """Compare filename contents with XMLSTR, or contents of other"""
+ try:
+ f = file(filename, "rb")
+ s = f.read()
+ except (IOError, OSError):
+ logging.warning("File %s does not exist" % filename)
+ return False
+ if other is None:
+ return s == self.XMLSTR
+ else:
+ other_f = file(other, "rb")
+ other_s = other_f.read()
+ return s == other_s
+
+
class test_ElementTree(xml_test_data):
def test_bundled_elementtree(self):
@@ -88,7 +105,6 @@ class test_TempXMLFile(xml_test_data):
tmpf.seek(0)
stuff = tmpf.read()
self.assertEqual(stuff, self.XMLSTR)
- del tmpf
def test_TempXMLFile_implicit(self):
@@ -111,22 +127,6 @@ class test_XMLBackup(xml_test_data):
class_to_test = xml_utils.XMLBackup
-
- def is_same_contents(self, filename, other=None):
- try:
- f = file(filename, "rb")
- s = f.read()
- except (IOError, OSError):
- logging.warning("File %s does not exist" % filename)
- return False
- if other is None:
- return s == self.XMLSTR
- else:
- other_f = file(other, "rb")
- other_s = other_f.read()
- return s == other_s
-
-
def test_backup_filename(self):
xmlbackup = self.class_to_test(self.XMLFILE)
self.assertEqual(xmlbackup.sourcefilename, self.XMLFILE)
@@ -245,8 +245,6 @@ class test_XMLTreeFile(test_XMLBackup):
self.assertFalse(self.is_same_contents(bu_tmps.name, tmps.name))
self.assertTrue(self.is_same_contents(bu_tmpf.name, bu_tmps.name))
self.assertFalse(self.is_same_contents(tmpf.name, tmps.name))
- del bu_tmpf
- del bu_tmps
def test_write_default(self):
@@ -297,6 +295,22 @@ class test_XMLTreeFile(test_XMLBackup):
self.assertTrue(self.is_same_contents(otherfile.name))
+ def get_xpath_elements(self, target_path_string):
+ xmlbackup = self.class_to_test(self.XMLSTR)
+ target_element = xmlbackup.find(target_path_string)
+ test_path_string = xmlbackup.get_xpath(target_element)
+ test_element = xmlbackup.find(test_path_string)
+ return (target_element, test_element)
+
+
+ def test_get_xpath(self):
+ # 2.6 ElementPath doesn't support predicates as in 2.7 :(
+ # (it blindly returns the first match)
+ self.assertEqual(*self.get_xpath_elements('guest/arch/wordsize'))
+ self.assertEqual(*self.get_xpath_elements('guest/arch/machine'))
+ self.assertEqual(*self.get_xpath_elements('host/cpu/arch'))
+
+
class test_templatized_xml(xml_test_data):
def setUp(self):
diff --git a/client/tests b/client/tests
index 4ebba12..de7e51b 160000
--- a/client/tests
+++ b/client/tests
@@ -1 +1 @@
-Subproject commit 4ebba1264bdcddfe738c109e3fde5395278ba31f
+Subproject commit de7e51b6d917f4d30214eb9845f542c0f6a60b5f
_______________________________________________
Autotest-kernel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/autotest-kernel
