From 443e3ef90b6bfece9617d0b2ff580fa4e2fbc6a5 Mon Sep 17 00:00:00 2001 From: Marcus Huewe Date: Wed, 3 Jun 2020 21:06:26 +0200 Subject: [PATCH] tests: Ignore the ordering of attributes in XML documents Old xml.etree.cElementTree versions (python2) reorder the attributes while recent xml.etree.cElementTree versions (python3) keep the document order. Example: python3: >>> ET.tostring(ET.fromstring('')) b'' >>> python2: >>> ET.tostring(ET.fromstring('')) '' >>> So far, the testsuite compared two serialized XML documents via a simple string comparison. For instance via, self.assertEqual(actual_serialized_xml, expected_serialized_xml) where the expected_serialized_xml is, for instance, a hardcoded str. Obviously, this would only work for python2 or python3. In order to support both python versions, we first parse both XML documents and then compare the corresponding trees (this is OK because we do not care about comments etc.). A related issue is the way how the testsuite compares data that is "send" to the API. So far, this was a plain bytes comparison. Again, this won't work in case of XML documents (see above). Moreover, we have currently no notion to "indicate" that the transmitted data is an XML document. As a workaround, we keep the plain bytes comparison and in case it fails, we try an xml comparison (see above) as a last resort. Strictly speaking, this is "wrong" (there might be cases (in the future) where we want to ensure that the transmitted XML data is bit identical to a fixture file) but a reasonable comprise for now. Fixes: #751 ("[python3.8] Testsuite fails") --- tests/common.py | 50 ++++++++++++++++++++++++++++++++++++++++--- tests/test_request.py | 36 +++++++++++++++---------------- 2 files changed, 65 insertions(+), 21 deletions(-) diff --git a/tests/common.py b/tests/common.py index 16715006..5521c412 100644 --- a/tests/common.py +++ b/tests/common.py @@ -42,6 +42,30 @@ def urlcompare(url, *args): return True + +def xml_equal(actual, exp): + try: + actual_xml = ET.fromstring(actual) + exp_xml = ET.fromstring(exp) + except ET.ParseError: + return False + todo = [(actual_xml, exp_xml)] + while todo: + actual_xml, exp_xml = todo.pop(0) + if actual_xml.tag != exp_xml.tag: + return False + if actual_xml.attrib != exp_xml.attrib: + return False + if actual_xml.text != exp_xml.text: + return False + if actual_xml.tail != exp_xml.tail: + return False + if len(actual_xml) != len(exp_xml): + return False + todo.extend(list(zip(actual_xml, exp_xml))) + return True + + class RequestWrongOrder(Exception): """raised if an unexpected request is issued to urllib2""" def __init__(self, url, exp_url, method, exp_method): @@ -98,7 +122,14 @@ class MyHTTPHandler(HTTPHandler): if hasattr(data, 'read'): data = data.read() if data != exp: - raise RequestDataMismatch(req.get_full_url(), repr(data), repr(exp)) + # We do not have a notion to explicitly mark xml content. In case + # of xml, we do not care about the exact xml representation (for + # now). Hence, if both, data and exp, are xml and are "equal", + # everything is fine (for now); otherwise, error out + # (of course, this is problematic if we want to ensure that XML + # documents are bit identical...) + if not xml_equal(data, exp): + raise RequestDataMismatch(req.get_full_url(), repr(data), repr(exp)) return self.__get_response(req.get_full_url(), **kwargs) def __get_response(self, url, **kwargs): @@ -192,14 +223,27 @@ class OscTestCase(unittest.TestCase): def _check_digests(self, fname, *skipfiles): fname = os.path.join(self._get_fixtures_dir(), fname) - self.assertEqual(open(os.path.join('.osc', '_files'), 'r').read(), open(fname, 'r').read()) - root = ET.parse(fname).getroot() + with open(os.path.join('.osc', '_files'), 'r') as f: + files_act = f.read() + with open(fname, 'r') as f: + files_exp = f.read() + self.assertXMLEqual(files_act, files_exp) + root = ET.fromstring(files_act) for i in root.findall('entry'): if i.get('name') in skipfiles: continue self.assertTrue(os.path.exists(os.path.join('.osc', i.get('name')))) self.assertEqual(osc.core.dgst(os.path.join('.osc', i.get('name'))), i.get('md5')) + def assertXMLEqual(self, act, exp): + if xml_equal(act, exp): + return + # ok, xmls are different, hence, assertEqual is expected to fail + # (we just use it in order to get a "nice" error message) + self.assertEqual(act, exp) + # not reached (unless assertEqual is overridden in an incompatible way) + raise RuntimeError('assertEqual assumptions violated') + def assertEqualMultiline(self, got, exp): if (got + exp).find('\n') == -1: self.assertEqual(got, exp) diff --git a/tests/test_request.py b/tests/test_request.py index 96b5ba40..aa05aab7 100644 --- a/tests/test_request.py +++ b/tests/test_request.py @@ -38,7 +38,7 @@ class TestRequest(OscTestCase): """ - self.assertEqual(exp, r.to_str()) + self.assertXMLEqual(exp, r.to_str()) def test_createsr_with_option(self): """create a submitrequest with option""" @@ -67,7 +67,7 @@ class TestRequest(OscTestCase): """ - self.assertEqual(exp, r.to_str()) + self.assertXMLEqual(exp, r.to_str()) def test_createsr_missing_tgt_package(self): """create a submitrequest with missing target package""" @@ -88,7 +88,7 @@ class TestRequest(OscTestCase): """ - self.assertEqual(exp, r.to_str()) + self.assertXMLEqual(exp, r.to_str()) def test_createsr_invalid_argument(self): """create a submitrequest with invalid action argument""" @@ -117,7 +117,7 @@ class TestRequest(OscTestCase): """ - self.assertEqual(exp, r.to_str()) + self.assertXMLEqual(exp, r.to_str()) def test_create_add_role_group(self): """create an add_role request (group element)""" @@ -138,7 +138,7 @@ class TestRequest(OscTestCase): """ - self.assertEqual(exp, r.to_str()) + self.assertXMLEqual(exp, r.to_str()) def test_create_add_role_person_group(self): """create an add_role request (person+group element)""" @@ -161,7 +161,7 @@ class TestRequest(OscTestCase): """ - self.assertEqual(exp, r.to_str()) + self.assertXMLEqual(exp, r.to_str()) def test_create_set_bugowner_project(self): """create a set_bugowner request for a project""" @@ -179,7 +179,7 @@ class TestRequest(OscTestCase): """ - self.assertEqual(exp, r.to_str()) + self.assertXMLEqual(exp, r.to_str()) def test_create_set_bugowner_package(self): """create a set_bugowner request for a package""" @@ -197,7 +197,7 @@ class TestRequest(OscTestCase): """ - self.assertEqual(exp, r.to_str()) + self.assertXMLEqual(exp, r.to_str()) def test_create_delete_project(self): """create a delete request for a project""" @@ -213,7 +213,7 @@ class TestRequest(OscTestCase): """ - self.assertEqual(exp, r.to_str()) + self.assertXMLEqual(exp, r.to_str()) def test_create_delete_package(self): """create a delete request for a package""" @@ -229,7 +229,7 @@ class TestRequest(OscTestCase): """ - self.assertEqual(exp, r.to_str()) + self.assertXMLEqual(exp, r.to_str()) def test_create_change_devel(self): """create a change devel request""" @@ -248,7 +248,7 @@ class TestRequest(OscTestCase): """ - self.assertEqual(exp, r.to_str()) + self.assertXMLEqual(exp, r.to_str()) def test_action_from_xml1(self): """create action from xml""" @@ -266,7 +266,7 @@ class TestRequest(OscTestCase): self.assertEqual(action.person_role, 'reader') self.assertEqual(action.group_name, 'group') self.assertEqual(action.group_role, 'reviewer') - self.assertEqual(xml, action.to_str()) + self.assertXMLEqual(xml, action.to_str()) def test_action_from_xml2(self): """create action from xml""" @@ -288,7 +288,7 @@ class TestRequest(OscTestCase): self.assertEqual(action.opt_sourceupdate, 'cleanup') self.assertEqual(action.opt_updatelink, '1') self.assertTrue(action.src_rev is None) - self.assertEqual(xml, action.to_str()) + self.assertXMLEqual(xml, action.to_str()) def test_action_from_xml3(self): """create action from xml (with acceptinfo element)""" @@ -312,7 +312,7 @@ class TestRequest(OscTestCase): self.assertEqual(action.acceptinfo_xsrcmd5, 'ffffffffffffffffffffffffffffffff') self.assertTrue(action.acceptinfo_osrcmd5 is None) self.assertTrue(action.acceptinfo_oxsrcmd5 is None) - self.assertEqual(xml, action.to_str()) + self.assertXMLEqual(xml, action.to_str()) def test_action_from_xml_unknown_type(self): """try to create action from xml with unknown type""" @@ -350,7 +350,7 @@ class TestRequest(OscTestCase): self.assertEqual(r.description, 'this is a\nvery long\ndescription') self.assertTrue(len(r.statehistory) == 1) self.assertTrue(len(r.reviews) == 0) - self.assertEqual(xml, r.to_str()) + self.assertXMLEqual(xml, r.to_str()) def test_read_request2(self): """read in a request (with reviews)""" @@ -389,7 +389,7 @@ class TestRequest(OscTestCase): self.assertEqual(r.creator, 'creator') self.assertTrue(len(r.statehistory) == 1) self.assertTrue(len(r.reviews) == 1) - self.assertEqual(xml, r.to_str()) + self.assertXMLEqual(xml, r.to_str()) def test_read_request3(self): """read in a request (with an "empty" comment+description)""" @@ -426,7 +426,7 @@ class TestRequest(OscTestCase): """ - self.assertEqual(exp, r.to_str()) + self.assertXMLEqual(exp, r.to_str()) def test_request_list_view1(self): """test the list_view method""" @@ -559,7 +559,7 @@ Comment: """ """ - self.assertEqual(exp, r.to_str()) + self.assertXMLEqual(exp, r.to_str()) def test_get_actions(self): """test get_actions method"""