Issue 5383 - Add tests for the Chrome and Firefox packagers

tests/tools.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 os
+import shutil
+import zipfile
+import json
+import difflib
+
+from xml.etree import ElementTree
+
+from buildtools.build import processArgs
+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_opt, srcdir, buildnum=None, keyfile=None):
+    """Run a build process."""
+    if build_opt == 'build':
+        build_args = ['build']
+    elif build_opt == 'release':
+        build_args = ['build', '-r']
+    else:
+        build_args = ['devenv']
+
+    args = ['build.py', '-t', ext_type] + build_args
+
+    if keyfile:
+        args += ['-k', keyfile]
+    if buildnum:
+        args += ['-b', buildnum]
+
+    processArgs(str(srcdir), args)
+
+
+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):
+    """Assert equal content in two manifests in XML format."""
+    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))
+        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.
+    strs_a = sorted(get_leafs_string(ElementTree.fromstring(a)))
+    strs_b = sorted(get_leafs_string(ElementTree.fromstring(b)))
+
+    diff = list(difflib.unified_diff(strs_a, strs_b, n=0))
+    assert len(diff) == 0, '\n'.join(diff)
+
+
+def compare_json(a, b):
+    """Assert equal content in two manifests in JSON format.
+
+    Compare the content of two JSON strings, respecting the order of items in
+    a list as well as possible duplicates.
+    Raise a unified diff if equality could not be asserted.
+    """
+    json_a = json.dumps(json.loads(a), sort_keys=True, indent=0).split('\n')
+    json_b = json.dumps(json.loads(b), sort_keys=True, indent=0).split('\n')
+
+    diff = list(difflib.unified_diff(json_a, json_b, n=0))

[Sebastian Noack, 2017/09/22 01:56:05]

This logic is duplicated. Why not just move it to assert_manifest_content()?

[tlucas, 2017/09/22 09:02:20]

See
https://codereview.adblockplus.org/29501558/diff2/29541651:29543665/tests/too...
in LEFT line 177 - if you don't mind, i would stick to this.

[Sebastian Noack, 2017/09/22 18:31:04]

I don't see how that discussion is related to the suggestion here. I am merely
suggesting to make compare_{xml|json} return a and b, so that we can move the
duplicated code to assert_manifest_content(). This doesn't seem to contradict
anything discussed earlier.

[Sebastian Noack, 2017/09/23 21:11:41]

If you just make make compare_{xml|json} return a and b (as I suggested above),
and then move following to compare_json(), I don't see how this would give any
less useful error message:

  diff = list(difflib.unified_diff(a, b, n=0))
  assert len(diff) == 0, '\n'.join(diff)

In the original code, you did the comparison before using the assert statement,
so the error message would only show False, rather than the expected and actual
result that are compared. But that isn't the case if you do it like I suggest.

[Sebastian Noack, 2017/09/25 20:21:19]

Yes, that is essentially what I suggested.

+    assert len(diff) == 0, '\n'.join(diff)
+
+
+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':
+            compare_xml(manifest, fp.read())
+        else:
+            compare_json(manifest, fp.read())