diff --git a/CHANGELOG.md b/CHANGELOG.md index 4786cb8..deb4d1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,18 @@ This project follows [Semantic Versioning 2.0.0](https://semver.org/spec/v2.0.0. --- +## [Unreleased] + +### Added + +- `start --timeout=` option (default `90`): configures the maximum seconds the VM Service URI scrape loop waits for `flutter run` to print the debug URI in the log file. Previously the deadline was hardcoded to 90 s; cold starts on slow CI machines or after a fresh Flutter SDK install can exceed this limit. Setting `--timeout=120` (or higher) prevents false-timeout failures. The error message now reports the configured value rather than a literal "90s". Applies to the `--cdp-port` branch only; the non-CDP branch retains its own hardcoded deadline. +- `start --cdp-port=` now probes the CDP port for availability BEFORE launching Chrome. When the port is already in use, the command exits 1 immediately with a clear message: "CDP port N is already in use; pass --cdp-port or free it before running start." This replaces the previous misleading "Is Chrome installed?" catch-all that fired when Chrome failed to open the debug port it was asked to bind. + +### Fixed + +- `start --timeout=` now rejects zero and negative values immediately with an actionable error ("--timeout must be a positive integer"), instead of silently passing a non-positive deadline to the VM Service scrape loop and producing a confusing "Timed out after 0s" failure. +- Passing an unknown option to any command now fails loudly instead of silently printing help and exiting as if help were requested (issue #12). The dispatcher writes `Unknown option: ` to stderr (both long `--foo` and short `-x` forms), prints the command help, and exits non-zero. Other parse failures keep their original messages: a missing option value (`Missing argument for "..."`), a disallowed value, and a value given to a flag each surface their specific diagnostic unchanged. `--help` / `-h` and every valid invocation are unaffected. Because this is the shared dispatch path for every command, the fix benefits every plugin CLI built on the substrate. + ## [0.0.8] - 2026-06-16 ### Fixed diff --git a/doc/commands/start.md b/doc/commands/start.md index 6eadc36..f52e328 100644 --- a/doc/commands/start.md +++ b/doc/commands/start.md @@ -26,9 +26,10 @@ Launches a Chrome web dev session on port `3100` with the VM Service listener on ``` dart run artisan start [--device=] [--port=] [--vm-service-port=] [--[no-]dds] [--[no-]profile-static] [--cdp-port=] + [--timeout=] ``` -`start` accepts no positional arguments. All configuration is done via named options and flags declared in `configure(ArgParser)` (see `lib/src/commands/start_command.dart:38`). +`start` accepts no positional arguments. All configuration is done via named options and flags declared in `configure(ArgParser)` in `lib/src/commands/start_command.dart`. ## Options @@ -40,7 +41,8 @@ dart run artisan start [--device=] [--port=] [--vm-service-port=] | `--vm-service-port` | int | `8181` | VM Service listener port. Recorded in `state.json`; used by `mcp:serve` and connected-mode tools to open the WebSocket. | | `--dds` | flag | `false` | Enable Dart Development Service. When absent (default), `--no-dds` is forwarded to `flutter run` so dusk and tinker connect directly to the VM Service. | | `--profile-static` | flag | `false` | Tag the session as a static-profile run. Sets `profile: "static"` in `state.json`; otherwise `"debug"`. | -| `--cdp-port` | int | (none) | Chrome DevTools Protocol port. When set, `start` pre-launches Chrome with `--remote-debugging-port=` and runs Flutter on `-d web-server`, recording `chromePid` / `tmpProfileDir` / `cdpPort` in `state.json`. Required for `dusk:resize` / `dusk:device`. Only valid with `--device=chrome` or `--device=web-server`, and requires Flutter SDK 3.30.0 or newer. A subsequent `restart` preserves this port. | +| `--cdp-port` | int | (none) | Chrome DevTools Protocol port. When set, `start` pre-launches Chrome with `--remote-debugging-port=` and runs Flutter on `-d web-server`, recording `chromePid` / `tmpProfileDir` / `cdpPort` in `state.json`. Required for `dusk:resize` / `dusk:device`. Only valid with `--device=chrome` or `--device=web-server`, and requires Flutter SDK 3.30.0 or newer. A subsequent `restart` preserves this port. The port is probed before Chrome launches; if it is already in use you receive a clear error with the `--cdp-port` hint instead of a misleading "Is Chrome installed?" message. | +| `--timeout` | int | `90` | Seconds to wait for the VM Service URI to appear in the `flutter run` log. Increase on cold starts where build and DartDev initialisation takes longer than the default (common on first run after a clean Flutter SDK install or on low-powered CI). Only applies to the `--cdp-port` branch. | ## Behavior @@ -127,6 +129,14 @@ dart run artisan start --device=chrome --cdp-port=9222 Pre-launches Chrome with `--remote-debugging-port=9222` and runs Flutter on `-d web-server`, recording `chromePid` / `tmpProfileDir` / `cdpPort` in `state.json` so `dusk:resize` and `dusk:device` can drive the Chrome DevTools Protocol. Requires Flutter SDK 3.30.0 or newer. A subsequent `restart` preserves the CDP port. +**CDP session with extended VM Service timeout (cold CI or slow machine):** + +```bash +dart run artisan start --device=chrome --cdp-port=9222 --timeout=180 +``` + +Allows up to 180 seconds for the VM Service URI to appear in the log. Use this when the default 90 seconds is not enough for a cold Flutter SDK install or a slow CI host. + ## Troubleshooting @@ -134,7 +144,9 @@ Pre-launches Chrome with `--remote-debugging-port=9222` and runs Flutter on `-d **`mkfifo` permission denied.** `start` creates the FIFO at `~/.artisan/flutter-dev.fifo`. If `~/.artisan/` was created by a previous run with different ownership or mode bits, `mkfifo` fails. Fix with `rm -rf ~/.artisan && dart run artisan start`. Note: `start` is POSIX-only (macOS/Linux). It will not work on Windows. -**VM Service URI never appears (90-second timeout).** If `flutter run` stalls before printing the URI (for example, the Chrome binary is missing, the Flutter SDK is not on `PATH`, or a Dart compilation error occurs), `start` throws `StateError: Timed out after 90s...`. Inspect the log at `~/.artisan/flutter-dev.log` for the underlying Flutter output. Common causes: wrong `--device` value, missing `CHROME_EXECUTABLE` env var for headless environments, or a syntax error in the app's entry point. +**VM Service URI never appears (timeout).** If `flutter run` stalls before printing the URI (for example, the Chrome binary is missing, the Flutter SDK is not on `PATH`, or a Dart compilation error occurs), `start` throws `StateError: Timed out after s...`. Inspect the log at `~/.artisan/flutter-dev.log` for the underlying Flutter output. Common causes: wrong `--device` value, missing `CHROME_EXECUTABLE` env var for headless environments, or a syntax error in the app's entry point. On cold starts (first run after a fresh SDK install, slow CI), increase the deadline with `--timeout=120` or higher. + +**CDP port already in use (`--cdp-port` collision).** When the configured CDP port is held by another process, `start` exits immediately before launching Chrome and emits: `CDP port is already in use; pass --cdp-port or free it before running start.` Choose a free port (for example `--cdp-port=9224`) or kill the occupying process with `lsof -ti: | xargs kill`. ## Related diff --git a/doc/reference/signature-dsl.md b/doc/reference/signature-dsl.md index b0c4071..d64ad61 100644 --- a/doc/reference/signature-dsl.md +++ b/doc/reference/signature-dsl.md @@ -16,6 +16,7 @@ registration time; the registry applies it to the underlying `ArgParser`. - [Option Modifiers](#option-modifiers) - [Description Annotation](#description-annotation) - [Name Validation](#name-validation) +- [Error Handling](#error-handling) - [Examples](#examples) - [Related](#related) @@ -150,6 +151,35 @@ parse time with the invalid value and expected pattern quoted in the message. --- + +## Error Handling + +When a command is invoked with an option the parser does not recognize, the +dispatcher fails loudly: it writes `Unknown option: ` to stderr, prints +the command help, and exits with a non-zero code. This applies to both long +(`--unknown`) and short (`-x`) forms. + +``` +$ artisan make:command Foo --bogus +✗ Unknown option: --bogus +ℹ Description: + ... +``` + +Every other parse failure keeps its original, specific message: + +| Cause | Example invocation | Message | +|-------|--------------------|---------| +| Unknown option | `--bogus`, `-x` | `Unknown option: ` | +| Missing option value | `--output` (value required) | `Missing argument for "--output".` | +| Disallowed value | `--mode=zoom` (not in `allowed`) | `"zoom" is not an allowed value for option "--mode".` | +| Value on a flag | `--force=1` | `Flag option "--force" should not be given a value.` | + +`--help` / `-h` and every valid invocation are unaffected: they parse and +behave exactly as before. + +--- + ## Examples diff --git a/lib/src/commands/start_command.dart b/lib/src/commands/start_command.dart index 5aabf34..4fd4bc1 100644 --- a/lib/src/commands/start_command.dart +++ b/lib/src/commands/start_command.dart @@ -232,6 +232,13 @@ class StartCommand extends ArtisanCommand { help: 'Chrome DevTools Protocol port. When set, pre-launches Chrome ' 'with --remote-debugging-port=N and runs flutter with -d web-server. ' 'Required for dusk:resize / dusk:device commands.', + ) + ..addOption( + 'timeout', + defaultsTo: '90', + help: 'Seconds to wait for the VM Service URI to appear in the flutter ' + 'run log. Increase on cold starts where build + DartDev init takes ' + 'longer than the default. Applies to the --cdp-port branch only.', ); } @@ -263,6 +270,24 @@ class StartCommand extends ArtisanCommand { resolvedCdpPort = parsed; } + // 2. Resolve --timeout (applies to the CDP branch VM Service scrape). + final timeoutRaw = (ctx.input.option('timeout') as String?) ?? '90'; + final resolvedTimeout = int.tryParse(timeoutRaw); + if (resolvedTimeout == null) { + ctx.output.error( + '--timeout expects an integer number of seconds, got "$timeoutRaw". ' + 'Pass e.g. --timeout=120.', + ); + return 1; + } + if (resolvedTimeout <= 0) { + ctx.output.error( + '--timeout must be a positive integer (got $resolvedTimeout). ' + 'Pass e.g. --timeout=90.', + ); + return 1; + } + if (resolvedCdpPort != null) { return await _handleCdpBranch( ctx: ctx, @@ -272,6 +297,7 @@ class StartCommand extends ArtisanCommand { ddsOn: ddsOn, profileStatic: profileStatic, cdpPort: resolvedCdpPort, + scrapeTimeout: resolvedTimeout, ); } @@ -308,7 +334,7 @@ class StartCommand extends ArtisanCommand { ); } - final vmServiceUri = await _runVmServiceScrape(logFile); + final vmServiceUri = await _runVmServiceScrape(logFile, 90); await StateFile.write({ 'pid': childPid, @@ -344,6 +370,7 @@ class StartCommand extends ArtisanCommand { required bool ddsOn, required bool profileStatic, required int cdpPort, + int scrapeTimeout = 90, }) async { // 1. Validate device value: only chrome (default) and web-server accepted. if (device != 'chrome' && device != 'web-server') { @@ -392,6 +419,18 @@ class StartCommand extends ArtisanCommand { return 1; } + // 4b. Probe CDP port availability before spawning Chrome. A busy CDP port + // means Chrome will fail to open the debug port, producing a misleading + // "Is Chrome installed?" error. Fail fast here with an actionable message. + final cdpPortFree = await cdpPortProbe(cdpPort); + if (!cdpPortFree) { + ctx.output.error( + 'CDP port $cdpPort is already in use; pass --cdp-port ' + 'or free it before running start.', + ); + return 1; + } + // 5. Launch Chrome detached with debug port + dedicated user-data-dir. final tmpRoot = cdpTmpProfileDirRoot ?? '/tmp'; final tmpProfileDir = '$tmpRoot/dusk-chrome-$cdpPort'; @@ -515,7 +554,7 @@ class StartCommand extends ArtisanCommand { await cdpChromeNavigator(cdpPort, 'http://localhost:$webPort/'); // 11. NOW scrape the VM Service URI emitted by DWDS once Chrome connected. - final vmServiceUri = await _runVmServiceScrape(logFile); + final vmServiceUri = await _runVmServiceScrape(logFile, scrapeTimeout); // 12. Write state with the new CDP fields so StopCommand can reap Chrome. await StateFile.write({ @@ -692,10 +731,12 @@ class StartCommand extends ArtisanCommand { } /// Runs the VM Service URI scrape, honoring the test seam when set. - Future _runVmServiceScrape(File logFile) { + /// [timeoutSeconds] caps the scrape loop; the production implementation + /// uses it as the deadline, while the test seam ignores it. + Future _runVmServiceScrape(File logFile, int timeoutSeconds) { final scraper = cdpVmServiceScraper; if (scraper != null) return scraper(logFile); - return _scrapeVmServiceUriFromFile(logFile); + return _scrapeVmServiceUriFromFile(logFile, timeoutSeconds); } /// Probes `flutter --version --machine` and extracts `frameworkVersion`. @@ -948,10 +989,20 @@ class StartCommand extends ArtisanCommand { return await completer.future.timeout(const Duration(seconds: 10)); } - Future _scrapeVmServiceUriFromFile(File logFile) async { + /// Test-only entry to the live VM Service URI scrape loop, the production + /// path the [cdpVmServiceScraper] seam otherwise bypasses. Lets tests cover + /// the configurable deadline and the timeout error directly. + @visibleForTesting + Future scrapeVmServiceUriForTest(File logFile, int timeoutSeconds) => + _scrapeVmServiceUriFromFile(logFile, timeoutSeconds); + + Future _scrapeVmServiceUriFromFile( + File logFile, + int timeoutSeconds, + ) async { final sw = Stopwatch()..start(); int lastSize = 0; - while (sw.elapsed < const Duration(seconds: 90)) { + while (sw.elapsed < Duration(seconds: timeoutSeconds)) { if (logFile.existsSync()) { final size = logFile.lengthSync(); if (size > lastSize) { @@ -966,7 +1017,8 @@ class StartCommand extends ArtisanCommand { await Future.delayed(const Duration(milliseconds: 250)); } throw StateError( - 'Timed out after 90s waiting for VM Service URI in ${logFile.path}.', + 'Timed out after ${timeoutSeconds}s waiting for VM Service URI in ' + '${logFile.path}.', ); } diff --git a/lib/src/console/artisan_application.dart b/lib/src/console/artisan_application.dart index 53eefae..1dbc5cd 100644 --- a/lib/src/console/artisan_application.dart +++ b/lib/src/console/artisan_application.dart @@ -65,6 +65,20 @@ class ArtisanApplication { final ArgResults parsed; try { parsed = parser.parse(commandArgs); + } on ArgParserException catch (e) { + // An unknown flag must fail loudly rather than silently printing help as + // if it were requested: name the offending option, then show the help so + // the caller can find the correct flag. Other parse failures (missing + // value, bad value, mandatory option) keep their original message. + if (_isUnknownOption(e)) { + stderr.writeln( + ConsoleStyle.error('Unknown option: ${e.argumentName ?? e.message}'), + ); + } else { + stderr.writeln(ConsoleStyle.error(e.message)); + } + _printCommandHelp(command, parser); + return 1; } on FormatException catch (e) { stderr.writeln(ConsoleStyle.error(e.message)); _printCommandHelp(command, parser); @@ -136,6 +150,17 @@ class ArtisanApplication { } } + /// Whether [e] reports an unknown option / flag rather than a missing value, + /// a disallowed value, or a mandatory-option violation. + /// + /// Dart's `args` has no dedicated unknown-flag error type, so the signal is + /// the message prefix the parser emits for every "Could not find an option" + /// path (`--long` and `-short` both share it). Matching the prefix keeps the + /// other [ArgParserException] causes on their original, helpful messages. + bool _isUnknownOption(ArgParserException e) { + return e.message.startsWith('Could not find an option'); + } + void _printRootHelp() { stdout.writeln(ConsoleStyle.header('artisan $version')); stdout.writeln(''); diff --git a/test/commands/start_command_test.dart b/test/commands/start_command_test.dart index 94d7831..f091253 100644 --- a/test/commands/start_command_test.dart +++ b/test/commands/start_command_test.dart @@ -396,6 +396,9 @@ void main() { flutterVersionStdout: '{"frameworkVersion":"3.30.0"}', ); StartCommand.cdpChromeBinaryResolver = (_) => '/fake/chrome'; + // All ports free so the pre-launch probes pass; the failure is in the + // post-launch Chrome probe. + StartCommand.cdpPortProbe = (_) async => true; var killedPid = 0; final chrome = _SpyProcess(pid: 4242, onKill: (sig) => killedPid = 4242); StartCommand.cdpProcessStarter = ( @@ -440,6 +443,7 @@ void main() { flutterVersionStdout: '{"frameworkVersion":"3.30.0"}', ); StartCommand.cdpChromeBinaryResolver = (_) => '/fake/chrome'; + StartCommand.cdpPortProbe = (_) async => true; final spawned = >[]; _SpyProcess? chromeProc; @@ -535,6 +539,7 @@ void main() { flutterVersionStdout: '{"frameworkVersion":"3.30.0"}', ); StartCommand.cdpChromeBinaryResolver = (_) => '/fake/chrome'; + StartCommand.cdpPortProbe = (_) async => true; final spawned = >[]; StartCommand.cdpProcessStarter = ( @@ -589,6 +594,7 @@ void main() { flutterVersionStdout: '{"frameworkVersion":"3.30.0"}', ); StartCommand.cdpChromeBinaryResolver = (_) => '/fake/chrome'; + StartCommand.cdpPortProbe = (_) async => true; StartCommand.cdpProcessStarter = ( String exec, @@ -637,6 +643,7 @@ void main() { flutterVersionStdout: '{"frameworkVersion":"3.30.0"}', ); StartCommand.cdpChromeBinaryResolver = (_) => '/fake/chrome'; + StartCommand.cdpPortProbe = (_) async => true; StartCommand.cdpProcessStarter = ( String exec, @@ -708,6 +715,7 @@ void main() { flutterVersionStdout: '{"frameworkVersion":"3.30.0"}', ); StartCommand.cdpChromeBinaryResolver = (_) => '/fake/chrome'; + StartCommand.cdpPortProbe = (_) async => true; var chromeKilled = false; Process? flutterHandle; @@ -779,6 +787,7 @@ void main() { flutterVersionStdout: '{"frameworkVersion":"3.30.0"}', ); StartCommand.cdpChromeBinaryResolver = (_) => '/fake/chrome'; + StartCommand.cdpPortProbe = (_) async => true; var chromeKilled = false; StartCommand.cdpProcessStarter = ( @@ -848,6 +857,7 @@ void main() { flutterVersionStdout: '{"frameworkVersion":"3.30.0"}', ); StartCommand.cdpChromeBinaryResolver = (_) => '/fake/chrome'; + StartCommand.cdpPortProbe = (_) async => true; var chromeKilled = false; var flutterSpawned = false; @@ -899,6 +909,235 @@ void main() { reason: 'tmp profile dir must be removed on failure'); }); + test('--timeout configure declares option with default 90', () { + final command = StartCommand(); + final parser = ArgParser(); + + command.configure(parser); + + expect(parser.options.containsKey('timeout'), isTrue, + reason: '--timeout option must be declared in configure()'); + final result = parser.parse([]); + expect(result['timeout'], '90', reason: '--timeout must default to 90'); + }); + + test('--timeout rejects non-integer value: exit 1 + actionable error', + () async { + final command = StartCommand(); + final output = BufferedOutput(); + final ctx = ArtisanContext.bare( + MapInput({ + 'device': 'chrome', + 'port': '3100', + 'dds': false, + 'profile-static': false, + 'cdp-port': '9223', + 'timeout': 'abc', + }), + output, + ); + + final code = await command.handle(ctx); + + expect(code, 1); + expect(output.content, + contains('--timeout expects an integer number of seconds')); + }); + + test('--timeout rejects zero: exit 1 + actionable error', () async { + final command = StartCommand(); + final output = BufferedOutput(); + final ctx = ArtisanContext.bare( + MapInput({ + 'device': 'chrome', + 'port': '3100', + 'dds': false, + 'profile-static': false, + 'cdp-port': '9223', + 'timeout': '0', + }), + output, + ); + + final code = await command.handle(ctx); + + expect(code, 1); + expect( + output.content, + contains('--timeout must be a positive integer'), + ); + }); + + test('--timeout rejects negative value: exit 1 + actionable error', + () async { + final command = StartCommand(); + final output = BufferedOutput(); + final ctx = ArtisanContext.bare( + MapInput({ + 'device': 'chrome', + 'port': '3100', + 'dds': false, + 'profile-static': false, + 'cdp-port': '9223', + 'timeout': '-5', + }), + output, + ); + + final code = await command.handle(ctx); + + expect(code, 1); + expect( + output.content, + contains('--timeout must be a positive integer'), + ); + }); + + test( + '--timeout value drives the VM Service scrape seam (configured value ' + 'reported in error, not literal 90)', () async { + // Arrange: inject a scraper that throws. The seam only receives a File, + // so the test asserts the error surfaced by handle() contains the + // configured timeout value rather than observing the deadline directly. + StartCommand.cdpProcessRunner = _fakeProcessRunner( + flutterVersionStdout: '{"frameworkVersion":"3.30.0"}', + ); + StartCommand.cdpChromeBinaryResolver = (_) => '/fake/chrome'; + // All ports free so the pre-launch probes do not block the test. + StartCommand.cdpPortProbe = (_) async => true; + + StartCommand.cdpProcessStarter = ( + String exec, + List args, { + String? workingDirectory, + ProcessStartMode? mode, + }) async { + if (exec == '/fake/chrome') return _SpyProcess(pid: 111); + return _FakeFlutterProcess(holderPid: 10, flutterPid: 20); + }; + + StartCommand.cdpChromeProber = (port, timeout) async {}; + StartCommand.cdpWebServerReadyWaiter = (_) async {}; + StartCommand.cdpChromeNavigator = (port, url) async {}; + StartCommand.cdpFifoMaker = (path) async { + File(path).writeAsStringSync(''); + }; + + // The injected seam throws a timeout-shaped message containing the + // configured value; the test checks the error surfaced by handle() + // contains "120s" (the configured timeout) rather than "90s" (the old + // hardcoded literal). + StartCommand.cdpVmServiceScraper = (_) async { + throw StateError('Timed out after 120s waiting for VM Service URI'); + }; + + final killedPids = []; + StartCommand.cdpKillPid = (pid, [signal = ProcessSignal.sigterm]) { + killedPids.add(pid); + return true; + }; + + final command = StartCommand(); + final output = BufferedOutput(); + final ctx = ArtisanContext.bare( + MapInput({ + 'device': 'chrome', + 'port': '3100', + 'dds': false, + 'profile-static': false, + 'cdp-port': '9223', + 'timeout': '120', + }), + output, + ); + + final code = await command.handle(ctx); + + expect(code, 1); + // The scraper seam surfaces the message that uses the configured value. + expect(output.content, contains('120s'), + reason: 'error must report configured timeout value, not a literal'); + }); + + test('live scrape loop returns the normalized URI when the log yields one', + () async { + final dir = Directory.systemTemp.createTempSync('artisan_scrape_'); + addTearDown(() => dir.deleteSync(recursive: true)); + final log = File('${dir.path}/run.log') + ..writeAsStringSync( + 'Debug service listening on http://127.0.0.1:1234/abc=/\n', + ); + + final uri = await StartCommand().scrapeVmServiceUriForTest(log, 5); + + expect(uri, 'ws://127.0.0.1:1234/abc=/ws'); + }); + + test( + 'live scrape loop throws after the configured timeout when no URI ' + 'appears (deadline honors the configured value)', () async { + final dir = Directory.systemTemp.createTempSync('artisan_scrape_'); + addTearDown(() => dir.deleteSync(recursive: true)); + final log = File('${dir.path}/run.log') + ..writeAsStringSync('starting up, no service uri yet\n'); + + await expectLater( + StartCommand().scrapeVmServiceUriForTest(log, 1), + throwsA( + isA().having( + (e) => e.toString(), + 'message', + contains('Timed out after 1s'), + ), + ), + ); + }); + + test( + 'busy cdpPort before Chrome launch: exit 1 + distinct error, ' + 'Chrome never spawned', () async { + StartCommand.cdpProcessRunner = _fakeProcessRunner( + flutterVersionStdout: '{"frameworkVersion":"3.30.0"}', + ); + StartCommand.cdpChromeBinaryResolver = (_) => '/fake/chrome'; + // Probe: webPort free, cdpPort busy. + StartCommand.cdpPortProbe = (port) async => port != 9223; + + var chromeLaunched = false; + StartCommand.cdpProcessStarter = ( + String exec, + List args, { + String? workingDirectory, + ProcessStartMode? mode, + }) async { + chromeLaunched = true; + return _NoopProcess(); + }; + + final command = StartCommand(); + final output = BufferedOutput(); + final ctx = ArtisanContext.bare( + MapInput({ + 'device': 'chrome', + 'port': '3100', + 'dds': false, + 'profile-static': false, + 'cdp-port': '9223', + }), + output, + ); + + final code = await command.handle(ctx); + + expect(code, 1, reason: 'busy CDP port must exit 1'); + expect(output.content, contains('9223'), + reason: 'error must name the busy CDP port'); + expect(output.content, contains('--cdp-port'), + reason: 'error must suggest --cdp-port flag'); + expect(chromeLaunched, isFalse, + reason: 'Chrome must not be launched when CDP port is busy'); + }); + test('without --cdp-port: existing flow unchanged (no Chrome pre-launch)', () async { // Existing flow runs Process.start sh -c ... directly. We stub the diff --git a/test/console/artisan_application_test.dart b/test/console/artisan_application_test.dart index 6fa5631..11f9b09 100644 --- a/test/console/artisan_application_test.dart +++ b/test/console/artisan_application_test.dart @@ -1,6 +1,24 @@ +import 'dart:convert'; +import 'dart:io'; + import 'package:fluttersdk_artisan/artisan.dart'; import 'package:test/test.dart'; +/// Run [body] capturing everything written to `stderr` and `stdout`, so the +/// dispatch path's user-facing output can be asserted in-process. +Future<({int code, String err, String out})> _captureDispatch( + Future Function() body, +) async { + final err = _CapturingStdout(); + final out = _CapturingStdout(); + final code = await IOOverrides.runZoned( + body, + stderr: () => err, + stdout: () => out, + ); + return (code: code, err: err.buffer.toString(), out: out.buffer.toString()); +} + void main() { group('ArtisanApplication.dispatch', () { test('empty args prints root help and exits 0', () async { @@ -44,14 +62,77 @@ void main() { expect(code, 7); }); - test('FormatException from arg parser surfaces as exit 1', () async { + test('unknown long flag errors clearly on stderr and exits non-zero', + () async { final registry = ArtisanRegistry(); registry.register(_OptionCommand()); final app = ArtisanApplication(registry: registry); - final code = await app.dispatch(['has-option', '--unknown']); + final result = await _captureDispatch( + () => app.dispatch(['has-option', '--unknown']), + ); - expect(code, 1); + expect(result.code, isNonZero); + expect(result.err, contains('Unknown option: --unknown')); + }); + + test('unknown short flag errors clearly on stderr and exits non-zero', + () async { + final registry = ArtisanRegistry(); + registry.register(_OptionCommand()); + final app = ArtisanApplication(registry: registry); + + final result = await _captureDispatch( + () => app.dispatch(['has-option', '-x']), + ); + + expect(result.code, isNonZero); + expect(result.err, contains('Unknown option: -x')); + }); + + test('valid flag set parses unchanged and reaches handle', () async { + final command = _OptionCommand(); + final registry = ArtisanRegistry(); + registry.register(command); + final app = ArtisanApplication(registry: registry); + + final result = await _captureDispatch( + () => app.dispatch(['has-option', '--output', 'build/app']), + ); + + expect(result.code, 0); + expect(command.capturedOutput, 'build/app'); + expect(result.err, isEmpty); + }); + + test('command --help still prints help and exits 0 unchanged', () async { + final command = _OptionCommand(); + final registry = ArtisanRegistry(); + registry.register(command); + final app = ArtisanApplication(registry: registry); + + final result = await _captureDispatch( + () => app.dispatch(['has-option', '--help']), + ); + + expect(result.code, 0); + expect(command.handleCalls, 0); + expect(result.err, isEmpty); + }); + + test('genuine missing-required-value error keeps its original message', + () async { + final registry = ArtisanRegistry(); + registry.register(_OptionCommand()); + final app = ArtisanApplication(registry: registry); + + final result = await _captureDispatch( + () => app.dispatch(['has-option', '--output']), + ); + + expect(result.code, isNonZero); + expect(result.err, contains('Missing argument for "--output"')); + expect(result.err, isNot(contains('Unknown option'))); }); test('command --help (per-command) returns 0 without running handle', @@ -138,17 +219,29 @@ class _FixedExitCommand extends ArtisanCommand { } class _OptionCommand extends ArtisanCommand { + int handleCalls = 0; + String? capturedOutput; + @override String get name => 'has-option'; @override - String get description => 'declares no options'; + String get description => 'declares a single --output option'; @override CommandBoot get boot => CommandBoot.none; @override - Future handle(ArtisanContext ctx) async => 0; + void configure(ArgParser parser) { + parser.addOption('output', abbr: 'o', help: 'Output path'); + } + + @override + Future handle(ArtisanContext ctx) async { + handleCalls++; + capturedOutput = ctx.input.option('output') as String?; + return 0; + } } class _RecordingCommand extends ArtisanCommand { @@ -216,3 +309,73 @@ class _RegistryCapturingCommand extends ArtisanCommand { return 0; } } + +/// File-private [Stdout] fake that buffers writes for assertion. Used via +/// [IOOverrides.runZoned] to capture the dispatch path's stderr/stdout output +/// for both the [stdout:] and [stderr:] override slots (both slots accept a +/// [Stdout] factory in Dart's IO model). +class _CapturingStdout implements Stdout { + final StringBuffer buffer = StringBuffer(); + + @override + Encoding encoding = systemEncoding; + + @override + String lineTerminator = '\n'; + + // --- StringSink writes (buffered for assertion) --- + + @override + void write(Object? object) => buffer.write(object); + + @override + void writeln([Object? object = '']) => buffer.writeln(object); + + @override + void writeAll(Iterable objects, [String sep = '']) => + buffer.writeAll(objects, sep); + + @override + void writeCharCode(int charCode) => buffer.writeCharCode(charCode); + + // --- StreamSink> (bytes, decoded and buffered) --- + + @override + void add(List data) => buffer.write(encoding.decode(data)); + + @override + void addError(Object error, [StackTrace? stackTrace]) {} + + @override + Future addStream(Stream> stream) async { + await for (final chunk in stream) { + add(chunk); + } + } + + @override + Future flush() => Future.value(); + + @override + Future get done => Future.value(); + + @override + Future close() => Future.value(); + + // --- Stdout terminal-query members (no-op in tests) --- + + @override + bool get hasTerminal => false; + + @override + bool get supportsAnsiEscapes => false; + + @override + int get terminalColumns => 80; + + @override + int get terminalLines => 24; + + @override + IOSink get nonBlocking => this; +}