diff --git a/packages/shorebird_cli/lib/src/doctor.dart b/packages/shorebird_cli/lib/src/doctor.dart index 5ae7062a1..ec4ffc2ea 100644 --- a/packages/shorebird_cli/lib/src/doctor.dart +++ b/packages/shorebird_cli/lib/src/doctor.dart @@ -35,6 +35,7 @@ class Doctor { AndroidInternetPermissionValidator(), MacosNetworkEntitlementValidator(), ShorebirdYamlAssetValidator(), + TrackedLockFilesValidator(), ]; /// Run the provided [validators]. If [applyFixes] is `true`, any validation diff --git a/packages/shorebird_cli/lib/src/executables/git.dart b/packages/shorebird_cli/lib/src/executables/git.dart index b645853e7..c9b53d0c1 100644 --- a/packages/shorebird_cli/lib/src/executables/git.dart +++ b/packages/shorebird_cli/lib/src/executables/git.dart @@ -1,3 +1,4 @@ +// cspell:words unmatch import 'dart:io'; import 'package:scoped_deps/scoped_deps.dart'; @@ -153,4 +154,40 @@ class Git { return (await symbolicRef(directory: directory)) .replaceAll('refs/heads/', ''); } + + /// Whether [directory] is part of a git repository. + Future 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 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; + } } diff --git a/packages/shorebird_cli/lib/src/validators/tracked_lock_files_validator.dart b/packages/shorebird_cli/lib/src/validators/tracked_lock_files_validator.dart new file mode 100644 index 000000000..bdeb3b546 --- /dev/null +++ b/packages/shorebird_cli/lib/src/validators/tracked_lock_files_validator.dart @@ -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> 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 = []; + 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 _fileExistsAndIsNotTracked(File file) async { + return file.existsSync() && !(await git.isFileTracked(file: file)); + } +} diff --git a/packages/shorebird_cli/lib/src/validators/validators.dart b/packages/shorebird_cli/lib/src/validators/validators.dart index b97e38a31..8be09685e 100644 --- a/packages/shorebird_cli/lib/src/validators/validators.dart +++ b/packages/shorebird_cli/lib/src/validators/validators.dart @@ -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 { diff --git a/packages/shorebird_cli/test/src/executables/git_test.dart b/packages/shorebird_cli/test/src/executables/git_test.dart index 47e9149e0..f9ddc198d 100644 --- a/packages/shorebird_cli/test/src/executables/git_test.dart +++ b/packages/shorebird_cli/test/src/executables/git_test.dart @@ -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); + }); + }); + }); }); } diff --git a/packages/shorebird_cli/test/src/validators/tracked_lock_files_validator_test.dart b/packages/shorebird_cli/test/src/validators/tracked_lock_files_validator_test.dart new file mode 100644 index 000000000..dc87d889c --- /dev/null +++ b/packages/shorebird_cli/test/src/validators/tracked_lock_files_validator_test.dart @@ -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 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); + }); + }); + }); + }); +}