fix(shorebird_cli): gracefully handle failure to generate an aab diff (#922)
This commit is contained in:
@@ -2,8 +2,10 @@ import 'dart:convert';
|
||||
import 'dart:io';
|
||||
|
||||
import 'package:archive/archive_io.dart';
|
||||
import 'package:collection/collection.dart';
|
||||
import 'package:shorebird_cli/src/archive_analysis/android_archive_differ.dart';
|
||||
import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart';
|
||||
import 'package:shorebird_cli/src/archive_analysis/archive_differ.dart';
|
||||
import 'package:shorebird_cli/src/archive_analysis/mf_reader.dart';
|
||||
|
||||
/// Finds differences between two AABs.
|
||||
@@ -45,10 +47,14 @@ class AabDiffer extends AndroidArchiveDiffer {
|
||||
String _metaInfMfContent(File aab) {
|
||||
final inputStream = InputFileStream(aab.path);
|
||||
final archive = ZipDecoder().decodeBuffer(inputStream);
|
||||
return utf8.decode(
|
||||
archive.files
|
||||
.firstWhere((file) => file.name == 'META-INF/MANIFEST.MF')
|
||||
.content as List<int>,
|
||||
final manifestFile = archive.files.firstWhereOrNull(
|
||||
(file) => file.name == 'META-INF/MANIFEST.MF',
|
||||
);
|
||||
|
||||
if (manifestFile == null) {
|
||||
throw DiffFailedException();
|
||||
}
|
||||
|
||||
return utf8.decode(manifestFile.content as List<int>);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart';
|
||||
|
||||
/// Thrown when an [ArchiveDiffer] fails to generate a [FileSetDiff].
|
||||
class DiffFailedException implements Exception {}
|
||||
|
||||
/// Computes content differences between two archives.
|
||||
abstract class ArchiveDiffer {
|
||||
/// Files that have been added, removed, or that have changed between the
|
||||
|
||||
@@ -5,6 +5,7 @@ import 'package:http/http.dart' as http;
|
||||
import 'package:mason_logger/mason_logger.dart';
|
||||
import 'package:path/path.dart' as p;
|
||||
import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart';
|
||||
import 'package:shorebird_cli/src/archive_analysis/archive_differ.dart';
|
||||
import 'package:shorebird_cli/src/cache.dart';
|
||||
import 'package:shorebird_cli/src/code_push_client_wrapper.dart';
|
||||
import 'package:shorebird_cli/src/command.dart';
|
||||
@@ -242,10 +243,19 @@ https://github.com/shorebirdtech/shorebird/issues/472
|
||||
|
||||
downloadReleaseArtifactProgress.complete();
|
||||
|
||||
final contentDiffs = _aabDiffer.changedFiles(
|
||||
releaseAabPath,
|
||||
bundlePath,
|
||||
);
|
||||
FileSetDiff contentDiffs;
|
||||
try {
|
||||
contentDiffs = _aabDiffer.changedFiles(
|
||||
releaseAabPath,
|
||||
bundlePath,
|
||||
);
|
||||
} on DiffFailedException catch (_) {
|
||||
logger.warn(
|
||||
'''
|
||||
Could not determine whether patch contains asset changes. If you have added or removed assets, you will need to create a new release.''',
|
||||
);
|
||||
contentDiffs = FileSetDiff.empty();
|
||||
}
|
||||
|
||||
logger.detail('aab content differences: $contentDiffs');
|
||||
|
||||
|
||||
Binary file not shown.
@@ -1,5 +1,6 @@
|
||||
import 'package:path/path.dart' as p;
|
||||
import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart';
|
||||
import 'package:shorebird_cli/src/archive_analysis/archive_differ.dart';
|
||||
import 'package:test/test.dart';
|
||||
|
||||
void main() {
|
||||
@@ -13,6 +14,7 @@ void main() {
|
||||
p.join(aabFixturesBasePath, 'changed_kotlin.aab');
|
||||
final changedDartAndAssetAabPath =
|
||||
p.join(aabFixturesBasePath, 'changed_dart_and_asset.aab');
|
||||
final noManifestAabPath = p.join(aabFixturesBasePath, 'no_manifest.aab');
|
||||
|
||||
late AabDiffer differ;
|
||||
|
||||
@@ -38,6 +40,13 @@ void main() {
|
||||
});
|
||||
});
|
||||
|
||||
test('does not crash if MANIFEST.MF is missing', () {
|
||||
expect(
|
||||
() => differ.changedFiles(baseAabPath, noManifestAabPath),
|
||||
throwsA(isA<DiffFailedException>()),
|
||||
);
|
||||
});
|
||||
|
||||
group('contentDifferences', () {
|
||||
test('detects no differences between the same aab', () {
|
||||
expect(differ.changedFiles(baseAabPath, baseAabPath), isEmpty);
|
||||
|
||||
@@ -8,6 +8,7 @@ import 'package:path/path.dart' as p;
|
||||
import 'package:platform/platform.dart';
|
||||
import 'package:scoped/scoped.dart';
|
||||
import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart';
|
||||
import 'package:shorebird_cli/src/archive_analysis/archive_differ.dart';
|
||||
import 'package:shorebird_cli/src/auth/auth.dart';
|
||||
import 'package:shorebird_cli/src/bundletool.dart';
|
||||
import 'package:shorebird_cli/src/cache.dart' show Cache, cacheRef;
|
||||
@@ -622,6 +623,27 @@ https://github.com/shorebirdtech/shorebird/issues/472
|
||||
expect(exitCode, ExitCode.software.code);
|
||||
});
|
||||
|
||||
test(
|
||||
'prints warning if differ cannot determine patch differences',
|
||||
() async {
|
||||
when(() => aabDiffer.changedFiles(any(), any()))
|
||||
.thenThrow(DiffFailedException());
|
||||
final tempDir = setUpTempDir();
|
||||
setUpTempArtifacts(tempDir);
|
||||
final exitCode = await IOOverrides.runZoned(
|
||||
() => runWithOverrides(command.run),
|
||||
getCurrentDirectory: () => tempDir,
|
||||
);
|
||||
|
||||
expect(exitCode, ExitCode.success.code);
|
||||
verify(
|
||||
() => logger.warn(
|
||||
'''Could not determine whether patch contains asset changes. If you have added or removed assets, you will need to create a new release.''',
|
||||
),
|
||||
).called(1);
|
||||
},
|
||||
);
|
||||
|
||||
test('prompts user to continue when Java/Kotlin code changes are detected',
|
||||
() async {
|
||||
when(() => aabDiffer.containsPotentiallyBreakingNativeDiffs(any()))
|
||||
|
||||
Reference in New Issue
Block a user