From b249d76237252997c7e538a20ea35e0497bc3b44 Mon Sep 17 00:00:00 2001 From: Felix Angelov Date: Wed, 9 Aug 2023 15:21:21 -0500 Subject: [PATCH] fix(shorebird_cli): fix reported flutter version (#1077) --- packages/shorebird_cli/lib/src/git.dart | 9 +- .../lib/src/shorebird_flutter.dart | 27 +++- .../shorebird_flutter_validator.dart | 6 +- packages/shorebird_cli/test/src/git_test.dart | 30 +++++ .../test/src/shorebird_flutter_test.dart | 121 +++++++++++++++--- .../shorebird_flutter_validator_test.dart | 15 +-- 6 files changed, 171 insertions(+), 37 deletions(-) diff --git a/packages/shorebird_cli/lib/src/git.dart b/packages/shorebird_cli/lib/src/git.dart index 88efa1aa..58200cc6 100644 --- a/packages/shorebird_cli/lib/src/git.dart +++ b/packages/shorebird_cli/lib/src/git.dart @@ -93,8 +93,15 @@ class Git { required String directory, required String format, required String pattern, + String? pointsAt, }) async { - final arguments = ['for-each-ref', '--format', format, pattern]; + final arguments = [ + 'for-each-ref', + if (pointsAt != null) ...['--points-at', pointsAt], + '--format', + format, + pattern + ]; final result = await process.run( executable, arguments, diff --git a/packages/shorebird_cli/lib/src/shorebird_flutter.dart b/packages/shorebird_cli/lib/src/shorebird_flutter.dart index bf5e3d1a..8df07497 100644 --- a/packages/shorebird_cli/lib/src/shorebird_flutter.dart +++ b/packages/shorebird_cli/lib/src/shorebird_flutter.dart @@ -64,20 +64,17 @@ class ShorebirdFlutter { return status.isEmpty; } - /// Returns the current Shorebird Flutter version. + /// Returns the current system Flutter version. /// Throws a [ProcessException] if the version check fails. /// Returns `null` if the version check succeeds but the version cannot be /// parsed. - /// - /// If [useVendedFlutter] is `true`, the vended Flutter is used instead of - /// the system Flutter. Defaults to true. - Future getVersion({bool useVendedFlutter = true}) async { + Future getSystemVersion() async { const args = ['--version']; final result = await process.run( executable, args, runInShell: true, - useVendedFlutter: useVendedFlutter, + useVendedFlutter: false, ); if (result.exitCode != 0) { @@ -96,6 +93,24 @@ class ShorebirdFlutter { return match?.group(1); } + /// Returns the current Shorebird Flutter version. + /// Throws a [ProcessException] if the version check fails. + /// Returns `null` if the version check succeeds but the version cannot be + /// parsed. + Future getVersion() async { + final result = await git.forEachRef( + pointsAt: shorebirdEnv.flutterRevision, + format: '%(refname:short)', + pattern: 'refs/remotes/origin/flutter_release/*', + directory: _workingDirectory(), + ); + + return LineSplitter.split(result) + .map((e) => e.replaceFirst('origin/flutter_release/', '')) + .toList() + .firstOrNull; + } + Future> getVersions({String? revision}) async { final result = await git.forEachRef( format: '%(refname:short)', diff --git a/packages/shorebird_cli/lib/src/validators/shorebird_flutter_validator.dart b/packages/shorebird_cli/lib/src/validators/shorebird_flutter_validator.dart index c106564d..faaa0a10 100644 --- a/packages/shorebird_cli/lib/src/validators/shorebird_flutter_validator.dart +++ b/packages/shorebird_cli/lib/src/validators/shorebird_flutter_validator.dart @@ -116,9 +116,9 @@ This can cause unexpected behavior if you are switching between the tools and th Future _getFlutterVersion({bool useVendedFlutter = true}) async { final String? version; try { - version = await shorebirdFlutter.getVersion( - useVendedFlutter: useVendedFlutter, - ); + version = useVendedFlutter + ? await shorebirdFlutter.getVersion() + : await shorebirdFlutter.getSystemVersion(); } on ProcessException catch (error) { throw FlutterValidationException( 'Flutter version check did not complete successfully. ${error.message}', diff --git a/packages/shorebird_cli/test/src/git_test.dart b/packages/shorebird_cli/test/src/git_test.dart index af54136c..c4e77ac9 100644 --- a/packages/shorebird_cli/test/src/git_test.dart +++ b/packages/shorebird_cli/test/src/git_test.dart @@ -223,6 +223,36 @@ origin/flutter_release/3.10.6'''; ).called(1); }); + test('executes correct command w/points-at', () async { + const pointsAt = 'revision'; + when(() => processResult.stdout).thenReturn(output); + await expectLater( + runWithOverrides( + () => git.forEachRef( + pointsAt: pointsAt, + pattern: pattern, + format: format, + directory: directory, + ), + ), + completion(equals(output.trim())), + ); + verify( + () => process.run( + 'git', + [ + 'for-each-ref', + '--points-at', + pointsAt, + '--format', + format, + pattern + ], + workingDirectory: directory, + ), + ).called(1); + }); + test('throws ProcessException if process exits with error', () async { const error = 'oops'; when(() => processResult.exitCode).thenReturn(ExitCode.software.code); diff --git a/packages/shorebird_cli/test/src/shorebird_flutter_test.dart b/packages/shorebird_cli/test/src/shorebird_flutter_test.dart index 511a9a59..d095b11b 100644 --- a/packages/shorebird_cli/test/src/shorebird_flutter_test.dart +++ b/packages/shorebird_cli/test/src/shorebird_flutter_test.dart @@ -81,6 +81,14 @@ void main() { directory: any(named: 'directory'), ), ).thenAnswer((_) async => flutterRevision); + when( + () => git.forEachRef( + directory: any(named: 'directory'), + pointsAt: any(named: 'pointsAt'), + format: any(named: 'format'), + pattern: any(named: 'pattern'), + ), + ).thenAnswer((_) async => 'origin/flutter_release/3.10.6'); when(() => shorebirdEnv.flutterDirectory).thenReturn(flutterDirectory); when(() => shorebirdEnv.flutterRevision).thenReturn(flutterRevision); when( @@ -88,35 +96,45 @@ void main() { 'flutter', ['--version'], runInShell: true, - useVendedFlutter: any(named: 'useVendedFlutter'), + useVendedFlutter: false, ), ).thenAnswer((_) async => processResult); when(() => processResult.exitCode).thenReturn(0); }); - group('getVersion', () { + group('getSystemVersion', () { test('throws ProcessException when process exits with non-zero code', () async { const error = 'oops'; when(() => processResult.exitCode).thenReturn(ExitCode.software.code); when(() => processResult.stderr).thenReturn(error); await expectLater( - runWithOverrides(shorebirdFlutter.getVersion), + runWithOverrides(shorebirdFlutter.getSystemVersion), throwsA(isA()), ); verify( - () => process.run('flutter', ['--version'], runInShell: true), + () => process.run( + 'flutter', + ['--version'], + runInShell: true, + useVendedFlutter: false, + ), ).called(1); }); test('returns null when cannot parse version', () async { when(() => processResult.stdout).thenReturn(''); await expectLater( - runWithOverrides(shorebirdFlutter.getVersion), + runWithOverrides(shorebirdFlutter.getSystemVersion), completion(isNull), ); verify( - () => process.run('flutter', ['--version'], runInShell: true), + () => process.run( + 'flutter', + ['--version'], + runInShell: true, + useVendedFlutter: false, + ), ).called(1); }); @@ -127,21 +145,9 @@ Framework • revision f468f3366c (4 weeks ago) • 2023-07-12 15:19:05 -0700 Engine • revision cdbeda788a Tools • Dart 3.0.6 • DevTools 2.23.1'''); await expectLater( - runWithOverrides(shorebirdFlutter.getVersion), + runWithOverrides(shorebirdFlutter.getSystemVersion), completion(equals('3.10.6')), ); - verify( - () => process.run('flutter', ['--version'], runInShell: true), - ).called(1); - }); - - test('uses system flutter when specified', () async { - await expectLater( - runWithOverrides( - () => shorebirdFlutter.getVersion(useVendedFlutter: false), - ), - completes, - ); verify( () => process.run( 'flutter', @@ -153,6 +159,83 @@ Tools • Dart 3.0.6 • DevTools 2.23.1'''); }); }); + group('getVersion', () { + test('throws ProcessException when process exits with non-zero code', + () async { + const error = 'oops'; + when( + () => git.forEachRef( + directory: any(named: 'directory'), + pointsAt: any(named: 'pointsAt'), + format: any(named: 'format'), + pattern: any(named: 'pattern'), + ), + ).thenThrow( + ProcessException( + 'git', + [ + 'for-each-ref', + '--format', + '%(refname:short)', + 'refs/remotes/origin/flutter_release/*' + ], + error, + ExitCode.software.code, + ), + ); + await expectLater( + runWithOverrides(shorebirdFlutter.getVersion), + throwsA(isA()), + ); + verify( + () => git.forEachRef( + directory: p.join(flutterDirectory.parent.path, flutterRevision), + pointsAt: flutterRevision, + format: '%(refname:short)', + pattern: 'refs/remotes/origin/flutter_release/*', + ), + ).called(1); + }); + + test('returns null when cannot parse version', () async { + when( + () => git.forEachRef( + directory: any(named: 'directory'), + pointsAt: any(named: 'pointsAt'), + format: any(named: 'format'), + pattern: any(named: 'pattern'), + ), + ).thenAnswer((_) async => ''); + await expectLater( + runWithOverrides(shorebirdFlutter.getVersion), + completion(isNull), + ); + verify( + () => git.forEachRef( + directory: p.join(flutterDirectory.parent.path, flutterRevision), + pointsAt: flutterRevision, + format: '%(refname:short)', + pattern: 'refs/remotes/origin/flutter_release/*', + ), + ).called(1); + }); + + test('returns version when able to parse the string', () async { + await expectLater( + runWithOverrides(shorebirdFlutter.getVersion), + completion(equals('3.10.6')), + ); + verify( + () => git.forEachRef( + directory: p.join(flutterDirectory.parent.path, flutterRevision), + pointsAt: flutterRevision, + format: '%(refname:short)', + pattern: 'refs/remotes/origin/flutter_release/*', + ), + ).called(1); + }); + }); + group('getVersions', () { const format = '%(refname:short)'; const pattern = 'refs/remotes/origin/flutter_release/*'; diff --git a/packages/shorebird_cli/test/src/validators/shorebird_flutter_validator_test.dart b/packages/shorebird_cli/test/src/validators/shorebird_flutter_validator_test.dart index d026464d..bdb5d567 100644 --- a/packages/shorebird_cli/test/src/validators/shorebird_flutter_validator_test.dart +++ b/packages/shorebird_cli/test/src/validators/shorebird_flutter_validator_test.dart @@ -59,9 +59,10 @@ void main() { ).thenReturn(flutterDirectory(tempDir)); when(() => platform.environment).thenReturn({}); when( - () => shorebirdFlutter.getVersion( - useVendedFlutter: any(named: 'useVendedFlutter'), - ), + () => shorebirdFlutter.getVersion(), + ).thenAnswer((_) async => flutterVersion); + when( + () => shorebirdFlutter.getSystemVersion(), ).thenAnswer((_) async => flutterVersion); validator = ShorebirdFlutterValidator(); @@ -115,7 +116,7 @@ void main() { ' major and minor but different patch versions', () async { when( - () => shorebirdFlutter.getVersion(useVendedFlutter: false), + () => shorebirdFlutter.getSystemVersion(), ).thenAnswer((_) async => '3.7.10'); final results = await runWithOverrides( @@ -131,7 +132,7 @@ void main() { 'than shorebird flutter', () async { when( - () => shorebirdFlutter.getVersion(useVendedFlutter: false), + () => shorebirdFlutter.getSystemVersion(), ).thenAnswer((_) async => '3.8.9'); final results = await runWithOverrides(validator.validate); @@ -170,9 +171,7 @@ void main() { ); test('throws exception if path flutter version lookup fails', () async { - when( - () => shorebirdFlutter.getVersion(useVendedFlutter: false), - ).thenThrow( + when(() => shorebirdFlutter.getSystemVersion()).thenThrow( const ProcessException( 'flutter', ['--version'],