Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Apr 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions bin/internal/shared.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class DoctorCommand extends ShorebirdCommand with ShorebirdVersionMixin {
ShorebirdVersionValidator(
isShorebirdVersionCurrent: isShorebirdVersionCurrent,
),
ShorebirdFlutterValidator(runProcess: runProcess),
AndroidInternetPermissionValidator(),
];
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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');
}
}
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export 'android_internet_permission_validator.dart';
export 'shorebird_flutter_validator.dart';
export 'shorebird_version_validator.dart';
36 changes: 25 additions & 11 deletions packages/shorebird_cli/lib/src/shorebird_paths.dart
Original file line number Diff line number Diff line change
@@ -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;

/// The root of the Shorebird-vended Flutter git checkout.
static Directory get flutterDirectory => Directory(
p.join(
shorebirdRoot.path,
'bin',
'cache',
'flutter',
),
);

/// Path to the Shorebird-vended Flutter binary.
static String get flutterBinaryPath => p.join(
shorebirdRoot.path,
'bin',
'cache',
'flutter',
'bin',
'flutter',
/// The Shorebird-vended Flutter binary.
static File get flutterBinaryFile => File(
p.join(
flutterDirectory.path,
'bin',
'flutter',
),
);
}
2 changes: 1 addition & 1 deletion packages/shorebird_cli/lib/src/shorebird_process.dart
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ abstract class ShorebirdProcess {

static String _resolveExecutable(String executable) {
if (executable == 'flutter') {
return ShorebirdPaths.flutterBinaryPath;
return ShorebirdPaths.flutterBinaryFile.path;
}

return executable;
Expand Down
1 change: 1 addition & 0 deletions packages/shorebird_cli/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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'));
});
});
}
12 changes: 7 additions & 5 deletions packages/shorebird_cli/test/src/shorebird_process_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
Expand Down