My git send-email is misbehaving, attaching the patch mail...
-- Chris Evich, RHCA, RHCE, RHCDS, RHCSS Quality Assurance Engineer e-mail: cevich + `@' + redhat.com o: 1-888-RED-HAT1 x44214
From: Chris Evich <[email protected]> Date: Thu, 30 Aug 2012 14:46:58 +0000 Subject: [PATCH] client: xml_utils cleanup / enhancements Cc: Chris Evich <[email protected]> + ElementTree's remove method only operates on an element, and normally it is a child element you need to remove. Add remove() to XMLTreeFile that utilizes a method for locating the parent element. Signed-off-by: Yu Mingfei<[email protected]> + The automatic removal of temporary files behavior was inconsistent between various classes. Special methods moved into base-class to unify behavior. + Unit tests updated to confirm exceptional exits retain specially-named backup files for debugging. Normal exits auto-delete, and unittests fail if auto-delete doesn't happen as expected. + TemplateXML class, __init__ method is very sensitive to XMLTreeFile __init__behavior. Cleaned up comments to make this more clear. + Added XMLTreeFile.backup_copy() method to make producing additional temporary backups easy. Added unittests that confirm backup files behave correctly. + Added XMLTreeFile.remove_by_xpath() method to enhance Yu's remove method and make removing elements by name/path convenient. Signed-off-by: Chris Evich <[email protected]> --- client/shared/xml_utils.py | 129 +++++++++++++++++++++++----------- client/shared/xml_utils_unittest.py | 76 ++++++++++++++++---- 2 files changed, 148 insertions(+), 57 deletions(-) diff --git a/client/shared/xml_utils.py b/client/shared/xml_utils.py index 966ce4e..2caf0df 100644 --- a/client/shared/xml_utils.py +++ b/client/shared/xml_utils.py @@ -47,9 +47,10 @@ import logging from autotest.client.shared import ElementTree -# Used by unittests +# Also used by unittests TMPPFX='xml_utils_temp_' TMPSFX='.xml' +EXSFX='_exception_retained' class TempXMLFile(file): """ @@ -70,24 +71,42 @@ class TempXMLFile(file): super(TempXMLFile, self).__init__(path, mode, buffer) - def __exit__(self, exc_type, exc_value, traceback): + def _info(self): """ - Always remove temporary file on module exit. + 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) - self.__del__() - super(TempXMLFile, self).__exit__(exc_type, exc_value, traceback) - - def __del__(self): + def unlink(self): """ - Remove temporary file on instance delete. + Unconditionaly delete file, ignoring related exceptions """ - try: os.unlink(self.name) - except OSError: - pass # don't care + self.close() + except (OSError, IOError): + pass # don't care if delete fails + + + def __exit__(self, exc_type, exc_value, traceback): + """ + unlink temporary backup on unexceptional module exit. + """ + + # there was an exception + if None not in (exc_type, exc_value, traceback): + os.rename(self.name, self.name + EXSFX) + self.unlink() # safe if file was renamed + + + def __del__(self): + """ + unlink temporary file on instance delete. + """ + + self.unlink() class XMLBackup(TempXMLFile): @@ -108,6 +127,11 @@ class XMLBackup(TempXMLFile): self.backup() + def __del__(self): + self.sourcefilename = None + super(XMLBackup, self).__del__() + + def backup(self): """ Overwrite temporary backup with contents of original source. @@ -130,30 +154,6 @@ class XMLBackup(TempXMLFile): self.seek(0) - def _info(self): - logging.info("Retaining backup of %s in %s", self.sourcefilename, - self.name) - - - def __exit__(self, exc_type, exc_value, traceback): - """ - Remove temporary backup on unexceptional module exit. - """ - - if exc_type is None and exc_value is None and traceback is None: - super(XMLBackup, self).__del__() - else: - self._info() - - - def __del__(self): - """ - Remove temporary file on instance delete. - """ - - self._info() - - class XMLTreeFile(ElementTree.ElementTree, XMLBackup): """ Combination of ElementTree root and auto-cleaned XML backup file. @@ -174,22 +174,67 @@ class XMLTreeFile(ElementTree.ElementTree, XMLBackup): # If it's a string, use auto-delete TempXMLFile # to hold the original content. try: + # Test if xml is a valid filename self.sourcebackupfile = open(xml, "rb") - # Prevent source modification self.sourcebackupfile.close() except (IOError, OSError): - # this will auto-delete when XMLBase instance goes out of scope + # Assume xml is a string that needs a temporary source file self.sourcebackupfile = TempXMLFile() self.sourcebackupfile.write(xml) - # Prevent source modification self.sourcebackupfile.close() - xml = self.sourcebackupfile.name - XMLBackup.__init__(self, sourcefilename=xml) + # sourcebackupfile now safe to use for base class initialization + XMLBackup.__init__(self, self.sourcebackupfile.name) ElementTree.ElementTree.__init__(self, element=None, file=self.name) - # Ensure parsed content matches file content + # Required for TemplateXML class to work self.write() - self.flush() + self.flush() # make sure it's on-disk + + + def backup_copy(self): + """Return a copy of instance, including copies of files""" + return self.__class__(self.name) + + + def get_parent_map(self, element=None): + """ + Return a child to parent mapping dictionary + + param: element: Search only below this element + """ + # Comprehension loop over all children in all parents + return dict((c, p) for p in self.getiterator(element) for c in p) + + + def get_parent(self, element, relative_root=None): + """ + Return the parent node of an element or None + + param: element: Element to retrieve parent of + param: relative_root: Search only below this element + """ + try: + return self.get_parent_map(relative_root)[element] + except KeyError: + return None + + + def remove(self, element): + """ + Removes a matching subelement. + + @param: element: element to be removed. + """ + self.get_parent(element).remove(element) + + + def remove_by_xpath(self, xpath): + """ + Remove an element found by xpath + + @param: xpath: element name or path to remove + """ + self.remove(self.find(xpath)) def write(self, filename=None, encoding="UTF-8"): diff --git a/client/shared/xml_utils_unittest.py b/client/shared/xml_utils_unittest.py index 58a03d5..05ffd97 100755 --- a/client/shared/xml_utils_unittest.py +++ b/client/shared/xml_utils_unittest.py @@ -1,6 +1,6 @@ #!/usr/bin/python -import unittest, tempfile, os, glob +import unittest, tempfile, os, glob, logging try: import autotest.common as common @@ -12,7 +12,17 @@ from autotest.client.shared import xml_utils, ElementTree class xml_test_data(unittest.TestCase): + def get_tmp_files(self, prefix, sufix): + path_string = os.path.join('/tmp', "%s*%s" % (prefix, sufix)) + return glob.glob(path_string) + def setUp(self): + # Previous testing may have failed / left behind extra files + for filename in self.get_tmp_files(xml_utils.TMPPFX, xml_utils.TMPSFX): + os.unlink(filename) + for filename in self.get_tmp_files(xml_utils.TMPPFX, + xml_utils.TMPSFX + xml_utils.EXSFX): + os.unlink(filename) # Compacted to save excess scrolling self.TEXT_REPLACE_KEY="TEST_XML_TEXT_REPLACE" self.XMLSTR="""<?xml version='1.0' encoding='UTF-8'?><capabilities><host> @@ -45,11 +55,10 @@ class xml_test_data(unittest.TestCase): def tearDown(self): - for filename in glob.glob(os.path.join('/tmp', "%s*%s" % - (xml_utils.TMPPFX, xml_utils.TMPSFX) - )): - os.unlink(filename) - + os.unlink(self.XMLFILE) + leftovers = self.get_tmp_files(xml_utils.TMPPFX, xml_utils.TMPSFX) + if len(leftovers) > 0: + self.fail('Leftover files: %s' % str(leftovers)) def canonicalize_test_xml(self): et = ElementTree.parse(self.XMLFILE) @@ -102,10 +111,20 @@ class test_XMLBackup(xml_test_data): class_to_test = xml_utils.XMLBackup - def is_same_contents(self, filename): - f = file(filename, "rb") - s = f.read() - return s == self.XMLSTR + + 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): @@ -149,9 +168,7 @@ class test_XMLBackup(xml_test_data): tmpf = self.class_to_test(self.XMLFILE) return tmpf.name filename = out_of_scope_xmlbackup() - # DOES NOT delete - self.assertTrue(self.is_same_contents(filename)) - os.unlink(filename) + self.assertRaises(OSError, os.unlink, filename) def test_TempXMLBackup_exception_exit(self): @@ -159,8 +176,8 @@ class test_XMLBackup(xml_test_data): filename = tmpf.name # simulate exception exit DOES NOT DELETE tmpf.__exit__(Exception, "foo", "bar") - self.assertTrue(self.is_same_contents(filename)) - os.unlink(filename) + self.assertTrue(self.is_same_contents(filename + xml_utils.EXSFX)) + os.unlink(filename + xml_utils.EXSFX) def test_TempXMLBackup_unexception_exit(self): @@ -175,6 +192,16 @@ class test_XMLTreeFile(test_XMLBackup): class_to_test = xml_utils.XMLTreeFile + def test_sourcebackupfile_closed_file(self): + xml = self.class_to_test(self.XMLFILE) + self.assertRaises(ValueError, xml.sourcebackupfile.write, 'foobar') + + + def test_sourcebackupfile_closed_string(self): + xml = self.class_to_test(self.XMLSTR) + self.assertRaises(ValueError, xml.sourcebackupfile.write, 'foobar') + + def test_init_str(self): xml = self.class_to_test(self.XMLSTR) self.assert_(xml.sourcefilename is not None) @@ -203,6 +230,25 @@ class test_XMLTreeFile(test_XMLBackup): self.assertTrue(self.is_same_contents(xmlbackup.name)) + def test_backup_backup_and_remove(self): + tmpf = self.class_to_test(self.XMLFILE) + tmps = self.class_to_test(self.XMLSTR) + bu_tmpf = tmpf.backup_copy() + bu_tmps = tmps.backup_copy() + self.assertTrue(self.is_same_contents(bu_tmpf.name, tmpf.name)) + self.assertTrue(self.is_same_contents(bu_tmps.name, tmps.name)) + tmpf.remove_by_xpath('guest/arch/wordsize') + tmps.find('guest/arch/wordsize').text = 'FOOBAR' + tmpf.write() + tmps.write() + self.assertFalse(self.is_same_contents(bu_tmpf.name, tmpf.name)) + 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): xmlbackup = self.class_to_test(self.XMLFILE) wordsize = xmlbackup.find('guest/arch/wordsize') -- 1.7.4.1
_______________________________________________ Autotest-kernel mailing list [email protected] https://www.redhat.com/mailman/listinfo/autotest-kernel
