fix(shorebird_cli): handle exceptions during validation more graceful…

…ly (#359)
shorebirdtech · Apr 24, 2023 · 624dafc · 624dafc
1 parent cc97ca2
commit 624dafc
Show file tree

Hide file tree

Showing 9 changed files with 94 additions and 21 deletions.
diff --git a/packages/shorebird_cli/lib/src/commands/build/build_apk_command.dart b/packages/shorebird_cli/lib/src/commands/build/build_apk_command.dart
@@ -3,9 +3,9 @@ import 'dart:io';
 import 'package:mason_logger/mason_logger.dart';
 import 'package:shorebird_cli/src/auth_logger_mixin.dart';
 import 'package:shorebird_cli/src/command.dart';
-import 'package:shorebird_cli/src/flutter_validation_mixin.dart';
 import 'package:shorebird_cli/src/shorebird_build_mixin.dart';
 import 'package:shorebird_cli/src/shorebird_config_mixin.dart';
+import 'package:shorebird_cli/src/shorebird_validation_mixin.dart';
 
 /// {@template build_apk_command}
 ///

diff --git a/packages/shorebird_cli/lib/src/commands/build/build_app_bundle_command.dart b/packages/shorebird_cli/lib/src/commands/build/build_app_bundle_command.dart
@@ -3,9 +3,9 @@ import 'dart:io';
 import 'package:mason_logger/mason_logger.dart';
 import 'package:shorebird_cli/src/auth_logger_mixin.dart';
 import 'package:shorebird_cli/src/command.dart';
-import 'package:shorebird_cli/src/flutter_validation_mixin.dart';
 import 'package:shorebird_cli/src/shorebird_build_mixin.dart';
 import 'package:shorebird_cli/src/shorebird_config_mixin.dart';
+import 'package:shorebird_cli/src/shorebird_validation_mixin.dart';
 
 /// {@template build_app_bundle_command}
 ///

diff --git a/packages/shorebird_cli/lib/src/commands/build/build_command.dart b/packages/shorebird_cli/lib/src/commands/build/build_command.dart
@@ -1,8 +1,8 @@
 import 'package:shorebird_cli/src/command.dart';
 import 'package:shorebird_cli/src/commands/build/build.dart';
-import 'package:shorebird_cli/src/flutter_validation_mixin.dart';
 import 'package:shorebird_cli/src/shorebird_build_mixin.dart';
 import 'package:shorebird_cli/src/shorebird_config_mixin.dart';
+import 'package:shorebird_cli/src/shorebird_validation_mixin.dart';
 
 /// {@template build_command}
 ///

diff --git a/packages/shorebird_cli/lib/src/commands/patch_command.dart b/packages/shorebird_cli/lib/src/commands/patch_command.dart
@@ -7,10 +7,10 @@ import 'package:mason_logger/mason_logger.dart';
 import 'package:path/path.dart' as p;
 import 'package:shorebird_cli/src/auth_logger_mixin.dart';
 import 'package:shorebird_cli/src/command.dart';
-import 'package:shorebird_cli/src/flutter_validation_mixin.dart';
 import 'package:shorebird_cli/src/shorebird_build_mixin.dart';
 import 'package:shorebird_cli/src/shorebird_config_mixin.dart';
 import 'package:shorebird_cli/src/shorebird_create_app_mixin.dart';
+import 'package:shorebird_cli/src/shorebird_validation_mixin.dart';
 import 'package:shorebird_code_push_client/shorebird_code_push_client.dart';
 
 /// Metadata about a patch artifact that we are about to upload.

diff --git a/packages/shorebird_cli/lib/src/commands/release_command.dart b/packages/shorebird_cli/lib/src/commands/release_command.dart
@@ -6,10 +6,10 @@ import 'package:mason_logger/mason_logger.dart';
 import 'package:path/path.dart' as p;
 import 'package:shorebird_cli/src/auth_logger_mixin.dart';
 import 'package:shorebird_cli/src/command.dart';
-import 'package:shorebird_cli/src/flutter_validation_mixin.dart';
 import 'package:shorebird_cli/src/shorebird_build_mixin.dart';
 import 'package:shorebird_cli/src/shorebird_config_mixin.dart';
 import 'package:shorebird_cli/src/shorebird_create_app_mixin.dart';
+import 'package:shorebird_cli/src/shorebird_validation_mixin.dart';
 import 'package:shorebird_code_push_client/shorebird_code_push_client.dart';
 
 /// {@template release_command}

diff --git a/packages/shorebird_cli/lib/src/commands/run_command.dart b/packages/shorebird_cli/lib/src/commands/run_command.dart
@@ -3,8 +3,8 @@ import 'dart:convert';
 import 'package:mason_logger/mason_logger.dart';
 import 'package:shorebird_cli/src/auth_logger_mixin.dart';
 import 'package:shorebird_cli/src/command.dart';
-import 'package:shorebird_cli/src/flutter_validation_mixin.dart';
 import 'package:shorebird_cli/src/shorebird_config_mixin.dart';
+import 'package:shorebird_cli/src/shorebird_validation_mixin.dart';
 
 /// {@template run_command}
 /// `shorebird run`

diff --git a/...cli/lib/src/flutter_validation_mixin.dart → ...i/lib/src/shorebird_validation_mixin.dart b/...cli/lib/src/flutter_validation_mixin.dart → ...i/lib/src/shorebird_validation_mixin.dart
diff --git a/packages/shorebird_cli/lib/src/validators/shorebird_flutter_validator.dart b/packages/shorebird_cli/lib/src/validators/shorebird_flutter_validator.dart
@@ -55,10 +55,33 @@ class ShorebirdFlutterValidator extends Validator {
       );
     }
 
-    final shorebirdFlutterVersion = await _shorebirdFlutterVersion(process);
-    final pathFlutterVersion = await _pathFlutterVersion(process);
+    String? shorebirdFlutterVersion;
+    try {
+      shorebirdFlutterVersion = await _shorebirdFlutterVersion(process);
+    } catch (error) {
+      issues.add(
+        ValidationIssue(
+          severity: ValidationIssueSeverity.error,
+          message: 'Failed to determine Shorebird Flutter version. $error',
+        ),
+      );
+    }
+
+    String? pathFlutterVersion;
+    try {
+      pathFlutterVersion = await _pathFlutterVersion(process);
+    } catch (error) {
+      issues.add(
+        ValidationIssue(
+          severity: ValidationIssueSeverity.error,
+          message: 'Failed to determine path Flutter version. $error',
+        ),
+      );
+    }
 
-    if (shorebirdFlutterVersion != pathFlutterVersion) {
+    if (shorebirdFlutterVersion != null &&
+        pathFlutterVersion != null &&
+        shorebirdFlutterVersion != pathFlutterVersion) {
       final message = '''
 The version of Flutter that Shorebird includes and the Flutter on your path are different.
 \tShorebird Flutter: $shorebirdFlutterVersion
@@ -147,7 +170,7 @@ This can cause unexpected behavior if you are switching between the tools and th
     final match = _flutterVersionRegex.firstMatch(output);
     if (match == null) {
       throw FlutterValidationException(
-        'Could not find version match in $output',
+        'Could not find version number in $output',
       );
     }
 

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
@@ -15,7 +15,7 @@ class _MockPlatform extends Mock implements Platform {}
 class _MockShorebirdProcess extends Mock implements ShorebirdProcess {}
 
 void main() {
-  group('ShorebirdFlutterValidator', () {
+  group(ShorebirdFlutterValidator, () {
     const flutterRevision = '45fc514f1a9c347a3af76b02baf980a4d88b7879';
     const gitStatusMessage = '''
 HEAD detached at 45fc514f
@@ -196,26 +196,76 @@ Tools • Dart 2.19.6 • DevTools 2.20.1
       },
     );
 
-    test('throws exception if flutter version output is malformed', () async {
+    test('throws exception if path flutter version output is malformed',
+        () async {
       when(() => pathFlutterVersionProcessResult.stdout)
           .thenReturn('OH NO THERE IS NO FLUTTER VERSION HERE');
 
-      expect(() async => validator.validate(shorebirdProcess), throwsException);
+      final results = await validator.validate(shorebirdProcess);
+
+      expect(results, hasLength(1));
+      expect(
+        results[0],
+        isA<ValidationIssue>().having(
+          (exception) => exception.message,
+          'message',
+          contains('Failed to determine path Flutter version'),
+        ),
+      );
     });
 
-    test('prints stderr output and throws if version check fails', () async {
+    test('prints stderr output and throws if path Flutterversion check fails',
+        () async {
       when(() => pathFlutterVersionProcessResult.exitCode).thenReturn(1);
       when(() => pathFlutterVersionProcessResult.stderr)
           .thenReturn('error getting Flutter version');
 
+      final results = await validator.validate(shorebirdProcess);
+
+      expect(results, hasLength(1));
       expect(
-        () async => validator.validate(shorebirdProcess),
-        throwsA(
-          isA<FlutterValidationException>().having(
-            (e) => e.message,
-            'message',
-            contains('error getting Flutter version'),
-          ),
+        results[0],
+        isA<ValidationIssue>().having(
+          (exception) => exception.message,
+          'message',
+          contains('Failed to determine path Flutter version'),
+        ),
+      );
+    });
+
+    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 validator.validate(shorebirdProcess);
+
+      expect(results, hasLength(1));
+      expect(
+        results[0],
+        isA<ValidationIssue>().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 validator.validate(shorebirdProcess);
+
+      expect(results, hasLength(1));
+      expect(
+        results[0],
+        isA<ValidationIssue>().having(
+          (exception) => exception.message,
+          'message',
+          contains('Failed to determine Shorebird Flutter version'),
         ),
       );
     });