-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[camera_android_camerax] Migrate check-readiness skill from bash to Dart #11943
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
Merged
Merged
Changes from 42 commits
Commits
Show all changes
52 commits
Select commit
Hold shift + click to select a range
ee20fa7
feat: Set up agent skills management architecture
reidbaker 26b37ae
feat: Add check-readiness skill
reidbaker d825767
feat: Add local check-readiness skill and configure linter
reidbaker f7f0646
test: ensure tracked skills cannot be published accidentally
reidbaker ea27e43
build: pin dart_skills_lint to support individual_skills
reidbaker 9ac31a2
test: rewrite tracked skills test to use ValidationSession API
reidbaker 87cfaf1
test: fix analyzer warnings
reidbaker acbcc3e
build: update dart_skills_lint to 8f85e82b on main
reidbaker 7eabf5f
test: rewrite tracked skills test to use yaml parsing, address PR com…
reidbaker 9e0d9e5
test: revert yaml parsing, use public ValidationSession from repo
reidbaker 6b68664
test: revert to ValidationSession and use ignore implementation_imports
reidbaker 0942453
test: replace yaml parsing with custom skill rule EnforceTrackedSkill…
reidbaker 9471107
test: extract EnforceTrackedSkillsInternalRule to its own file with docs
reidbaker c4e832f
ci: allow dart_skills_lint in packages
reidbaker acc34a5
test: fix internal rule validation feedback
reidbaker 288e5ec
Fix missing copyright headers in check-readiness agent skill
reidbaker 43be3d1
Support .pubignore in publish-check
reidbaker 8d17949
Add tests for publish_check pubignore parsing
reidbaker 81c1c52
Fix .gitignore for check-readiness skill and symlinked skills
reidbaker 438537c
Revert un-ignoring of symlinked skills
reidbaker f64190b
Add TODO for tracking dart-lang/pub issue 4841
reidbaker 14d7cfc
Group pubignore tracking tests under issue 4841
reidbaker d7f024e
Move TODO out of group name
reidbaker bc37ae6
Fix analyzer warnings
reidbaker 16e3f28
Fix .gitignore un-ignore rule overreach
reidbaker 83638ff
Rename EnforceTrackedSkillsInternalRule to EnforceTrackedSkillsPreven…
reidbaker d93c81e
Move third-party skills to third_party/skill-repos/ and update symlin…
reidbaker 7c080f9
Move third-party README to third_party/skill-repos/ and update refere…
reidbaker 51ecf16
Canonicalize paths for EnforceTrackedSkillsPreventPublishingRule to f…
reidbaker 2271f28
Add path dependency to camera_android_camerax dev_dependencies
reidbaker da6b889
feat: migrate check-readiness skill from bash to dart with comprehens…
reidbaker 0129f97
address code review feedback
reidbaker 4169386
Fix dart analyze behavior for .agents/skills
reidbaker f17e516
Support .pubignore parsing in flutter_plugin_tools to skip hygiene ch…
reidbaker 6c9ec74
Merge remote-tracking branch 'origin/main' into convert-check-sh-to-dart
reidbaker a2d86d0
Remove migrated check.sh script
reidbaker abd0a6b
Format code
reidbaker d92048e
Exempt .agents/ changes from version and CHANGELOG validations
reidbaker 710a71d
Address code review feedback on pubignore matching and symlink following
reidbaker f48a349
Remove redundant followLinks argument in analyze_command
reidbaker 60e836a
Resolve all linter warnings in repository_package
reidbaker 4355658
Fix check-readiness license headers and resolve Windows test failure
reidbaker c528a1a
Fix check-readiness Windows path handling and restore analyze-skills-…
reidbaker aa402df
Address review feedback on convert-check-sh-to-dart
reidbaker 1a330b2
Address feedback on check-readiness root resolution and add unit test…
reidbaker b6ace4d
Improve failure summary details for analyze command
reidbaker 8604918
Add regression test for both main and skills analyze failures
reidbaker 4db9387
Fix omit_obvious_local_variable_types lint in analyze_command.dart
reidbaker bf182b1
Refactor checkReadiness to report all failures and avoid early exit
reidbaker 7840313
Address stuartmorgan-g feedback on check_readiness and errors initial…
reidbaker 7299da7
Inline _canRunCommand helper in ReadinessChecker
reidbaker e1fc69a
Restrict check-readiness tests to VM platform via dart_test.yaml
reidbaker File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
17 changes: 17 additions & 0 deletions
17
packages/camera/camera_android_camerax/.agents/skills/check-readiness/bin/check.dart
|
reidbaker marked this conversation as resolved.
Outdated
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| // Copyright 2013 The Flutter Authors | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'dart:io'; | ||
|
|
||
| import 'package:check_readiness/check_readiness.dart'; | ||
|
|
||
| Future<void> main(List<String> args) async { | ||
| // Since this tool is executed via `dart run` from the package root, | ||
| // the current directory is the workspace root. | ||
| final String workspaceRoot = Directory.current.path; | ||
|
reidbaker marked this conversation as resolved.
Outdated
|
||
|
|
||
| final checker = ReadinessChecker(); | ||
| final bool isReady = await checker.checkReadiness(workspaceRoot); | ||
| exitCode = isReady ? 0 : 1; | ||
| } | ||
140 changes: 140 additions & 0 deletions
140
...ges/camera/camera_android_camerax/.agents/skills/check-readiness/lib/check_readiness.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| // Copyright 2013 The Flutter Authors | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'dart:io'; | ||
|
|
||
| import 'package:file/file.dart'; | ||
| import 'package:file/local.dart'; | ||
| import 'package:path/path.dart' as p; | ||
| import 'package:process/process.dart'; | ||
|
|
||
| /// Checks if the environment is ready for new work. | ||
| class ReadinessChecker { | ||
| /// Creates a new ReadinessChecker. | ||
| ReadinessChecker({ | ||
| FileSystem? fileSystem, | ||
| ProcessManager? processManager, | ||
| void Function(Object?)? log, | ||
| }) : _fileSystem = fileSystem ?? const LocalFileSystem(), | ||
| _processManager = processManager ?? const LocalProcessManager(), | ||
| _log = log ?? ((Object? msg) => stdout.writeln(msg)); | ||
|
|
||
| final FileSystem _fileSystem; | ||
| final ProcessManager _processManager; | ||
| final void Function(Object?) _log; | ||
|
|
||
| /// Runs all readiness checks. | ||
| /// | ||
| /// Returns `true` if ready, `false` otherwise. | ||
| Future<bool> checkReadiness(String workspaceRoot) async { | ||
| _log('Checking if environment is ready for new work...'); | ||
|
|
||
| if (!await _checkSymlinks(workspaceRoot)) { | ||
| return false; | ||
| } | ||
| if (!await _checkGitState(workspaceRoot)) { | ||
| return false; | ||
| } | ||
| if (!await _checkFlutterAndDart()) { | ||
| return false; | ||
| } | ||
| if (!await _checkDependencies(workspaceRoot)) { | ||
| return false; | ||
| } | ||
|
|
||
| _log('Environment is fully ready!'); | ||
| return true; | ||
| } | ||
|
|
||
| Future<bool> _checkSymlinks(String workspaceRoot) async { | ||
| _log('1. Checking skill symlinks...'); | ||
| final Directory agentsDir = _fileSystem.directory(p.join(workspaceRoot, '.agents', 'skills')); | ||
| if (!agentsDir.existsSync()) { | ||
| // If it doesn't exist, there are no broken symlinks. | ||
| _log('All symlinks resolve correctly.'); | ||
| return true; | ||
| } | ||
|
|
||
| final brokenLinks = <String>[]; | ||
| await for (final FileSystemEntity entity | ||
| in agentsDir.list(recursive: true, followLinks: false)) { | ||
| if (entity is Link) { | ||
| if (_fileSystem.typeSync(entity.path) == FileSystemEntityType.notFound) { | ||
| brokenLinks.add(entity.path); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (brokenLinks.isNotEmpty) { | ||
| _log('Error: Found broken symlinks in .agents/skills:'); | ||
| brokenLinks.forEach(_log); | ||
| return false; | ||
| } | ||
|
|
||
| _log('All symlinks resolve correctly.'); | ||
| return true; | ||
| } | ||
|
|
||
| Future<bool> _checkGitState(String workspaceRoot) async { | ||
| _log('2. Checking git state...'); | ||
| final ProcessResult result; | ||
| try { | ||
| result = await _processManager.run( | ||
| ['git', 'status', '--porcelain'], | ||
| workingDirectory: workspaceRoot, | ||
| ); | ||
| } on ProcessException catch (e) { | ||
| _log('Error: Failed to run git status. Is git installed and on the PATH?'); | ||
| _log(e.toString()); | ||
| return false; | ||
| } | ||
| if (result.exitCode != 0) { | ||
| _log('Error: Failed to run git status.'); | ||
| return false; | ||
| } | ||
| final String stdoutStr = (result.stdout as String).trim(); | ||
| if (stdoutStr.isNotEmpty) { | ||
| _log( | ||
| 'Error: Git working directory is not clean. Please commit or stash your changes before starting new work.'); | ||
| return false; | ||
| } | ||
| _log('Git working directory is clean.'); | ||
| return true; | ||
| } | ||
|
|
||
| Future<bool> _checkFlutterAndDart() async { | ||
| _log('3. Checking Flutter and Dart...'); | ||
| if (!_canRunCommand('flutter')) { | ||
| _log("Error: 'flutter' is not on the PATH."); | ||
| return false; | ||
| } | ||
| if (!_canRunCommand('dart')) { | ||
| _log("Error: 'dart' is not on the PATH."); | ||
| return false; | ||
| } | ||
| _log('Flutter and Dart are on the PATH.'); | ||
| return true; | ||
| } | ||
|
|
||
| bool _canRunCommand(String command) { | ||
| // A simple check using ProcessManager's canRun | ||
|
reidbaker marked this conversation as resolved.
Outdated
|
||
| // NOTE: ProcessManager.canRun exists if we use process package > certain version | ||
|
reidbaker marked this conversation as resolved.
Outdated
|
||
| // Let's implement a safe check | ||
|
reidbaker marked this conversation as resolved.
Outdated
|
||
| return _processManager.canRun(command); | ||
| } | ||
|
|
||
| Future<bool> _checkDependencies(String workspaceRoot) async { | ||
| _log('4. Checking dependencies in camera_android_camerax...'); | ||
| final ProcessResult result = await _processManager.run( | ||
| ['flutter', 'pub', 'get'], | ||
| workingDirectory: workspaceRoot, | ||
| ); | ||
| if (result.exitCode != 0) { | ||
| _log('Error: Failed to resolve dependencies.'); | ||
| return false; | ||
| } | ||
| _log('Dependencies are resolved and ready.'); | ||
| return true; | ||
| } | ||
| } | ||
16 changes: 16 additions & 0 deletions
16
packages/camera/camera_android_camerax/.agents/skills/check-readiness/pubspec.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| name: check_readiness | ||
| description: A tool to check if the repository is ready for new work, intended only for use in the check-readiness skill. | ||
| version: 0.1.0 | ||
| publish_to: none | ||
|
|
||
| environment: | ||
| sdk: ^3.0.0 | ||
|
|
||
| dev_dependencies: | ||
| build_runner: ^2.15.0 | ||
| mockito: ^5.7.0 | ||
| test: ^1.24.0 | ||
| dependencies: | ||
|
reidbaker marked this conversation as resolved.
|
||
| file: ^7.0.1 | ||
| path: ^1.9.1 | ||
| process: ^5.0.5 | ||
55 changes: 0 additions & 55 deletions
55
packages/camera/camera_android_camerax/.agents/skills/check-readiness/scripts/check.sh
This file was deleted.
Oops, something went wrong.
144 changes: 144 additions & 0 deletions
144
packages/camera/camera_android_camerax/.agents/skills/check-readiness/test/check_test.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,144 @@ | ||
| // Copyright 2013 The Flutter Authors | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'dart:async'; | ||
| import 'dart:convert'; | ||
| import 'dart:io'; | ||
|
|
||
| import 'package:check_readiness/check_readiness.dart'; | ||
| import 'package:file/memory.dart'; | ||
| import 'package:file/src/interface/directory.dart'; | ||
| import 'package:file/src/interface/link.dart'; | ||
| import 'package:path/path.dart' as p; | ||
| import 'package:process/process.dart'; | ||
| import 'package:test/test.dart'; | ||
|
|
||
| class FakeProcessManager implements ProcessManager { | ||
| final Map<String, bool> canRunMock = {}; | ||
| final Map<String, ProcessResult> runMock = {}; | ||
| final List<List<String>> runInvocations = []; | ||
|
|
||
| @override | ||
| bool canRun(dynamic executable, {String? workingDirectory}) { | ||
| return canRunMock[executable as String] ?? true; | ||
| } | ||
|
|
||
| @override | ||
| Future<ProcessResult> run( | ||
| List<dynamic> command, { | ||
| String? workingDirectory, | ||
| Map<String, String>? environment, | ||
| bool includeParentEnvironment = true, | ||
| bool runInShell = false, | ||
| Encoding? stdoutEncoding = systemEncoding, | ||
| Encoding? stderrEncoding = systemEncoding, | ||
| }) async { | ||
| final List<String> cmdList = command.cast<String>(); | ||
| runInvocations.add(cmdList); | ||
| final String key = cmdList.join(' '); | ||
| if (runMock.containsKey(key)) { | ||
| return runMock[key]!; | ||
| } | ||
| return ProcessResult(0, 0, '', ''); | ||
| } | ||
|
|
||
| // The rest of the interface is unimplemented. | ||
| @override | ||
| dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); | ||
| } | ||
|
|
||
| void main() { | ||
| late MemoryFileSystem fileSystem; | ||
| late FakeProcessManager processManager; | ||
| late ReadinessChecker checker; | ||
| late String workspaceRoot; | ||
| final List<String> printLogs = []; | ||
|
|
||
| setUp(() { | ||
| fileSystem = MemoryFileSystem.test(); | ||
| processManager = FakeProcessManager(); | ||
| checker = ReadinessChecker( | ||
| fileSystem: fileSystem, | ||
| processManager: processManager, | ||
| log: (Object? message) => printLogs.add(message.toString()), | ||
| ); | ||
| workspaceRoot = '/workspace'; | ||
| printLogs.clear(); | ||
| }); | ||
|
|
||
| /// Runs the checker and captures prints | ||
| Future<bool> runChecker() async { | ||
| return checker.checkReadiness(workspaceRoot); | ||
| } | ||
|
|
||
| test('passes when everything is correct', () async { | ||
| // Setup empty skills dir (no broken symlinks) | ||
| fileSystem.directory(p.join(workspaceRoot, '.agents', 'skills')).createSync(recursive: true); | ||
|
|
||
| // Git returns clean | ||
| processManager.runMock['git status --porcelain'] = ProcessResult(0, 0, '', ''); | ||
|
|
||
| final bool result = await runChecker(); | ||
| expect(result, isTrue); | ||
| expect(printLogs, contains('Environment is fully ready!')); | ||
| }); | ||
|
|
||
| test('fails when a broken symlink is present', () async { | ||
| final Directory skillsDir = fileSystem.directory(p.join(workspaceRoot, '.agents', 'skills')) | ||
| ..createSync(recursive: true); | ||
|
|
||
| // MemoryFileSystem supports links | ||
| final Link link = fileSystem.link(p.join(skillsDir.path, 'broken_link')); | ||
| link.createSync('non_existent_target'); | ||
|
|
||
| final bool result = await runChecker(); | ||
| expect(result, isFalse); | ||
| expect( | ||
| printLogs.any((line) => line.contains('Found broken symlinks in .agents/skills:')), isTrue); | ||
| }); | ||
|
|
||
| test('fails when git is dirty', () async { | ||
| fileSystem.directory(p.join(workspaceRoot, '.agents', 'skills')).createSync(recursive: true); | ||
|
|
||
| processManager.runMock['git status --porcelain'] = ProcessResult(0, 0, ' M file.txt\n', ''); | ||
|
|
||
| final bool result = await runChecker(); | ||
| expect(result, isFalse); | ||
| expect( | ||
| printLogs, | ||
| contains( | ||
| 'Error: Git working directory is not clean. Please commit or stash your changes before starting new work.')); | ||
| }); | ||
|
|
||
| test('fails when flutter is missing', () async { | ||
| fileSystem.directory(p.join(workspaceRoot, '.agents', 'skills')).createSync(recursive: true); | ||
|
|
||
| processManager.canRunMock['flutter'] = false; | ||
|
|
||
| final bool result = await runChecker(); | ||
| expect(result, isFalse); | ||
| expect(printLogs, contains("Error: 'flutter' is not on the PATH.")); | ||
| }); | ||
|
|
||
| test('fails when dart is missing', () async { | ||
| fileSystem.directory(p.join(workspaceRoot, '.agents', 'skills')).createSync(recursive: true); | ||
|
|
||
| processManager.canRunMock['dart'] = false; | ||
|
|
||
| final bool result = await runChecker(); | ||
| expect(result, isFalse); | ||
| expect(printLogs, contains("Error: 'dart' is not on the PATH.")); | ||
| }); | ||
|
|
||
| test('fails when flutter pub get fails', () async { | ||
| fileSystem.directory(p.join(workspaceRoot, '.agents', 'skills')).createSync(recursive: true); | ||
|
|
||
| processManager.runMock['git status --porcelain'] = ProcessResult(0, 0, '', ''); | ||
| processManager.runMock['flutter pub get'] = ProcessResult(0, 1, '', 'Error'); | ||
|
|
||
| final bool result = await runChecker(); | ||
| expect(result, isFalse); | ||
| expect(printLogs, contains('Error: Failed to resolve dependencies.')); | ||
| }); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - camera_android_camerax |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.