Skip to content

Commit

Permalink
feat(shorebird_cli): add doctor check that lockfiles are tracked in s…
Browse files Browse the repository at this point in the history
…ource control (#2738)

Co-authored-by: Felix Angelov <[email protected]>
  • Loading branch information
bryanoltman and felangel authored Jan 3, 2025
1 parent bcdf128 commit 10322b9
Show file tree
Hide file tree
Showing 6 changed files with 311 additions and 0 deletions.
1 change: 1 addition & 0 deletions packages/shorebird_cli/lib/src/doctor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class Doctor {
AndroidInternetPermissionValidator(),
MacosNetworkEntitlementValidator(),
ShorebirdYamlAssetValidator(),
TrackedLockFilesValidator(),
];

/// Run the provided [validators]. If [applyFixes] is `true`, any validation
Expand Down
37 changes: 37 additions & 0 deletions packages/shorebird_cli/lib/src/executables/git.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// cspell:words unmatch
import 'dart:io';

import 'package:scoped_deps/scoped_deps.dart';
Expand Down Expand Up @@ -153,4 +154,40 @@ class Git {
return (await symbolicRef(directory: directory))
.replaceAll('refs/heads/', '');
}

/// Whether [directory] is part of a git repository.
Future<bool> isGitRepo({required Directory directory}) async {
try {
// [git] throws if the command's exit code is nonzero, which is what we're
// checking for here.
await git(
['status'],
workingDirectory: directory.path,
);
} on Exception {
return false;
}

return true;
}

/// Whether [file] is tracked by its git repository.
Future<bool> isFileTracked({required File file}) async {
try {
// [git] throws if the command's exit code is nonzero, which is what we're
// checking for here.
await git(
[
'ls-files',
'--error-unmatch',
file.absolute.path,
],
workingDirectory: file.parent.path,
);
} on Exception {
return false;
}

return true;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import 'dart:io';

import 'package:path/path.dart' as p;
import 'package:shorebird_cli/src/executables/git.dart';
import 'package:shorebird_cli/src/shorebird_env.dart';
import 'package:shorebird_cli/src/validators/validators.dart';

/// Checks that .lock files (pubspec.lock, Podfile.lock) are tracked in source
/// control if they exist and if the project is part of a git repository.
class TrackedLockFilesValidator extends Validator {
@override
String get description => 'Lock files are tracked in source control';

@override
bool canRunInCurrentContext() => shorebirdEnv.hasPubspecYaml;

@override
Future<List<ValidationIssue>> validate() async {
final projectRoot = shorebirdEnv.getFlutterProjectRoot();
if (projectRoot == null) {
return [];
}

final isGitRepo = await git.isGitRepo(directory: projectRoot);
if (!isGitRepo) {
// Don't return an issue if the project is not tracked in git. The user
// may be using a different source control system.
return [];
}

final lockFilePaths = [
'pubspec.lock',
p.join('ios', 'Podfile.lock'),
p.join('macos', 'Podfile.lock'),
];

final warnings = <ValidationIssue>[];
for (final path in lockFilePaths) {
final file = File(p.join(projectRoot.path, path));
if (await _fileExistsAndIsNotTracked(file)) {
warnings.add(
ValidationIssue.warning(
message:
'''$path is not tracked in source control. We recommend tracking lock files in source control to avoid unexpected dependency version changes.''',
),
);
}
}

return warnings;
}

/// Returns true if [file] exists but is not tracked in git.
Future<bool> _fileExistsAndIsNotTracked(File file) async {
return file.existsSync() && !(await git.isFileTracked(file: file));
}
}
1 change: 1 addition & 0 deletions packages/shorebird_cli/lib/src/validators/validators.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export 'flavor_validator.dart';
export 'macos_network_entitlement_validator.dart';
export 'shorebird_version_validator.dart';
export 'shorebird_yaml_asset_validator.dart';
export 'tracked_lock_files_validator.dart';

/// Severity level of a [ValidationIssue].
enum ValidationIssueSeverity {
Expand Down
58 changes: 58 additions & 0 deletions packages/shorebird_cli/test/src/executables/git_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -516,5 +516,63 @@ refs/heads/main
);
});
});

group('isGitRepo', () {
group('when git exits with code 0', () {
setUp(() {
when(() => processResult.exitCode).thenReturn(ExitCode.success.code);
});

test('returns true', () async {
final directory = Directory.current;
final result = await runWithOverrides(
() => git.isGitRepo(directory: directory),
);
expect(result, isTrue);
});
});

group('when git exits with nonzero code', () {
setUp(() {
when(() => processResult.exitCode).thenReturn(ExitCode.software.code);
});

test('returns true', () async {
final directory = Directory.current;
final result = await runWithOverrides(
() => git.isGitRepo(directory: directory),
);
expect(result, isFalse);
});
});
});

group('isFileTracked', () {
group('when git exits with code 0', () {
setUp(() {
when(() => processResult.exitCode).thenReturn(ExitCode.success.code);
});

test('returns true', () async {
final result = await runWithOverrides(
() => git.isFileTracked(file: File('file')),
);
expect(result, isTrue);
});
});

group('when git exits with nonzero code', () {
setUp(() {
when(() => processResult.exitCode).thenReturn(ExitCode.software.code);
});

test('returns true', () async {
final result = await runWithOverrides(
() => git.isFileTracked(file: File('file')),
);
expect(result, isFalse);
});
});
});
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
import 'dart:io';

import 'package:mocktail/mocktail.dart';
import 'package:path/path.dart' as p;
import 'package:scoped_deps/scoped_deps.dart';
import 'package:shorebird_cli/src/executables/git.dart';
import 'package:shorebird_cli/src/shorebird_env.dart';
import 'package:shorebird_cli/src/validators/tracked_lock_files_validator.dart';
import 'package:shorebird_cli/src/validators/validators.dart';
import 'package:test/test.dart';

import '../mocks.dart';

void main() {
group(TrackedLockFilesValidator, () {
late Git git;
late ShorebirdEnv shorebirdEnv;
late Directory projectRoot;

late TrackedLockFilesValidator validator;

R runWithOverrides<R>(R Function() body) {
return runScoped(
body,
values: {
gitRef.overrideWith(() => git),
shorebirdEnvRef.overrideWith(() => shorebirdEnv),
},
);
}

setUpAll(() {
registerFallbackValue(Directory(''));
registerFallbackValue(File(''));
});

setUp(() {
git = MockGit();
shorebirdEnv = MockShorebirdEnv();
projectRoot = Directory.systemTemp.createTempSync();

when(() => shorebirdEnv.getFlutterProjectRoot()).thenReturn(projectRoot);

validator = TrackedLockFilesValidator();
});

test('has a non-empty description', () {
expect(validator.description, isNotEmpty);
});

group('canRunInCurrentContext', () {
group('when a pubspec.yaml file exists', () {
setUp(() {
when(() => shorebirdEnv.hasPubspecYaml).thenReturn(true);
});

test('returns true', () {
expect(runWithOverrides(validator.canRunInCurrentContext), isTrue);
});
});

group('when a pubspec.yaml file does not exist', () {
setUp(() {
when(() => shorebirdEnv.hasPubspecYaml).thenReturn(false);
});

test('returns false', () {
expect(runWithOverrides(validator.canRunInCurrentContext), isFalse);
});
});
});

group('validate', () {
group('when no project root is found', () {
setUp(() {
when(() => shorebirdEnv.getFlutterProjectRoot()).thenReturn(null);
});

test('returns an empty list', () async {
final issues = await runWithOverrides(() => validator.validate());
expect(issues, isEmpty);
});
});

group('when project is not tracked in git', () {
setUp(() {
when(
() => git.isGitRepo(directory: any(named: 'directory')),
).thenAnswer((_) async => false);
});

test('returns no issues', () async {
final issues = await runWithOverrides(() => validator.validate());
expect(issues, isEmpty);
});
});

group('when a lock file does not exist', () {
setUp(() {
when(
() => git.isGitRepo(directory: any(named: 'directory')),
).thenAnswer((_) async => true);
});

test('does not warn about lock file not being tracked', () async {
final issues = await runWithOverrides(() => validator.validate());
expect(issues, isEmpty);
});
});

group('when a lock file exists but is not tracked', () {
setUp(() {
when(
() => git.isGitRepo(directory: any(named: 'directory')),
).thenAnswer((_) async => true);
when(
() => git.isFileTracked(file: any(named: 'file')),
).thenAnswer((_) async => false);

File(p.join(projectRoot.path, 'pubspec.lock')).createSync();
});

test('recommends adding lock file to source control', () async {
final issues = await runWithOverrides(() => validator.validate());
expect(issues, hasLength(1));
expect(
issues.first,
equals(
ValidationIssue.warning(
message:
'''pubspec.lock is not tracked in source control. We recommend tracking lock files in source control to avoid unexpected dependency version changes.''',
),
),
);
});
});

group('when lock file exists and is tracked', () {
setUp(() {
when(
() => git.isGitRepo(directory: any(named: 'directory')),
).thenAnswer((_) async => true);
when(
() => git.isFileTracked(file: any(named: 'file')),
).thenAnswer((_) async => true);

File(p.join(projectRoot.path, 'pubspec.lock')).createSync();
});

test('returns no issues', () async {
final issues = await runWithOverrides(() => validator.validate());
expect(issues, isEmpty);
});
});
});
});
}

0 comments on commit 10322b9

Please sign in to comment.