feat: Add doctor validator to ensure no issues exist with Flutter install (#230)

bin/internal/shared.sh

@@ -10,8 +10,6 @@ unset CDPATH
 
 # Either clones or pulls the Shorebird Flutter repository, depending on whether FLUTTER_PATH exists.
 function update_flutter {
-  # TODO(bryanoltman): add doctor check that we're on stable
-  # TODO(bryanoltman): add doctor check for modified files
   if [[ -d "$FLUTTER_PATH" ]]; then
     git --git-dir="$FLUTTER_PATH/.git" pull
   else

packages/shorebird_cli/lib/src/commands/doctor_command.dart

@@ -23,6 +23,7 @@ class DoctorCommand extends ShorebirdCommand with ShorebirdVersionMixin {
           ShorebirdVersionValidator(
             isShorebirdVersionCurrent: isShorebirdVersionCurrent,
           ),
+          ShorebirdFlutterValidator(runProcess: runProcess),
           AndroidInternetPermissionValidator(),
         ];
   }

packages/shorebird_cli/lib/src/doctor/validators/shorebird_flutter_validator.dart

@@ -0,0 +1,68 @@
+import 'package:shorebird_cli/src/doctor/doctor_validator.dart';
+import 'package:shorebird_cli/src/shorebird_paths.dart';
+import 'package:shorebird_cli/src/shorebird_process.dart';
+
+class ShorebirdFlutterValidator extends DoctorValidator {
+  ShorebirdFlutterValidator({required this.runProcess});
+
+  final RunProcess runProcess;
+
+  // coverage:ignore-start
+  @override
+  String get description => 'Flutter install is correct';
+  // coverage:ignore-end
+
+  @override
+  Future<List<ValidationIssue>> validate() async {
+    if (!ShorebirdPaths.flutterDirectory.existsSync()) {
+      return [
+        ValidationIssue(
+          severity: ValidationIssueSeverity.error,
+          message:
+              'No Flutter directory found at ${ShorebirdPaths.flutterDirectory}',
+        ),
+      ];
+    }
+
+    if (!await _flutterDirectoryIsClean()) {
+      return [
+        ValidationIssue(
+          severity: ValidationIssueSeverity.warning,
+          message: '${ShorebirdPaths.flutterDirectory} has local modifications',
+        ),
+      ];
+    }
+
+    if (!await _flutterDirectoryTracksStable()) {
+      return [
+        ValidationIssue(
+          severity: ValidationIssueSeverity.warning,
+          message:
+              '${ShorebirdPaths.flutterDirectory} is not on the "stable" branch',
+        ),
+      ];
+    }
+
+    return [];
+  }
+
+  Future<bool> _flutterDirectoryIsClean() async {
+    final result = await runProcess(
+      'git',
+      ['status'],
+      workingDirectory: ShorebirdPaths.flutterDirectory.path,
+    );
+    return result.stdout
+        .toString()
+        .contains('nothing to commit, working tree clean');
+  }
+
+  Future<bool> _flutterDirectoryTracksStable() async {
+    final result = await runProcess(
+      'git',
+      ['--no-pager', 'branch'],
+      workingDirectory: ShorebirdPaths.flutterDirectory.path,
+    );
+    return result.stdout.toString().contains('* stable');
+  }
+}

packages/shorebird_cli/lib/src/doctor/validators/validators.dart

@@ -1,2 +1,3 @@
 export 'android_internet_permission_validator.dart';
+export 'shorebird_flutter_validator.dart';
 export 'shorebird_version_validator.dart';

packages/shorebird_cli/lib/src/shorebird_paths.dart

@@ -1,21 +1,35 @@
-import 'dart:io';
+import 'dart:io' hide Platform;
 
+import 'package:meta/meta.dart';
 import 'package:path/path.dart' as p;
+import 'package:platform/platform.dart';
 
 abstract class ShorebirdPaths {
+  @visibleForTesting
+  static Platform platform = const LocalPlatform();
+
   /// The root directory of the Shorebird install.
   ///
   /// Assumes we are running from $ROOT/bin/cache.
-  static Directory shorebirdRoot =
-      File(Platform.script.toFilePath()).parent.parent.parent;
+  static Directory get shorebirdRoot =>
+      File(platform.script.toFilePath()).parent.parent.parent;
 
-  /// Path to the Shorebird-vended Flutter binary.
-  static String get flutterBinaryPath => p.join(
-        shorebirdRoot.path,
-        'bin',
-        'cache',
-        'flutter',
-        'bin',
-        'flutter',
+  /// The root of the Shorebird-vended Flutter git checkout.
+  static Directory get flutterDirectory => Directory(
+        p.join(
+          shorebirdRoot.path,
+          'bin',
+          'cache',
+          'flutter',
+        ),
+      );
+
+  /// The Shorebird-vended Flutter binary.
+  static File get flutterBinaryFile => File(
+        p.join(
+          flutterDirectory.path,
+          'bin',
+          'flutter',
+        ),
       );
 }

packages/shorebird_cli/lib/src/shorebird_process.dart

@@ -50,7 +50,7 @@ abstract class ShorebirdProcess {
 
   static String _resolveExecutable(String executable) {
     if (executable == 'flutter') {
-      return ShorebirdPaths.flutterBinaryPath;
+      return ShorebirdPaths.flutterBinaryFile.path;
     }
 
     return executable;

packages/shorebird_cli/pubspec.yaml

@@ -23,6 +23,7 @@ dependencies:
   mason_logger: ^0.2.4
   meta: ^1.9.0
   path: ^1.8.3
+  platform: ^3.1.0
   pubspec_parse: ^1.2.2
   shorebird_code_push_client:
     path: ../shorebird_code_push_client

packages/shorebird_cli/test/src/commands/doctor/validators/shorebird_flutter_validator_test.dart

@@ -0,0 +1,120 @@
+import 'dart:io' hide Platform;
+
+import 'package:collection/collection.dart';
+import 'package:mocktail/mocktail.dart';
+import 'package:path/path.dart' as p;
+import 'package:platform/platform.dart';
+import 'package:shorebird_cli/src/doctor/doctor_validator.dart';
+import 'package:shorebird_cli/src/doctor/validators/validators.dart';
+import 'package:shorebird_cli/src/shorebird_paths.dart';
+import 'package:test/test.dart';
+
+class _MockProcessResult extends Mock implements ProcessResult {}
+
+class _MockPlatform extends Mock implements Platform {}
+
+void main() {
+  group('ShorebirdFlutterValidator', () {
+    const gitStatusMessage = """
+On branch stable
+Your branch is up to date with 'origin/stable'.
+
+nothing to commit, working tree clean
+""";
+
+    const gitBranchMessage = '''
+  main
+* stable
+''';
+
+    late ShorebirdFlutterValidator validator;
+    late Directory tempDir;
+    late ProcessResult gitBranchProcessResult;
+    late ProcessResult gitStatusProcessResult;
+
+    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;
+    }
+
+    setUp(() {
+      tempDir = setupTempDirectory();
+
+      ShorebirdPaths.platform = _MockPlatform();
+      when(() => ShorebirdPaths.platform.script)
+          .thenReturn(shorebirdScriptFile(tempDir).uri);
+
+      gitBranchProcessResult = _MockProcessResult();
+      gitStatusProcessResult = _MockProcessResult();
+
+      validator = ShorebirdFlutterValidator(
+        runProcess: (
+          executable,
+          arguments, {
+          bool runInShell = false,
+          workingDirectory,
+        }) async {
+          if (executable == 'git') {
+            if (arguments.equals(['status'])) {
+              return gitStatusProcessResult;
+            } else if (arguments.equals(['--no-pager', 'branch'])) {
+              return gitBranchProcessResult;
+            }
+          }
+          return _MockProcessResult();
+        },
+      );
+
+      when(() => gitBranchProcessResult.stdout).thenReturn(gitBranchMessage);
+      when(() => gitStatusProcessResult.stdout).thenReturn(gitStatusMessage);
+    });
+
+    test('returns no issues when the Flutter install is good', () async {
+      final results = await validator.validate();
+
+      expect(results, isEmpty);
+    });
+
+    test('errors when Flutter does not exist', () async {
+      flutterDirectory(tempDir).deleteSync();
+
+      final results = await validator.validate();
+
+      expect(results, hasLength(1));
+      expect(results.first.severity, ValidationIssueSeverity.error);
+      expect(results.first.message, contains('No Flutter directory found'));
+    });
+
+    test('warns when Flutter has local modifications', () async {
+      when(() => gitStatusProcessResult.stdout)
+          .thenReturn('Changes not staged for commit');
+
+      final results = await validator.validate();
+
+      expect(results, hasLength(1));
+      expect(results.first.severity, ValidationIssueSeverity.warning);
+      expect(results.first.message, contains('has local modifications'));
+    });
+
+    test('warns when Flutter does not track stable', () async {
+      when(() => gitBranchProcessResult.stdout).thenReturn('''
+* main
+  stable
+''');
+
+      final results = await validator.validate();
+
+      expect(results, hasLength(1));
+      expect(results.first.severity, ValidationIssueSeverity.warning);
+      expect(results.first.message, contains('is not on the "stable" branch'));
+    });
+  });
+}

packages/shorebird_cli/test/src/shorebird_process_test.dart

@@ -94,11 +94,13 @@ void main() {
       test('replaces "flutter" with our local flutter', () async {
         await ShorebirdProcess.start('flutter', ['run'], runInShell: true);
 
-        verify(() => processWrapper.start(
-              'flutter/bin/flutter',
-              ['run'],
-              runInShell: true,
-            )).called(1);
+        verify(
+          () => processWrapper.start(
+            'flutter/bin/flutter',
+            ['run'],
+            runInShell: true,
+          ),
+        ).called(1);
       });
     });
   });