diff --git a/packages/shorebird_cli/lib/src/shorebird_flutter.dart b/packages/shorebird_cli/lib/src/shorebird_flutter.dart index b65c2fa6..bf5e3d1a 100644 --- a/packages/shorebird_cli/lib/src/shorebird_flutter.dart +++ b/packages/shorebird_cli/lib/src/shorebird_flutter.dart @@ -68,9 +68,17 @@ class ShorebirdFlutter { /// Throws a [ProcessException] if the version check fails. /// Returns `null` if the version check succeeds but the version cannot be /// parsed. - Future getVersion() async { + /// + /// If [useVendedFlutter] is `true`, the vended Flutter is used instead of + /// the system Flutter. Defaults to true. + Future getVersion({bool useVendedFlutter = true}) async { const args = ['--version']; - final result = await process.run(executable, args, runInShell: true); + final result = await process.run( + executable, + args, + runInShell: true, + useVendedFlutter: useVendedFlutter, + ); if (result.exitCode != 0) { throw ProcessException( 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 eff2b2b8..c106564d 100644 --- a/packages/shorebird_cli/lib/src/validators/shorebird_flutter_validator.dart +++ b/packages/shorebird_cli/lib/src/validators/shorebird_flutter_validator.dart @@ -1,7 +1,7 @@ import 'package:shorebird_cli/src/platform.dart'; -import 'package:shorebird_cli/src/process.dart'; import 'package:shorebird_cli/src/shorebird_env.dart'; import 'package:shorebird_cli/src/shorebird_flutter.dart'; +import 'package:shorebird_cli/src/third_party/flutter_tools/lib/flutter_tools.dart'; import 'package:shorebird_cli/src/validators/validators.dart'; import 'package:version/version.dart'; @@ -18,8 +18,6 @@ class FlutterValidationException implements Exception { class ShorebirdFlutterValidator extends Validator { ShorebirdFlutterValidator(); - final _flutterVersionRegex = RegExp(r'Flutter (\d+.\d+.\d+)'); - @override String get description => 'Flutter install is correct'; @@ -53,7 +51,7 @@ class ShorebirdFlutterValidator extends Validator { String? shorebirdFlutterVersionString; try { - shorebirdFlutterVersionString = await _shorebirdFlutterVersion(process); + shorebirdFlutterVersionString = await _getFlutterVersion(); } catch (error) { issues.add( ValidationIssue( @@ -65,7 +63,9 @@ class ShorebirdFlutterValidator extends Validator { String? pathFlutterVersionString; try { - pathFlutterVersionString = await _pathFlutterVersion(process); + pathFlutterVersionString = await _getFlutterVersion( + useVendedFlutter: false, + ); } catch (error) { issues.add( ValidationIssue( @@ -113,42 +113,24 @@ This can cause unexpected behavior if you are switching between the tools and th return issues; } - Future _shorebirdFlutterVersion(ShorebirdProcess process) { - return _getFlutterVersion(process: process); - } - - Future _pathFlutterVersion(ShorebirdProcess process) { - return _getFlutterVersion( - process: process, - useVendedFlutter: false, - ); - } - - Future _getFlutterVersion({ - required ShorebirdProcess process, - bool useVendedFlutter = true, - }) async { - final result = await process.run( - 'flutter', - ['--version'], - useVendedFlutter: useVendedFlutter, - runInShell: true, - ); - - if (result.exitCode != 0) { + Future _getFlutterVersion({bool useVendedFlutter = true}) async { + final String? version; + try { + version = await shorebirdFlutter.getVersion( + useVendedFlutter: useVendedFlutter, + ); + } on ProcessException catch (error) { throw FlutterValidationException( - 'Flutter version check did not complete successfully. ${result.stderr}', + 'Flutter version check did not complete successfully. ${error.message}', ); } - final output = result.stdout.toString(); - final match = _flutterVersionRegex.firstMatch(output); - if (match == null) { - throw FlutterValidationException( - 'Could not find version number in $output', + if (version == null) { + throw const FlutterValidationException( + 'Could not detect version number in output', ); } - return match.group(1)!; + return version; } } diff --git a/packages/shorebird_cli/test/src/shorebird_flutter_test.dart b/packages/shorebird_cli/test/src/shorebird_flutter_test.dart index 6c5c53b9..511a9a59 100644 --- a/packages/shorebird_cli/test/src/shorebird_flutter_test.dart +++ b/packages/shorebird_cli/test/src/shorebird_flutter_test.dart @@ -84,7 +84,12 @@ void main() { when(() => shorebirdEnv.flutterDirectory).thenReturn(flutterDirectory); when(() => shorebirdEnv.flutterRevision).thenReturn(flutterRevision); when( - () => process.run('flutter', ['--version'], runInShell: true), + () => process.run( + 'flutter', + ['--version'], + runInShell: true, + useVendedFlutter: any(named: 'useVendedFlutter'), + ), ).thenAnswer((_) async => processResult); when(() => processResult.exitCode).thenReturn(0); }); @@ -129,6 +134,23 @@ Tools • Dart 3.0.6 • DevTools 2.23.1'''); () => 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', + ['--version'], + runInShell: true, + useVendedFlutter: false, + ), + ).called(1); + }); }); group('getVersions', () { 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 4f1e8ecb..d026464d 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 @@ -5,45 +5,24 @@ import 'package:path/path.dart' as p; import 'package:platform/platform.dart'; import 'package:scoped/scoped.dart'; import 'package:shorebird_cli/src/platform.dart'; -import 'package:shorebird_cli/src/process.dart'; import 'package:shorebird_cli/src/shorebird_env.dart'; import 'package:shorebird_cli/src/shorebird_flutter.dart'; import 'package:shorebird_cli/src/validators/validators.dart'; import 'package:test/test.dart'; -class _MockProcessResult extends Mock implements ShorebirdProcessResult {} - class _MockPlatform extends Mock implements Platform {} class _MockShorebirdEnv extends Mock implements ShorebirdEnv {} class _MockShorebirdFlutter extends Mock implements ShorebirdFlutter {} -class _MockShorebirdProcess extends Mock implements ShorebirdProcess {} - void main() { group(ShorebirdFlutterValidator, () { const flutterRevision = '45fc514f1a9c347a3af76b02baf980a4d88b7879'; - - const pathFlutterVersionMessage = ''' -Flutter 3.7.9 • channel unknown • unknown source -Framework • revision 62bd79521d (7 days ago) • 2023-03-30 10:59:36 -0700 -Engine • revision ec975089ac -Tools • Dart 2.19.6 • DevTools 2.20.1 -'''; - - const shorebirdFlutterVersionMessage = ''' -Flutter 3.7.9 • channel stable • https://github.com/shorebirdtech/flutter.git -Framework • revision 62bd79521d (7 days ago) • 2023-03-30 10:59:36 -0700 -Engine • revision ec975089ac -Tools • Dart 2.19.6 • DevTools 2.20.1 -'''; + const flutterVersion = '3.7.9'; late ShorebirdFlutterValidator validator; late Directory tempDir; - late ShorebirdProcessResult pathFlutterVersionProcessResult; - late ShorebirdProcessResult shorebirdFlutterVersionProcessResult; - late ShorebirdProcess shorebirdProcess; late ShorebirdEnv shorebirdEnv; late ShorebirdFlutter shorebirdFlutter; late Platform platform; @@ -53,7 +32,6 @@ Tools • Dart 2.19.6 • DevTools 2.20.1 () => body(), values: { platformRef.overrideWith(() => platform), - processRef.overrideWith(() => shorebirdProcess), shorebirdEnvRef.overrideWith(() => shorebirdEnv), shorebirdFlutterRef.overrideWith(() => shorebirdFlutter), }, @@ -63,12 +41,8 @@ Tools • Dart 2.19.6 • DevTools 2.20.1 Directory flutterDirectory(Directory root) => Directory(p.join(root.path, 'bin', 'cache', 'flutter')); - File shorebirdScriptFile(Directory root) => - File(p.join(root.path, 'bin', 'cache', 'shorebird.snapshot')); - Directory setupTempDirectory() { final tempDir = Directory.systemTemp.createTempSync(); - shorebirdScriptFile(tempDir).createSync(recursive: true); flutterDirectory(tempDir).createSync(recursive: true); return tempDir; } @@ -83,12 +57,12 @@ Tools • Dart 2.19.6 • DevTools 2.20.1 when( () => shorebirdEnv.flutterDirectory, ).thenReturn(flutterDirectory(tempDir)); - when(() => platform.script).thenReturn(shorebirdScriptFile(tempDir).uri); when(() => platform.environment).thenReturn({}); - - pathFlutterVersionProcessResult = _MockProcessResult(); - shorebirdFlutterVersionProcessResult = _MockProcessResult(); - shorebirdProcess = _MockShorebirdProcess(); + when( + () => shorebirdFlutter.getVersion( + useVendedFlutter: any(named: 'useVendedFlutter'), + ), + ).thenAnswer((_) async => flutterVersion); validator = ShorebirdFlutterValidator(); when( @@ -96,30 +70,6 @@ Tools • Dart 2.19.6 • DevTools 2.20.1 revision: any(named: 'revision'), ), ).thenAnswer((_) async => true); - when( - () => shorebirdProcess.run( - 'flutter', - ['--version'], - runInShell: any(named: 'runInShell'), - ), - ).thenAnswer((_) async => shorebirdFlutterVersionProcessResult); - when( - () => shorebirdProcess.run( - 'flutter', - ['--version'], - useVendedFlutter: false, - runInShell: any(named: 'runInShell'), - ), - ).thenAnswer((_) async => pathFlutterVersionProcessResult); - - when(() => pathFlutterVersionProcessResult.stdout) - .thenReturn(pathFlutterVersionMessage); - when(() => pathFlutterVersionProcessResult.stderr).thenReturn(''); - when(() => pathFlutterVersionProcessResult.exitCode).thenReturn(0); - when(() => shorebirdFlutterVersionProcessResult.stdout) - .thenReturn(shorebirdFlutterVersionMessage); - when(() => shorebirdFlutterVersionProcessResult.stderr).thenReturn(''); - when(() => shorebirdFlutterVersionProcessResult.exitCode).thenReturn(0); }); test('has a non-empty description', () { @@ -164,9 +114,9 @@ Tools • Dart 2.19.6 • DevTools 2.20.1 'does not warn if flutter version and shorebird flutter version have same' ' major and minor but different patch versions', () async { - when(() => pathFlutterVersionProcessResult.stdout).thenReturn( - pathFlutterVersionMessage.replaceAll('3.7.9', '3.7.10'), - ); + when( + () => shorebirdFlutter.getVersion(useVendedFlutter: false), + ).thenAnswer((_) async => '3.7.10'); final results = await runWithOverrides( () => validator.validate(), @@ -180,9 +130,9 @@ Tools • Dart 2.19.6 • DevTools 2.20.1 'warns when path flutter version has different major or minor version ' 'than shorebird flutter', () async { - when(() => pathFlutterVersionProcessResult.stdout).thenReturn( - pathFlutterVersionMessage.replaceAll('3.7.9', '3.8.9'), - ); + when( + () => shorebirdFlutter.getVersion(useVendedFlutter: false), + ).thenAnswer((_) async => '3.8.9'); final results = await runWithOverrides(validator.validate); @@ -219,10 +169,17 @@ Tools • Dart 2.19.6 • DevTools 2.20.1 }, ); - test('throws exception if path flutter version output is malformed', - () async { - when(() => pathFlutterVersionProcessResult.stdout) - .thenReturn('OH NO THERE IS NO FLUTTER VERSION HERE'); + test('throws exception if path flutter version lookup fails', () async { + when( + () => shorebirdFlutter.getVersion(useVendedFlutter: false), + ).thenThrow( + const ProcessException( + 'flutter', + ['--version'], + 'OH NO THERE IS NO FLUTTER VERSION HERE', + 1, + ), + ); final results = await runWithOverrides(validator.validate); @@ -237,48 +194,18 @@ Tools • Dart 2.19.6 • DevTools 2.20.1 ); }); - test('prints stderr output and throws if path Flutterversion check fails', + test('throws exception if shorebird flutter version lookup fails', () async { - when(() => pathFlutterVersionProcessResult.exitCode).thenReturn(1); - when(() => pathFlutterVersionProcessResult.stderr) - .thenReturn('error getting Flutter version'); - - final results = await runWithOverrides(validator.validate); - - expect(results, hasLength(1)); - expect( - results[0], - isA().having( - (exception) => exception.message, - 'message', - contains('Failed to determine path Flutter version'), + when( + () => shorebirdFlutter.getVersion(), + ).thenThrow( + const ProcessException( + 'flutter', + ['--version'], + 'OH NO THERE IS NO FLUTTER VERSION HERE', + 1, ), ); - }); - - test('throws exception if shorebird flutter version output is malformed', - () async { - when(() => shorebirdFlutterVersionProcessResult.stdout) - .thenReturn('OH NO THERE IS NO FLUTTER VERSION HERE'); - - final results = await runWithOverrides(validator.validate); - - expect(results, hasLength(1)); - expect( - results[0], - isA().having( - (exception) => exception.message, - 'message', - contains('Failed to determine Shorebird Flutter version'), - ), - ); - }); - - test('prints stderr output and throws if path Flutterversion check fails', - () async { - when(() => shorebirdFlutterVersionProcessResult.exitCode).thenReturn(1); - when(() => shorebirdFlutterVersionProcessResult.stderr) - .thenReturn('error getting Flutter version'); final results = await runWithOverrides(validator.validate);