Moves "track" and "connection" options to the api-level

CHANGELOG.md

@@ -6,6 +6,11 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Changed
 * Separates read-only logic from logic that requires a transaction
+* `--service-account` and `--credentials` is now only required if `--do-not-contact-google-play` isn't provided
+* `push_apk` and `update_apk_description` now accept types for `track` and `connection`, rather than mutually-exclusive primitive parameters 
+
+### Fixed
+* `check_rollout` no longer ignores `--do-not-contact-google-play`
 
 ## [4.1.0] - 2019-07-10
 ### Removed

mozapkpublisher/check_rollout.py

@@ -9,7 +9,7 @@
 
 from argparse import ArgumentParser
 from mozapkpublisher.common.googleplay import add_general_google_play_arguments, \
-    ReadOnlyGooglePlay, GooglePlayConnection
+    ReadOnlyGooglePlay, connection_for_options
 
 DAY = 24 * 60 * 60
 
@@ -18,7 +18,7 @@
 
 def check_rollout(google_play, days):
     """Check if package_name has a release on staged rollout for too long"""
-    track_status = google_play.get_track_status(track='production')
+    track_status = google_play.get_rollout_status()
     releases = track_status['releases']
     for release in releases:
         if release['status'] == 'inProgress':
@@ -39,11 +39,10 @@ def main():
     parser.add_argument('--days', help='The time before we warn about incomplete staged rollout of a release (default: 7)',
                         type=int, default=7)
     config = parser.parse_args()
+    connection = connection_for_options(config.contact_google_play, config.service_account,
+                                        config.google_play_credentials_file)
 
-    google_play = ReadOnlyGooglePlay.create(GooglePlayConnection.open(
-        config.service_account,
-        config.google_play_credentials_file.name
-    ), 'org.mozilla.firefox')
+    google_play = ReadOnlyGooglePlay.create(connection, 'org.mozilla.firefox')
     for (release, age) in check_rollout(google_play, config.days):
         print('fennec {} is on staged rollout at {}% but it shipped {} days ago'.format(
               release['name'], int(release['userFraction'] * 100), int(age / DAY)))

mozapkpublisher/common/googleplay.py

@@ -30,10 +30,33 @@
 logger = logging.getLogger(__name__)
 
 
+class RolloutTrack:
+    def __init__(self, percentage):
+        if not (1.0 >= percentage > 0):
+            raise ValueError('Rollout percentage must be (0.0, 1.0]')
+        self.name = 'rollout'
+        self.percentage = percentage
+
+    def __eq__(self, other):
+        if isinstance(other, RolloutTrack):
+            return self.name == other.name
+        return False
+
+
+class StaticTrack:
+    def __init__(self, name):
+        self.name = name
+
+    def __eq__(self, other):
+        if isinstance(other, StaticTrack):
+            return self.name == other.name
+        return False
+
+
 def add_general_google_play_arguments(parser):
-    parser.add_argument('--service-account', help='The service account email', required=True)
+    parser.add_argument('--service-account', help='The service account email')
     parser.add_argument('--credentials', dest='google_play_credentials_file', type=argparse.FileType(mode='rb'),
-                        help='The p12 authentication file', required=True)
+                        help='The p12 authentication file')
 
     parser.add_argument('--commit', action='store_true',
                         help='Commit changes onto Google Play. This action cannot be reverted.')
@@ -98,15 +121,21 @@ def get_edit_resource():
         update_mock.update = lambda *args, **kwargs: _ExecuteDummy('fake-update-response')
         edit_service_mock.tracks = lambda *args, **kwargs: update_mock
         edit_service_mock.listings = lambda *args, **kwargs: update_mock
-        edit_service_mock.apklistings = lambda *args, **kwargs: update_mock
 
         return edit_service_mock
 
 
 def connection_for_options(contact_google_play, service_account, credentials_file):
     if contact_google_play:
+        if service_account is None or credentials_file is None:
+            raise WrongArgumentGiven("Either provide '--service-account' and '--credentials', or avoid communication "
+                                     "with the real Google Play with '--do-not-contact-google-play'")
         return GooglePlayConnection.open(service_account, credentials_file.name)
     else:
+        if service_account is not None or credentials_file is not None:
+            raise WrongArgumentGiven("When using '--do-not-contact-google-play', do not use '--service-account' or "
+                                     "'--credentials'")
+
         logger.warning('Not a single request to Google Play will be made, since `contact_google_play` was set')
         return MockGooglePlayConnection()
 
@@ -122,13 +151,13 @@ def __init__(self, edit_resource, edit_id, package_name):
         self._edit_id = edit_id
         self._package_name = package_name
 
-    def get_track_status(self, track):
+    def get_rollout_status(self):
         response = self._edit_resource.tracks().get(
             editId=self._edit_id,
-            track=track,
+            track='production',
             packageName=self._package_name
         ).execute()
-        logger.debug('Track "{}" has status: {}'.format(track, response))
+        logger.debug('Track "production" has status: {}'.format(response))
         return response
 
     @staticmethod
@@ -177,25 +206,21 @@ def upload_apk(self, apk_path):
                     return
             raise
 
-    def update_track(self, track, version_codes, rollout_percentage=None):
+    def update_track(self, track, version_codes):
         body = {
             u'releases': [{
                 u'status': 'completed',
                 u'versionCodes': version_codes,
             }],
         }
-        if rollout_percentage is not None:
-            if rollout_percentage < 0 or rollout_percentage > 100:
-                raise WrongArgumentGiven(
-                    'rollout percentage must be between 0 and 100. Value given: {}'.format(
-                        rollout_percentage))
 
-            body[u'userFraction'] = rollout_percentage / 100.0  # Ensure float in Python 2
+        if isinstance(track, RolloutTrack):
+            body[u'userFraction'] = track.percentage
 
         response = self._edit_resource.tracks().update(
-            editId=self._edit_id, track=track, packageName=self._package_name, body=body
+            editId=self._edit_id, track=track.name, packageName=self._package_name, body=body
         ).execute()
-        logger.info('Track "{}" updated with: {}'.format(track, body))
+        logger.info('Track "{}" updated with: {}'.format(track.name, body))
         logger.debug('Track update response: {}'.format(response))
 
     def update_listings(self, language, title, full_description, short_description):
@@ -210,26 +235,16 @@ def update_listings(self, language, title, full_description, short_description):
         logger.info(u'Listing for language "{}" has been updated with: {}'.format(language, body))
         logger.debug(u'Listing response: {}'.format(response))
 
-    def update_whats_new(self, language, apk_version_code, whats_new):
-        response = self._edit_resource.apklistings().update(
-            editId=self._edit_id, packageName=self._package_name, language=language,
-            apkVersionCode=apk_version_code, body={'recentChanges': whats_new}
-        ).execute()
-        logger.info(u'What\'s new listing for ("{}", "{}") has been updated to: "{}"'.format(
-            language, apk_version_code, whats_new
-        ))
-        logger.debug(u'Apk listing response: {}'.format(response))
-
     @staticmethod
     @contextmanager
-    def transaction(connection, package_name, do_not_commit=False):
+    def transaction(connection, package_name, commit):
         edit_resource = connection.get_edit_resource()
         edit_id = edit_resource.insert(body={}, packageName=package_name).execute()['id']
         google_play = WritableGooglePlay(edit_resource, edit_id, package_name)
         yield google_play
-        if do_not_commit:
-            logger.warning('Transaction not committed, since `do_not_commit` was set')
-        else:
+        if commit:
             edit_resource.commit(editId=edit_id, packageName=package_name)
             logger.info('Changes committed')
             logger.debug('edit_id "{}" for "{}" has been committed'.format(edit_id, package_name))
+        else:
+            logger.warning('Transaction not committed, since `do_not_commit` was set')

mozapkpublisher/push_apk.py

@@ -6,20 +6,17 @@
 from mozapkpublisher.common import googleplay, main_logging
 from mozapkpublisher.common.apk import add_apk_checks_arguments, extract_and_check_apks_metadata
 from mozapkpublisher.common.exceptions import WrongArgumentGiven
-from mozapkpublisher.common.googleplay import WritableGooglePlay, connection_for_options
+from mozapkpublisher.common.googleplay import WritableGooglePlay, connection_for_options, RolloutTrack, StaticTrack
 
 logger = logging.getLogger(__name__)
 
 
 def push_apk(
     apks,
-    service_account,
-    google_play_credentials_file,
+    connection,
     track,
     expected_package_names,
-    rollout_percentage=None,
     commit=True,
-    contact_google_play=True,
     skip_check_ordered_version_codes=False,
     skip_check_multiple_locales=False,
     skip_check_same_locales=False,
@@ -29,16 +26,10 @@ def push_apk(
 
     Args:
         apks: list of APK files
-        service_account: Google Play service account
-        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
+        connection (typing.Union[GooglePlayConnection, MockGooglePlayConnection]): connection to Google Play
+        track (typing.Union[StaticTrack, RolloutTrack]): Google Play track to deploy to (e.g.: StaticTrack("nightly")).
         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
-        contact_google_play (bool): `False` to avoid communicating with Google Play. Useful if you're using mock
-            credentials.
         skip_checks_fennec (bool): skip Fennec-specific checks
         skip_check_same_locales (bool): skip check to ensure all APKs have the same locales
         skip_check_multiple_locales (bool): skip check to ensure all APKs have more than one locale
@@ -49,11 +40,6 @@ def push_apk(
     # We want to tune down some logs, even when push_apk() isn't called from the command line
     main_logging.init()
 
-    if track == 'rollout' and rollout_percentage is None:
-        raise WrongArgumentGiven("When using track='rollout', rollout percentage must be provided too")
-    if rollout_percentage is not None and track != 'rollout':
-        raise WrongArgumentGiven("When using rollout-percentage, track must be set to rollout")
-
     apks_metadata_per_paths = extract_and_check_apks_metadata(
         apks,
         expected_package_names,
@@ -63,22 +49,16 @@ def push_apk(
         skip_check_ordered_version_codes,
     )
 
-    # TODO make programmatic usage of this library provide a "GooglePlayConnection" object, rather
-    # than having to provide redundant information like "service_account" and "credentials" when
-    # "contact_google_play" is false
-    connection = connection_for_options(contact_google_play, service_account, google_play_credentials_file)
-
     # Each distinct product must be uploaded in different Google Play transaction, so we split them
     # by package name here.
     split_apk_metadata = _split_apk_metadata_per_package_name(apks_metadata_per_paths)
     for (package_name, apks_metadata) in split_apk_metadata.items():
-        with WritableGooglePlay.transaction(connection, package_name,
-                                            do_not_commit=not commit) as google_play:
+        with WritableGooglePlay.transaction(connection, package_name, commit) as google_play:
             for path, metadata in apks_metadata_per_paths.items():
                 google_play.upload_apk(path)
 
             all_version_codes = _get_ordered_version_codes(apks_metadata_per_paths)
-            google_play.update_track(track, all_version_codes, rollout_percentage)
+            google_play.update_track(track, all_version_codes)
 
 
 def _split_apk_metadata_per_package_name(apks_metadata_per_paths):
@@ -119,16 +99,25 @@ def main():
 
     config = parser.parse_args()
 
+    if config.track == 'rollout':
+        if config.rollout_percentage is None:
+            raise WrongArgumentGiven("When using '--track rollout', '--rollout-percentage' must be provided too")
+        track = RolloutTrack(config.rollout_percentage / 100.0)
+    else:
+        if config.rollout_percentage is not None:
+            raise WrongArgumentGiven("When using '--rollout-percentage', you must have '--track rollout'")
+        track = StaticTrack(config.track)
+
+    connection = connection_for_options(config.contact_google_play, config.service_account,
+                                        config.google_play_credentials_file)
+
     try:
         push_apk(
             config.apks,
-            config.service_account,
-            config.google_play_credentials_file,
-            config.track,
+            connection,
+            track,
             config.expected_package_names,
-            config.rollout_percentage,
             config.commit,
-            config.contact_google_play,
             config.skip_check_ordered_version_codes,
             config.skip_check_multiple_locales,
             config.skip_check_same_locales,

mozapkpublisher/test/common/test_googleplay.py

@@ -13,7 +13,7 @@
 from mozapkpublisher.common.exceptions import WrongArgumentGiven
 from mozapkpublisher.common.googleplay import add_general_google_play_arguments, \
     WritableGooglePlay, MockGooglePlayConnection, ReadOnlyGooglePlay, GooglePlayConnection, \
-    connection_for_options
+    connection_for_options, StaticTrack, RolloutTrack
 from mozapkpublisher.test import does_not_raise
 
 
@@ -30,10 +30,29 @@ def test_add_general_google_play_arguments():
     assert config.service_account == 'dummy@dummy'
 
 
+@pytest.mark.parametrize('contact_google_play,service_account,credentials_file', (
+        (False, 'account', None),
+        (False, None, 'file'),
+        (False, 'account', 'file'),
+        (True, None, None),
+        (True, 'account', None),
+        (True, None, 'file'),
+))
+def test_connection_for_options_invalid_options(contact_google_play, service_account, credentials_file):
+    with pytest.raises(WrongArgumentGiven):
+        connection_for_options(contact_google_play, service_account, credentials_file)
+
+
 @patch.object(googleplay, 'MockGooglePlayConnection')
-def test_connection_for_options_contact(mock):
-    connection_for_options(False, '', MagicMock)
-    mock.assert_called_with()
+def test_connection_for_options_do_not_contact(mock):
+    connection_for_options(False, None, None)
+    mock.assert_called()
+
+
+@patch.object(googleplay.GooglePlayConnection, 'open')
+def test_connection_for_options_do_contact(mock):
+    connection_for_options(True, '', MagicMock())
+    mock.assert_called()
 
 
 @patch.object(googleplay.ServiceAccountCredentials, 'from_p12_keyfile')
@@ -68,7 +87,7 @@ def test_writable_google_play_commit_transaction():
     connection = MockGooglePlayConnection()
     mock_edits_resource = MagicMock()
     connection.get_edit_resource = lambda: mock_edits_resource
-    with WritableGooglePlay.transaction(connection, 'package.name') as _:
+    with WritableGooglePlay.transaction(connection, 'package.name', True) as _:
         pass
 
     mock_edits_resource.commit.assert_called_with(editId=ANY, packageName='package.name')
@@ -78,7 +97,7 @@ def test_writable_google_play_argument_do_not_commit_transaction():
     connection = MockGooglePlayConnection()
     mock_edits_resource = MagicMock()
     connection.get_edit_resource = lambda: mock_edits_resource
-    with WritableGooglePlay.transaction(connection, 'package.name', do_not_commit=True) as _:
+    with WritableGooglePlay.transaction(connection, 'package.name', False) as _:
         pass
 
     mock_edits_resource.commit.assert_not_called()
@@ -109,7 +128,7 @@ def test_get_track_status(edit_resource_mock):
     edit_resource_mock.tracks().get.reset_mock()
 
     google_play = ReadOnlyGooglePlay(edit_resource_mock, 1, 'dummy_package_name')
-    assert google_play.get_track_status(track='production') == release_data
+    assert google_play.get_rollout_status() == release_data
     edit_resource_mock.tracks().get.assert_called_once_with(
         editId=1,
         track='production',
@@ -178,7 +197,7 @@ def test_upload_apk_does_not_error_out_when_apk_is_already_published(edit_resour
 def test_update_track(edit_resource_mock):
     google_play = WritableGooglePlay(edit_resource_mock, 1, 'dummy_package_name')
 
-    google_play.update_track('alpha', ['2015012345', '2015012347'])
+    google_play.update_track(StaticTrack('alpha'), ['2015012345', '2015012347'])
     edit_resource_mock.tracks().update.assert_called_once_with(
         editId=google_play._edit_id,
         packageName='dummy_package_name',
@@ -192,7 +211,7 @@ def test_update_track(edit_resource_mock):
     )
 
     edit_resource_mock.tracks().update.reset_mock()
-    google_play.update_track('rollout', ['2015012345', '2015012347'], rollout_percentage=1)
+    google_play.update_track(RolloutTrack(0.01), ['2015012345', '2015012347'])
     edit_resource_mock.tracks().update.assert_called_once_with(
         editId=google_play._edit_id,
         packageName='dummy_package_name',
@@ -207,12 +226,10 @@ def test_update_track(edit_resource_mock):
     )
 
 
-@pytest.mark.parametrize('invalid_percentage', (-1, 101))
+@pytest.mark.parametrize('invalid_percentage', (-0.01, 0.0, 1.01))
 def test_update_track_should_refuse_wrong_percentage(edit_resource_mock, invalid_percentage):
-    google_play = WritableGooglePlay(edit_resource_mock, 1, 'dummy_package_name')
-
-    with pytest.raises(WrongArgumentGiven):
-        google_play.update_track('rollout', ['2015012345', '2015012347'], invalid_percentage)
+    with pytest.raises(ValueError):
+        RolloutTrack(invalid_percentage)
 
 
 def test_update_listings(edit_resource_mock):
@@ -234,16 +251,3 @@ def test_update_listings(edit_resource_mock):
             'shortDescription': 'Short',
         }
     )
-
-
-def test_update_whats_new(edit_resource_mock):
-    google_play = WritableGooglePlay(edit_resource_mock, 1, 'dummy_package_name')
-
-    google_play.update_whats_new('en-GB', '2015012345', 'Check out this cool feature!')
-    edit_resource_mock.apklistings().update.assert_called_once_with(
-        editId=google_play._edit_id,
-        packageName='dummy_package_name',
-        language='en-GB',
-        apkVersionCode='2015012345',
-        body={'recentChanges': 'Check out this cool feature!'}
-    )

mozapkpublisher/test/test_check_rollout.py

@@ -22,7 +22,7 @@ def set_up_mocks(_requests_mock, tracks):
                         status_code=404)
 
     google_play_mock = create_autospec(googleplay.ReadOnlyGooglePlay)
-    google_play_mock.get_track_status.return_value = tracks
+    google_play_mock.get_rollout_status.return_value = tracks
     return google_play_mock
 
 

mozapkpublisher/test/test_push_apk.py

@@ -1,5 +1,7 @@
 from contextlib import contextmanager
 
+from mozapkpublisher.common.exceptions import WrongArgumentGiven
+
 import mozapkpublisher
 import os
 import pytest
@@ -10,7 +12,7 @@
 from tempfile import NamedTemporaryFile
 
 from mozapkpublisher.common import googleplay
-from mozapkpublisher.common.exceptions import WrongArgumentGiven
+from mozapkpublisher.common.googleplay import StaticTrack, RolloutTrack, MockGooglePlayConnection
 from mozapkpublisher.push_apk import (
     push_apk,
     main,
@@ -20,12 +22,9 @@
 from unittest.mock import patch
 
 
-credentials = NamedTemporaryFile()
 apk_x86 = NamedTemporaryFile()
 apk_arm = NamedTemporaryFile()
-
 APKS = [apk_x86, apk_arm]
-SERVICE_ACCOUNT = 'foo@developer.gserviceaccount.com'
 
 
 @pytest.fixture
@@ -82,26 +81,6 @@ def fake_transaction(_, __, do_not_commit):
     monkeypatch_.setattr('mozapkpublisher.push_apk.extract_and_check_apks_metadata', _metadata)
 
 
-def test_invalid_rollout_percentage():
-    with pytest.raises(WrongArgumentGiven):
-        # missing percentage
-        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, [], rollout_percentage=valid_percentage)
-
-
-def test_valid_rollout_percentage(writable_google_play_mock, monkeypatch):
-    set_up_mocks(monkeypatch, writable_google_play_mock)
-    valid_percentage = 50
-
-    push_apk(APKS, SERVICE_ACCOUNT, credentials, 'rollout', [], rollout_percentage=valid_percentage, contact_google_play=False)
-    writable_google_play_mock.update_track.assert_called_once_with('rollout', ['0', '1'], valid_percentage)
-    writable_google_play_mock.update_track.reset_mock()
-
-
 def test_get_ordered_version_codes():
     assert _get_ordered_version_codes({
         'x86': {
@@ -116,12 +95,12 @@ def test_get_ordered_version_codes():
 def test_upload_apk(writable_google_play_mock, monkeypatch):
     set_up_mocks(monkeypatch, writable_google_play_mock)
 
-    push_apk(APKS, SERVICE_ACCOUNT, credentials, 'alpha', [], contact_google_play=False)
+    push_apk(APKS, MockGooglePlayConnection(), StaticTrack('alpha'), [])
 
     for apk_file in (apk_arm, apk_x86):
         writable_google_play_mock.upload_apk.assert_any_call(apk_file.name)
 
-    writable_google_play_mock.update_track.assert_called_once_with('alpha', ['0', '1'], None)
+    writable_google_play_mock.update_track.assert_called_once_with(StaticTrack('alpha'), ['0', '1'])
 
 
 def test_get_distinct_package_name_apk_metadata():
@@ -168,7 +147,7 @@ def test_push_apk_tunes_down_logs(monkeypatch):
     monkeypatch.setattr('mozapkpublisher.push_apk.extract_and_check_apks_metadata', MagicMock())
     monkeypatch.setattr('mozapkpublisher.push_apk._split_apk_metadata_per_package_name', MagicMock())
 
-    push_apk(APKS, SERVICE_ACCOUNT, credentials, 'alpha', [], contact_google_play=False)
+    push_apk(APKS, MockGooglePlayConnection(), StaticTrack('alpha'), [])
 
     main_logging_mock.init.assert_called_once_with()
 
@@ -180,31 +159,49 @@ def test_main_bad_arguments_status_code(monkeypatch):
     assert exception.value.code == 2
 
 
-def test_main(monkeypatch):
-    incomplete_args = [
-        '--package-name', 'org.mozilla.fennec_aurora', '--track', 'alpha',
-        '--service-account', 'foo@developer.gserviceaccount.com',
-    ]
-
-    monkeypatch.setattr(sys, 'argv', incomplete_args)
+@pytest.mark.parametrize('flags', (
+        (['--track', 'rollout']),
+        (['--track', 'production', '--rollout-percentage', '50']),
+        (['--track', 'nightly', '--rollout-percentage', '1']),
+))
+def test_parse_invalid_track(monkeypatch, flags):
+    file = os.path.join(os.path.dirname(__file__), 'data', 'blob')
+    args = [
+        'script',
+        '--expected-package-name', 'org.mozilla.fennec_aurora', '--do-not-contact-google-play'
+    ] + flags + [file]
+    monkeypatch.setattr(sys, 'argv', args)
 
-    with pytest.raises(SystemExit):
+    with pytest.raises(WrongArgumentGiven):
         main()
 
+
+@pytest.mark.parametrize('flags,expected_track', (
+        (['--track', 'rollout', '--rollout-percentage', '50'], RolloutTrack(0.50)),
+        (['--track', 'production'], StaticTrack('production')),
+        (['--track', 'nightly'], StaticTrack('nightly')),
+))
+def test_parse_valid_track(monkeypatch, flags, expected_track):
     file = os.path.join(os.path.dirname(__file__), 'data', 'blob')
-    fail_manual_validation_args = [
+    args = [
         'script',
-        '--track', 'rollout',
+        '--expected-package-name', 'org.mozilla.fennec_aurora', '--do-not-contact-google-play'
+    ] + flags + [file]
+    monkeypatch.setattr(sys, 'argv', args)
+
+    with patch.object(mozapkpublisher.push_apk, 'push_apk') as mock_push_apk:
+        main()
+        mock_push_apk.assert_called_once()
+        assert mock_push_apk.call_args[0][2] == expected_track
+
+
+def test_main(monkeypatch):
+    incomplete_args = [
+        'script', '--expected-package-name', 'org.mozilla.fennec_aurora', '--track', 'alpha',
         '--service-account', 'foo@developer.gserviceaccount.com',
-        '--credentials', file,
-        '--expected-package-name', 'org.mozilla.fennec_aurora',
-        file
     ]
 
-    with patch.object(mozapkpublisher.push_apk, 'push_apk', wraps=mozapkpublisher.push_apk.push_apk) as mock_push_apk:
-        monkeypatch.setattr(sys, 'argv', fail_manual_validation_args)
-
-        with pytest.raises(SystemExit):
-            main()
+    monkeypatch.setattr(sys, 'argv', incomplete_args)
 
-        assert mock_push_apk.called
+    with pytest.raises(SystemExit):
+        main()

mozapkpublisher/test/test_update_apk_description.py

@@ -8,6 +8,8 @@
 from tempfile import NamedTemporaryFile
 
 from mozapkpublisher.common import googleplay, store_l10n
+from mozapkpublisher.common.exceptions import WrongArgumentGiven
+from mozapkpublisher.common.googleplay import MockGooglePlayConnection
 from mozapkpublisher.update_apk_description import main, update_apk_description
 
 
@@ -27,7 +29,7 @@ def test_update_apk_description_force_locale(monkeypatch):
     })
     monkeypatch.setattr(store_l10n, '_translate_moz_locate_into_google_play_one', lambda locale: 'google_play_locale')
 
-    update_apk_description('org.mozilla.firefox_beta', 'en-US', False, 'foo@developer.gserviceaccount.com', credentials, False)
+    update_apk_description(MockGooglePlayConnection(), 'org.mozilla.firefox_beta', 'en-US', False)
 
     google_play_mock.update_listings.assert_called_once_with(
         'google_play_locale',
@@ -41,13 +43,14 @@ def test_update_apk_description_force_locale(monkeypatch):
 
 def test_main(monkeypatch):
     incomplete_args = [
+        'script',
         '--package-name', 'org.mozilla.firefox_beta',
         '--service-account', 'foo@developer.gserviceaccount.com',
     ]
 
     monkeypatch.setattr(sys, 'argv', incomplete_args)
 
-    with pytest.raises(SystemExit):
+    with pytest.raises(WrongArgumentGiven):
         main()
 
     complete_args = [
@@ -57,5 +60,6 @@ def test_main(monkeypatch):
         '--credentials', os.path.join(os.path.dirname(__file__), 'data', 'blob')
     ]
     monkeypatch.setattr(sys, 'argv', complete_args)
-    monkeypatch.setattr(mozapkpublisher.update_apk_description, 'update_apk_description', lambda _, __, ___, ____, _____, ______: None)
+    monkeypatch.setattr(mozapkpublisher.update_apk_description, 'update_apk_description', lambda _, __, ___, ____: None)
+    monkeypatch.setattr(mozapkpublisher.update_apk_description, 'connection_for_options', lambda _, __, ___: None)
     main()

mozapkpublisher/update_apk_description.py

@@ -10,10 +10,8 @@
 logger = logging.getLogger(__name__)
 
 
-def update_apk_description(package_name, force_locale, commit, service_account, google_play_credentials_file,
-                           contact_google_play):
-    connection = connection_for_options(contact_google_play, service_account, google_play_credentials_file)
-    with WritableGooglePlay.transaction(connection, package_name, do_not_commit=not commit) as google_play:
+def update_apk_description(connection, package_name, force_locale, commit):
+    with WritableGooglePlay.transaction(connection, package_name, commit) as google_play:
         moz_locales = [force_locale] if force_locale else None
         l10n_strings = store_l10n.get_translations_per_google_play_locale_code(package_name, moz_locales)
         create_or_update_listings(google_play, l10n_strings)
@@ -48,8 +46,9 @@ def main():
                         help='The Google play name of the app', required=True)
     parser.add_argument('--force-locale', help='Force to a specific locale (instead of all)')
     config = parser.parse_args()
-    update_apk_description(config.package_name, config.force_locale, config.commit, config.service_account,
-                           config.google_play_credentials_file, config.contact_google_play)
+    connection = connection_for_options(config.contact_google_play, config.service_account,
+                                        config.google_play_credentials_file)
+    update_apk_description(connection, config.package_name, config.force_locale, config.commit)
 
 
 __name__ == '__main__' and main()