feat: configurable start --timeout, CDP port probe, strict unknown-flag handling#33
Merged
Merged
Conversation
…n-flag handling Adds a --timeout option to start so the VM Service URI log-scrape deadline is configurable for cold macOS/iOS boots; the timeout error reports the configured value (#6). Probes the CDP port before launching Chrome and emits a clear port-in-use message instead of the misleading 'Is Chrome installed?' fallback (#7). Unknown command flags now error and exit non-zero across the whole CLI substrate instead of silently printing help; valid invocations, --help, and genuine missing-value errors are unchanged (#12).
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR improves CLI robustness and the start command’s CDP workflow by making VM Service scrape timeouts configurable, failing fast on CDP port collisions, and ensuring unknown flags consistently produce non-zero exits with clear diagnostics.
Changes:
- Added
start --timeout=<n>(default90) and threaded it through the CDP-branch VM Service URI scrape and timeout error message. - Probed
--cdp-portavailability before launching Chrome to emit an actionable “port in use” error instead of a misleading Chrome-install diagnostic. - Updated the dispatcher to treat unknown flags as errors (stderr + help + non-zero) and added tests/docs/changelog entries for the new behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/console/artisan_application_test.dart | Adds in-process stdout/stderr capture and new tests asserting strict unknown-flag handling. |
| test/commands/start_command_test.dart | Extends coverage for --timeout parsing/defaults and CDP-port prelaunch probe behavior. |
| lib/src/console/artisan_application.dart | Implements strict unknown-flag failure behavior in the shared dispatch path. |
| lib/src/commands/start_command.dart | Adds --timeout, threads it into the CDP scrape path, and probes CDP port availability pre-Chrome launch. |
| doc/reference/signature-dsl.md | Documents new unknown-option error handling behavior. |
| doc/commands/start.md | Documents --timeout and CDP-port collision troubleshooting. |
| CHANGELOG.md | Adds [Unreleased] entries for --timeout, CDP port probing, and strict unknown-flag handling. |
- start --timeout: reject zero and negative values with a clear error (--timeout must be a positive integer). Previously a zero value silently reached the VM Service scrape loop and produced a confusing 'Timed out after 0s' failure. Two new TDD tests cover both the zero and negative cases. - _CapturingStdout (artisan_application_test.dart): replace the noSuchMethod blanket override with explicit implementations for all Stdout members (flush, done, close, addError, addStream, hasTerminal, supportsAnsiEscapes, terminalColumns, terminalLines, nonBlocking, lineTerminator). The fake now qualifies as a valid IOOverrides.runZoned stderr: override without relying on noSuchMethod fallback, so the unknown-option test asserts against genuinely captured stderr output. - doc/commands/start.md: remove the drifted start_command.dart:38 line reference from the Synopsis section (line 38 is now a typedef, not configure). Reference the file without a line number. - CHANGELOG.md: add Fixed entry for the non-positive --timeout rejection under [Unreleased].
Adds a @VisibleForTesting entry to the production _scrapeVmServiceUriFromFile loop (otherwise bypassed by the cdpVmServiceScraper seam) plus two tests: a fast success case and a 1s timeout case. Covers the configurable-deadline and timeout-error lines flagged by codecov/patch.
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.
Fixes REPORT friction items #6, #7, #12.
start --timeout=<n>(default 90) makes the VM Service URI log-scrape deadline configurable for cold macOS/iOS boots; the timeout error reports the configured value instead of a literal "90s".start --cdp-portnow probes the CDP port before launching Chrome and emits a clear port-in-use message instead of the misleading "Is Chrome installed?" catch-all.--help, and genuine missing-value errors are unchanged. Benefits every plugin CLI on the substrate.TDD throughout. analyze 0, format 0-diff,
dart test1177 pass, coverage >= 80%. Doc + CHANGELOG synced.