Merge pull request #212 from mitchhentges/194-remove-package-name-check

Fixes #194, removes "skip package-name check" option

CHANGELOG.md

@@ -2,6 +2,10 @@
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
+## [4.1.0] - 2019-07-10
+### Removed
+* `--skip-check-package-names`. When pushing or checking an APK, expected package names must always be provided
+
 ## [4.0.0] - 2019-07-09
 ### Changed
 * `EditService` now calls the Google Play API v3

mozapkpublisher/check_apks.py

@@ -20,7 +20,6 @@ def main():
     extract_and_check_apks_metadata(
         config.apks,
         config.expected_package_names,
-        config.skip_check_package_names,
         config.skip_checks_fennec,
         config.skip_check_multiple_locales,
         config.skip_check_same_locales,

mozapkpublisher/common/apk/__init__.py

@@ -1,8 +1,6 @@
 import argparse
 
 from mozapkpublisher.common.apk.checker import (
-    AnyPackageNamesCheck,
-    ExpectedPackageNamesCheck,
     cross_check_apks,
 )
 from mozapkpublisher.common.apk.extractor import extract_metadata
@@ -22,42 +20,28 @@ def add_apk_checks_arguments(parser):
     parser.add_argument('--skip-checks-fennec', action='store_true',
                         help='Skip checks that are Fennec-specific (ini-checking, checking '
                              'version-to-package-name compliance)')
-
-    expected_package_names_group = parser.add_mutually_exclusive_group(required=True)
-    expected_package_names_group.add_argument('--expected-package-name', dest='expected_package_names',
-                                              action='append',
-                                              help='Package names apks are expected to match')
-    expected_package_names_group.add_argument('--skip-check-package-names', action='store_true',
-                                              help='Skip assertion that apks match a specified package name')
+    parser.add_argument('--expected-package-name', dest='expected_package_names',
+                        action='append',
+                        help='Package names apks are expected to match',
+                        required=True)
 
 
 def extract_and_check_apks_metadata(
     apks,
     expected_package_names,
-    skip_check_package_names,
     skip_checks_fennec,
     skip_check_multiple_locales,
     skip_check_same_locales,
     skip_check_ordered_version_codes,
 ):
-    if expected_package_names and not skip_check_package_names:
-        package_names_check = ExpectedPackageNamesCheck(expected_package_names)
-    elif not expected_package_names and skip_check_package_names:
-        package_names_check = AnyPackageNamesCheck()
-    else:
-        raise ValueError(
-            'Either expected_package_names or skip_check_package_names must be truthy. '
-            'Values: {}'.format((expected_package_names, skip_check_package_names))
-        )
-
     apks_paths = [apk.name for apk in apks]
     apks_metadata_per_paths = {
         apk_path: extract_metadata(apk_path, not skip_checks_fennec)
         for apk_path in apks_paths
     }
     cross_check_apks(
         apks_metadata_per_paths,
-        package_names_check,
+        expected_package_names,
         skip_checks_fennec,
         skip_check_multiple_locales,
         skip_check_same_locales,

mozapkpublisher/common/apk/checker.py

@@ -15,27 +15,10 @@
 _ARCHITECTURE_ORDER_REGARDING_VERSION_CODE = ('armeabi-v7a', 'arm64-v8a', 'x86', 'x86_64')
 
 
-class ExpectedPackageNamesCheck:
-    def __init__(self, expected_product_types):
-        self.expected_product_types = expected_product_types
-
-    def validate(self, apks_metadata_per_paths):
-        types = set([metadata['package_name'] for metadata in apks_metadata_per_paths.values()])
-
-        if not types == set(self.expected_product_types):
-            raise BadSetOfApks('Expected product types {}, found {}'.format(self.expected_product_types, types))
-        logger.info('Expected product types {} found'.format(self.expected_product_types))
-
-
-class AnyPackageNamesCheck:
-    def validate(self, _):
-        pass
-
-
-def cross_check_apks(apks_metadata_per_paths, package_names_check, skip_checks_fennec, skip_check_multiple_locales,
+def cross_check_apks(apks_metadata_per_paths, expected_package_names, skip_checks_fennec, skip_check_multiple_locales,
                      skip_check_same_locales, skip_check_ordered_version_codes):
     logger.info("Checking APKs' metadata and content...")
-    package_names_check.validate(apks_metadata_per_paths)
+    _check_package_names(expected_package_names, apks_metadata_per_paths)
 
     if not skip_checks_fennec:
         singular_apk_metadata = list(apks_metadata_per_paths.values())[0]
@@ -59,6 +42,15 @@ def cross_check_apks(apks_metadata_per_paths, package_names_check, skip_checks_f
     logger.info('APKs are sane!')
 
 
+def _check_package_names(expected_package_names, apks_metadata_per_paths):
+    types = set([metadata['package_name'] for metadata in apks_metadata_per_paths.values()])
+
+    if not types == set(expected_package_names):
+        raise BadSetOfApks(
+            'Expected package names {}, found {}'.format(expected_package_names, types))
+    logger.info('Found valid package names {}'.format(types))
+
+
 def _check_piece_of_metadata_is_unique(key, pretty_key, apks_metadata_per_paths):
     all_items = [metadata[key] for metadata in apks_metadata_per_paths.values()]
     unique_items = filter_out_identical_values(all_items)

mozapkpublisher/push_apk.py

@@ -16,7 +16,6 @@ def push_apk(
     google_play_credentials_file,
     track,
     expected_package_names,
-    skip_check_package_names=False,
     rollout_percentage=None,
     commit=True,
     contact_google_play=True,
@@ -33,8 +32,7 @@ def push_apk(
         google_play_credentials_file: Credentials file to authenticate to Google Play
         track (str): Google Play track to deploy to (e.g.: "nightly"). If "rollout" is chosen, the parameter
             `rollout_percentage` must be specified as well
-        expected_package_names (str): defines what the expected package name must be.
-        skip_check_package_names (bool): skip the checked package name check. Raises if `expected_package_names` is truthy,
+        expected_package_names (list of str): defines what the expected package name must be.
         rollout_percentage (int): percentage of users to roll out this update to. Must be a number between [0-100].
             This option is only valid if `track` is set to "rollout"
         commit (bool): `False` to do a dry-run
@@ -58,7 +56,6 @@ def push_apk(
     apks_metadata_per_paths = extract_and_check_apks_metadata(
         apks,
         expected_package_names,
-        skip_check_package_names,
         skip_checks_fennec,
         skip_check_multiple_locales,
         skip_check_same_locales,
@@ -149,7 +146,6 @@ def main():
             config.google_play_credentials_file,
             config.track,
             config.expected_package_names,
-            config.skip_check_package_names,
             config.rollout_percentage,
             config.commit,
             config.contact_google_play,

mozapkpublisher/test/common/apk/test_checker.py

@@ -10,8 +10,7 @@
     _check_apks_version_codes_are_correctly_ordered,
     _check_all_apks_are_multi_locales,
     _check_all_architectures_and_api_levels_are_present,
-    ExpectedPackageNamesCheck,
-)
+    _check_package_names)
 from mozapkpublisher.common.exceptions import NotMultiLocaleApk, BadApk, BadSetOfApks
 
 
@@ -50,12 +49,11 @@
     }
 }, ['org.mozilla.focus', 'org.mozilla.klar', 'org.mozilla.reference.browser'], True)))
 def test_check_correct_apk_package_names(apks_metadata_per_paths, product_types, should_fail):
-    expected_names = ExpectedPackageNamesCheck(product_types)
     if should_fail:
         with pytest.raises(BadSetOfApks):
-            expected_names.validate(apks_metadata_per_paths)
+            _check_package_names(product_types, apks_metadata_per_paths)
     else:
-        expected_names.validate(apks_metadata_per_paths)
+        _check_package_names(product_types, apks_metadata_per_paths)
 
 
 @pytest.mark.parametrize('apks_metadata_per_paths, package_names_check, skip_checks_fennec, skip_check_multiple_locales, skip_check_same_locales, skip_check_ordered_version_codes', ((   # noqa
@@ -97,7 +95,7 @@ def test_check_correct_apk_package_names(apks_metadata_per_paths, product_types,
             'version_code': '2015523300',
         },
     },
-    ExpectedPackageNamesCheck(['org.mozilla.firefox']),
+    ['org.mozilla.firefox'],
     False,
     False,
     False,
@@ -159,7 +157,7 @@ def test_check_correct_apk_package_names(apks_metadata_per_paths, product_types,
             'version_code': '2015605651',
         },
     },
-    ExpectedPackageNamesCheck(['org.mozilla.fennec_aurora']),
+    ['org.mozilla.fennec_aurora'],
     False,
     False,
     False,
@@ -239,7 +237,7 @@ def test_check_correct_apk_package_names(apks_metadata_per_paths, product_types,
             'version_code': '2015605655',
         },
     },
-    ExpectedPackageNamesCheck(['org.mozilla.fennec_aurora']),
+    ['org.mozilla.fennec_aurora'],
     False,
     False,
     False,
@@ -319,7 +317,7 @@ def test_check_correct_apk_package_names(apks_metadata_per_paths, product_types,
             'version_code': '4',
         },
     },
-    ExpectedPackageNamesCheck(['org.mozilla.firefox_beta']),
+    ['org.mozilla.firefox_beta'],
     False,
     False,
     False,
@@ -399,7 +397,7 @@ def test_check_correct_apk_package_names(apks_metadata_per_paths, product_types,
             'version_code': '4',
         },
     },
-    ExpectedPackageNamesCheck(['org.mozilla.firefox']),
+    ['org.mozilla.firefox'],
     False,
     False,
     False,
@@ -419,7 +417,7 @@ def test_check_correct_apk_package_names(apks_metadata_per_paths, product_types,
             'version_code': '11'
         }
     },
-    ExpectedPackageNamesCheck(['org.mozilla.focus', 'org.mozilla.klar']),
+    ['org.mozilla.focus', 'org.mozilla.klar'],
     True,
     True,
     True,

mozapkpublisher/test/common/apk/test_init.py

@@ -1,10 +1,7 @@
 import argparse
-import pytest
 import tempfile
 
-from unittest.mock import MagicMock
-
-from mozapkpublisher.common.apk import add_apk_checks_arguments, extract_and_check_apks_metadata
+from mozapkpublisher.common.apk import add_apk_checks_arguments
 
 
 def test_add_apk_checks_arguments():
@@ -18,46 +15,3 @@ def test_add_apk_checks_arguments():
         assert config.apks[0].name == f.name
 
     assert config.expected_package_names == ['some.package.name']
-
-
-@pytest.mark.parametrize('expected_package_names, skip_check_package_names, raises', (
-    ('some.package.name', False, False),
-    ('', True, False),
-    ('', False, True),
-    ('some.package.name', True, True),
-))
-def test_extract_and_check_apks_metadata(monkeypatch, expected_package_names, skip_check_package_names, raises):
-    monkeypatch.setattr(
-        'mozapkpublisher.common.apk.extract_metadata',
-        lambda _, __: {'some': 'metadata'}
-    )
-    monkeypatch.setattr(
-        'mozapkpublisher.common.apk.cross_check_apks',
-        lambda *args, **kwargs: None
-    )
-
-    apk_mock = MagicMock()
-    apk_mock.name = '/some/path'
-    apks = [apk_mock]
-
-    if raises:
-        with pytest.raises(ValueError):
-            extract_and_check_apks_metadata(
-                apks,
-                expected_package_names,
-                skip_check_package_names,
-                True,
-                True,
-                True,
-                True,
-            )
-    else:
-        assert extract_and_check_apks_metadata(
-            apks,
-            expected_package_names,
-            skip_check_package_names,
-            True,
-            True,
-            True,
-            True,
-        ) == {'/some/path': {'some': 'metadata'}}

mozapkpublisher/test/test_push_apk.py

@@ -8,7 +8,6 @@
 from tempfile import NamedTemporaryFile
 
 from mozapkpublisher.common import googleplay
-from mozapkpublisher.common.apk.checker import AnyPackageNamesCheck
 from mozapkpublisher.common.exceptions import WrongArgumentGiven
 from mozapkpublisher.push_apk import (
     push_apk,
@@ -80,19 +79,19 @@ def _metadata(*args, **kwargs):
 def test_invalid_rollout_percentage(edit_service_mock, monkeypatch):
     with pytest.raises(WrongArgumentGiven):
         # missing percentage
-        push_apk(APKS, SERVICE_ACCOUNT, credentials, 'rollout', AnyPackageNamesCheck())
+        push_apk(APKS, SERVICE_ACCOUNT, credentials, 'rollout', [])
 
     valid_percentage = 1
     invalid_track = 'production'
     with pytest.raises(WrongArgumentGiven):
-        push_apk(APKS, SERVICE_ACCOUNT, credentials, invalid_track, AnyPackageNamesCheck(), rollout_percentage=valid_percentage)
+        push_apk(APKS, SERVICE_ACCOUNT, credentials, invalid_track, [], rollout_percentage=valid_percentage)
 
 
 def test_valid_rollout_percentage(edit_service_mock, monkeypatch):
     set_up_mocks(monkeypatch, edit_service_mock)
     valid_percentage = 50
 
-    push_apk(APKS, SERVICE_ACCOUNT, credentials, 'rollout', AnyPackageNamesCheck(), rollout_percentage=valid_percentage)
+    push_apk(APKS, SERVICE_ACCOUNT, credentials, 'rollout', [], rollout_percentage=valid_percentage)
     edit_service_mock.update_track.assert_called_once_with('rollout', ['0', '1'], valid_percentage)
     edit_service_mock.update_track.reset_mock()
 
@@ -111,7 +110,7 @@ def test_get_ordered_version_codes():
 def test_upload_apk(edit_service_mock, monkeypatch):
     set_up_mocks(monkeypatch, edit_service_mock)
 
-    push_apk(APKS, SERVICE_ACCOUNT, credentials, 'alpha', AnyPackageNamesCheck())
+    push_apk(APKS, SERVICE_ACCOUNT, credentials, 'alpha', [])
 
     for apk_file in (apk_arm, apk_x86):
         edit_service_mock.upload_apk.assert_any_call(apk_file.name)
@@ -165,7 +164,7 @@ def test_push_apk_tunes_down_logs(monkeypatch):
     monkeypatch.setattr('mozapkpublisher.push_apk._split_apk_metadata_per_package_name', MagicMock())
     monkeypatch.setattr('mozapkpublisher.push_apk._upload_apks', MagicMock())
 
-    push_apk(APKS, SERVICE_ACCOUNT, credentials, 'alpha', AnyPackageNamesCheck(), contact_google_play=False)
+    push_apk(APKS, SERVICE_ACCOUNT, credentials, 'alpha', [], contact_google_play=False)
 
     main_logging_mock.init.assert_called_once_with()
 
@@ -194,7 +193,7 @@ def test_main(monkeypatch):
         '--track', 'rollout',
         '--service-account', '[email protected]',
         '--credentials', file,
-        '--skip-check-package-names',
+        '--expected-package-name', 'org.mozilla.fennec_aurora',
         file
     ]
 

version.txt

@@ -1 +1 @@
-4.0.0
+4.1.0