Deprecation: Ignore --skip-package-validation option for `app-store-c…

…onnect publish` (#174)

* Ignore --skip-package-validation option for app-store-connect publish

* Fix tests

Update expected default behaviour for app-store-connect publish
with respect changed package validation logic.

* Update docs

* Update changelog and version

CHANGELOG.md

@@ -1,3 +1,13 @@
+Version 0.14.0
+-------------
+
+**Deprecations**
+- Option `--skip-package-validation` of `app-store-connect publish` is ignored and shows a deprecation warning. [PR #174](https://github.com/codemagic-ci-cd/cli-tools/pull/170)
+
+**Features**
+- Change `app-store-connect publish` not to run package validation by default before uploading it to App Store Connect. [PR #174](https://github.com/codemagic-ci-cd/cli-tools/pull/170)
+- Add new option `--enable-package-validation` to action `app-store-connect publish` that turns on package validation before it is uploaded to App Store Connect. [PR #174](https://github.com/codemagic-ci-cd/cli-tools/pull/170)
+
 Version 0.13.2
 -------------
 

docs/app-store-connect/publish.md

@@ -17,6 +17,7 @@ app-store-connect publish [-h] [--log-stream STREAM] [--no-color] [--version] [-
     [--path APPLICATION_PACKAGE_PATH_PATTERNS]
     [--apple-id APPLE_ID]
     [--password APP_SPECIFIC_PASSWORD]
+    [--enable-package-validation]
     [--skip-package-validation]
     [--skip-package-upload]
     [--max-build-processing-wait MAX_BUILD_PROCESSING_WAIT]
@@ -56,10 +57,14 @@ App Store Connect username used for application package validation and upload if
 
 
 App-specific password used for application package validation and upload if App Store Connect API Key is not specified. Used together with --apple-id and should match pattern `abcd-abcd-abcd-abcd`. Create an app-specific password in the Security section of your Apple ID account. Learn more from https://support.apple.com/en-us/HT204397. If not given, the value will be checked from the environment variable `APP_SPECIFIC_PASSWORD`. Alternatively to entering `APP_SPECIFIC_PASSWORD` in plaintext, it may also be specified using the `@env:` prefix followed by an environment variable name, or the `@file:` prefix followed by a path to the file containing the value. Example: `@env:<variable>` uses the value in the environment variable named `<variable>`, and `@file:<file_path>` uses the value from the file at `<file_path>`.
+##### `--enable-package-validation, -ev`
+
+
+Validate package before uploading it to App Store Connect. Use this switch to enable running `altool --validate-app` before uploading the package to App Store connect. If not given, the value will be checked from the environment variable `APP_STORE_CONNECT_ENABLE_PACKAGE_VALIDATION`.
 ##### `--skip-package-validation, -sv`
 
 
-Skip package validation before uploading it to App Store Connect. Use this switch to opt out from running `altool --validate-app` before uploading package to App Store connect. If not given, the value will be checked from the environment variable `APP_STORE_CONNECT_SKIP_PACKAGE_VALIDATION`.
+Deprecated. Starting from version `0.14.0` package validation before uploading it to App Store Connect is disabled by default. If not given, the value will be checked from the environment variable `APP_STORE_CONNECT_SKIP_PACKAGE_VALIDATION`.
 ##### `--skip-package-upload, -su`
 
 

src/codemagic/__version__.py

@@ -1,5 +1,5 @@
 __title__ = 'codemagic-cli-tools'
 __description__ = 'CLI tools used in Codemagic builds'
-__version__ = '0.13.2'
+__version__ = '0.14.0'
 __url__ = 'https://github.com/codemagic-ci-cd/cli-tools'
 __licence__ = 'GNU General Public License v3.0'

src/codemagic/tools/_app_store_connect/actions/publish_action.py

@@ -267,7 +267,8 @@ def publish(
             submit_to_testflight: Optional[bool] = None,
             submit_to_app_store: Optional[bool] = None,
             # Package upload and validation arguments
-            skip_package_validation: Optional[bool] = None,
+            skip_package_validation: Optional[bool] = None,  # Deprecated
+            enable_package_validation: Optional[bool] = None,
             skip_package_upload: Optional[bool] = None,
             altool_retries_count: Optional[Types.AltoolRetriesCount] = None,
             altool_retry_wait: Optional[Types.AltoolRetryWait] = None,
@@ -277,6 +278,9 @@ def publish(
         """
         Publish application packages to App Store, submit them to Testflight, and release to the groups of beta testers
         """
+        if bool(skip_package_validation):
+            self._log_skip_validation_deprecation()
+
         self._validate_publishing_arguments(
             apple_id,
             app_specific_password,
@@ -294,7 +298,7 @@ def publish(
                 self._publish_application_package(
                     altool,
                     application_package,
-                    skip_package_validation,
+                    enable_package_validation,
                     skip_package_upload,
                     Types.AltoolRetriesCount.resolve_value(altool_retries_count),
                     Types.AltoolRetryWait.resolve_value(altool_retry_wait),
@@ -318,11 +322,22 @@ def publish(
         if failed_packages:
             raise AppStoreConnectError(f'Failed to publish {", ".join(failed_packages)}')
 
+    def _log_skip_validation_deprecation(self):
+        flag = PublishArgument.SKIP_PACKAGE_VALIDATION.flag
+        message = (
+            f'{Colors.YELLOW("Deprecation warning!")} Support for {Colors.WHITE(flag)} is deprecated'
+            'and this flag will be removed in future releases.'
+            '\n'
+            f'Starting from version 0.14.0 package validation '
+            f'is disabled by default and using {Colors.WHITE(flag)} has no effect.'
+        )
+        self.logger.info(message)
+
     def _publish_application_package(
             self,
             altool: Altool,
             application_package: Union[Ipa, MacOsPackage],
-            skip_package_validation: Optional[bool],
+            enable_package_validation: Optional[bool],
             skip_package_upload: Optional[bool],
             altool_retries: int,
             altool_retry_wait: float,
@@ -333,10 +348,8 @@ def _publish_application_package(
         self.logger.info(Colors.BLUE('\nPublish "%s" to App Store Connect'), application_package.path)
         self.logger.info(application_package.get_text_summary())
 
-        if not bool(skip_package_validation):
+        if bool(enable_package_validation):
             self._validate_artifact_with_altool(altool, application_package.path, altool_retries, altool_retry_wait)
-        else:
-            self.logger.info(Colors.YELLOW('\nSkip validating "%s" for App Store Connect'), application_package.path)
 
         if not bool(skip_package_upload):
             self._upload_artifact_with_altool(altool, application_package.path, altool_retries, altool_retry_wait)

src/codemagic/tools/_app_store_connect/arguments.py

@@ -98,6 +98,10 @@ class AppStoreConnectSkipPackageValidation(cli.TypedCliArgument[bool]):
         argument_type = bool
         environment_variable_key = 'APP_STORE_CONNECT_SKIP_PACKAGE_VALIDATION'
 
+    class AppStoreConnectEnablePackageValidation(cli.TypedCliArgument[bool]):
+        argument_type = bool
+        environment_variable_key = 'APP_STORE_CONNECT_ENABLE_PACKAGE_VALIDATION'
+
     class AppStoreConnectSkipPackageUpload(cli.TypedCliArgument[bool]):
         argument_type = bool
         environment_variable_key = 'APP_STORE_CONNECT_SKIP_PACKAGE_UPLOAD'
@@ -684,14 +688,28 @@ class PublishArgument(cli.Argument):
         ),
         argparse_kwargs={'required': False},
     )
+    ENABLE_PACKAGE_VALIDATION = cli.ArgumentProperties(
+        key='enable_package_validation',
+        flags=('--enable-package-validation', '-ev'),
+        type=Types.AppStoreConnectEnablePackageValidation,
+        description=(
+            'Validate package before uploading it to App Store Connect. '
+            'Use this switch to enable running `altool --validate-app` before uploading '
+            'the package to App Store connect'
+        ),
+        argparse_kwargs={
+            'required': False,
+            'action': 'store_true',
+        },
+    )
     SKIP_PACKAGE_VALIDATION = cli.ArgumentProperties(
         key='skip_package_validation',
         flags=('--skip-package-validation', '-sv'),
         type=Types.AppStoreConnectSkipPackageValidation,
         description=(
-            'Skip package validation before uploading it to App Store Connect. '
-            'Use this switch to opt out from running `altool --validate-app` before uploading '
-            'package to App Store connect'
+            f'{Colors.BOLD("Deprecated")}. '
+            f'Starting from version `0.14.0` package validation before '
+            'uploading it to App Store Connect is disabled by default.'
         ),
         argparse_kwargs={
             'required': False,
@@ -1241,6 +1259,7 @@ class ArgumentGroups:
         BuildArgument.PROCESSING_STATE,
     )
     PACKAGE_UPLOAD_ARGUMENTS = (
+        PublishArgument.ENABLE_PACKAGE_VALIDATION,
         PublishArgument.SKIP_PACKAGE_VALIDATION,
         PublishArgument.SKIP_PACKAGE_UPLOAD,
     )

tests/tools/app_store_connect/test_publish_action.py

@@ -127,7 +127,7 @@ def test_publish_action_testflight_with_localization(publishing_namespace_kwargs
         )
 
         mock_get_packages.assert_called_with(patterns)
-        mock_validate.assert_called()
+        mock_validate.assert_not_called()
         mock_upload.assert_called()
         mock_wait_until_build_is_processed.assert_called_with(build, Types.MaxBuildProcessingWait.default_value)
         mock_submit_to_testflight.assert_called_with(build.id, max_build_processing_wait=0)
@@ -171,7 +171,7 @@ def test_publish_action_app_store_submit(publishing_namespace_kwargs):
         )
 
         mock_get_packages.assert_called_with(patterns)
-        mock_validate.assert_called()
+        mock_validate.assert_not_called()
         mock_upload.assert_called()
         mock_wait_until_build_is_processed.assert_called_with(build, 5)
         mock_submit_to_testflight.assert_not_called()
@@ -199,12 +199,12 @@ def test_publish_action_app_store_submit(publishing_namespace_kwargs):
 
 
 @mock.patch('codemagic.tools._app_store_connect.actions.publish_action.Altool')
-@pytest.mark.parametrize('skip_package_validation, should_validate', [
-    (True, False),
-    (False, True),
-    (None, True),
+@pytest.mark.parametrize('enable_package_validation, should_validate', [
+    (True, True),
+    (False, False),
+    (None, False),
 ])
-def test_publish_action_skip_validation(_mock_altool, namespace_kwargs, skip_package_validation, should_validate):
+def test_publish_action_enable_validation(_mock_altool, namespace_kwargs, enable_package_validation, should_validate):
     asc = AppStoreConnect.from_cli_args(argparse.Namespace(**namespace_kwargs))
     with mock.patch.object(AppStoreConnect, 'find_paths') as mock_find_paths, \
             mock.patch.object(AppStoreConnect, '_get_publishing_application_packages') as mock_get_packages, \
@@ -218,7 +218,7 @@ def test_publish_action_skip_validation(_mock_altool, namespace_kwargs, skip_pac
             application_package_path_patterns=[ipa_path],
             apple_id='name@example.com',
             app_specific_password=Types.AppSpecificPassword('xxxx-yyyy-zzzz-wwww'),
-            skip_package_validation=skip_package_validation,
+            enable_package_validation=enable_package_validation,
         )
 
         mock_upload.assert_called()
@@ -281,7 +281,7 @@ def test_add_build_to_beta_groups(publishing_namespace_kwargs):
         )
 
         mock_get_packages.assert_called_with(patterns)
-        mock_validate.assert_called()
+        mock_validate.assert_not_called()
         mock_upload.assert_called()
         mock_wait_until_build_is_processed.assert_called_with(build, Types.MaxBuildProcessingWait.default_value)
         mock_submit_to_testflight.assert_called_with(build.id, max_build_processing_wait=0)