Noissue - Make comparable_xml safe against ambiguous tags

tests/test_packagerWebExt.py

 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import difflib
 import json
 import os
 import re
 import shutil
 import zipfile
@@ skipping 222 matching lines @@
     files['ext/c.js'] = 'var this_is_c;'
 
     tmpdir.mkdir('lib').join('b.js').write(files['lib/b.js'])
     ext_dir = tmpdir.mkdir('ext')
     ext_dir.join('a.js').write(files['ext/a.js'])
     ext_dir.join('c.js').write(files['ext/c.js'])
 
     return files
 
 
-def comparable_xml(xml):
-    """Create a nonambiguous representation of a given XML tree.
-
-    Note that this function is not safe against ambiguous tags
-    containing differently distributed children, e.g.:
-
-    '<a><b><c/></b><b><d/></b></a>'
-    vs.
-    '<a><b/><b><c/><d/></b></a>'
-
-    This is irrelevant for comparing the XML used by the tests of this
-    module.
-    """
-    def get_leafs_string(tree):
-        """Recursively build a string representing all xml leaf-nodes."""
-        root_str = '{}|{}|{}'.format(tree.tag, tree.tail, tree.text).strip()
-        result = []
-
-        if len(tree) > 0:
-            for subtree in tree:
-                for leaf in get_leafs_string(subtree):
-                    result.append('{}__{}'.format(root_str, leaf))
-        for k, v in tree.attrib.items():
-            result.append('{}__{}:{}'.format(root_str, k, v))
-        else:
-            result.append(root_str)
-        return result
-
-    # XML data itself shall not be sorted, hence we can safely sort
-    # our string representations in order to produce a valid unified diff.
-    return sorted(get_leafs_string(ElementTree.fromstring(xml)))
-
-
 def comparable_json(json_string):
     """Create a nonambiguous representation of a given JSON string."""
     return json.dumps(
         json.loads(json_string), sort_keys=True, indent=0
     ).split('\n')
 
 
+def comparable_xml(xml):
+    """Create a nonambiguous representation of a given XML tree."""
+    def strip(s):
+        if s is None:
+            return ''
+        return s.strip()
+
+    def transform(elt):
+        subs = sorted(transform(s) for s in elt)
+        attrs = tuple('{}={}'.format(k, v) for k, v in elt.attrib.items())

[Vasily Kuznetsov, 2017/10/23 12:48:20]

Since you're doing a `comparable_json` later, we can just use `elt.attrib`
instead of this tuple comprehension thingy and `comparable_json` will handle
those dicts just fine, right?

[tlucas, 2017/10/23 13:21:36]

Good point - Done.

+        return (elt.tag, strip(elt.tail), strip(elt.text), attrs, subs)
+
+    return comparable_json(json.dumps(transform(ElementTree.fromstring(xml))))

[Vasily Kuznetsov, 2017/10/23 12:48:20]

Its kind of a pity that we're doing `json.dumps` here just to immediately
`json.loads` it in `comparable_json` but I guess it's just a few extra CPU
cycles so who cares.

[tlucas, 2017/10/23 13:21:36]

No, you are right - dumps(loads(dumps())) is definitely kind of ugly - Done.

+
+
 def assert_manifest_content(manifest, expected_path):
     extension = os.path.basename(expected_path).split('.')[-1]
 
     with open(expected_path, 'r') as fp:
         if extension == 'xml':
             generated = comparable_xml(manifest)
             expected = comparable_xml(fp.read())
         else:
             generated = comparable_json(manifest)
             expected = comparable_json(fp.read())
@@ skipping 183 matching lines @@
             expected = os.path.join(
                 os.path.dirname(__file__),
                 'expecteddata',
                 filename.format(name, ext),
             )
 
             assert_manifest_content(
                 package.read(os.path.join(folder, '{}.{}'.format(name, ext))),
                 expected,
             )