From 08f5441f24fcce24b05646ecd78f3eeb604c6bed Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Fri, 25 Oct 2024 17:41:10 -0400 Subject: [PATCH] feat: add gradle steps to appbundle build progress (#2579) --- cspell.config.yaml | 1 + .../lib/src/artifact_builder.dart | 38 +++++- .../src/commands/patch/android_patcher.dart | 5 +- .../commands/release/android_releaser.dart | 6 +- .../lib/src/logging/detail_progress.dart | 77 +++++++++++ .../lib/src/logging/logging.dart | 1 + .../test/src/artifact_builder_test.dart | 95 ++++++++++++-- .../commands/patch/android_patcher_test.dart | 7 + .../release/android_releaser_test.dart | 8 ++ .../src/logging/detail_progress_test.dart | 122 ++++++++++++++++++ packages/shorebird_cli/test/src/mocks.dart | 2 + 11 files changed, 344 insertions(+), 18 deletions(-) create mode 100644 packages/shorebird_cli/lib/src/logging/detail_progress.dart create mode 100644 packages/shorebird_cli/test/src/logging/detail_progress_test.dart diff --git a/cspell.config.yaml b/cspell.config.yaml index 676350fd..b0c53de1 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -99,6 +99,7 @@ words: - udid # Unique Device Identifier - unawaited - unmockable + - unsets - upvote - usbmuxd - vmcode diff --git a/packages/shorebird_cli/lib/src/artifact_builder.dart b/packages/shorebird_cli/lib/src/artifact_builder.dart index cedf86df..3cdf6699 100644 --- a/packages/shorebird_cli/lib/src/artifact_builder.dart +++ b/packages/shorebird_cli/lib/src/artifact_builder.dart @@ -1,5 +1,6 @@ // cspell:words endtemplate aabs ipas appbundle bryanoltman codesign xcarchive // cspell:words xcframework +import 'dart:convert'; import 'dart:io'; import 'package:mason_logger/mason_logger.dart'; @@ -86,6 +87,7 @@ class ArtifactBuilder { Iterable? targetPlatforms, List args = const [], String? base64PublicKey, + DetailProgress? buildProgress, }) async { await _runShorebirdBuildCommand(() async { const executable = 'flutter'; @@ -100,20 +102,46 @@ class ArtifactBuilder { ...args, ]; - final result = await process.run( + final buildProcess = await process.start( executable, arguments, runInShell: true, environment: base64PublicKey?.toPublicKeyEnv(), ); - if (result.exitCode != ExitCode.success.code) { - throw ArtifactBuildException( - 'Failed to build: ${result.stderr}', - ); + // Android builds are a series of gradle tasks that are all logged in + // this format. We can use the 'Task :' line to get the current task + // being run. + final gradleTaskRegex = RegExp(r'^\[.*\] \> (Task :.*)$'); + buildProcess.stdout + .transform(utf8.decoder) + .transform(const LineSplitter()) + .listen((line) { + if (buildProgress == null) { + return; + } + final captured = gradleTaskRegex.firstMatch(line)?.group(1); + if (captured != null) { + buildProgress.updateDetailMessage(captured); + } + }); + + final stderrLines = await buildProcess.stderr + .transform(utf8.decoder) + .transform(const LineSplitter()) + .toList(); + final stdErr = stderrLines.join('\n'); + final exitCode = await buildProcess.exitCode; + if (exitCode != ExitCode.success.code) { + throw ArtifactBuildException('Failed to build: $stdErr'); } }); + // If we've been updating the progress with gradle tasks, reset it to the + // original base message so as not to leave the user with a confusing + // message. + buildProgress?.updateDetailMessage(null); + final projectRoot = shorebirdEnv.getShorebirdProjectRoot()!; try { return shorebirdAndroidArtifacts.findAab( diff --git a/packages/shorebird_cli/lib/src/commands/patch/android_patcher.dart b/packages/shorebird_cli/lib/src/commands/patch/android_patcher.dart index 9f83d15b..744bdea4 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/android_patcher.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/android_patcher.dart @@ -71,8 +71,8 @@ class AndroidPatcher extends Patcher { Future buildPatchArtifact({String? releaseVersion}) async { final File aabFile; final flutterVersionString = await shorebirdFlutter.getVersionAndRevision(); - final buildProgress = - logger.progress('Building patch with Flutter $flutterVersionString'); + final buildProgress = logger + .detailProgress('Building patch with Flutter $flutterVersionString'); try { aabFile = await artifactBuilder.buildAppBundle( @@ -81,6 +81,7 @@ class AndroidPatcher extends Patcher { args: argResults.forwardedArgs + buildNameAndNumberArgsFromReleaseVersion(releaseVersion), base64PublicKey: argResults.encodedPublicKey, + buildProgress: buildProgress, ); buildProgress.complete(); } on ArtifactBuildException catch (error) { diff --git a/packages/shorebird_cli/lib/src/commands/release/android_releaser.dart b/packages/shorebird_cli/lib/src/commands/release/android_releaser.dart index ea264437..cd3fb2a2 100644 --- a/packages/shorebird_cli/lib/src/commands/release/android_releaser.dart +++ b/packages/shorebird_cli/lib/src/commands/release/android_releaser.dart @@ -100,8 +100,9 @@ Please comment and upvote ${link(uri: Uri.parse('https://github.com/shorebirdtec final flutterVersionString = await shorebirdFlutter.getVersionAndRevision(); - final buildAppBundleProgress = logger - .progress('Building app bundle with Flutter $flutterVersionString'); + final buildAppBundleProgress = logger.detailProgress( + 'Building app bundle with Flutter $flutterVersionString', + ); final File aab; @@ -114,6 +115,7 @@ Please comment and upvote ${link(uri: Uri.parse('https://github.com/shorebirdtec targetPlatforms: architectures, args: argResults.forwardedArgs, base64PublicKey: base64PublicKey, + buildProgress: buildAppBundleProgress, ); } on ArtifactBuildException catch (e) { buildAppBundleProgress.fail(e.message); diff --git a/packages/shorebird_cli/lib/src/logging/detail_progress.dart b/packages/shorebird_cli/lib/src/logging/detail_progress.dart new file mode 100644 index 00000000..3f1965a5 --- /dev/null +++ b/packages/shorebird_cli/lib/src/logging/detail_progress.dart @@ -0,0 +1,77 @@ +import 'package:mason_logger/mason_logger.dart'; + +/// {@template detail_progress} +/// A [Progress] wrapper that allows for maintaining a base message (e.g., a +/// task title) while updating the message with more specific information, +/// rendered in dark gray. +/// {@endtemplate} +class DetailProgress implements Progress { + /// {@macro detail_progress} + DetailProgress._({ + required Progress progress, + required String primaryMessage, + }) : _progress = progress, + _primaryMessage = primaryMessage; + + String _primaryMessage; + String? _detailMessage; + final Progress _progress; + + /// Updates the main message of the progress with the given [message]. This is + /// roughly equivalent to calling [Progress.update], except that this will + /// preserve the detail message if one exists. + void updatePrimaryMessage(String message) { + _primaryMessage = message; + _updateImpl(); + } + + /// Updates the detail message of the progress with the given [message]. + void updateDetailMessage(String? message) { + _detailMessage = message; + _updateImpl(); + } + + @override + void update(String update) { + _primaryMessage = update; + _detailMessage = null; + _updateImpl(); + } + + void _updateImpl() { + final detailMessage = _detailMessage; + if (detailMessage != null) { + _progress.update('$_primaryMessage ${darkGray.wrap(detailMessage)}'); + } else { + _progress.update(_primaryMessage); + } + } + + @override + void cancel() { + _progress.cancel(); + } + + @override + void complete([String? update]) { + _progress.complete(update ?? _primaryMessage); + } + + @override + void fail([String? update]) { + _progress.fail(update ?? _primaryMessage); + } +} + +/// {@template detail_progress_logger} +/// Adds a method to [Logger] to create an [DetailProgress] instance. +/// {@endtemplate} +extension DetailProgressLogger on Logger { + /// {@macro detail_progress_logger} + DetailProgress detailProgress(String primaryMessage) { + return DetailProgress._( + progress: progress(primaryMessage), + primaryMessage: primaryMessage, + ); + } +} diff --git a/packages/shorebird_cli/lib/src/logging/logging.dart b/packages/shorebird_cli/lib/src/logging/logging.dart index e9711e82..0fa9b4d2 100644 --- a/packages/shorebird_cli/lib/src/logging/logging.dart +++ b/packages/shorebird_cli/lib/src/logging/logging.dart @@ -1,2 +1,3 @@ +export 'detail_progress.dart'; export 'logging_stdout.dart'; export 'shorebird_logger.dart'; diff --git a/packages/shorebird_cli/test/src/artifact_builder_test.dart b/packages/shorebird_cli/test/src/artifact_builder_test.dart index b407ae9f..0e90ac4d 100644 --- a/packages/shorebird_cli/test/src/artifact_builder_test.dart +++ b/packages/shorebird_cli/test/src/artifact_builder_test.dart @@ -1,3 +1,4 @@ +import 'dart:convert'; import 'dart:io'; import 'package:mason_logger/mason_logger.dart'; @@ -39,6 +40,7 @@ void main() { late ShorebirdProcessResult buildProcessResult; late ShorebirdProcessResult pubGetProcessResult; late ArtifactBuilder builder; + late Process buildProcess; R runWithOverrides(R Function() body) { return runScoped( @@ -71,6 +73,7 @@ void main() { shorebirdArtifacts = MockShorebirdArtifacts(); shorebirdEnv = MockShorebirdEnv(); shorebirdProcess = MockShorebirdProcess(); + buildProcess = MockProcess(); when( () => shorebirdProcess.run( @@ -89,6 +92,13 @@ void main() { runInShell: any(named: 'runInShell'), ), ).thenAnswer((_) async => buildProcessResult); + when( + () => shorebirdProcess.start( + any(), + any(), + runInShell: any(named: 'runInShell'), + ), + ).thenAnswer((_) async => buildProcess); when(() => buildProcessResult.exitCode).thenReturn(ExitCode.success.code); when(() => buildProcessResult.stdout).thenReturn( ''' @@ -175,13 +185,28 @@ Either run `flutter pub get` manually, or follow the steps in ${cannotRunInVSCod flavor: any(named: 'flavor'), ), ).thenReturn(File('app-release.aab')); + when(() => buildProcess.stdout).thenAnswer( + (_) => Stream.fromIterable( + [ + 'Some build output', + ].map(utf8.encode), + ), + ); + when(() => buildProcess.stderr).thenAnswer( + (_) => Stream.fromIterable( + [ + 'Some build output', + ].map(utf8.encode), + ), + ); + when(() => buildProcess.exitCode).thenAnswer((_) async => 0); }); test('invokes the correct flutter build command', () async { await runWithOverrides(() => builder.buildAppBundle()); verify( - () => shorebirdProcess.run( + () => shorebirdProcess.start( 'flutter', ['build', 'appbundle', '--release'], runInShell: any(named: 'runInShell'), @@ -201,7 +226,7 @@ Either run `flutter pub get` manually, or follow the steps in ${cannotRunInVSCod ); verify( - () => shorebirdProcess.run( + () => shorebirdProcess.start( 'flutter', [ 'build', @@ -223,7 +248,7 @@ Either run `flutter pub get` manually, or follow the steps in ${cannotRunInVSCod setUp(() { when( - () => shorebirdProcess.run( + () => shorebirdProcess.start( 'flutter', [ 'build', @@ -238,7 +263,7 @@ Either run `flutter pub get` manually, or follow the steps in ${cannotRunInVSCod 'SHOREBIRD_PUBLIC_KEY': base64PublicKey, }, ), - ).thenAnswer((_) async => buildProcessResult); + ).thenAnswer((_) async => buildProcess); }); test('adds the SHOREBIRD_PUBLIC_KEY to the environment', () async { @@ -252,7 +277,7 @@ Either run `flutter pub get` manually, or follow the steps in ${cannotRunInVSCod ); verify( - () => shorebirdProcess.run( + () => shorebirdProcess.start( 'flutter', [ 'build', @@ -329,11 +354,63 @@ Either run `flutter pub get` manually, or follow the steps in ${cannotRunInVSCod }); }); + group('when output contains gradle task names', () { + late DetailProgress progress; + + setUp(() { + progress = MockDetailProgress(); + + when(() => buildProcess.stdout).thenAnswer( + (_) => Stream.fromIterable( + [ + 'Some build output', + '[ ] > Task :app:bundleRelease', + 'More build output', + '[ ] > Task :app:someOtherTask', + 'Even more build output', + ] + .map((line) => '$line${Platform.lineTerminator}') + .map(utf8.encode), + ), + ); + when(() => buildProcess.stderr).thenAnswer( + (_) => Stream.fromIterable( + ['Some build output'].map(utf8.encode), + ), + ); + }); + + test('updates progress with gradle task names', () async { + await expectLater( + runWithOverrides( + () => builder.buildAppBundle( + buildProgress: progress, + ), + ), + completes, + ); + + // Required to trigger stdout stream events + await pumpEventQueue(); + + // Ensure we update the progress in the correct order and with the + // correct messages, and reset to the base message after the build + // completes. + verifyInOrder( + [ + () => progress.updateDetailMessage('Task :app:bundleRelease'), + () => progress.updateDetailMessage('Task :app:someOtherTask'), + () => progress.updateDetailMessage(null), + ], + ); + }); + }); + group('after a build', () { group('when the build is successful', () { setUp(() { - when(() => buildProcessResult.exitCode) - .thenReturn(ExitCode.success.code); + when(() => buildProcess.exitCode) + .thenAnswer((_) async => ExitCode.success.code); }); verifyCorrectFlutterPubGet( @@ -342,8 +419,8 @@ Either run `flutter pub get` manually, or follow the steps in ${cannotRunInVSCod group('when the build fails', () { setUp(() { - when(() => buildProcessResult.exitCode) - .thenReturn(ExitCode.software.code); + when(() => buildProcess.exitCode) + .thenAnswer((_) async => ExitCode.software.code); }); verifyCorrectFlutterPubGet( diff --git a/packages/shorebird_cli/test/src/commands/patch/android_patcher_test.dart b/packages/shorebird_cli/test/src/commands/patch/android_patcher_test.dart index a5c65069..3ff0f969 100644 --- a/packages/shorebird_cli/test/src/commands/patch/android_patcher_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/android_patcher_test.dart @@ -281,6 +281,7 @@ void main() { targetPlatforms: any(named: 'targetPlatforms'), args: any(named: 'args'), base64PublicKey: any(named: 'base64PublicKey'), + buildProgress: any(named: 'buildProgress'), ), ).thenAnswer((_) async => aabFile); }); @@ -293,7 +294,10 @@ void main() { () => artifactBuilder.buildAppBundle( flavor: any(named: 'flavor'), target: any(named: 'target'), + targetPlatforms: any(named: 'targetPlatforms'), args: any(named: 'args'), + base64PublicKey: any(named: 'base64PublicKey'), + buildProgress: any(named: 'buildProgress'), ), ).thenThrow(exception); when(() => logger.progress(any())).thenReturn(progress); @@ -350,6 +354,7 @@ Looked in: named: 'args', that: containsAll(['--build-name=1.2.3', '--build-number=4']), ), + buildProgress: any(named: 'buildProgress'), ), ).called(1); }); @@ -369,6 +374,7 @@ Looked in: verify( () => artifactBuilder.buildAppBundle( args: ['--verbose'], + buildProgress: any(named: 'buildProgress'), ), ).called(1); }); @@ -401,6 +407,7 @@ Looked in: flavor: any(named: 'flavor'), target: any(named: 'target'), base64PublicKey: 'public_key_encoded', + buildProgress: any(named: 'buildProgress'), ), ).called(1); }); diff --git a/packages/shorebird_cli/test/src/commands/release/android_releaser_test.dart b/packages/shorebird_cli/test/src/commands/release/android_releaser_test.dart index d29caaf0..bdeef8c5 100644 --- a/packages/shorebird_cli/test/src/commands/release/android_releaser_test.dart +++ b/packages/shorebird_cli/test/src/commands/release/android_releaser_test.dart @@ -289,6 +289,7 @@ To change the version of this release, change your app's version in your pubspec target: any(named: 'target'), targetPlatforms: any(named: 'targetPlatforms'), args: any(named: 'args'), + buildProgress: any(named: 'buildProgress'), ), ).thenAnswer((_) async => aabFile); when( @@ -320,6 +321,7 @@ To change the version of this release, change your app's version in your pubspec target: any(named: 'target'), targetPlatforms: any(named: 'targetPlatforms'), args: any(named: 'args'), + buildProgress: any(named: 'buildProgress'), ), ).thenThrow(ArtifactBuildException('Uh oh')); }); @@ -375,6 +377,7 @@ To change the version of this release, change your app's version in your pubspec () => artifactBuilder.buildAppBundle( targetPlatforms: Arch.values, args: ['--verbose'], + buildProgress: any(named: 'buildProgress'), ), ).called(1); }); @@ -389,6 +392,7 @@ To change the version of this release, change your app's version in your pubspec () => artifactBuilder.buildAppBundle( targetPlatforms: Arch.values, args: [], + buildProgress: any(named: 'buildProgress'), ), ).called(1); }); @@ -429,6 +433,7 @@ To change the version of this release, change your app's version in your pubspec target: target, targetPlatforms: Arch.values, args: [], + buildProgress: any(named: 'buildProgress'), ), ).called(1); verify( @@ -462,6 +467,7 @@ To change the version of this release, change your app's version in your pubspec targetPlatforms: any(named: 'targetPlatforms'), args: any(named: 'args'), base64PublicKey: any(named: 'base64PublicKey'), + buildProgress: any(named: 'buildProgress'), ), ).thenAnswer((_) async => aabFile); when( @@ -492,6 +498,7 @@ To change the version of this release, change your app's version in your pubspec targetPlatforms: any(named: 'targetPlatforms'), args: any(named: 'args'), base64PublicKey: base64PublicKey, + buildProgress: any(named: 'buildProgress'), ), ).called(1); }, @@ -516,6 +523,7 @@ To change the version of this release, change your app's version in your pubspec targetPlatforms: any(named: 'targetPlatforms'), args: any(named: 'args'), base64PublicKey: base64PublicKey, + buildProgress: any(named: 'buildProgress'), ), ).called(1); diff --git a/packages/shorebird_cli/test/src/logging/detail_progress_test.dart b/packages/shorebird_cli/test/src/logging/detail_progress_test.dart new file mode 100644 index 00000000..123b92fd --- /dev/null +++ b/packages/shorebird_cli/test/src/logging/detail_progress_test.dart @@ -0,0 +1,122 @@ +import 'package:mason_logger/mason_logger.dart'; +import 'package:mocktail/mocktail.dart'; +import 'package:scoped_deps/scoped_deps.dart'; +import 'package:shorebird_cli/src/logging/logging.dart'; +import 'package:test/test.dart'; + +import '../mocks.dart'; + +void main() { + group(DetailProgress, () { + late ShorebirdLogger logger; + late Progress progress; + late DetailProgress detailProgress; + + R runWithOverrides(R Function() body) { + return runScoped( + body, + values: { + loggerRef.overrideWith(() => logger), + }, + ); + } + + setUp(() { + logger = MockShorebirdLogger(); + progress = MockProgress(); + + when(() => logger.progress(any())).thenReturn(progress); + + detailProgress = runWithOverrides(() => logger.detailProgress('title')); + }); + + group('updatePrimaryMessage', () { + group('when no detail message is set', () { + test('updates the primary message', () { + detailProgress.updatePrimaryMessage('new title'); + verify(() => progress.update('new title')).called(1); + }); + }); + + group('when a detail message is present', () { + setUp(() { + detailProgress.updateDetailMessage('detail'); + }); + + test('updates the primary message and detail message', () { + detailProgress.updatePrimaryMessage('new title'); + verify( + () => progress.update('new title ${darkGray.wrap('detail')}'), + ).called(1); + }); + }); + }); + + group('updateDetailMessage', () { + test('updates the detail message', () { + detailProgress.updateDetailMessage('new detail'); + verify( + () => progress.update('title ${darkGray.wrap('new detail')}'), + ).called(1); + }); + + test('can unset the detail message', () { + detailProgress.updateDetailMessage(null); + verify(() => progress.update('title')).called(1); + }); + }); + + group('update', () { + test('updates the primary message', () { + detailProgress.update('new title'); + verify(() => progress.update('new title')).called(1); + }); + + group('when a detail message is set', () { + setUp(() { + detailProgress.updateDetailMessage('detail'); + }); + + test('unsets the detail message', () { + detailProgress.update('new title'); + verify(() => progress.update('new title')).called(1); + }); + }); + }); + + group('cancel', () { + test('cancels the progress', () { + detailProgress.cancel(); + verify(() => progress.cancel()).called(1); + }); + }); + + group('complete', () { + test('completes the progress', () { + detailProgress.complete(); + verify(() => progress.complete('title')).called(1); + }); + + test('completes the progress with an update', () { + detailProgress.complete('update'); + verify(() => progress.complete('update')).called(1); + }); + }); + + group('fail', () { + test('fails the progress', () { + detailProgress.fail(); + verify(() => progress.fail('title')).called(1); + }); + + test('fails the progress with an update', () { + detailProgress.fail('update'); + verify(() => progress.fail('update')).called(1); + }); + }); + + test('implements Progress', () { + expect(detailProgress, isA()); + }); + }); +} diff --git a/packages/shorebird_cli/test/src/mocks.dart b/packages/shorebird_cli/test/src/mocks.dart index 07fb7c00..95918606 100644 --- a/packages/shorebird_cli/test/src/mocks.dart +++ b/packages/shorebird_cli/test/src/mocks.dart @@ -81,6 +81,8 @@ class MockCodePushClientWrapper extends Mock implements CodePushClientWrapper {} class MockCodeSigner extends Mock implements CodeSigner {} +class MockDetailProgress extends Mock implements DetailProgress {} + class MockDevicectl extends Mock implements Devicectl {} class MockDirectory extends Mock implements Directory {}