Issue 5383 - Add tests for the Chrome and Firefox packagers

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 json
-import re
+import zipfile
+from xml.etree import ElementTree
+from struct import unpack
 
 import pytest
+from Crypto.Signature import PKCS1_v1_5
+from Crypto.PublicKey import RSA
+from Crypto.Hash import SHA
 
 from buildtools import packager
-from buildtools.tests.tools import DirContent

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

These imports also can be combined.

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

Acknowledged.

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

Done.

-from buildtools.tests.tools import ZipContent
-from buildtools.tests.tools import copy_metadata
-from buildtools.tests.tools import run_webext_build
-from buildtools.tests.tools import assert_all_locales_present
-from buildtools.tests.tools import assert_manifest_content
-from buildtools.tests.tools import locale_files
-from buildtools.tests.conftest import ALL_LANGUAGES
-
+from buildtools.packagerChrome import defaultLocale
+from buildtools.build import processArgs
 
 LOCALES_MODULE = {
     'test.Foobar': {
         'message': 'Ensuring dict-copy from modules for $domain$',
         'description': 'test description',
         'placeholders': {'content': '$1', 'example': 'www.adblockplus.org'}
     }
 }
 
-DTD_TEST = ('<!ENTITY access.key "access key(&amp;a)">'
-            '<!ENTITY ampersand "foo &amp;-bar">')
-
-PROPERTIES_TEST = 'description=very descriptive!'
-
-
-@pytest.fixture
-def gecko_import(tmpdir):
-    tmpdir.mkdir('_imp').mkdir('en-US').join('gecko.dtd').write(DTD_TEST)
+ALL_LANGUAGES = ['en_US', 'de', 'it']
+
+MESSAGES_EN_US = json.dumps({
+    'name': {'message': 'Adblock Plus'},
+    'name_devbuild': {'message': 'devbuild-marker'},
+    'description': {
+        'message': 'Adblock Plus is the most popular ad blocker ever, '
+                   'and also supports websites by not blocking '
+                   'unobstrusive ads by default (configurable).'
+    },
+})
+
+
+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):
+    """Provide 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):
+        """Construct ZipContent object 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):
+        """Construct a DirContent object handling <path>."""
+        self._path = path
+        super(DirContent, self).__init__()
+
+    def _close(self):
+        pass
+
+    def namelist(self):
+        """Generate a list of filenames."""
+        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, keyfile=None):
+    """Run a build process."""
+    cmd_mapping = {
+        'devenv': ['devenv'],
+        'development_build': ['build', '-b', '1337'],
+        'release_build': ['build', '-r'],
+    }
+
+    args = ['build.py', '-t', ext_type] + cmd_mapping[build_opt]
+
+    if keyfile:
+        args += ['-k', keyfile]
+
+    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, prefix):
+    names = {x for x in package.namelist() if
+             x.startswith(os.path.join(prefix, '_locales'))}
+    assert names == {
+        os.path.join(prefix, '_locales', lang, 'messages.json')
+        for lang in ALL_LANGUAGES
+    }
+
+
+@pytest.fixture
+def srcdir(tmpdir):
+    """Source directory for building the package."""
+    for size in ['44', '50', '150']:
+        path = tmpdir.join('chrome', 'icons', 'abp-{}.png'.format(size))
+        path.write(size, ensure=True)
+
+    tmpdir.join('bar.json').write(json.dumps({}))
+    return tmpdir
 
 
 @pytest.fixture
 def locale_modules(tmpdir):
     mod_dir = tmpdir.mkdir('_modules')
-    lang_dir = mod_dir.mkdir('en-US')
+    lang_dir = mod_dir.mkdir('en_US')
     lang_dir.join('module.json').write(json.dumps(LOCALES_MODULE))
-    lang_dir.join('unit.properties').write(json.dumps(PROPERTIES_TEST))
 
 
 @pytest.fixture
 def icons(srcdir):
     icons_dir = srcdir.mkdir('icons')
     for filename in ['abp-16.png', 'abp-19.png', 'abp-53.png']:
         shutil.copy(
             os.path.join(os.path.dirname(__file__), filename),
             os.path.join(str(icons_dir), filename),
         )
 
 
 @pytest.fixture
 def all_lang_locales(tmpdir):
     return locale_files(ALL_LANGUAGES, '_locales', tmpdir)
 
 
 @pytest.fixture
 def chrome_metadata(tmpdir):
     filename = 'metadata.chrome'
     copy_metadata(filename, tmpdir)
 
 
 @pytest.fixture
 def gecko_webext_metadata(tmpdir, chrome_metadata):
-    filename = 'metadata.gecko-webext'
+    filename = 'metadata.gecko'
     copy_metadata(filename, tmpdir)
 
 
 @pytest.fixture
+def edge_metadata(tmpdir):
+    filename = 'metadata.edge'
+    copy_metadata(filename, tmpdir)
+
+    return packager.readMetadata(str(tmpdir), 'edge')
+
+
+@pytest.fixture
 def keyfile(tmpdir):
-    """Test-privatekey for signing chrome release-package"""
+    """Test-privatekey for signing chrome release-package."""
     return os.path.join(os.path.dirname(__file__), 'chrome_rsa.pem')
 
 
 @pytest.fixture
 def lib_files(tmpdir):
     files = packager.Files(['lib'], set())
-    files['ext/a.js'] = 'var bar;'
+    files['ext/a.js'] = 'require("./c.js");\nvar bar;'
     files['lib/b.js'] = 'var foo;'
+    files['ext/c.js'] = 'var this_is_c;'
 
     tmpdir.mkdir('lib').join('b.js').write(files['lib/b.js'])
-    tmpdir.mkdir('ext').join('a.js').write(files['ext/a.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 assert_gecko_locale_conversion(package):
-    locale = json.loads(package.read('_locales/en_US/messages.json'))
-
-    assert locale.get('test_Foobar', {}) == LOCALES_MODULE['test.Foobar']
-    assert locale.get('access_key', {}) == {'message': 'access key'}
-    assert locale.get('ampersand', {}) == {'message': 'foo -bar'}
-    assert locale.get('_description', {}) == {'message': 'very descriptive!"'}
-
-
-def assert_convert_js(package, excluded=False):
-    libfoo = package.read('lib/foo.js')
+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 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())
+
+    diff = list(difflib.unified_diff(generated, expected, n=0))
+    assert len(diff) == 0, '\n'.join(diff)
+
+
+def assert_webpack_bundle(package, prefix, excluded=False):
+    libfoo = package.read(os.path.join(prefix, 'lib/foo.js'))
+    libfoomap = package.read(os.path.join(prefix, 'lib/foo.js.map'))
 
     assert 'var bar;' in libfoo
-    assert 'require.modules["ext_a"]' in libfoo
+    assert 'webpack:///./ext/a.js' in libfoomap
+
+    assert 'var this_is_c;' in libfoo
+    assert 'webpack:///./ext/c.js' in libfoomap
 
     assert ('var foo;' in libfoo) != excluded
-    assert ('require.modules["b"]' in libfoo) != excluded
-
-
-def assert_devenv_scripts(package, devenv):
-    manifest = json.loads(package.read('manifest.json'))
+    assert ('webpack:///./lib/b.js' in libfoomap) != excluded
+
+
+def assert_devenv_scripts(package, prefix, devenv):
+    manifest = json.loads(package.read(os.path.join(prefix, 'manifest.json')))
     filenames = package.namelist()
     scripts = [
-            'ext/common.js',
-            'ext/background.js',
+        'ext/common.js',
+        'ext/background.js',
     ]
 
+    assert (os.path.join(prefix, 'qunit/index.html') in filenames) == devenv
+    assert (os.path.join(prefix, 'devenvPoller__.js') in filenames) == devenv
+    assert (os.path.join(prefix, 'devenvVersion__') in filenames) == devenv
+
     if devenv:
-        assert 'qunit/index.html' in filenames
-        assert 'devenvPoller__.js' in filenames
-        assert 'devenvVersion__' in filenames
-
-        assert '../ext/common.js' in package.read('qunit/index.html')
-        assert '../ext/background.js' in package.read('qunit/index.html')
+        quint_index = package.read(os.path.join(prefix, 'qunit/index.html'))
+        assert '../ext/common.js' in quint_index
+        assert '../ext/background.js' in quint_index
 
         assert set(manifest['background']['scripts']) == set(
             scripts + ['devenvPoller__.js']
         )
     else:
-        assert 'qunit/index.html' not in filenames
-
         assert set(manifest['background']['scripts']) == set(scripts)
 
 
-def assert_base_files(package):
+def assert_base_files(package, platform, prefix):
     filenames = set(package.namelist())
 
-    assert 'bar.json' in filenames
-    assert 'manifest.json' in filenames
-    assert 'lib/foo.js' in filenames
-    assert 'foo/logo_50.png' in filenames
-    assert 'icons/logo_150.png' in filenames
+    if platform == 'edge':
+        assert 'AppxManifest.xml' in filenames
+        assert 'AppxBlockMap.xml' in filenames
+        assert '[Content_Types].xml' in filenames
+
+        assert package.read('Assets/logo_44.png') == '44'
+        assert package.read('Extension/icons/abp-44.png') == '44'
+
+    assert os.path.join(prefix, 'bar.json') in filenames
+    assert os.path.join(prefix, 'manifest.json') in filenames
+    assert os.path.join(prefix, 'lib/foo.js') in filenames
+    assert os.path.join(prefix, 'foo/logo_50.png') in filenames
+    assert os.path.join(prefix, 'icons/logo_150.png') in filenames
 
 
 def assert_chrome_signature(filename, keyfile):
-    from struct import unpack

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

Is there any specific reason to have these imports are inside of the function?

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

Not at all - merely a result of a speedy adjustment - will change :)

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

Done.

-    from Crypto.Hash import SHA
-    from Crypto.PublicKey import RSA
-    from Crypto.Signature import PKCS1_v1_5
-
     with open(filename, 'r') as fp:
         content = fp.read()
 
     _, _, l_pubkey, l_signature = unpack('<4sIII', content[:16])
     signature = content[16 + l_pubkey: 16 + l_pubkey + l_signature]
 
     digest = SHA.new()
     with open(keyfile, 'r') as fp:
         rsa_key = RSA.importKey(fp.read())
 
@@ skipping 15 matching lines @@
     # every language.
     for match in re.finditer(r'__MSG_(\S+)__', manifest):
         name = match.group(1)
 
         for other in translations[1:]:
             assert translations[0][name]['message'] == other[name]['message']
 
 
 @pytest.mark.usefixtures(
     'all_lang_locales',
-    'gecko_import',
     'locale_modules',
     'icons',
     'lib_files',
     'chrome_metadata',
+    'gecko_webext_metadata',
+    'edge_metadata',
 )
-@pytest.mark.parametrize('dev_build_release', ['build', 'devenv', 'release'])
-def test_build_chrome(dev_build_release, keyfile, tmpdir, srcdir, capsys):
-    from buildtools import packagerChrome
-    release = dev_build_release == 'release'
-    devenv = dev_build_release == 'devenv'
-
-    run_webext_build('chrome', dev_build_release, srcdir, packagerChrome,
-                     keyfile if release else None)
+@pytest.mark.parametrize('platform,command', [
+    ('chrome', 'development_build'),
+    ('chrome', 'devenv'),
+    ('chrome', 'release_build'),
+    ('gecko', 'development_build'),
+    ('gecko', 'devenv'),
+    ('gecko', 'release_build'),
+    ('edge', 'development_build'),
+    ('edge', 'devenv'),
+    ('edge', 'release_build'),
+])
+def test_build_webext(platform, command, keyfile, tmpdir, srcdir, capsys):
+    release = command == 'release_build'
+    devenv = command == 'devenv'
+
+    if platform == 'chrome' and release:
+        key = keyfile
+    else:
+        key = None
+
+    manifests = {
+        'gecko': [('', 'manifest', 'json')],
+        'chrome': [('', 'manifest', 'json')],
+        'edge': [('', 'AppxManifest', 'xml'),
+                 ('Extension', 'manifest', 'json')],
+    }
+
+    filenames = {
+        'gecko': 'adblockplusfirefox-1.2.3{}.xpi',
+        'chrome': 'adblockpluschrome-1.2.3{{}}.{}'.format(
+            {True: 'crx', False: 'zip'}[release]
+        ),
+        'edge': 'adblockplusedge-1.2.3{}.appx',
+    }
+
+    if platform == 'edge':
+        prefix = 'Extension'
+    else:
+        prefix = ''
+
+    run_webext_build(platform, command, srcdir, keyfile=key)
 
     # The makeIcons() in packagerChrome.py should warn about non-square
     # icons via stderr.
     out, err = capsys.readouterr()
     assert 'icon should be square' in err
 
     if devenv:
         content_class = DirContent
-        out_file_path = os.path.join(str(srcdir), 'devenv.chrome')
+        out_file_path = os.path.join(str(srcdir), 'devenv.' + platform)
     else:
         content_class = ZipContent
-        out_file = 'adblockpluschrome-1.2.3'
-        if not release:
-            out_file += '.0'
 
         if release:
-            out_file += '.crx'
+            add_version = ''
         else:
-            out_file += '.zip'
+            add_version = '.1337'
+
+        out_file = filenames[platform].format(add_version)
 
         out_file_path = os.path.abspath(os.path.join(
             os.path.dirname(__file__), os.pardir, out_file))
-
         assert os.path.exists(out_file_path)
 
-    if release:
+    if release and platform == 'chrome':
         assert_chrome_signature(out_file_path, keyfile)
 
     with content_class(out_file_path) as package:
-        assert_base_files(package)
-        assert_devenv_scripts(package, devenv)
-        assert_all_locales_present(package, '_locales')
-        assert_locale_upfix(package)
-        assert_gecko_locale_conversion(package)
-        assert_convert_js(package)
-        expected = os.path.join(
-            os.path.dirname(__file__),
-            'expecteddata',
-            'manifest_chrome_{}.json'.format(dev_build_release),
-        )
-        assert_manifest_content(package.read('manifest.json'), expected)
-
-
-@pytest.mark.usefixtures(
-    'all_lang_locales',
-    'locale_modules',
-    'gecko_import',
-    'icons',
-    'lib_files',
-    'gecko_webext_metadata',
-)
-@pytest.mark.parametrize('dev_build_release', ['build', 'devenv', 'release'])
-def test_build_gecko_webext(dev_build_release, tmpdir, srcdir, capsys):

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

This test seems very similar to the previous one apart from some file naming
differences. What do you think about merging them and using parametrization to
switch between the two versions of the file names (and other minor difference)?
Seems like this wouldn't make things much more complicated and we would
eliminate a lot of code duplication.

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

I agree - will do (also i fear the complexity of the resulting test function
might be a bit over the top)

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

Done.

-    from buildtools import packagerChrome
-    release = dev_build_release == 'release'
-    devenv = dev_build_release == 'devenv'
-
-    run_webext_build('gecko-webext', dev_build_release, srcdir, packagerChrome)
-
-    # The makeIcons() in packagerChrome.py should warn about non-square
-    # icons via stderr.
-    out, err = capsys.readouterr()
-    assert 'icon should be square' in err
-
-    if devenv:
-        content_class = DirContent
-        out_file_path = os.path.join(str(srcdir), 'devenv.gecko-webext')
-    else:
-        content_class = ZipContent
-        out_file = 'adblockplusfirefox-1.2.3{}.xpi'.format(
-            '.0' if not release else ''
-        )
-
-        out_file_path = os.path.abspath(os.path.join(
-            os.path.dirname(__file__), os.pardir, out_file))
-
-        assert os.path.exists(out_file_path)
-
-    with content_class(out_file_path) as package:
-        assert_base_files(package)
-        assert_devenv_scripts(package, devenv)
-        assert_all_locales_present(package, '_locales')
-        assert_gecko_locale_conversion(package)
-        assert_convert_js(package, True)
-
-        expected = os.path.join(
-            os.path.dirname(__file__),
-            'expecteddata',
-            'manifest_gecko-webext_{}.json'.format(dev_build_release),
-        )
-        assert_manifest_content(package.read('manifest.json'), expected)
+        assert_base_files(package, platform, prefix)
+        assert_all_locales_present(package, prefix)
+        assert_webpack_bundle(package, prefix, platform == 'gecko')
+
+        if platform == 'chrome':
+            assert_locale_upfix(package)
+
+        assert_devenv_scripts(package, prefix, devenv)
+
+        for folder, name, ext in manifests[platform]:
+            filename = '{{}}_{}_{}.{{}}'.format(platform, command)
+            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,
+            )