From 5ab5b2c97fa7ea4b9d2009f67f82455ea0c9cebc Mon Sep 17 00:00:00 2001 From: borjandev <64651636+borjandev@users.noreply.github.com> Date: Tue, 23 Apr 2024 22:48:07 +0200 Subject: [PATCH] feat(shorebird_cli): add `ShorebirdYamlAssetValidator` (#1907) Co-authored-by: Bryan Oltman Co-authored-by: Felix Angelov --- packages/shorebird_cli/bin/shorebird.dart | 2 + .../lib/src/commands/init_command.dart | 33 +-- packages/shorebird_cli/lib/src/doctor.dart | 1 + .../shorebird_cli/lib/src/pubspec_editor.dart | 54 +++++ .../shorebird_yaml_asset_validator.dart | 41 ++++ .../lib/src/validators/validators.dart | 1 + .../test/src/commands/init_command_test.dart | 85 +------- packages/shorebird_cli/test/src/mocks.dart | 3 + .../test/src/pubspec_editor_test.dart | 201 ++++++++++++++++++ .../shorebird_yaml_asset_validator_test.dart | 117 ++++++++++ 10 files changed, 429 insertions(+), 109 deletions(-) create mode 100644 packages/shorebird_cli/lib/src/pubspec_editor.dart create mode 100644 packages/shorebird_cli/lib/src/validators/shorebird_yaml_asset_validator.dart create mode 100644 packages/shorebird_cli/test/src/pubspec_editor_test.dart create mode 100644 packages/shorebird_cli/test/src/validators/shorebird_yaml_asset_validator_test.dart diff --git a/packages/shorebird_cli/bin/shorebird.dart b/packages/shorebird_cli/bin/shorebird.dart index 04e4f76d..d3b96ddc 100644 --- a/packages/shorebird_cli/bin/shorebird.dart +++ b/packages/shorebird_cli/bin/shorebird.dart @@ -17,6 +17,7 @@ import 'package:shorebird_cli/src/os/os.dart'; import 'package:shorebird_cli/src/patch_diff_checker.dart'; import 'package:shorebird_cli/src/platform.dart'; import 'package:shorebird_cli/src/platform/platform.dart'; +import 'package:shorebird_cli/src/pubspec_editor.dart'; import 'package:shorebird_cli/src/shorebird_artifacts.dart'; import 'package:shorebird_cli/src/shorebird_env.dart'; import 'package:shorebird_cli/src/shorebird_flutter.dart'; @@ -53,6 +54,7 @@ Future main(List args) async { patchDiffCheckerRef, platformRef, processRef, + pubspecEditorRef, shorebirdArtifactsRef, shorebirdEnvRef, shorebirdFlutterRef, diff --git a/packages/shorebird_cli/lib/src/commands/init_command.dart b/packages/shorebird_cli/lib/src/commands/init_command.dart index b87abb77..76af1738 100644 --- a/packages/shorebird_cli/lib/src/commands/init_command.dart +++ b/packages/shorebird_cli/lib/src/commands/init_command.dart @@ -10,10 +10,10 @@ import 'package:shorebird_cli/src/doctor.dart'; import 'package:shorebird_cli/src/executables/executables.dart'; import 'package:shorebird_cli/src/logger.dart'; import 'package:shorebird_cli/src/platform.dart'; +import 'package:shorebird_cli/src/pubspec_editor.dart'; import 'package:shorebird_cli/src/shorebird_env.dart'; import 'package:shorebird_cli/src/shorebird_validator.dart'; import 'package:shorebird_code_push_client/shorebird_code_push_client.dart'; -import 'package:yaml/yaml.dart'; import 'package:yaml_edit/yaml_edit.dart'; /// {@template init_command} @@ -210,9 +210,7 @@ Please make sure you are running "shorebird init" from within your Flutter proje ); if (!shorebirdEnv.pubspecContainsShorebirdYaml) { - _addShorebirdYamlToPubspecAssets( - shorebirdEnv.getPubspecYamlFile(cwd: projectRoot), - ); + pubspecEditor.addShorebirdYamlToPubspecAssets(); } logger.info( @@ -314,31 +312,4 @@ app_id: return ShorebirdYaml(appId: appId); } - - void _addShorebirdYamlToPubspecAssets(File pubspecFile) { - final pubspecContents = pubspecFile.readAsStringSync(); - final yaml = loadYaml(pubspecContents, sourceUrl: pubspecFile.uri) as Map; - final editor = YamlEditor(pubspecContents); - if (!yaml.containsKey('flutter') || yaml['flutter'] == null) { - editor.update( - ['flutter'], - { - 'assets': ['shorebird.yaml'], - }, - ); - } else { - if (!(yaml['flutter'] as Map).containsKey('assets')) { - editor.update(['flutter', 'assets'], ['shorebird.yaml']); - } else { - final assets = (yaml['flutter'] as Map)['assets'] as List; - if (!assets.contains('shorebird.yaml')) { - editor.update(['flutter', 'assets'], [...assets, 'shorebird.yaml']); - } - } - } - - if (editor.edits.isEmpty) return; - - pubspecFile.writeAsStringSync(editor.toString()); - } } diff --git a/packages/shorebird_cli/lib/src/doctor.dart b/packages/shorebird_cli/lib/src/doctor.dart index 76f1f4da..02d02925 100644 --- a/packages/shorebird_cli/lib/src/doctor.dart +++ b/packages/shorebird_cli/lib/src/doctor.dart @@ -28,6 +28,7 @@ class Doctor { ShorebirdFlutterValidator(), AndroidInternetPermissionValidator(), StorageAccessValidator(), + ShorebirdYamlAssetValidator(), ]; /// Run the provided [validators]. If [applyFixes] is `true`, any validation diff --git a/packages/shorebird_cli/lib/src/pubspec_editor.dart b/packages/shorebird_cli/lib/src/pubspec_editor.dart new file mode 100644 index 00000000..ac84897a --- /dev/null +++ b/packages/shorebird_cli/lib/src/pubspec_editor.dart @@ -0,0 +1,54 @@ +import 'package:scoped/scoped.dart'; +import 'package:shorebird_cli/src/shorebird_env.dart'; +import 'package:yaml/yaml.dart'; +import 'package:yaml_edit/yaml_edit.dart'; + +/// A reference to a [PubspecEditor] instance. +final pubspecEditorRef = create(PubspecEditor.new); + +/// The [PubspecEditor] instance available in the current zone. +PubspecEditor get pubspecEditor => read(pubspecEditorRef); + +/// {@template pubspec_editor} +/// A class that exposes APIs to edit the current project's `pubspec.yaml`. +/// {@endtemplate} +class PubspecEditor { + /// Adds shorebird.yaml to the assets section of the pubspec.yaml file. + /// Does nothing if the pubspec.yaml file already contains shorebird.yaml. + /// Does nothing if a flutter project root cannot be found. + void addShorebirdYamlToPubspecAssets() { + if (shorebirdEnv.pubspecContainsShorebirdYaml) return; + + final root = shorebirdEnv.getFlutterProjectRoot(); + // TODO(felangel): this should throw an exception instead of returning + // to make it explicit that the edit operation failed. + if (root == null) return; + + final pubspecFile = shorebirdEnv.getPubspecYamlFile(cwd: root); + final pubspecContents = pubspecFile.readAsStringSync(); + final editor = YamlEditor(pubspecContents); + final yaml = loadYaml(pubspecContents, sourceUrl: pubspecFile.uri) as Map; + + if (!yaml.containsKey('flutter') || yaml['flutter'] == null) { + editor.update( + ['flutter'], + { + 'assets': ['shorebird.yaml'], + }, + ); + } else { + if (!(yaml['flutter'] as Map).containsKey('assets')) { + editor.update(['flutter', 'assets'], ['shorebird.yaml']); + } else { + final assets = (yaml['flutter'] as Map)['assets'] as List; + if (!assets.contains('shorebird.yaml')) { + editor.update(['flutter', 'assets'], [...assets, 'shorebird.yaml']); + } + } + } + + if (editor.edits.isEmpty) return; + + pubspecFile.writeAsStringSync(editor.toString()); + } +} diff --git a/packages/shorebird_cli/lib/src/validators/shorebird_yaml_asset_validator.dart b/packages/shorebird_cli/lib/src/validators/shorebird_yaml_asset_validator.dart new file mode 100644 index 00000000..bcd391e1 --- /dev/null +++ b/packages/shorebird_cli/lib/src/validators/shorebird_yaml_asset_validator.dart @@ -0,0 +1,41 @@ +import 'package:shorebird_cli/src/pubspec_editor.dart'; +import 'package:shorebird_cli/src/shorebird_env.dart'; +import 'package:shorebird_cli/src/validators/validators.dart'; + +/// Verifies that the shorebird.yaml is found in pubspec.yaml assets. +class ShorebirdYamlAssetValidator extends Validator { + @override + String get description => 'shorebird.yaml found in pubspec.yaml assets'; + + @override + bool canRunInCurrentContext() => shorebirdEnv.hasPubspecYaml; + + @override + String get incorrectContextMessage => ''' +The pubspec.yaml file does not exist. +The command you are running must be run within a Flutter app project.'''; + + @override + Future> validate() async { + if (!canRunInCurrentContext()) { + return [ + const ValidationIssue( + severity: ValidationIssueSeverity.error, + message: 'No pubspec.yaml file found', + ), + ]; + } + + if (shorebirdEnv.pubspecContainsShorebirdYaml) { + return []; + } + + return [ + ValidationIssue( + severity: ValidationIssueSeverity.error, + message: 'No shorebird.yaml found in pubspec.yaml assets', + fix: () => pubspecEditor.addShorebirdYamlToPubspecAssets(), + ), + ]; + } +} diff --git a/packages/shorebird_cli/lib/src/validators/validators.dart b/packages/shorebird_cli/lib/src/validators/validators.dart index a9b101fb..d009cb40 100644 --- a/packages/shorebird_cli/lib/src/validators/validators.dart +++ b/packages/shorebird_cli/lib/src/validators/validators.dart @@ -7,6 +7,7 @@ import 'package:shorebird_cli/src/shorebird_process.dart'; export 'android_internet_permission_validator.dart'; export 'shorebird_flutter_validator.dart'; export 'shorebird_version_validator.dart'; +export 'shorebird_yaml_asset_validator.dart'; export 'storage_access_validator.dart'; /// Severity level of a [ValidationIssue]. diff --git a/packages/shorebird_cli/test/src/commands/init_command_test.dart b/packages/shorebird_cli/test/src/commands/init_command_test.dart index 1bd8681f..72250034 100644 --- a/packages/shorebird_cli/test/src/commands/init_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/init_command_test.dart @@ -14,6 +14,7 @@ import 'package:shorebird_cli/src/doctor.dart'; import 'package:shorebird_cli/src/executables/executables.dart'; import 'package:shorebird_cli/src/logger.dart'; import 'package:shorebird_cli/src/platform.dart'; +import 'package:shorebird_cli/src/pubspec_editor.dart'; import 'package:shorebird_cli/src/shorebird_env.dart'; import 'package:shorebird_cli/src/shorebird_process.dart'; import 'package:shorebird_cli/src/shorebird_validator.dart'; @@ -45,6 +46,7 @@ environment: late Logger logger; late Platform platform; late Progress progress; + late PubspecEditor pubspecEditor; late ShorebirdEnv shorebirdEnv; late ShorebirdValidator shorebirdValidator; late XcodeBuild xcodeBuild; @@ -60,6 +62,7 @@ environment: loggerRef.overrideWith(() => logger), platformRef.overrideWith(() => platform), processRef.overrideWith(() => process), + pubspecEditorRef.overrideWith(() => pubspecEditor), shorebirdEnvRef.overrideWith(() => shorebirdEnv), shorebirdValidatorRef.overrideWith(() => shorebirdValidator), xcodeBuildRef.overrideWith(() => xcodeBuild), @@ -80,6 +83,7 @@ environment: shorebirdYamlFile = MockFile(); pubspecYamlFile = MockFile(); projectRoot = Directory.systemTemp.createTempSync(); + pubspecEditor = MockPubspecEditor(); logger = MockLogger(); platform = MockPlatform(); progress = MockProgress(); @@ -903,85 +907,10 @@ flutter: ); }); - test('creates flutter.assets and adds shorebird.yaml', () async { + test('ensures that addShorebirdYamlToPubspecAssets is called', () async { + when(() => shorebirdEnv.pubspecContainsShorebirdYaml).thenReturn(false); await runWithOverrides(command.run); - verify( - () => pubspecYamlFile.writeAsStringSync( - any( - that: equals(''' -$pubspecYamlContent -flutter: - assets: - - shorebird.yaml -'''), - ), - ), - ); - }); - - test('creates assets and adds shorebird.yaml (empty)', () async { - when(() => pubspecYamlFile.readAsStringSync()).thenReturn(''' -$pubspecYamlContent -flutter: -'''); - await runWithOverrides(command.run); - verify( - () => pubspecYamlFile.writeAsStringSync( - any( - that: equals(''' -$pubspecYamlContent -flutter: - assets: - - shorebird.yaml -'''), - ), - ), - ); - }); - - test('creates assets and adds shorebird.yaml (non-empty)', () async { - when(() => pubspecYamlFile.readAsStringSync()).thenReturn(''' -$pubspecYamlContent -flutter: - uses-material-design: true -'''); - await runWithOverrides(command.run); - verify( - () => pubspecYamlFile.writeAsStringSync( - any( - that: equals(''' -$pubspecYamlContent -flutter: - assets: - - shorebird.yaml - uses-material-design: true -'''), - ), - ), - ); - }); - - test('adds shorebird.yaml to assets', () async { - when(() => pubspecYamlFile.readAsStringSync()).thenReturn(''' -$pubspecYamlContent -flutter: - assets: - - some/asset.txt -'''); - await runWithOverrides(command.run); - verify( - () => pubspecYamlFile.writeAsStringSync( - any( - that: equals(''' -$pubspecYamlContent -flutter: - assets: - - some/asset.txt - - shorebird.yaml -'''), - ), - ), - ).called(1); + verify(pubspecEditor.addShorebirdYamlToPubspecAssets).called(1); }); test('fixes fixable validation errors', () async { diff --git a/packages/shorebird_cli/test/src/mocks.dart b/packages/shorebird_cli/test/src/mocks.dart index 72fa976d..49a48a88 100644 --- a/packages/shorebird_cli/test/src/mocks.dart +++ b/packages/shorebird_cli/test/src/mocks.dart @@ -23,6 +23,7 @@ import 'package:shorebird_cli/src/executables/executables.dart'; import 'package:shorebird_cli/src/os/os.dart'; import 'package:shorebird_cli/src/patch_diff_checker.dart'; import 'package:shorebird_cli/src/platform/platform.dart'; +import 'package:shorebird_cli/src/pubspec_editor.dart'; import 'package:shorebird_cli/src/shorebird_artifacts.dart'; import 'package:shorebird_cli/src/shorebird_env.dart'; import 'package:shorebird_cli/src/shorebird_flutter.dart'; @@ -117,6 +118,8 @@ class MockProcess extends Mock implements Process {} class MockProgress extends Mock implements Progress {} +class MockPubspecEditor extends Mock implements PubspecEditor {} + class MockRelease extends Mock implements Release {} class MockReleaseArtifact extends Mock implements ReleaseArtifact {} diff --git a/packages/shorebird_cli/test/src/pubspec_editor_test.dart b/packages/shorebird_cli/test/src/pubspec_editor_test.dart new file mode 100644 index 00000000..fdd92b00 --- /dev/null +++ b/packages/shorebird_cli/test/src/pubspec_editor_test.dart @@ -0,0 +1,201 @@ +import 'dart:io'; + +import 'package:mocktail/mocktail.dart'; +import 'package:path/path.dart' as p; +import 'package:scoped/scoped.dart'; +import 'package:shorebird_cli/src/pubspec_editor.dart'; +import 'package:shorebird_cli/src/shorebird_env.dart'; +import 'package:test/test.dart'; + +import 'mocks.dart'; + +class _FakeDirectory extends Fake implements Directory {} + +void main() { + group(PubspecEditor, () { + late ShorebirdEnv shorebirdEnv; + late PubspecEditor pubspecEditor; + + R runWithOverrides(R Function() body) { + return runScoped( + () => body(), + values: { + shorebirdEnvRef.overrideWith(() => shorebirdEnv), + }, + ); + } + + setUpAll(() { + registerFallbackValue(_FakeDirectory()); + }); + + setUp(() { + shorebirdEnv = MockShorebirdEnv(); + pubspecEditor = PubspecEditor(); + }); + + group('addShorebirdYamlToPubspecAssets', () { + group('when shorebird.yaml is part of the pubspec.yaml assets', () { + setUp(() { + when( + () => shorebirdEnv.pubspecContainsShorebirdYaml, + ).thenReturn(true); + }); + + test('does nothing', () { + expect( + () => runWithOverrides( + pubspecEditor.addShorebirdYamlToPubspecAssets, + ), + returnsNormally, + ); + verifyNever(() => shorebirdEnv.getFlutterProjectRoot()); + }); + }); + + group('when shorebird.yaml is not part of the pubspec.yaml assets', () { + setUp(() { + when( + () => shorebirdEnv.pubspecContainsShorebirdYaml, + ).thenReturn(false); + }); + + group('when a flutter project root cannot be found', () { + setUp(() { + when(() => shorebirdEnv.getFlutterProjectRoot()).thenReturn(null); + }); + + test('does nothing', () { + expect( + () => runWithOverrides( + pubspecEditor.addShorebirdYamlToPubspecAssets, + ), + returnsNormally, + ); + verify(() => shorebirdEnv.getFlutterProjectRoot()).called(1); + }); + }); + + group('when a flutter project root can be found', () { + const basePubspecContents = ''' +name: test +version: 1.0.0 +environment: + sdk: ">=2.19.0 <3.0.0"'''; + late Directory tempDir; + late File pubspecFile; + + setUp(() { + tempDir = Directory.systemTemp.createTempSync(); + pubspecFile = File(p.join(tempDir.path, 'pubspec.yaml')); + when( + () => shorebirdEnv.getFlutterProjectRoot(), + ).thenReturn(tempDir); + when( + () => shorebirdEnv.getPubspecYamlFile(cwd: any(named: 'cwd')), + ).thenReturn(pubspecFile); + }); + + test('creates flutter.assets and adds shorebird.yaml', () { + pubspecFile + ..createSync() + ..writeAsStringSync(basePubspecContents); + IOOverrides.runZoned( + () => runWithOverrides( + pubspecEditor.addShorebirdYamlToPubspecAssets, + ), + getCurrentDirectory: () => tempDir, + ); + expect( + pubspecFile.readAsStringSync(), + equals(''' +$basePubspecContents +flutter: + assets: + - shorebird.yaml +'''), + ); + }); + + test('creates assets and adds shorebird.yaml (empty flutter)', () { + pubspecFile + ..createSync() + ..writeAsStringSync(''' +$basePubspecContents +flutter: +'''); + IOOverrides.runZoned( + () => runWithOverrides( + pubspecEditor.addShorebirdYamlToPubspecAssets, + ), + getCurrentDirectory: () => tempDir, + ); + expect( + pubspecFile.readAsStringSync(), + equals( + ''' +$basePubspecContents +flutter: + assets: + - shorebird.yaml +''', + ), + ); + }); + test('creates assets and adds shorebird.yaml (non-empty flutter)', + () { + pubspecFile + ..createSync() + ..writeAsStringSync(''' +$basePubspecContents +flutter: + uses-material-design: true +'''); + IOOverrides.runZoned( + () => runWithOverrides( + pubspecEditor.addShorebirdYamlToPubspecAssets, + ), + getCurrentDirectory: () => tempDir, + ); + expect( + pubspecFile.readAsStringSync(), + equals(''' +$basePubspecContents +flutter: + assets: + - shorebird.yaml + uses-material-design: true +'''), + ); + }); + test('adds shorebird.yaml to assets (existing assets)', () { + pubspecFile + ..createSync() + ..writeAsStringSync(''' +$basePubspecContents +flutter: + assets: + - some/asset.txt +'''); + IOOverrides.runZoned( + () => runWithOverrides( + pubspecEditor.addShorebirdYamlToPubspecAssets, + ), + getCurrentDirectory: () => tempDir, + ); + expect( + pubspecFile.readAsStringSync(), + equals(''' +$basePubspecContents +flutter: + assets: + - some/asset.txt + - shorebird.yaml +'''), + ); + }); + }); + }); + }); + }); +} diff --git a/packages/shorebird_cli/test/src/validators/shorebird_yaml_asset_validator_test.dart b/packages/shorebird_cli/test/src/validators/shorebird_yaml_asset_validator_test.dart new file mode 100644 index 00000000..93035315 --- /dev/null +++ b/packages/shorebird_cli/test/src/validators/shorebird_yaml_asset_validator_test.dart @@ -0,0 +1,117 @@ +import 'package:mocktail/mocktail.dart'; +import 'package:scoped/scoped.dart'; +import 'package:shorebird_cli/src/pubspec_editor.dart'; +import 'package:shorebird_cli/src/shorebird_env.dart'; +import 'package:shorebird_cli/src/validators/validators.dart'; +import 'package:test/test.dart'; + +import '../mocks.dart'; + +void main() { + group(ShorebirdYamlAssetValidator, () { + late ShorebirdEnv shorebirdEnv; + late PubspecEditor pubspecEditor; + + R runWithOverrides(R Function() body) { + return runScoped( + body, + values: { + shorebirdEnvRef.overrideWith(() => shorebirdEnv), + pubspecEditorRef.overrideWith(() => pubspecEditor), + }, + ); + } + + setUp(() { + shorebirdEnv = MockShorebirdEnv(); + pubspecEditor = MockPubspecEditor(); + }); + + test('has a non-empty description', () { + expect(ShorebirdYamlAssetValidator().description, isNotEmpty); + }); + + group('canRunInContext', () { + test('returns false if no pubspec.yaml file exists', () { + when(() => shorebirdEnv.hasPubspecYaml).thenReturn(false); + final result = runWithOverrides( + () => ShorebirdYamlAssetValidator().canRunInCurrentContext(), + ); + expect(result, isFalse); + }); + + test('returns true if a pubspec.yaml file exists', () { + when(() => shorebirdEnv.hasPubspecYaml).thenReturn(true); + final result = runWithOverrides( + () => ShorebirdYamlAssetValidator().canRunInCurrentContext(), + ); + expect(result, isTrue); + }); + }); + + group('validate', () { + test( + 'returns with no errors if pubspec.yaml has shorebird.yaml in assets', + () async { + when(() => shorebirdEnv.hasPubspecYaml).thenReturn(true); + when( + () => shorebirdEnv.pubspecContainsShorebirdYaml, + ).thenReturn(true); + final results = await runWithOverrides( + ShorebirdYamlAssetValidator().validate, + ); + expect(results.map((res) => res.severity), isEmpty); + }, + ); + + test('returns an error if pubspec.yaml file does not exist', () async { + when(() => shorebirdEnv.hasPubspecYaml).thenReturn(false); + final results = await runWithOverrides( + ShorebirdYamlAssetValidator().validate, + ); + expect(results, hasLength(1)); + expect(results.first.severity, ValidationIssueSeverity.error); + expect( + results.first.message, + startsWith('No pubspec.yaml file found'), + ); + expect(results.first.fix, isNull); + }); + + test('returns error if shorebird.yaml is missing from assets', () async { + when(() => shorebirdEnv.hasPubspecYaml).thenReturn(true); + when(() => shorebirdEnv.pubspecContainsShorebirdYaml).thenReturn(false); + final results = await runWithOverrides( + ShorebirdYamlAssetValidator().validate, + ); + expect(results, hasLength(1)); + expect( + results.first, + equals( + const ValidationIssue( + severity: ValidationIssueSeverity.error, + message: 'No shorebird.yaml found in pubspec.yaml assets', + ), + ), + ); + }); + }); + + group('fix', () { + test('adds shorebird.yaml to pubspec.yaml', () async { + when(() => shorebirdEnv.hasPubspecYaml).thenReturn(true); + when(() => shorebirdEnv.pubspecContainsShorebirdYaml).thenReturn(false); + when( + () => pubspecEditor.addShorebirdYamlToPubspecAssets(), + ).thenAnswer((_) {}); + final results = await runWithOverrides( + ShorebirdYamlAssetValidator().validate, + ); + expect(results, hasLength(1)); + expect(results.first.fix, isNotNull); + await runWithOverrides(() => results.first.fix!()); + verify(pubspecEditor.addShorebirdYamlToPubspecAssets).called(1); + }); + }); + }); +}