feat: added default port number for --dart-vm-service-port (#697)

* feat: added default port number

* test: improved test name

* refactor: moved port to single variable

* feat: recommend using `--dart-vm-service-port`  on VM port failure (#698)

* refactor: moved tests to multiple_dev_test.dart

* feat: added recommendation

* refactor: fixed typo

* feat: added "try"

* docs: removed example

* refactor: removed superfluos comment

* feat: used .fail over .cancel

* feat: add debugger info

* test: hit coverage

* refactor: removed moving e2e

* test: included process.fail verification

* test: simplified output

* test: removed registerFallbackValue

* test: adde "helpful" on test name

* test: reason typo

* refactor: simplified killing logic

* refactor: removed Please

---------

Co-authored-by: Renan <[email protected]>

Author: alestiago

packages/dart_frog_cli/lib/src/commands/dev/dev.dart

@@ -41,6 +41,13 @@ typedef Exit = dynamic Function(int exitCode);
 /// Regex for detecting warnings in the output of `dart run`.
 final _warningRegex = RegExp(r'^.*:\d+:\d+: Warning: .*', multiLine: true);
 
+/// Regex for detecting when the `dart_frog dev` fails to run for using a
+/// Dart VM Service with an already used port.
+final _dartVmServiceAlreadyInUseErrorRegex = RegExp(
+  '^Could not start the VM service: localhost:.* is already in use.',
+  multiLine: true,
+);
+
 RestorableDirectoryGeneratorTarget _defaultGeneratorTarget(Logger? logger) {
   return RestorableDirectoryGeneratorTarget(
     io.Directory(
@@ -84,10 +91,13 @@ class DevCommand extends DartFrogCommand {
       ..addOption(
         'dart-vm-service-port',
         abbr: 'd',
+        defaultsTo: _defaultDartVmServicePort,
         help: 'Which port number the dart vm service should listen on.',
       );
   }
 
+  static const _defaultDartVmServicePort = '8181';
+
   final void Function(io.Directory) _ensureRuntimeCompatibility;
   final DirectoryWatcherBuilder _directoryWatcher;
   final GeneratorBuilder _generator;
@@ -117,7 +127,8 @@ class DevCommand extends DartFrogCommand {
     var reloading = false;
     var hotReloadEnabled = false;
     final port = io.Platform.environment['PORT'] ?? results['port'] as String;
-    final dartVmServicePort = results['dart-vm-service-port'] as String?;
+    final dartVmServicePort = (results['dart-vm-service-port'] as String?) ??
+        _defaultDartVmServicePort;
     final target = _generatorTarget(logger);
     final generator = await _generator(dartFrogDevServerBundle);
 
@@ -147,9 +158,7 @@ class DevCommand extends DartFrogCommand {
       logger.detail('[codegen] reload complete.');
     }
 
-    final enableVmServiceFlag = '--enable-vm-service'
-        '${dartVmServicePort == null ? "" : "=$dartVmServicePort"}';
-
+    final enableVmServiceFlag = '--enable-vm-service=$dartVmServicePort';
     Future<void> serve() async {
       logger.detail(
         '''[process] dart $enableVmServiceFlag ${path.join('.dart_frog', 'server.dart')}''',
@@ -174,16 +183,24 @@ class DevCommand extends DartFrogCommand {
         final message = utf8.decode(_).trim();
         if (message.isEmpty) return;
 
-        /// Do not kill the process if the error is a warning from the SDK.
+        final isDartVMServiceAlreadyInUseError =
+            _dartVmServiceAlreadyInUseErrorRegex.hasMatch(message);
         final isSDKWarning = _warningRegex.hasMatch(message);
 
-        if (isSDKWarning) {
+        if (isDartVMServiceAlreadyInUseError) {
+          logger.err(
+            '$message '
+            '''Try specifying a different port using the `--dart-vm-service-port` argument when running `dart_frog dev`.''',
+          );
+        } else if (isSDKWarning) {
+          /// Do not kill the process if the error is a warning from the SDK.
           logger.warn(message);
         } else {
           logger.err(message);
         }
 
-        if (!hotReloadEnabled && !isSDKWarning) {
+        if ((!hotReloadEnabled && !isSDKWarning) ||
+            isDartVMServiceAlreadyInUseError) {
           await _killProcess(process);
           logger.detail('[process] exit(1)');
           _exit(1);

packages/dart_frog_cli/test/src/commands/dev/dev_test.dart

@@ -587,8 +587,7 @@ void main() {
     });
 
     test(
-      'when dart vm port not specified, --enable-vm-service option should '
-      'be called without any port number value',
+      '''when --dart-vm-service-port is not specified, calls --enable-vm-service with port 8181''',
       () async {
         late List<String> receivedArgs;
         command.testArgResults = argResults;
@@ -634,7 +633,7 @@ void main() {
         )..testArgResults = argResults;
         final exitCode = await command.run();
         expect(exitCode, equals(ExitCode.success.code));
-        expect(receivedArgs[0], equals('--enable-vm-service'));
+        expect(receivedArgs[0], equals('--enable-vm-service=8181'));
         verify(
           () => generatorHooks.preGen(
             vars: <String, dynamic>{'port': '8080'},
@@ -902,6 +901,84 @@ lib/my_model.g.dart:53:20: Warning: Operand of null-aware operation '!' has type
       expect(processRunCalls, isEmpty);
       verify(() => process.kill()).called(1);
     });
+
+    test(
+        '''kills process with helpful message when Dart VM is already in use''',
+        () async {
+      final generatorHooks = _MockGeneratorHooks();
+      final processRunCalls = <List<String>>[];
+      int? exitCode;
+      when(
+        () => generatorHooks.preGen(
+          vars: any(named: 'vars'),
+          workingDirectory: any(named: 'workingDirectory'),
+          onVarsChanged: any(named: 'onVarsChanged'),
+        ),
+      ).thenAnswer((invocation) async {
+        (invocation.namedArguments[const Symbol('onVarsChanged')] as void
+                Function(Map<String, dynamic> vars))
+            .call(<String, dynamic>{});
+      });
+      when(
+        () => generator.generate(
+          any(),
+          vars: any(named: 'vars'),
+          fileConflictResolution: FileConflictResolution.overwrite,
+        ),
+      ).thenAnswer((_) async => []);
+      when(() => generator.hooks).thenReturn(generatorHooks);
+      when(() => process.stdout).thenAnswer((_) => const Stream.empty());
+      const errorMessage =
+          'Could not start the VM service: localhost:8181 is already in use.';
+      when(() => process.stderr).thenAnswer(
+        (_) => Stream.value(utf8.encode(errorMessage)),
+      );
+      when(() => process.kill()).thenReturn(true);
+      when(
+        () => directoryWatcher.events,
+      ).thenAnswer((_) => StreamController<WatchEvent>().stream);
+      when(() => sigint.watch()).thenAnswer((_) => const Stream.empty());
+      command = DevCommand(
+        logger: logger,
+        ensureRuntimeCompatibility: (_) {},
+        directoryWatcher: (_) => directoryWatcher,
+        generator: (_) async => generator,
+        exit: (code) => exitCode = code,
+        startProcess: (
+          String executable,
+          List<String> arguments, {
+          bool runInShell = false,
+        }) async {
+          return process;
+        },
+        sigint: sigint,
+      )
+        ..testArgResults = argResults
+        ..testRunProcess = (String executable, List<String> arguments) async {
+          processRunCalls.add([executable, ...arguments]);
+          return processResult;
+        };
+      command.run().ignore();
+
+      await untilCalled(() => process.kill());
+      expect(
+        exitCode,
+        equals(1),
+        reason: 'Should exit when VM service is already in use.',
+      );
+      expect(
+        processRunCalls,
+        isEmpty,
+        reason: 'Should not run the serve process.',
+      );
+      verify(() => process.kill()).called(1);
+      verify(
+        () => logger.err(
+          '$errorMessage '
+          '''Try specifying a different port using the `--dart-vm-service-port` argument when running `dart_frog dev`.''',
+        ),
+      ).called(1);
+    });
   });
 
   group('CachedFile', () {