From 2bb8db1f651c71e046b263fa506d6dbc6720e4a4 Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Wed, 20 Mar 2024 15:47:18 -0400 Subject: [PATCH] refactor(shorebird_cli): pull ExportOptionsPlist logic into new class (#1804) --- packages/shorebird_cli/bin/shorebird.dart | 2 + .../src/commands/patch/patch_ios_command.dart | 2 +- .../patch/patch_ios_framework_command.dart | 2 +- .../commands/release/release_ios_command.dart | 53 +---- packages/shorebird_cli/lib/src/ios.dart | 11 -- .../shorebird_cli/lib/src/platform/ios.dart | 153 +++++++++++++++ .../lib/src/platform/platform.dart | 1 + .../lib/src/shorebird_build_mixin.dart | 71 +------ .../build/build_ipa_command_test.dart | 7 + .../patch/patch_ios_command_test.dart | 5 + .../release/release_ios_command_test.dart | 182 ++---------------- packages/shorebird_cli/test/src/mocks.dart | 3 + .../test/src/platform/ios_test.dart | 158 +++++++++++++++ 13 files changed, 353 insertions(+), 297 deletions(-) delete mode 100644 packages/shorebird_cli/lib/src/ios.dart create mode 100644 packages/shorebird_cli/lib/src/platform/ios.dart create mode 100644 packages/shorebird_cli/lib/src/platform/platform.dart create mode 100644 packages/shorebird_cli/test/src/platform/ios_test.dart diff --git a/packages/shorebird_cli/bin/shorebird.dart b/packages/shorebird_cli/bin/shorebird.dart index 0c7ccd32..04e4f76d 100644 --- a/packages/shorebird_cli/bin/shorebird.dart +++ b/packages/shorebird_cli/bin/shorebird.dart @@ -16,6 +16,7 @@ import 'package:shorebird_cli/src/logger.dart'; import 'package:shorebird_cli/src/os/os.dart'; import 'package:shorebird_cli/src/patch_diff_checker.dart'; import 'package:shorebird_cli/src/platform.dart'; +import 'package:shorebird_cli/src/platform/platform.dart'; import 'package:shorebird_cli/src/shorebird_artifacts.dart'; import 'package:shorebird_cli/src/shorebird_env.dart'; import 'package:shorebird_cli/src/shorebird_flutter.dart'; @@ -45,6 +46,7 @@ Future main(List args) async { httpClientRef, idevicesyslogRef, iosDeployRef, + iosRef, javaRef, loggerRef, osInterfaceRef, diff --git a/packages/shorebird_cli/lib/src/commands/patch/patch_ios_command.dart b/packages/shorebird_cli/lib/src/commands/patch/patch_ios_command.dart index e2372d15..872d61e4 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patch_ios_command.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patch_ios_command.dart @@ -18,9 +18,9 @@ import 'package:shorebird_cli/src/engine_config.dart'; import 'package:shorebird_cli/src/executables/executables.dart'; import 'package:shorebird_cli/src/extensions/arg_results.dart'; import 'package:shorebird_cli/src/formatters/file_size_formatter.dart'; -import 'package:shorebird_cli/src/ios.dart'; import 'package:shorebird_cli/src/logger.dart'; import 'package:shorebird_cli/src/patch_diff_checker.dart'; +import 'package:shorebird_cli/src/platform/platform.dart'; import 'package:shorebird_cli/src/shorebird_artifact_mixin.dart'; import 'package:shorebird_cli/src/shorebird_artifacts.dart'; import 'package:shorebird_cli/src/shorebird_build_mixin.dart'; diff --git a/packages/shorebird_cli/lib/src/commands/patch/patch_ios_framework_command.dart b/packages/shorebird_cli/lib/src/commands/patch/patch_ios_framework_command.dart index 054811f3..557f099c 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patch_ios_framework_command.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patch_ios_framework_command.dart @@ -17,9 +17,9 @@ import 'package:shorebird_cli/src/doctor.dart'; import 'package:shorebird_cli/src/engine_config.dart'; import 'package:shorebird_cli/src/executables/aot_tools.dart'; import 'package:shorebird_cli/src/formatters/file_size_formatter.dart'; -import 'package:shorebird_cli/src/ios.dart'; import 'package:shorebird_cli/src/logger.dart'; import 'package:shorebird_cli/src/patch_diff_checker.dart'; +import 'package:shorebird_cli/src/platform/platform.dart'; import 'package:shorebird_cli/src/shorebird_artifact_mixin.dart'; import 'package:shorebird_cli/src/shorebird_artifacts.dart'; import 'package:shorebird_cli/src/shorebird_build_mixin.dart'; diff --git a/packages/shorebird_cli/lib/src/commands/release/release_ios_command.dart b/packages/shorebird_cli/lib/src/commands/release/release_ios_command.dart index b7ddf22b..dda485eb 100644 --- a/packages/shorebird_cli/lib/src/commands/release/release_ios_command.dart +++ b/packages/shorebird_cli/lib/src/commands/release/release_ios_command.dart @@ -12,6 +12,7 @@ import 'package:shorebird_cli/src/config/config.dart'; import 'package:shorebird_cli/src/doctor.dart'; import 'package:shorebird_cli/src/extensions/arg_results.dart'; import 'package:shorebird_cli/src/logger.dart'; +import 'package:shorebird_cli/src/platform/platform.dart'; import 'package:shorebird_cli/src/shorebird_artifact_mixin.dart'; import 'package:shorebird_cli/src/shorebird_build_mixin.dart'; import 'package:shorebird_cli/src/shorebird_env.dart'; @@ -20,9 +21,6 @@ import 'package:shorebird_cli/src/shorebird_validator.dart'; import 'package:shorebird_cli/src/validators/validators.dart'; import 'package:shorebird_code_push_client/shorebird_code_push_client.dart'; -const exportMethodArgName = 'export-method'; -const exportOptionsPlistArgName = 'export-options-plist'; - /// {@template release_ios_command} /// `shorebird release ios` /// Create new app releases for iOS. @@ -132,32 +130,14 @@ make smaller updates to your app. ); } - final exportPlistArg = results[exportOptionsPlistArgName] as String?; - if (exportPlistArg != null && results.wasParsed(exportMethodArgName)) { - logger.err( - '''Cannot specify both --$exportMethodArgName and --$exportOptionsPlistArgName.''', - ); + final File exportOptionsPlist; + try { + exportOptionsPlist = ios.exportOptionsPlistFromArgs(results); + } catch (error) { + logger.err('$error'); return ExitCode.usage.code; } - final File? exportOptionsPlist; - if (exportPlistArg != null) { - exportOptionsPlist = File(exportPlistArg); - try { - _validateExportOptionsPlist(exportOptionsPlist); - } catch (error) { - logger.err('$error'); - return ExitCode.usage.code; - } - } else if (results.wasParsed(exportMethodArgName)) { - final exportMethod = ExportMethod.values.firstWhere( - (element) => element.argName == results[exportMethodArgName] as String, - ); - exportOptionsPlist = createExportOptionsPlist(exportMethod: exportMethod); - } else { - exportOptionsPlist = null; - } - const releasePlatform = ReleasePlatform.ios; final flavor = results.findOption('flavor', argParser: argParser); final target = results.findOption('target', argParser: argParser); @@ -385,25 +365,4 @@ ${styleBold.wrap('Make sure to uncheck "Manage Version and Build Number", or els }, ); } - - /// Verifies that [exportOptionsPlistFile] exists and sets - /// manageAppVersionAndBuildNumber to false, which prevents Xcode from - /// changing the version number out from under us. - /// - /// Throws an exception if validation fails, exits normally if validation - /// succeeds. - void _validateExportOptionsPlist(File exportOptionsPlistFile) { - if (!exportOptionsPlistFile.existsSync()) { - throw Exception( - '''Export options plist file ${exportOptionsPlistFile.path} does not exist''', - ); - } - - final plist = Plist(file: exportOptionsPlistFile); - if (plist.properties['manageAppVersionAndBuildNumber'] != false) { - throw Exception( - '''Export options plist ${exportOptionsPlistFile.path} does not set manageAppVersionAndBuildNumber to false. This is required for shorebird to work.''', - ); - } - } } diff --git a/packages/shorebird_cli/lib/src/ios.dart b/packages/shorebird_cli/lib/src/ios.dart deleted file mode 100644 index 9a06f099..00000000 --- a/packages/shorebird_cli/lib/src/ios.dart +++ /dev/null @@ -1,11 +0,0 @@ -import 'package:mason_logger/mason_logger.dart'; -import 'package:shorebird_cli/src/logger.dart'; - -void showiOSStatusWarning() { - final url = link( - uri: Uri.parse('https://docs.shorebird.dev/status'), - ); - logger - ..warn('iOS support is beta. Some apps may run slower after patching.') - ..info('See $url for more information.'); -} diff --git a/packages/shorebird_cli/lib/src/platform/ios.dart b/packages/shorebird_cli/lib/src/platform/ios.dart new file mode 100644 index 00000000..9d61f272 --- /dev/null +++ b/packages/shorebird_cli/lib/src/platform/ios.dart @@ -0,0 +1,153 @@ +import 'dart:io'; + +import 'package:args/args.dart'; +import 'package:mason_logger/mason_logger.dart'; +import 'package:path/path.dart' as p; +import 'package:scoped/scoped.dart'; +import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart'; +import 'package:shorebird_cli/src/logger.dart'; + +const exportMethodArgName = 'export-method'; +const exportOptionsPlistArgName = 'export-options-plist'; + +void showiOSStatusWarning() { + final url = link( + uri: Uri.parse('https://docs.shorebird.dev/status'), + ); + logger + ..warn('iOS support is beta. Some apps may run slower after patching.') + ..info('See $url for more information.'); +} + +/// {@template export_method} +/// The method used to export the IPA. +/// {@endtemplate} +enum ExportMethod { + appStore('app-store', 'Upload to the App Store'), + adHoc( + 'ad-hoc', + ''' +Test on designated devices that do not need to be registered with the Apple developer account. + Requires a distribution certificate.''', + ), + development( + 'development', + '''Test only on development devices registered with the Apple developer account.''', + ), + enterprise( + 'enterprise', + 'Distribute an app registered with the Apple Developer Enterprise Program.', + ); + + /// {@macro export_method} + const ExportMethod(this.argName, this.description); + + /// The command-line argument name for this export method. + final String argName; + + /// A description of this method and how/when it should be used. + final String description; +} + +/// {@template invalid_export_options_plist_exception} +/// Thrown when an invalid export options plist is provided. +/// {@endtemplate} +class InvalidExportOptionsPlistException implements Exception { + /// {@macro invalid_export_options_plist_exception} + InvalidExportOptionsPlistException(this.message); + + final String message; +} + +/// A reference to a [Ios] instance. +final iosRef = create(Ios.new); + +/// The [Ios] instance available in the current zone. +Ios get ios => read(iosRef); + +class Ios { + File exportOptionsPlistFromArgs(ArgResults results) { + final exportPlistArg = results[exportOptionsPlistArgName] as String?; + if (exportPlistArg != null && results.wasParsed(exportMethodArgName)) { + throw ArgumentError( + '''Cannot specify both --$exportMethodArgName and --$exportOptionsPlistArgName.''', + ); + } + + final File? exportOptionsPlist; + if (exportPlistArg != null) { + exportOptionsPlist = File(exportPlistArg); + _validateExportOptionsPlist(exportOptionsPlist); + return exportOptionsPlist; + } + + final ExportMethod? exportMethod; + if (results.wasParsed(exportMethodArgName)) { + exportMethod = ExportMethod.values.firstWhere( + (element) => element.argName == results[exportMethodArgName] as String, + ); + } else { + exportMethod = null; + } + + return createExportOptionsPlist( + exportMethod: exportMethod ?? ExportMethod.appStore, + ); + } + + /// Creates an ExportOptions.plist file, which is used to tell xcodebuild to + /// not manage the app version and build number. If we don't do this, then + /// xcodebuild will increment the build number if it detects an App Store + /// Connect build with the same version and build number. This is a problem + /// for us when patching, as patches need to have the same version and build + /// number as the release they are patching. + /// See + /// https://developer.apple.com/forums/thread/690647?answerId=689925022#689925022 + File createExportOptionsPlist({ + ExportMethod? exportMethod, + }) { + exportMethod ??= ExportMethod.appStore; + final plistContents = ''' + + + + + manageAppVersionAndBuildNumber + + signingStyle + automatic + uploadBitcode + + method + ${exportMethod.argName} + + +'''; + final tempDir = Directory.systemTemp.createTempSync(); + final exportPlistFile = File(p.join(tempDir.path, 'ExportOptions.plist')) + ..createSync(recursive: true) + ..writeAsStringSync(plistContents); + return exportPlistFile; + } + + /// Verifies that [exportOptionsPlistFile] exists and sets + /// manageAppVersionAndBuildNumber to false, which prevents Xcode from + /// changing the version number out from under us. + /// + /// Throws an exception if validation fails, exits normally if validation + /// succeeds. + void _validateExportOptionsPlist(File exportOptionsPlistFile) { + if (!exportOptionsPlistFile.existsSync()) { + throw FileSystemException( + '''Export options plist file ${exportOptionsPlistFile.path} does not exist''', + ); + } + + final plist = Plist(file: exportOptionsPlistFile); + if (plist.properties['manageAppVersionAndBuildNumber'] != false) { + throw InvalidExportOptionsPlistException( + '''Export options plist ${exportOptionsPlistFile.path} does not set manageAppVersionAndBuildNumber to false. This is required for shorebird to work.''', + ); + } + } +} diff --git a/packages/shorebird_cli/lib/src/platform/platform.dart b/packages/shorebird_cli/lib/src/platform/platform.dart new file mode 100644 index 00000000..d3b2beb4 --- /dev/null +++ b/packages/shorebird_cli/lib/src/platform/platform.dart @@ -0,0 +1 @@ +export 'ios.dart'; diff --git a/packages/shorebird_cli/lib/src/shorebird_build_mixin.dart b/packages/shorebird_cli/lib/src/shorebird_build_mixin.dart index c747b729..8c3f869a 100644 --- a/packages/shorebird_cli/lib/src/shorebird_build_mixin.dart +++ b/packages/shorebird_cli/lib/src/shorebird_build_mixin.dart @@ -2,11 +2,11 @@ import 'dart:io'; import 'package:collection/collection.dart'; import 'package:mason_logger/mason_logger.dart'; -import 'package:path/path.dart' as p; import 'package:shorebird_cli/src/command.dart'; import 'package:shorebird_cli/src/engine_config.dart'; import 'package:shorebird_cli/src/logger.dart'; import 'package:shorebird_cli/src/os/operating_system_interface.dart'; +import 'package:shorebird_cli/src/platform/platform.dart'; import 'package:shorebird_cli/src/shorebird_artifacts.dart'; import 'package:shorebird_cli/src/shorebird_process.dart'; @@ -43,36 +43,6 @@ class BuildException implements Exception { final String message; } -/// {@template export_method} -/// The method used to export the IPA. -/// {@endtemplate} -enum ExportMethod { - appStore('app-store', 'Upload to the App Store'), - adHoc( - 'ad-hoc', - ''' -Test on designated devices that do not need to be registered with the Apple developer account. - Requires a distribution certificate.''', - ), - development( - 'development', - '''Test only on development devices registered with the Apple developer account.''', - ), - enterprise( - 'enterprise', - 'Distribute an app registered with the Apple Developer Enterprise Program.', - ); - - /// {@macro export_method} - const ExportMethod(this.argName, this.description); - - /// The command-line argument name for this export method. - final String argName; - - /// A description of this method and how/when it should be used. - final String description; -} - mixin ShorebirdBuildMixin on ShorebirdCommand { // This exists only so tests can get the full list. static const allAndroidArchitectures = { @@ -223,6 +193,8 @@ mixin ShorebirdBuildMixin on ShorebirdCommand { }) async { return _runShorebirdBuildCommand(() async { const executable = 'flutter'; + final exportOptionsPlistPath = + (exportOptionsPlist ?? ios.createExportOptionsPlist()).path; final arguments = [ 'build', 'ipa', @@ -230,8 +202,7 @@ mixin ShorebirdBuildMixin on ShorebirdCommand { if (flavor != null) '--flavor=$flavor', if (target != null) '--target=$target', if (!codesign) '--no-codesign', - if (codesign) - '''--export-options-plist=${(exportOptionsPlist ?? createExportOptionsPlist()).path}''', + if (codesign) '''--export-options-plist=$exportOptionsPlistPath''', ...results.rest, ]; @@ -262,40 +233,6 @@ mixin ShorebirdBuildMixin on ShorebirdCommand { }); } - /// Creates an ExportOptions.plist file, which is used to tell xcodebuild to - /// not manage the app version and build number. If we don't do this, then - /// xcodebuild will increment the build number if it detects an App Store - /// Connect build with the same version and build number. This is a problem - /// for us when patching, as patches need to have the same version and build - /// number as the release they are patching. - /// See - /// https://developer.apple.com/forums/thread/690647?answerId=689925022#689925022 - File createExportOptionsPlist({ - ExportMethod exportMethod = ExportMethod.appStore, - }) { - final plistContents = ''' - - - - - manageAppVersionAndBuildNumber - - signingStyle - automatic - uploadBitcode - - method - ${exportMethod.argName} - - -'''; - final tempDir = Directory.systemTemp.createTempSync(); - final exportPlistFile = File(p.join(tempDir.path, 'ExportOptions.plist')) - ..createSync(recursive: true) - ..writeAsStringSync(plistContents); - return exportPlistFile; - } - /// Builds a release iOS framework (.xcframework) for the current project. Future buildIosFramework() async { return _runShorebirdBuildCommand(() async { diff --git a/packages/shorebird_cli/test/src/commands/build/build_ipa_command_test.dart b/packages/shorebird_cli/test/src/commands/build/build_ipa_command_test.dart index 7e5ade47..388e1557 100644 --- a/packages/shorebird_cli/test/src/commands/build/build_ipa_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/build/build_ipa_command_test.dart @@ -1,3 +1,5 @@ +import 'dart:io'; + import 'package:args/args.dart'; import 'package:mason_logger/mason_logger.dart'; import 'package:mocktail/mocktail.dart'; @@ -7,6 +9,7 @@ import 'package:shorebird_cli/src/commands/build/build.dart'; import 'package:shorebird_cli/src/doctor.dart'; import 'package:shorebird_cli/src/logger.dart'; import 'package:shorebird_cli/src/os/operating_system_interface.dart'; +import 'package:shorebird_cli/src/platform/platform.dart'; import 'package:shorebird_cli/src/shorebird_env.dart'; import 'package:shorebird_cli/src/shorebird_process.dart'; import 'package:shorebird_cli/src/shorebird_validator.dart'; @@ -20,6 +23,7 @@ void main() { group(BuildIpaCommand, () { late ArgResults argResults; late Doctor doctor; + late Ios ios; late Logger logger; late OperatingSystemInterface operatingSystemInterface; late ShorebirdProcessResult buildProcessResult; @@ -35,6 +39,7 @@ void main() { body, values: { doctorRef.overrideWith(() => doctor), + iosRef.overrideWith(() => ios), loggerRef.overrideWith(() => logger), osInterfaceRef.overrideWith(() => operatingSystemInterface), processRef.overrideWith(() => shorebirdProcess), @@ -51,6 +56,7 @@ void main() { setUp(() { argResults = MockArgResults(); doctor = MockDoctor(); + ios = MockIos(); logger = MockLogger(); operatingSystemInterface = MockOperatingSystemInterface(); shorebirdProcess = MockShorebirdProcess(); @@ -79,6 +85,7 @@ void main() { ).thenAnswer((_) async => buildProcessResult); when(() => argResults['codesign']).thenReturn(true); when(() => argResults.rest).thenReturn([]); + when(() => ios.createExportOptionsPlist()).thenReturn(File('.')); when(() => logger.progress(any())).thenReturn(MockProgress()); when(() => logger.info(any())).thenReturn(null); when(() => operatingSystemInterface.which('flutter')) diff --git a/packages/shorebird_cli/test/src/commands/patch/patch_ios_command_test.dart b/packages/shorebird_cli/test/src/commands/patch/patch_ios_command_test.dart index 86a9627d..56492b27 100644 --- a/packages/shorebird_cli/test/src/commands/patch/patch_ios_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/patch_ios_command_test.dart @@ -22,6 +22,7 @@ import 'package:shorebird_cli/src/logger.dart'; import 'package:shorebird_cli/src/os/operating_system_interface.dart'; import 'package:shorebird_cli/src/patch_diff_checker.dart'; import 'package:shorebird_cli/src/platform.dart'; +import 'package:shorebird_cli/src/platform/ios.dart'; import 'package:shorebird_cli/src/shorebird_artifacts.dart'; import 'package:shorebird_cli/src/shorebird_build_mixin.dart'; import 'package:shorebird_cli/src/shorebird_env.dart'; @@ -153,6 +154,7 @@ flutter: late File releaseArtifactFile; late ShorebirdArtifacts shorebirdArtifacts; late Doctor doctor; + late Ios ios; late IosArchiveDiffer archiveDiffer; late Progress progress; late Logger logger; @@ -180,6 +182,7 @@ flutter: codePushClientWrapperRef.overrideWith(() => codePushClientWrapper), doctorRef.overrideWith(() => doctor), engineConfigRef.overrideWith(() => engineConfig), + iosRef.overrideWith(() => ios), shorebirdArtifactsRef.overrideWith(() => shorebirdArtifacts), loggerRef.overrideWith(() => logger), osInterfaceRef.overrideWith(() => operatingSystemInterface), @@ -271,6 +274,7 @@ flutter: codePushClientWrapper = MockCodePushClientWrapper(); doctor = MockDoctor(); engineConfig = MockEngineConfig(); + ios = MockIos(); shorebirdArtifacts = MockShorebirdArtifacts(); shorebirdRoot = Directory.systemTemp.createTempSync(); projectRoot = Directory.systemTemp.createTempSync(); @@ -404,6 +408,7 @@ flutter: ).thenAnswer((_) async {}); when(() => doctor.iosCommandValidators).thenReturn([flutterValidator]); when(() => engineConfig.localEngine).thenReturn(null); + when(() => ios.createExportOptionsPlist()).thenReturn(File('.')); when(flutterValidator.validate).thenAnswer((_) async => []); when(() => logger.confirm(any())).thenReturn(true); when(() => logger.progress(any())).thenReturn(progress); diff --git a/packages/shorebird_cli/test/src/commands/release/release_ios_command_test.dart b/packages/shorebird_cli/test/src/commands/release/release_ios_command_test.dart index f2a9ee76..d652f99a 100644 --- a/packages/shorebird_cli/test/src/commands/release/release_ios_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/release/release_ios_command_test.dart @@ -6,9 +6,7 @@ import 'package:mason_logger/mason_logger.dart'; import 'package:mocktail/mocktail.dart'; import 'package:path/path.dart' as p; import 'package:platform/platform.dart'; -import 'package:propertylistserialization/propertylistserialization.dart'; import 'package:scoped/scoped.dart'; -import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart'; import 'package:shorebird_cli/src/auth/auth.dart'; import 'package:shorebird_cli/src/code_push_client_wrapper.dart'; import 'package:shorebird_cli/src/commands/commands.dart'; @@ -17,7 +15,7 @@ import 'package:shorebird_cli/src/doctor.dart'; import 'package:shorebird_cli/src/logger.dart'; import 'package:shorebird_cli/src/os/operating_system_interface.dart'; import 'package:shorebird_cli/src/platform.dart'; -import 'package:shorebird_cli/src/shorebird_build_mixin.dart'; +import 'package:shorebird_cli/src/platform/platform.dart'; import 'package:shorebird_cli/src/shorebird_env.dart'; import 'package:shorebird_cli/src/shorebird_flutter.dart'; import 'package:shorebird_cli/src/shorebird_process.dart'; @@ -116,6 +114,7 @@ flutter: late Auth auth; late Progress progress; late Logger logger; + late Ios ios; late OperatingSystemInterface operatingSystemInterface; late ShorebirdProcessResult flutterBuildProcessResult; late ShorebirdProcessResult flutterPubGetProcessResult; @@ -133,6 +132,7 @@ flutter: authRef.overrideWith(() => auth), codePushClientWrapperRef.overrideWith(() => codePushClientWrapper), doctorRef.overrideWith(() => doctor), + iosRef.overrideWith(() => ios), loggerRef.overrideWith(() => logger), osInterfaceRef.overrideWith(() => operatingSystemInterface), platformRef.overrideWith(() => platform), @@ -179,6 +179,7 @@ flutter: } setUpAll(() { + registerFallbackValue(File('')); registerFallbackValue(ReleasePlatform.ios); registerFallbackValue(ReleaseStatus.draft); registerFallbackValue(FakeRelease()); @@ -196,6 +197,7 @@ flutter: operatingSystemInterface = MockOperatingSystemInterface(); progress = MockProgress(); logger = MockLogger(); + ios = MockIos(); flutterBuildProcessResult = MockProcessResult(); flutterPubGetProcessResult = MockProcessResult(); flutterValidator = MockShorebirdFlutterValidator(); @@ -247,20 +249,17 @@ flutter: when(() => argResults['arch']).thenReturn(arch); when(() => argResults['codesign']).thenReturn(true); when(() => argResults['platform']).thenReturn(releasePlatform); - when(() => argResults['export-options-plist']).thenReturn(null); - // This is the default value in ReleaseIosCommand. - when(() => argResults['export-method']).thenReturn( - ExportMethod.appStore.argName, - ); when(() => argResults.rest).thenReturn([]); when(() => argResults.wasParsed(any())).thenReturn(true); - when(() => argResults.wasParsed('export-method')).thenReturn(false); when(() => auth.isAuthenticated).thenReturn(true); when(() => logger.progress(any())).thenReturn(progress); when(() => logger.confirm(any())).thenReturn(true); when( () => logger.prompt(any(), defaultValue: any(named: 'defaultValue')), ).thenReturn(version); + when(() => ios.exportOptionsPlistFromArgs(argResults)).thenReturn( + File('.'), + ); when( () => operatingSystemInterface.which('flutter'), ).thenReturn('/path/to/flutter'); @@ -556,12 +555,10 @@ flutter: }); }); - group('when both export-method and export-options-plist are provided', () { + group('when exportOptionsPlistFromArgs throws exception', () { setUp(() { - when(() => argResults.wasParsed(exportMethodArgName)).thenReturn(true); - when( - () => argResults[exportOptionsPlistArgName], - ).thenReturn('/path/to/export.plist'); + when(() => ios.exportOptionsPlistFromArgs(argResults)) + .thenThrow(ArgumentError('bad args')); }); test('logs error and exits with usage code', () async { @@ -569,130 +566,7 @@ flutter: final exitCode = await runWithOverrides(command.run); expect(exitCode, equals(ExitCode.usage.code)); - verify( - () => logger.err( - 'Cannot specify both --export-method and --export-options-plist.', - ), - ).called(1); - }); - }); - - group('when export-method is provided', () { - setUp(() { - when(() => argResults.wasParsed(exportMethodArgName)).thenReturn(true); - when(() => argResults[exportMethodArgName]) - .thenReturn(ExportMethod.adHoc.argName); - when(() => argResults[exportOptionsPlistArgName]).thenReturn(null); - }); - - test('generates an export options plist with that export method', - () async { - setUpProjectRoot(); - await runWithOverrides(command.run); - - final capturedArgs = verify( - () => shorebirdProcess.run( - 'flutter', - captureAny(), - runInShell: any(named: 'runInShell'), - ), - ).captured.first as List; - final exportOptionsPlistFile = File( - capturedArgs - .whereType() - .firstWhere((arg) => arg.contains(exportOptionsPlistArgName)) - .split('=') - .last, - ); - final exportOptionsPlist = Plist(file: exportOptionsPlistFile); - expect( - exportOptionsPlist.properties['method'], - ExportMethod.adHoc.argName, - ); - }); - }); - - group('when export-options-plist is provided', () { - group('when file does not exist', () { - setUp(() { - when(() => argResults[exportOptionsPlistArgName]) - .thenReturn('/does/not/exist'); - }); - - test('exits with usage code', () async { - setUpProjectRoot(); - final exitCode = await runWithOverrides(command.run); - - expect(exitCode, equals(ExitCode.usage.code)); - verify( - () => logger.err( - 'Exception: Export options plist file /does/not/exist does not exist', - ), - ).called(1); - }); - }); - - group('when manageAppVersionAndBuildNumber is not set to false', () { - const exportPlistContent = ''' - - - - - - -'''; - - test('exits with usage code', () async { - setUpProjectRoot(); - final exportPlistFile = File( - p.join(projectRoot.path, 'export.plist'), - )..writeAsStringSync(exportPlistContent); - when( - () => argResults[exportOptionsPlistArgName], - ).thenReturn(exportPlistFile.path); - final exitCode = await runWithOverrides(command.run); - - expect(exitCode, equals(ExitCode.usage.code)); - verify( - () => logger.err( - '''Exception: Export options plist ${exportPlistFile.path} does not set manageAppVersionAndBuildNumber to false. This is required for shorebird to work.''', - ), - ).called(1); - }); - }); - }); - - group('when neither export-method nor export-options-plist is provided', - () { - setUp(() { - when(() => argResults.wasParsed(exportMethodArgName)).thenReturn(false); - when(() => argResults[exportOptionsPlistArgName]).thenReturn(null); - }); - - test('generates an export options plist with app-store export method', - () async { - setUpProjectRoot(); - await runWithOverrides(command.run); - - final capturedArgs = verify( - () => shorebirdProcess.run( - 'flutter', - captureAny(), - runInShell: any(named: 'runInShell'), - ), - ).captured.first as List; - final exportOptionsPlistFile = File( - capturedArgs - .whereType() - .firstWhere((arg) => arg.contains(exportOptionsPlistArgName)) - .split('=') - .last, - ); - final exportOptionsPlist = Plist(file: exportOptionsPlistFile); - expect( - exportOptionsPlist.properties['method'], - ExportMethod.appStore.argName, - ); + verify(() => logger.err('Invalid argument(s): bad args')).called(1); }); }); @@ -1127,38 +1001,6 @@ flavors: ).called(1); }); - test('provides appropriate ExportOptions.plist to build ipa command', - () async { - setUpProjectRoot(); - - final exitCode = await runWithOverrides(command.run); - - expect(exitCode, ExitCode.success.code); - final capturedArgs = verify( - () => shorebirdProcess.run( - 'flutter', - captureAny(), - runInShell: any(named: 'runInShell'), - ), - ).captured.first as List; - final exportOptionsPlistFile = File( - capturedArgs - .whereType() - .firstWhere((arg) => arg.contains('export-options-plist')) - .split('=') - .last, - ); - expect(exportOptionsPlistFile.existsSync(), isTrue); - final exportOptionsPlist = - PropertyListSerialization.propertyListWithString( - exportOptionsPlistFile.readAsStringSync(), - ) as Map; - expect(exportOptionsPlist['manageAppVersionAndBuildNumber'], isFalse); - expect(exportOptionsPlist['signingStyle'], 'automatic'); - expect(exportOptionsPlist['uploadBitcode'], isFalse); - expect(exportOptionsPlist['method'], 'app-store'); - }); - test('does not provide export options when codesign is false', () async { when(() => argResults['codesign']).thenReturn(false); setUpProjectRoot(); diff --git a/packages/shorebird_cli/test/src/mocks.dart b/packages/shorebird_cli/test/src/mocks.dart index 607efe89..6e70e102 100644 --- a/packages/shorebird_cli/test/src/mocks.dart +++ b/packages/shorebird_cli/test/src/mocks.dart @@ -22,6 +22,7 @@ import 'package:shorebird_cli/src/executables/devicectl/apple_device.dart'; import 'package:shorebird_cli/src/executables/executables.dart'; import 'package:shorebird_cli/src/os/os.dart'; import 'package:shorebird_cli/src/patch_diff_checker.dart'; +import 'package:shorebird_cli/src/platform/platform.dart'; import 'package:shorebird_cli/src/shorebird_artifacts.dart'; import 'package:shorebird_cli/src/shorebird_env.dart'; import 'package:shorebird_cli/src/shorebird_flutter.dart'; @@ -87,6 +88,8 @@ class MockIOSDeploy extends Mock implements IOSDeploy {} class MockIOSink extends Mock implements IOSink {} +class MockIos extends Mock implements Ios {} + class MockIosArchiveDiffer extends Mock implements IosArchiveDiffer {} class MockJava extends Mock implements Java {} diff --git a/packages/shorebird_cli/test/src/platform/ios_test.dart b/packages/shorebird_cli/test/src/platform/ios_test.dart new file mode 100644 index 00000000..21e9a812 --- /dev/null +++ b/packages/shorebird_cli/test/src/platform/ios_test.dart @@ -0,0 +1,158 @@ +import 'dart:io'; + +import 'package:args/args.dart'; +import 'package:mocktail/mocktail.dart'; +import 'package:path/path.dart' as p; +import 'package:propertylistserialization/propertylistserialization.dart'; +import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart'; +import 'package:shorebird_cli/src/platform/platform.dart'; +import 'package:test/test.dart'; + +import '../mocks.dart'; + +void main() { + group(Ios, () { + late Ios ios; + + setUp(() { + ios = Ios(); + }); + + group('exportOptionsPlistFromArgs', () { + late ArgResults argResults; + + setUp(() { + argResults = MockArgResults(); + + when(() => argResults.wasParsed(any())).thenReturn(false); + }); + + group('when both export-method and export-options-plist are provided', + () { + setUp(() { + when( + () => argResults.wasParsed(exportMethodArgName), + ).thenReturn(true); + when( + () => argResults[exportOptionsPlistArgName], + ).thenReturn('/path/to/export.plist'); + }); + + test('throws ArgumentError', () { + expect( + () => ios.exportOptionsPlistFromArgs(argResults), + throwsArgumentError, + ); + }); + }); + + group('when export-method is provided', () { + setUp(() { + when(() => argResults.wasParsed(exportMethodArgName)) + .thenReturn(true); + when(() => argResults[exportMethodArgName]) + .thenReturn(ExportMethod.adHoc.argName); + when(() => argResults[exportOptionsPlistArgName]).thenReturn(null); + }); + + test('generates an export options plist with that export method', + () async { + final exportOptionsPlistFile = ios.exportOptionsPlistFromArgs( + argResults, + ); + final exportOptionsPlist = Plist(file: exportOptionsPlistFile); + expect( + exportOptionsPlist.properties['method'], + ExportMethod.adHoc.argName, + ); + }); + }); + + group('when export-options-plist is provided', () { + group('when file does not exist', () { + setUp(() { + when( + () => argResults[exportOptionsPlistArgName], + ).thenReturn('/does/not/exist'); + }); + + test('throws a FileSystemException', () async { + expect( + () => ios.exportOptionsPlistFromArgs(argResults), + throwsA( + isA().having( + (e) => e.message, + 'message', + '''Export options plist file /does/not/exist does not exist''', + ), + ), + ); + }); + }); + + group('when manageAppVersionAndBuildNumber is not set to false', () { + const exportPlistContent = ''' + + + + + + +'''; + + test('throws InvalidExportOptionsPlistException', () async { + final tmpDir = Directory.systemTemp.createTempSync(); + final exportPlistFile = File( + p.join(tmpDir.path, 'export.plist'), + )..writeAsStringSync(exportPlistContent); + when( + () => argResults[exportOptionsPlistArgName], + ).thenReturn(exportPlistFile.path); + expect( + () => ios.exportOptionsPlistFromArgs(argResults), + throwsA( + isA().having( + (e) => e.message, + 'message', + '''Export options plist ${exportPlistFile.path} does not set manageAppVersionAndBuildNumber to false. This is required for shorebird to work.''', + ), + ), + ); + }); + }); + }); + + group('when neither export-method nor export-options-plist is provided', + () { + setUp(() { + when(() => argResults.wasParsed(exportMethodArgName)) + .thenReturn(false); + when(() => argResults[exportOptionsPlistArgName]).thenReturn(null); + }); + + test('generates an export options plist with app-store export method', + () async { + final exportOptionsPlistFile = + ios.exportOptionsPlistFromArgs(argResults); + final exportOptionsPlist = Plist(file: exportOptionsPlistFile); + expect( + exportOptionsPlist.properties['method'], + ExportMethod.appStore.argName, + ); + + final exportOptionsPlistMap = + PropertyListSerialization.propertyListWithString( + exportOptionsPlistFile.readAsStringSync(), + ) as Map; + expect( + exportOptionsPlistMap['manageAppVersionAndBuildNumber'], + isFalse, + ); + expect(exportOptionsPlistMap['signingStyle'], 'automatic'); + expect(exportOptionsPlistMap['uploadBitcode'], isFalse); + expect(exportOptionsPlistMap['method'], 'app-store'); + }); + }); + }); + }); +}