Issue 5383 - Add tests for the Chrome and Firefox packagers

tests/test_packagerChrome.py

Index: tests/test_packagerChrome.py
diff --git a/tests/test_packagerChrome.py b/tests/test_packagerChrome.py
new file mode 100644
index 0000000000000000000000000000000000000000..5ea24a7cf3d4eee44d00c0c206b2204bd593f3a9
--- /dev/null
+++ b/tests/test_packagerChrome.py
@@ -0,0 +1,354 @@
+# 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 re
+import json
+import zipfile
+
+from xml.etree import ElementTree
+
+import pytest
+
+from buildtools import packager, packagerChrome
+from buildtools.tests.tools import copy_metadata
+
+ICON_SIZES = [32, 48, 64, 256, 53]
+
+MINIMUM_MANIFEST = [

[Vasily Kuznetsov, 2017/08/11 16:46:02]

Would it perhaps be more elegant to make this a dictionary? Seems like the data
is more like a dictionary and we mostly seem to be using this as a dictionary.

[tlucas, 2017/08/14 14:23:16]

Done.

+    ('manifest_version', 2),
+    ('author', 'Eyeo GmbH'),
+    ('permissions', [
+        'tabs',
+        '<all_urls>',
+        'contextMenus',
+        'webRequest',
+        'webRequestBlocking',
+        'webNavigation',
+        'storage',
+        'unlimitedStorage',
+        'notifications',
+    ]),
+    ('minimum_chrome_version', '49.0'),
+    ('minimum_opera_version', '36.0'),
+    ('devtools_page', 'devtools.html'),
+    ('description', '__MSG_description__'),
+    ('default_locale', 'en_US'),
+    ('short_name', '__MSG_name__'),
+    ('storage', {'managed_schema': 'managed-storage-schema.json'}),
+    ('options_ui', {'open_in_tab': True, 'page': 'options.html'}),
+]
+
+
+@pytest.fixture
+def chrome_metadata(tmpdir):
+    filename = 'metadata.chrome'
+    copy_metadata(filename, tmpdir)
+
+    return packager.readMetadata(str(tmpdir), 'chrome')

[Vasily Kuznetsov, 2017/08/11 16:46:01]

Could we just read the metadata from the original file, without copying it, or
does it not work for some reason?

[tlucas, 2017/08/14 14:23:17]

It doesn't, since the underlying functions almost every time accesses paths
relative to the metadata-file,

+
+
+@pytest.fixture
+def gecko_webext_metadata(tmpdir, chrome_metadata):
+    filename = 'metadata.gecko-webext'
+    copy_metadata(filename, tmpdir)
+
+    # gecko-webext metadata inherits from chrome metadata, assure existence
+    assert chrome_metadata is not None

[Vasily Kuznetsov, 2017/08/11 16:46:02]

Do you really need this assert? Seems like if this fixture depends on the other,
the file would have been copied and the metadata would have been read.

Also, if the idea above, about reading the metadata from the original files in
the tests instead of their copies in `tmpdir` work, you don't even need to
depend on `chrome_metadata` fixture here.

[tlucas, 2017/08/14 14:23:17]

As mentioned above, using the original-file won't work. But the assert is not
necessary, removed it.

+
+    return packager.readMetadata(str(tmpdir), 'gecko-webext')
+
+
+@pytest.fixture
+def icons(tmpdir):
+    """Valid .png files for testing make_icons"""
+    from PIL import Image
+
+    paths = []
+    icon_dir = tmpdir.mkdir('tmp_icons')
+
+    for size in ICON_SIZES:
+        img_path = str(icon_dir.join('abp-{}.png'.format(size)))
+        img = Image.new('1', (size, size if size != 53 else size + 1), 0)
+        img.save(img_path)
+        paths.append(img_path)
+
+    return paths
+
+
+@pytest.fixture
+def keyfile(tmpdir):
+    """Test-privatekey for signing files"""
+
+    content = """-----BEGIN RSA PRIVATE KEY-----

[Vasily Kuznetsov, 2017/08/11 16:46:01]

What do you think about putting this into a keyfile in the tests directory and
just returning that path?

[tlucas, 2017/08/14 14:23:17]

Done.

+MIIEowIBAAKCAQEA0P06DVxOWZ97cvBZWz7DV0DYTFF7Cdrxnu/sCxnHp8IEq+sM
+rqodp2i8eTsddnthTnA93wiFiFXz6/9liQKrSBtogEGG2tmrT73jy5mdOehm7h6m
+Ujy6wLuZQiuY/RWJ6453/0H3bKCA8IlIdU8Pkkn5DJWH+YiiSUD01U5ZMLn8m/+O
+D/r9CLsgWoJV4VCXyzMItcc8hlRbiSe/3DQmZOPbF0Bd1Jv6lNA4RG0U8oA6q5s1
+5w3n//FjNOreVocyxgWWWGk/awlkJme1xn3yB3yh74b51FHsr9vhiTjcs+QNO1BS
+9fA/oavzDOs1Q4fLnTbKn6Jlg5OAjWtiq4vjSQIDAQABAoH/fojTnUNGLP1iwTTE
+5Xoay7l3PL4YwN7PbGvXfuEdAXV3Xp/yDc7yJWpEsyIXtKT/RX6v91oxf1qLVVhN
+Iad8DSyLGRyTie5Aywct5RgdGfKcX5AvI5uhdxAeuvGqr5Fa8ERSYzqNlDeZ2glE
+1cIIq4oeQIBI08zmdXPeyUemuNjK8TEWKOTMOjDLhav/DMviI1cvqp5VIh3RseMr
+1dwprXe0/LWuCh0BrCZZNH/0smd2etXEq06RhgnE2sZkowktNhSgc2pqls92FT88
+ObjI/jRXf6OwK7HxIJ1tMKlk/VCvAI2PNWi6E7IZbqN7Rg7859uWybGZkP/h1Qf5
+fRitAoGBAOJnP3kgavnrUJ482R8MNjOwtqbLf4obS5Y/olOHUi+mu2x+EwxgDtaE
+sXeMjoIAuZvDoAIG0Hn7+J5ZSYkXT9aXQy/odVWDIpFNFFXNFnS73sAWay/FsMJc
+NvgbbWJk9fCq23O7ihuSkfM+zh8dAZoHzV4aw3DsZj37yzuTmTJbAoGBAOxPM6Wy
+fENT8ECGl9d/j8QBV4LvjrfIbz/RcCxgZHG50ELnxCo7FiYRg3d2DjR6Q+EpjclX
+CC+IwCGGf+RBz53WzJPOBkz+plPZzrhhhtpBuWgegeF+TmYT9pPd9XRHJs5qFCak
+J3xxgBur3NDtejNLWBc4bgFfrv8hdYeuIKorAoGBAJplWNbssadvv1G6I0NWG5yS
+lW0X+Akh5iE3kiaucPDIHqa1L55P366RXUku1Hx5rBo6hWL8bK3TlM/ACjLwb+Ti
+0NHaEDJZtHgsfYKp0veWqyiJ4Vz5zzosktwOMEFaopIWooPBUETPZrLgkMaNDGuT
+iIz1aXUX8f2xOf6OAHpjAoGBAMNqq9c2xrQW0fNKWn8HKih4w5mv6WHqCrXHyO+p
+tualNqhdaUdTHXnVudYsdorHISMubeY2ZrqIZ/pRc6mbEsoAO6VvFp7NZ1aoI98u
+J4qOF3kW4WlMPiEEGUEmqDjELj88UfWygkTSx5Iaibzs5cVNZUeujqnsKpcpYDwX
+r0OJAoGBAIWgnx72WjcFmL1kgOu2kH8tH5+cIAb/THX8lnCulQIlxss8n26NMMDa
+Kmh5gC2SOAe92cXG6jCYHZqZtl5gM6xGky1B9j7TDrXVPj+QDcItCuNN4zrdtiah
+US0KNMZKEMjxsaaMDPHj/bP9ULLbMHvtfvt4EnBQoVCDSZdfeBvZ
+-----END RSA PRIVATE KEY-----"""
+
+    keyfile_path = str(tmpdir.mkdir('rsakey').join('keyfile'))
+
+    with open(keyfile_path, 'wb') as key_fp:
+        key_fp.write(content)
+
+    return keyfile_path
+
+
+@pytest.mark.parametrize('release', [True, False])
+@pytest.mark.parametrize('devenv', [True, False])
+@pytest.mark.parametrize('ext_type', ['chrome', 'gecko-webext'])
+def test_create_manifest(srcdir, base_files, release, devenv, ext_type,
+                         chrome_metadata, gecko_webext_metadata):
+
+    if ext_type == 'chrome':
+        metadata = chrome_metadata
+    else:
+        metadata = gecko_webext_metadata
+
+    version = packager.getBuildVersion(str(srcdir), metadata, release)
+    params = {
+        'type': ext_type,
+        'baseDir': str(srcdir),
+        'releaseBuild': release,
+        'version': version,
+        'devenv': devenv,
+        'metadata': metadata,
+    }
+    manifest = json.loads(packagerChrome.createManifest(params, base_files))
+
+    expected_values = MINIMUM_MANIFEST + [

[Vasily Kuznetsov, 2017/08/11 16:46:01]

What do you think about just doing these checks directly instead of adding them
to `expected_values`?

Something like:

    gecko_app = manifest['applications']['gecko']
    assert gecko_app['id'] == '{.....}'
    assert gecko_app['strick_min_version'] == '50.0'

    if release:
        assert manifest['name'] == '__MSG_name_release__'
        assert manifest['version'] == '1.2.3'
    else:
        assert manifest['name'] == '__MSG_name__'
        assert manifest['version'] == '1.2.3.0'
        assert gecko_app['update_url'] == DEV_UPDATE_URL

I think this style is a bit easier to follow and it's not really that much more
wordy.

[tlucas, 2017/08/14 14:23:16]

Looks definitely cleaner, done.

+        ('name', '__MSG_name_' + ('devbuild__' if not release else '_')),
+        ('version', '1.2.3' + ('.0' if not release else '')),
+    ]
+
+    if ext_type == 'gecko-webext':
+        expected_values += [
+            ('applications', {
+                'gecko': {
+                    'id': '{d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d}',
+                    'strict_min_version': '50.0',
+                    }
+                })
+        ]
+        if not release:
+            expected_values[-1][1]['gecko'].update({
+                    'update_url': ''.join((
+                        'https://downloads.adblockplus.org/devbuilds/',
+                        'adblockplusfirefox/updates.json'))
+                })
+
+    for key, value in expected_values:
+        assert manifest.get(key, None) == value
+
+
+def test_sign_binary(base_files, keyfile):
+    from Crypto.Hash import SHA
+    from Crypto.PublicKey import RSA
+    from Crypto.Signature import PKCS1_v1_5
+
+    digest = SHA.new()
+
+    signature = packagerChrome.signBinary(base_files.zipToString(), keyfile)
+
+    with open(keyfile, 'r') as fp:
+        rsa_key = RSA.importKey(fp.read())
+
+    signer = PKCS1_v1_5.new(rsa_key)
+
+    digest.update(base_files.zipToString())
+    assert signer.verify(digest, signature)
+
+
+def test_get_public_key(keyfile):
+    expected = (
+        '30820122300d06092a864886f70d01010105000382010f003082010a0282010100d0'
+        'fd3a0d5c4e599f7b72f0595b3ec35740d84c517b09daf19eefec0b19c7a7c204abeb'
+        '0caeaa1da768bc793b1d767b614e703ddf08858855f3ebff658902ab481b68804186'
+        'dad9ab4fbde3cb999d39e866ee1ea6523cbac0bb99422b98fd1589eb8e77ff41f76c'
+        'a080f08948754f0f9249f90c9587f988a24940f4d54e5930b9fc9bff8e0ffafd08bb'
+        '205a8255e15097cb3308b5c73c86545b8927bfdc342664e3db17405dd49bfa94d038'
+        '446d14f2803aab9b35e70de7fff16334eade568732c6059658693f6b09642667b5c6'
+        '7df2077ca1ef86f9d451ecafdbe18938dcb3e40d3b5052f5f03fa1abf30ceb354387'
+        'cb9d36ca9fa2658393808d6b62ab8be3490203010001')
+
+    publickey = packagerChrome.getPublicKey(keyfile)
+    assert publickey.encode('hex') == expected

[Vasily Kuznetsov, 2017/08/11 16:46:01]

I just had an idea, what if we just take an MD5 or SHA hash of the value and
compare it to the hash of the expected value? Then we wouldn't need to keep this
long constant in the test (only its hash), but I don't think the test would
become any less robust.

[tlucas, 2017/08/14 14:23:16]

Good point, i started using SHA-hashes in the other tests i currently work on -
would serve consistency too, Done.

+
+
+@pytest.mark.usefixtures('chrome_metadata')
+@pytest.mark.parametrize('devenv', [True, False])
+def test_get_package_files(srcdir, devenv):
+    filenames = [
+        'foo.html',
+        'foo.js',
+        'foo.json',
+        'foo.xml',
+    ]
+
+    for filename in filenames:
+        srcdir.join(filename).write('', ensure=True)
+
+    params = {
+        'baseDir': str(srcdir),
+        'devenv': devenv,
+    }
+
+    files = packagerChrome.getPackageFiles(params)
+
+    assert ('qunit' in files) == devenv
+
+    for expected in ['icons', 'ui', 'skin', 'ext', '_locales', 'lib',
+                     'jquery-ui'] + [name for name in filenames]:
+        assert expected in files
+
+
+def test_make_icons(icons, capsys):
+    files = packager.Files(set(), set())
+    for path in icons:
+        files[path] = open(path, 'r').read()
+    result = packagerChrome.makeIcons(files, icons)
+
+    out, err = capsys.readouterr()
+
+    assert 'should be square' in err
+    assert set(result) == set(ICON_SIZES)
+
+
+def test_create_script_page(srcdir, chrome_metadata):
+    params = {'metadata': chrome_metadata}
+
+    template = packagerChrome.createScriptPage(
+        params, 'testIndex.html.tmpl', ('general', 'testScripts')
+    )
+
+    template_scripts = {elem.attrib['src'] for elem in
+                        ElementTree.fromstring(template).iter('script')}
+
+    for src in chrome_metadata.get('general', 'testScripts').split():
+        assert src in template_scripts
+
+
+def test_convert_js(srcdir, base_files, chrome_metadata):
+    params = {
+        'type': 'chrome',
+        'metadata': chrome_metadata,
+    }
+
+    packagerChrome.convertJS(params, base_files)
+    source = base_files['lib/foo.js']
+
+    assert 'var foo;' in source
+    assert 'var bar;' in source
+
+
+def test_to_json():
+    data = {'bar': ['foo', 'foobar', 1, True]}
+    data_json = packagerChrome.toJson(data)
+
+    assert data_json.endswith('\n')
+    json.loads(data_json)

[Vasily Kuznetsov, 2017/08/11 16:46:01]

Maybe also compare it to the original?

[tlucas, 2017/08/14 14:23:17]

Done.

+
+
+def test_import_gecko_locales(tmpdir, srcdir, base_files, chrome_metadata):
+    version = packager.getBuildVersion(str(srcdir), chrome_metadata, False)
+    params = {
+        'type': 'chrome',
+        'baseDir': str(srcdir),
+        'releaseBuild': False,
+        'version': version,
+        'devenv': False,
+        'metadata': chrome_metadata,
+    }
+
+    data = {'name': 'foo', 'name_devbuild': 'bar'}
+
+    t_dir = tmpdir.mkdir('_trans').mkdir('en-US')
+    t_dir.join('test.properties').write(
+        '\n'.join('{}={}'.format(k, v) for k, v in data.items()))
+
+    packagerChrome.importGeckoLocales(params, base_files)
+    trans_data = json.loads(base_files['_locales/en_US/messages.json'])
+
+    for key, value in data.items():
+        assert trans_data.get('test_' + key, {}).get('message', '') == value
+
+
+def test_fix_translations_for_cws(base_files):
+    base_files['_locales/de/messages.json'] = packagerChrome.toJson({})
+    base_files['manifest.json'] = packagerChrome.toJson(dict(MINIMUM_MANIFEST))
+    packagerChrome.fixTranslationsForCWS(base_files)
+
+    ger_messages = json.loads(base_files['_locales/de/messages.json'])
+    eng_messages = json.loads(base_files['_locales/en_US/messages.json'])
+
+    # Check for inserted translations, none were present in 'de' beforehand
+    for match in re.finditer(r'__MSG_(\S+)__', base_files['manifest.json']):
+        name = match.group(1)
+        assert ger_messages[name]['message'] == eng_messages[name]['message']
+
+    # CWS enforces max-lengths on string
+    assert len(ger_messages['description']['message']) <= 132
+    assert len(ger_messages['name']['message']) <= 12
+
+
+@pytest.mark.usefixtures('base_files', 'gecko_webext_metadata',
+                         'chrome_metadata')
+@pytest.mark.parametrize('release', [True, False])
+@pytest.mark.parametrize('devenv', [True, False])
+@pytest.mark.parametrize('ext_type', ['chrome', 'gecko-webext'])
+@pytest.mark.parametrize('key', [True, False])
+def test_create_build(srcdir, tmpdir, release, devenv, ext_type,
+                      key, keyfile):
+
+    basefiles = [
+        'manifest.json',
+        'lib/foo.js',
+        'foo/logo_50.png',
+        '_locales/en_US/messages.json',
+        '_locales/en-US/test.properties',
+        'logo_44.png',
+        'icons/logo_150.png',
+    ]
+
+    out_file = str(tmpdir.join('out'))
+
+    packagerChrome.createBuild(
+        outFile=out_file,
+        baseDir=str(srcdir),
+        type=ext_type,
+        releaseBuild=release,
+        keyFile=keyfile if key else None,
+        devenv=devenv)
+
+    with zipfile.ZipFile(out_file, 'r') as zipfp:
+        zipfp.testzip()
+
+        filenames = [zipinfo.filename for zipinfo in zipfp.infolist()]
+        for filename in basefiles if not devenv else basefiles + [
+                    'devenvPoller__.js',
+                    'devenvVersion__',
+                    'qunit/index.html',
+                ]:
+            assert filename in filenames