Conversation
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What Changed
Risk Assessment
🚨 High: The change introduces a likely package-resolution break for the Dart CLI by pulling a Flutter SDK dependency into the root package, so it should not merge without an explicit packaging decision.
Testing
Exercised the refactored CLI command surface, command orchestration, runtime adapter selection, pilot runtime adapter/VM-service behavior, nested
pilot_runtimeFlutter tests, and manual end-user CLI/report generation evidence; the valid commands passed and the only failures were corrected test-runner/package-context attempts.Evidence: Generated timeline report
Evidence: test --help output
Launch the Target App Package and run a Scenario. Usage: flutter_pilot test [arguments] -h, --help Print this usage information. -d, --device Target Device id, exact name, or unique id/name prefix. --flavor Flutter flavor passed to flutter run. -t, --target Flutter app entrypoint file passed to flutter run. --until Run through a 1-based step number or step label. --print Print diagnostics after --until. [snapshot, widget-tree, errors] --json Print raw diagnostics as indented JSON. Run "flutter_pilot help" to see global options.Evidence: validate command output
Scenario is valid.Evidence: report command output
HTML report: /var/folders/1b/xbh8qb0d0j11fzyfm0qs0pf40000gn/T/no-mistakes-evidence/01KX0XMY4BTNEXM92FDNB8YA3Y/report-run/timeline.htmlPipeline
Updates from git push no-mistakes
⏭️ **intent** - skipped
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
⏭️ **Review** - skipped
pubspec.yaml:15- The root CLI package now has a direct path dependency onpilot_runtime, but that package depends on the Flutter SDK. This turns the previously Dart-only CLI into a package that cannot be resolved with the Dart SDK alone, contradicting the repo's documenteddart format/analyze/testworkflow and breaking users/environments without Flutter SDK package resolution. Consider splitting the non-Flutter VM-service client/protocol into a Dart-only package or keeping Flutter-only binding code out of the CLI dependency graph.packages/pilot_runtime/lib/src/pilot_runtime_client.dart:269-initialize()only mapsPilotRuntimeServiceExtensionMissingExceptioninto a typed initialization failure. VM Service connection errors, RPC errors other than missing extension, or unexpected service failures will escape as raw exceptions throughPilotRuntimeAdapter.initialize(), soScenarioRunnerwill not turn them into a run-level failure/report. Wrap lower-level handshake call failures inPilotRuntimeInitializationExceptionso the adapter consistently raisesRuntimeOperationException.✅ **Test** - passed
✅ No issues found.
dart test test/cli_test.dart test/test_command_executor_test.dart test/runtime_adapter_selection_test.dart test/pilot_runtime_adapter_test.dart test/pilot_runtime_vm_service_test.dartdart test packages/pilot_runtime/test(setup-mismatched attempt: failed because Flutter package tests require Flutter test runner/package context)flutter test packages/pilot_runtime/test(setup-mismatched attempt: failed because it resolved from the root package instead ofpackages/pilot_runtime)flutter test testfrompackages/pilot_runtimedart run bin/flutter_pilot.dart test --help > .../test-help.stdout 2> .../test-help.stderrdart run bin/flutter_pilot.dart validate examples/login_error.yaml > .../validate-valid.stdout 2> .../validate-valid.stderrdart run bin/flutter_pilot.dart report .../report-run > .../report-command.stdout 2> .../report-command.stderrVerified generatedtimeline.htmlcontainssplit_cli_command_modules,step-failed, andFinder matched no widgets.Removed transientpackages/pilot_runtime/pubspec.lockcreated by Flutter dependency resolution and recheckedgit status --short.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.