diff --git a/lib/src/better_command_runner/better_command_runner.dart b/lib/src/better_command_runner/better_command_runner.dart index 3d0c8a9..659dc39 100644 --- a/lib/src/better_command_runner/better_command_runner.dart +++ b/lib/src/better_command_runner/better_command_runner.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:args/args.dart'; import 'package:args/command_runner.dart'; import 'package:cli_tools/src/better_command_runner/exit_exception.dart'; @@ -134,14 +136,9 @@ class BetterCommandRunner extends CommandRunner { _onAnalyticsEvent = null; } - await _onBeforeRunCommand?.call(this); - - try { - await super.runCommand(topLevelResults); + unawaited(Future(() async { var command = topLevelResults.command; - if (command == null) { - _onAnalyticsEvent?.call(BetterCommandRunnerAnalyticsEvents.help); - } else { + if (command != null) { // Command name can only be null for top level results. // But since we are taking the name of a command from the top level // results there should always be a name specified. @@ -149,7 +146,26 @@ class BetterCommandRunner extends CommandRunner { _onAnalyticsEvent?.call( command.name ?? BetterCommandRunnerAnalyticsEvents.invalid, ); + return; + } + + // Checks if the command is valid (i.e. no unexpected arguments). + // If there are unexpected arguments this will trigger a [UsageException] + // which will be caught in the try catch around the super.runCommand call. + // Therefore, this ensures that the help event is not sent for + // commands that are invalid. + // Note that there are other scenarios that also trigger a [UsageException] + // so the try/catch statement can't be fully compensated for handled here. + var noUnexpectedArgs = topLevelResults.rest.isEmpty; + if (noUnexpectedArgs) { + _onAnalyticsEvent?.call(BetterCommandRunnerAnalyticsEvents.help); } + })); + + await _onBeforeRunCommand?.call(this); + + try { + await super.runCommand(topLevelResults); } on UsageException catch (e) { _logError?.call(e.toString()); _onAnalyticsEvent?.call(BetterCommandRunnerAnalyticsEvents.invalid); diff --git a/test/better_command_runner/analytics_test.dart b/test/better_command_runner/analytics_test.dart index afee6f2..a7bcee4 100644 --- a/test/better_command_runner/analytics_test.dart +++ b/test/better_command_runner/analytics_test.dart @@ -1,7 +1,11 @@ +import 'dart:async'; + import 'package:args/command_runner.dart'; import 'package:cli_tools/better_command_runner.dart'; import 'package:test/test.dart'; +import '../test_utils.dart'; + class MockCommand extends Command { static String commandName = 'mock-command'; @@ -23,6 +27,31 @@ class MockCommand extends Command { } } +class CompletableMockCommand extends Command { + static String commandName = 'completable-mock-command'; + + @override + String get description => 'Mock command used to test when process hangs'; + + Completer completer = Completer(); + + @override + void run() async { + await completer.future; + } + + @override + String get name => commandName; + + CompletableMockCommand() { + argParser.addOption( + 'name', + defaultsTo: 'serverpod', + allowed: ['serverpod'], + ); + } +} + void main() { late BetterCommandRunner runner; group('Given runner with null onAnalyticsEvent callback', () { @@ -79,6 +108,8 @@ void main() { var args = ['--no-${BetterCommandRunnerFlags.analytics}']; await runner.run(args); + await flushEventQueue(); + expect(runner.analyticsEnabled(), isFalse); }); @@ -92,6 +123,8 @@ void main() { // Ignore any exception } + await flushEventQueue(); + expect(events, hasLength(1)); expect(events.first, equals('invalid')); }); @@ -107,6 +140,8 @@ void main() { // Ignore any exception } + await flushEventQueue(); + expect(events, hasLength(1)); expect(events.first, equals('invalid')); }); @@ -115,6 +150,8 @@ void main() { () async { await runner.run([]); + await flushEventQueue(); + expect(events, hasLength(1)); expect(events.first, equals('help')); }); @@ -124,6 +161,28 @@ void main() { () async { await runner.run(['--${BetterCommandRunnerFlags.analytics}']); + await flushEventQueue(); + + expect(events, hasLength(1)); + expect(events.first, equals('help')); + }); + + test('when running with help command then "help" analytics event is sent.', + () async { + await runner.run(['help']); + + await flushEventQueue(); + + expect(events, hasLength(1)); + expect(events.first, equals('help')); + }); + + test('when running with help flag then "help" analytics event is sent.', + () async { + await runner.run(['--help']); + + await flushEventQueue(); + expect(events, hasLength(1)); expect(events.first, equals('help')); }); @@ -150,6 +209,8 @@ void main() { await runner.run(args); + await flushEventQueue(); + expect(events, hasLength(1)); expect(events.first, equals(MockCommand.commandName)); }); @@ -161,6 +222,8 @@ void main() { await runner.run(args); + await flushEventQueue(); + expect(events, hasLength(1)); expect(events.first, equals(MockCommand.commandName)); }); @@ -176,8 +239,75 @@ void main() { // Ignore any exception } + await flushEventQueue(); + expect(events, hasLength(1)); expect(events.first, equals('invalid')); }); }); + + group('Given runner with registered command and analytics enabled', () { + List events = []; + late CompletableMockCommand command; + setUp(() { + command = CompletableMockCommand(); + runner = BetterCommandRunner( + 'test', + 'this is a test cli', + onAnalyticsEvent: (event) => events.add(event), + )..addCommand(command); + assert(runner.analyticsEnabled()); + }); + + tearDown(() { + events = []; + }); + + test( + 'when running with registered command that hangs then command name is sent', + () async { + var args = [CompletableMockCommand.commandName]; + + unawaited(runner.run(args)); + + await flushEventQueue(); + + expect(events, hasLength(1)); + expect(events.first, equals(CompletableMockCommand.commandName)); + + command.completer.complete(); + }); + + test( + 'when running with registered command that hangs and option then command name is sent,', + () async { + var args = [CompletableMockCommand.commandName, '--name', 'serverpod']; + + unawaited(runner.run(args)); + + await flushEventQueue(); + + expect(events, hasLength(1)); + expect(events.first, equals(CompletableMockCommand.commandName)); + + command.completer.complete(); + }); + + test( + 'when running with registered command that hangs but invalid option then "invalid" analytics event is sent,', + () async { + var args = [CompletableMockCommand.commandName, '--name', 'invalid']; + + unawaited(runner.run(args).catchError((_) { + // Ignore parse error + })); + + await flushEventQueue(); + + expect(events, hasLength(1)); + expect(events.first, equals('invalid')); + + command.completer.complete(); + }); + }); } diff --git a/test/test_utils.dart b/test/test_utils.dart new file mode 100644 index 0000000..e5c0ca7 --- /dev/null +++ b/test/test_utils.dart @@ -0,0 +1,5 @@ +/// Test helper to flush the event queue. +/// Useful for waiting for async events to complete before continuing the test. +Future flushEventQueue() { + return Future.delayed(Duration.zero); +}