Issue 5383 - Add tests for the Chrome and Firefox packagers

tests/tools.py

Index: tests/tools.py
diff --git a/tests/tools.py b/tests/tools.py
new file mode 100644
index 0000000000000000000000000000000000000000..7f3611ab911e6ed478a530deceeeadabeb53437a
--- /dev/null
+++ b/tests/tools.py
@@ -0,0 +1,206 @@
+# 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 os
+import shutil
+import zipfile
+import json
+
+from xml.etree import ElementTree
+
+from buildtools.packagerChrome import defaultLocale
+from buildtools.tests.conftest import MESSAGES_EN_US
+
+
+ALL_LANGUAGES = ['en_US', 'de', 'it']
+
+
+class Content(object):
+    """Base class for a unified ZipFile / Directory interface.
+
+    Base class for providing a unified context manager interface for
+    accessing files. This class is subclassed by ZipContent and DirContent,
+    which provide the additional methods "namelist()" and "read(path)".
+    """
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, exc_type, exc_value, exc_tb):
+        self._close()
+
+
+class ZipContent(Content):
+    """Provides a unified context manager for ZipFile access.
+
+    Inherits the context manager API from Content.
+    If desired, the specified ZipFile is deleted on exiting the manager.
+    """
+
+    def __init__(self, zip_path, delete_on_close=True):
+        """Constructor of ZipContent handling the file <zip_path>.
+
+        The parameter 'delete_on_close' causes the context manager to
+        delete the handled ZipFile (specified by zip_path) if set to
+        True (default).
+        """
+
+        self._zip_path = zip_path
+        self._zip_file = zipfile.ZipFile(zip_path)
+        self._delete_on_close = delete_on_close
+        super(ZipContent, self).__init__()
+
+    def _close(self):
+        self._zip_file.close()
+        if self._delete_on_close:
+            # if specified, delete the handled file
+            os.remove(self._zip_path)
+
+    def namelist(self):
+        return self._zip_file.namelist()
+
+    def read(self, path):
+        return self._zip_file.read(path)
+
+
+class DirContent(Content):
+    """Provides a unified context manager for directory access.
+
+    Inherits the context managet API from Content.
+    """
+
+    def __init__(self, path):
+        """Constructor of DirContent handling <path>.
+        """
+
+        self._path = path
+        super(DirContent, self).__init__()
+
+    def _close(self):
+        pass
+
+    def namelist(self):
+        """Generate a list of filenames, as expected from ZipFile.nameslist().
+        """
+
+        result = []
+        for parent, directories, files in os.walk(self._path):
+            for filename in files:
+                file_path = os.path.join(parent, filename)
+                rel_path = os.path.relpath(file_path, self._path)
+                result.append(rel_path)
+        return result
+
+    def read(self, path):
+        content = None
+        with open(os.path.join(self._path, path)) as fp:
+            content = fp.read()
+        return content
+
+
+def copy_metadata(filename, tmpdir):
+    """Copy the used metadata to the used temporary directory."""
+    path = os.path.join(os.path.dirname(__file__), filename)
+    destination = str(tmpdir.join(filename))
+    shutil.copy(path, destination)
+
+
+def run_webext_build(ext_type, build_type, srcdir, packager_module,
+                     keyfile=None):
+    """Run a build process
+
+    Run a desired build process without unintenionally invoking the
+    releaseAutomation.
+    """
+    from buildtools.build import processArgs
+    args = ['build.py', '-t', ext_type, build_type]
+
+    if keyfile:
+        args += ['-k', keyfile]
+
+    if not build_type == 'release':
+        processArgs(str(srcdir), args)
+    else:
+        # We don't want to accidentaly run releaseAutomation in tests
+        packager_module.createBuild(
+            str(srcdir),
+            ext_type,
+            releaseBuild=True,
+            keyFile=keyfile,
+        )
+
+
+def locale_files(languages, rootfolder, srcdir):
+    """Generate example locales.
+
+    languages: tuple, list or set of languages to include
+    rootdir: folder-name to create the locale-folders in
+    """
+    for lang in languages:
+        subfolders = rootfolder.split(os.pathsep) + [lang, 'messages.json']
+        json_file = srcdir.ensure(*subfolders)
+        if lang == defaultLocale:
+            json_file.write(MESSAGES_EN_US)
+        else:
+            json_file.write('{}')
+
+
+def assert_all_locales_present(package, locales_path):
+    names = {x for x in package.namelist() if x.startswith(locales_path)}
+    assert names == {
+        os.path.join(locales_path, lang, 'messages.json')
+        for lang in ALL_LANGUAGES
+    }
+
+
+def compare_xml(a, b):
+    def get_leafs_string(tree):
+        """Recursively build a string representing all 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))
+        else:
+            result.append(root_str)
+        return result
+
+    tree_a = ElementTree.fromstring(a)
+    tree_b = ElementTree.fromstring(b)
+
+    return set(get_leafs_string(tree_a)) == set(get_leafs_string(tree_b))

[Vasily Kuznetsov, 2017/09/11 12:50:08]

This function and the one below seems to be only used in assertions. It's
probably better to just move the `assert`s into the functions -- this way pytest
will produce more helpful error messages with the diffs between the things we're
comparing.

[tlucas, 2017/09/12 11:32:12]

Acknowledged.

+
+
+def compare_json(a, b):

[Vasily Kuznetsov, 2017/09/11 12:50:08]

It seems like this function essentially compares two JSON strings ignoring the
order of items in lists. I think it would make sense to give it a docstring that
explains how this comparison differs from just `a == b` (and perhaps why it's
needed).

Also, I wonder if this is actually the best way to do this. What do you think
about traversing the deserialized data converting lists to sets and then just
comparing the results. Something like:

    def lst2set(obj):
        if isinstance(obj, list):
            return {lst2set(item) for item in obj}
        if isinstance(obj, dict):
            return {k, lst2set(v) for k, v in obj.items()}
        return obj

[tlucas, 2017/09/12 11:32:11]

I'm not sure if it would be a good idea to just ignore a list containing equal
items, e.g. [1,2,2,3] - on the other hand that would apply to a possibly desired
order too - what do you think about this?

[Sebastian Noack, 2017/09/12 21:55:26]

I agree, turning lists into sets when comparing the content of manifest.json
will potentially hide bugs. While for some values (e.g. permissions) the order
doesn't matter, for others like list of scripts that are executed in that order
the order matters. Also I cannot think of any scenario for any legit duplicate
in a list in manifest.json.

[tlucas, 2017/09/13 13:43:25]

Agreed - i came up with a different solution, please see in the patch set's
mail.

+    def get_leafs_string(obj):
+        """Recursively build a string representing all leaf-nodes"""
+        result = []
+        if isinstance(obj, list):
+            for item in obj:
+                for repr_str in get_leafs_string(item):
+                    result.append(':,' + repr_str)
+        elif isinstance(obj, dict):
+            for k, v in obj.items():
+                for repr_str in get_leafs_string(v):
+                    result.append('{}:{}'.format(k, repr_str))
+        else:
+            result = [str(obj)]
+
+        return result
+
+    tree_a = json.loads(a)
+    tree_b = json.loads(b)
+    return set(get_leafs_string(tree_a)) == set(get_leafs_string(tree_b))
+
+
+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':
+            assert compare_xml(manifest, fp.read())
+        else:
+            assert compare_json(manifest, fp.read())