From 4ff459b795fef148e3346eb37b9504f5f2a8c290 Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Tue, 19 Mar 2024 11:45:15 -0400 Subject: [PATCH] refactor(shorebird_cli): split PatchDiffChecker's force into allowNativeChanges and allowAssetChanges (#1800) --- .../src/commands/patch/patch_aar_command.dart | 3 +- .../commands/patch/patch_android_command.dart | 3 +- .../src/commands/patch/patch_ios_command.dart | 3 +- .../patch/patch_ios_framework_command.dart | 3 +- .../lib/src/patch_diff_checker.dart | 13 +++--- .../patch/patch_aar_command_test.dart | 18 +++++--- .../patch/patch_android_command_test.dart | 18 +++++--- .../patch/patch_ios_command_test.dart | 18 +++++--- .../patch_ios_framework_command_test.dart | 18 +++++--- .../test/src/patch_diff_checker_test.dart | 44 ++++++++++++------- 10 files changed, 92 insertions(+), 49 deletions(-) diff --git a/packages/shorebird_cli/lib/src/commands/patch/patch_aar_command.dart b/packages/shorebird_cli/lib/src/commands/patch/patch_aar_command.dart index 36842d92..9d1f3c36 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patch_aar_command.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patch_aar_command.dart @@ -201,7 +201,8 @@ Please re-run the release command for this version or create a new release.'''); releaseArtifact: await artifactManager .downloadFile(Uri.parse(releaseAarArtifact.url)), archiveDiffer: _archiveDiffer, - force: force, + allowAssetChanges: force, + allowNativeChanges: force, ); } on UserCancelledException { return ExitCode.success.code; diff --git a/packages/shorebird_cli/lib/src/commands/patch/patch_android_command.dart b/packages/shorebird_cli/lib/src/commands/patch/patch_android_command.dart index 1a633587..f2f295e0 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patch_android_command.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patch_android_command.dart @@ -278,7 +278,8 @@ Looked in: localArtifact: File(bundlePath), releaseArtifact: releaseAabArtifactFile, archiveDiffer: _archiveDiffer, - force: force, + allowAssetChanges: force, + allowNativeChanges: force, ); } on UserCancelledException { return ExitCode.success.code; 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 8df50183..0b1a37d7 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 @@ -235,7 +235,8 @@ Current Flutter Revision: $currentFlutterRevision localArtifactDirectory: Directory(archivePath), releaseArtifact: releaseArtifactZipFile, archiveDiffer: _archiveDiffer, - force: force, + allowAssetChanges: force, + allowNativeChanges: force, ); } on UserCancelledException { return ExitCode.success.code; 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 a1a858ef..f9069c55 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 @@ -205,7 +205,8 @@ Please re-run the release command for this version or create a new release.'''); localArtifactDirectory: Directory(getAppXcframeworkPath()), releaseArtifact: releaseArtifactZipFile, archiveDiffer: _archiveDiffer, - force: force, + allowAssetChanges: force, + allowNativeChanges: force, ); } on UserCancelledException { return ExitCode.success.code; diff --git a/packages/shorebird_cli/lib/src/patch_diff_checker.dart b/packages/shorebird_cli/lib/src/patch_diff_checker.dart index bfd22274..4a341a89 100644 --- a/packages/shorebird_cli/lib/src/patch_diff_checker.dart +++ b/packages/shorebird_cli/lib/src/patch_diff_checker.dart @@ -48,7 +48,8 @@ class PatchDiffChecker { required Directory localArtifactDirectory, required File releaseArtifact, required ArchiveDiffer archiveDiffer, - required bool force, + required bool allowAssetChanges, + required bool allowNativeChanges, }) async { final zipProgress = logger.progress('Compressing archive'); final zippedFile = await localArtifactDirectory.zipToTempFile(); @@ -58,7 +59,8 @@ class PatchDiffChecker { localArtifact: zippedFile, releaseArtifact: releaseArtifact, archiveDiffer: archiveDiffer, - force: force, + allowAssetChanges: allowAssetChanges, + allowNativeChanges: allowNativeChanges, ); } @@ -68,7 +70,8 @@ class PatchDiffChecker { required File localArtifact, required File releaseArtifact, required ArchiveDiffer archiveDiffer, - required bool force, + required bool allowAssetChanges, + required bool allowNativeChanges, }) async { final progress = logger.progress('Verifying patch can be applied to release'); @@ -102,7 +105,7 @@ class PatchDiffChecker { If you don't know why you're seeing this error, visit our troublshooting page at ${link(uri: Uri.parse('https://docs.shorebird.dev/troubleshooting#unexpected-native-changes'))}'''), ); - if (!force) { + if (!allowNativeChanges) { if (shorebirdEnv.isRunningOnCI) { throw UnpatchableChangeException(); } @@ -124,7 +127,7 @@ If you don't know why you're seeing this error, visit our troublshooting page at ), ); - if (!force) { + if (!allowAssetChanges) { if (shorebirdEnv.isRunningOnCI) { throw UnpatchableChangeException(); } diff --git a/packages/shorebird_cli/test/src/commands/patch/patch_aar_command_test.dart b/packages/shorebird_cli/test/src/commands/patch/patch_aar_command_test.dart index 04ef9a1a..931397c4 100644 --- a/packages/shorebird_cli/test/src/commands/patch/patch_aar_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/patch_aar_command_test.dart @@ -333,7 +333,8 @@ void main() { localArtifact: any(named: 'localArtifact'), releaseArtifact: any(named: 'releaseArtifact'), archiveDiffer: archiveDiffer, - force: any(named: 'force'), + allowAssetChanges: any(named: 'allowAssetChanges'), + allowNativeChanges: any(named: 'allowNativeChanges'), ), ).thenAnswer( (_) async => DiffStatus( @@ -638,7 +639,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifact: any(named: 'localArtifact'), releaseArtifact: any(named: 'releaseArtifact'), archiveDiffer: archiveDiffer, - force: any(named: 'force'), + allowAssetChanges: any(named: 'allowAssetChanges'), + allowNativeChanges: any(named: 'allowNativeChanges'), ), ).thenThrow(UserCancelledException()); setUpProjectRootArtifacts(); @@ -651,7 +653,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifact: any(named: 'localArtifact'), releaseArtifact: releaseArtifactFile, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ).called(1); verifyNever( @@ -677,7 +680,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifact: any(named: 'localArtifact'), releaseArtifact: any(named: 'releaseArtifact'), archiveDiffer: archiveDiffer, - force: any(named: 'force'), + allowAssetChanges: any(named: 'allowAssetChanges'), + allowNativeChanges: any(named: 'allowNativeChanges'), ), ).thenThrow(UnpatchableChangeException()); @@ -691,7 +695,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifact: any(named: 'localArtifact'), releaseArtifact: releaseArtifactFile, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ).called(1); verifyNever( @@ -786,7 +791,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifact: any(named: 'localArtifact'), releaseArtifact: any(named: 'releaseArtifact'), archiveDiffer: archiveDiffer, - force: any(named: 'force'), + allowAssetChanges: any(named: 'allowAssetChanges'), + allowNativeChanges: any(named: 'allowNativeChanges'), ), ).thenAnswer( (_) async => DiffStatus( diff --git a/packages/shorebird_cli/test/src/commands/patch/patch_android_command_test.dart b/packages/shorebird_cli/test/src/commands/patch/patch_android_command_test.dart index ac575229..d0a713f8 100644 --- a/packages/shorebird_cli/test/src/commands/patch/patch_android_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/patch_android_command_test.dart @@ -375,7 +375,8 @@ flutter: localArtifact: any(named: 'localArtifact'), releaseArtifact: any(named: 'releaseArtifact'), archiveDiffer: archiveDiffer, - force: any(named: 'force'), + allowAssetChanges: any(named: 'allowAssetChanges'), + allowNativeChanges: any(named: 'allowNativeChanges'), ), ).thenAnswer( (_) async => DiffStatus( @@ -728,7 +729,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifact: any(named: 'localArtifact'), releaseArtifact: any(named: 'releaseArtifact'), archiveDiffer: archiveDiffer, - force: any(named: 'force'), + allowAssetChanges: any(named: 'allowAssetChanges'), + allowNativeChanges: any(named: 'allowNativeChanges'), ), ).thenThrow(UserCancelledException()); setUpProjectRoot(); @@ -742,7 +744,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifact: any(named: 'localArtifact'), releaseArtifact: releaseArtifactFile, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ).called(1); verifyNever( @@ -768,7 +771,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifact: any(named: 'localArtifact'), releaseArtifact: any(named: 'releaseArtifact'), archiveDiffer: archiveDiffer, - force: any(named: 'force'), + allowAssetChanges: any(named: 'allowAssetChanges'), + allowNativeChanges: any(named: 'allowNativeChanges'), ), ).thenThrow(UnpatchableChangeException()); @@ -783,7 +787,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifact: any(named: 'localArtifact'), releaseArtifact: releaseArtifactFile, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ).called(1); verifyNever( @@ -875,7 +880,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifact: any(named: 'localArtifact'), releaseArtifact: any(named: 'releaseArtifact'), archiveDiffer: archiveDiffer, - force: any(named: 'force'), + allowAssetChanges: any(named: 'allowAssetChanges'), + allowNativeChanges: any(named: 'allowNativeChanges'), ), ).thenAnswer( (_) async => DiffStatus( 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 acb29990..20656013 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 @@ -490,7 +490,8 @@ flutter: localArtifactDirectory: any(named: 'localArtifactDirectory'), releaseArtifact: any(named: 'releaseArtifact'), archiveDiffer: archiveDiffer, - force: any(named: 'force'), + allowAssetChanges: any(named: 'allowAssetChanges'), + allowNativeChanges: any(named: 'allowNativeChanges'), ), ).thenAnswer( (_) async => DiffStatus( @@ -994,7 +995,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifactDirectory: any(named: 'localArtifactDirectory'), releaseArtifact: any(named: 'releaseArtifact'), archiveDiffer: archiveDiffer, - force: any(named: 'force'), + allowAssetChanges: any(named: 'allowAssetChanges'), + allowNativeChanges: any(named: 'allowNativeChanges'), ), ).thenThrow(UserCancelledException()); setUpProjectRoot(); @@ -1008,7 +1010,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifactDirectory: any(named: 'localArtifactDirectory'), releaseArtifact: releaseArtifactFile, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ).called(1); verifyNever( @@ -1071,7 +1074,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifactDirectory: any(named: 'localArtifactDirectory'), releaseArtifact: any(named: 'releaseArtifact'), archiveDiffer: archiveDiffer, - force: any(named: 'force'), + allowAssetChanges: any(named: 'allowAssetChanges'), + allowNativeChanges: any(named: 'allowNativeChanges'), ), ).thenThrow(UnpatchableChangeException()); setUpProjectRoot(); @@ -1085,7 +1089,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifactDirectory: any(named: 'localArtifactDirectory'), releaseArtifact: releaseArtifactFile, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ).called(1); verifyNever( @@ -1340,7 +1345,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifactDirectory: any(named: 'localArtifactDirectory'), releaseArtifact: any(named: 'releaseArtifact'), archiveDiffer: archiveDiffer, - force: any(named: 'force'), + allowAssetChanges: any(named: 'allowAssetChanges'), + allowNativeChanges: any(named: 'allowNativeChanges'), ), ).thenAnswer( (_) async => DiffStatus( diff --git a/packages/shorebird_cli/test/src/commands/patch/patch_ios_framework_command_test.dart b/packages/shorebird_cli/test/src/commands/patch/patch_ios_framework_command_test.dart index 6950a6de..a29bc4a4 100644 --- a/packages/shorebird_cli/test/src/commands/patch/patch_ios_framework_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/patch_ios_framework_command_test.dart @@ -396,7 +396,8 @@ flutter: localArtifactDirectory: any(named: 'localArtifactDirectory'), releaseArtifact: any(named: 'releaseArtifact'), archiveDiffer: archiveDiffer, - force: any(named: 'force'), + allowAssetChanges: any(named: 'allowAssetChanges'), + allowNativeChanges: any(named: 'allowNativeChanges'), ), ).thenAnswer( (_) async => DiffStatus( @@ -754,7 +755,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifactDirectory: any(named: 'localArtifactDirectory'), releaseArtifact: any(named: 'releaseArtifact'), archiveDiffer: archiveDiffer, - force: any(named: 'force'), + allowAssetChanges: any(named: 'allowAssetChanges'), + allowNativeChanges: any(named: 'allowNativeChanges'), ), ).thenThrow(UserCancelledException()); setUpProjectRoot(); @@ -768,7 +770,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifactDirectory: any(named: 'localArtifactDirectory'), releaseArtifact: releaseArtifactFile, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ).called(1); verifyNever( @@ -794,7 +797,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifactDirectory: any(named: 'localArtifactDirectory'), releaseArtifact: any(named: 'releaseArtifact'), archiveDiffer: archiveDiffer, - force: any(named: 'force'), + allowAssetChanges: any(named: 'allowAssetChanges'), + allowNativeChanges: any(named: 'allowNativeChanges'), ), ).thenThrow(UnpatchableChangeException()); setUpProjectRoot(); @@ -808,7 +812,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifactDirectory: any(named: 'localArtifactDirectory'), releaseArtifact: releaseArtifactFile, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ).called(1); verifyNever( @@ -877,7 +882,8 @@ Please re-run the release command for this version or create a new release.'''), localArtifactDirectory: any(named: 'localArtifactDirectory'), releaseArtifact: any(named: 'releaseArtifact'), archiveDiffer: archiveDiffer, - force: any(named: 'force'), + allowAssetChanges: any(named: 'allowAssetChanges'), + allowNativeChanges: any(named: 'allowNativeChanges'), ), ).thenAnswer( (_) async => DiffStatus( diff --git a/packages/shorebird_cli/test/src/patch_diff_checker_test.dart b/packages/shorebird_cli/test/src/patch_diff_checker_test.dart index 9b1df07e..71cc5240 100644 --- a/packages/shorebird_cli/test/src/patch_diff_checker_test.dart +++ b/packages/shorebird_cli/test/src/patch_diff_checker_test.dart @@ -97,7 +97,8 @@ void main() { localArtifactDirectory: localArtifactDirectory, releaseArtifact: releaseArtifact, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ); @@ -119,7 +120,8 @@ void main() { localArtifact: localArtifact, releaseArtifact: releaseArtifact, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ); @@ -141,26 +143,28 @@ void main() { ).called(1); }); - test('prompts user if force is false', () async { + test('prompts user if allowNativeChanges is false', () async { await runWithOverrides( () => patchDiffChecker.confirmUnpatchableDiffsIfNecessary( localArtifact: localArtifact, releaseArtifact: releaseArtifact, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ); verify(() => logger.confirm('Continue anyways?')).called(1); }); - test('does not prompt user if force is true', () async { + test('does not prompt user if allowNativeChanges is true', () async { await runWithOverrides( () => patchDiffChecker.confirmUnpatchableDiffsIfNecessary( localArtifact: localArtifact, releaseArtifact: releaseArtifact, archiveDiffer: archiveDiffer, - force: true, + allowAssetChanges: false, + allowNativeChanges: true, ), ); @@ -177,7 +181,8 @@ void main() { localArtifact: localArtifact, releaseArtifact: releaseArtifact, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ), throwsA( @@ -197,7 +202,8 @@ void main() { localArtifact: localArtifact, releaseArtifact: releaseArtifact, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ), throwsA(isA()), @@ -220,7 +226,8 @@ void main() { localArtifact: localArtifact, releaseArtifact: releaseArtifact, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ); @@ -234,26 +241,28 @@ void main() { ).called(1); }); - test('prompts user if force is false', () async { + test('prompts user if allowAssetChanges is false', () async { await runWithOverrides( () => patchDiffChecker.confirmUnpatchableDiffsIfNecessary( localArtifact: localArtifact, releaseArtifact: releaseArtifact, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ); verify(() => logger.confirm('Continue anyways?')).called(1); }); - test('does not prompt user if force is true', () async { + test('does not prompt user if allowAssetChanges is true', () async { await runWithOverrides( () => patchDiffChecker.confirmUnpatchableDiffsIfNecessary( localArtifact: localArtifact, releaseArtifact: releaseArtifact, archiveDiffer: archiveDiffer, - force: true, + allowAssetChanges: true, + allowNativeChanges: false, ), ); @@ -270,7 +279,8 @@ void main() { localArtifact: localArtifact, releaseArtifact: releaseArtifact, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ), throwsA(isA()), @@ -288,7 +298,8 @@ void main() { localArtifact: localArtifact, releaseArtifact: releaseArtifact, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ), throwsA(isA()), @@ -306,7 +317,8 @@ void main() { localArtifact: localArtifact, releaseArtifact: releaseArtifact, archiveDiffer: archiveDiffer, - force: false, + allowAssetChanges: false, + allowNativeChanges: false, ), ), completes,