Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Report analytics before running command. #8

Merged
merged 4 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions lib/src/better_command_runner/better_command_runner.dart
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -134,22 +136,36 @@ 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.
assert(command.name != null, 'Command name should never be null.');
_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]
hampuslavin marked this conversation as resolved.
Show resolved Hide resolved
// 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);
}
SandPod marked this conversation as resolved.
Show resolved Hide resolved
}));

await _onBeforeRunCommand?.call(this);

try {
await super.runCommand(topLevelResults);
} on UsageException catch (e) {
_logError?.call(e.toString());
_onAnalyticsEvent?.call(BetterCommandRunnerAnalyticsEvents.invalid);
SandPod marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
130 changes: 130 additions & 0 deletions test/better_command_runner/analytics_test.dart
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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<void> completer = Completer<void>();

@override
void run() async {
await completer.future;
}

@override
String get name => commandName;

CompletableMockCommand() {
argParser.addOption(
'name',
defaultsTo: 'serverpod',
allowed: <String>['serverpod'],
);
}
}

void main() {
late BetterCommandRunner runner;
group('Given runner with null onAnalyticsEvent callback', () {
Expand Down Expand Up @@ -79,6 +108,8 @@ void main() {
var args = ['--no-${BetterCommandRunnerFlags.analytics}'];
await runner.run(args);

await flushEventQueue();

expect(runner.analyticsEnabled(), isFalse);
});

Expand All @@ -92,6 +123,8 @@ void main() {
// Ignore any exception
}

await flushEventQueue();

expect(events, hasLength(1));
expect(events.first, equals('invalid'));
});
Expand All @@ -107,6 +140,8 @@ void main() {
// Ignore any exception
}

await flushEventQueue();

expect(events, hasLength(1));
expect(events.first, equals('invalid'));
});
Expand All @@ -115,6 +150,8 @@ void main() {
() async {
await runner.run([]);

await flushEventQueue();

expect(events, hasLength(1));
expect(events.first, equals('help'));
});
Expand All @@ -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'));
});
Expand All @@ -150,6 +209,8 @@ void main() {

await runner.run(args);

await flushEventQueue();

expect(events, hasLength(1));
expect(events.first, equals(MockCommand.commandName));
});
Expand All @@ -161,6 +222,8 @@ void main() {

await runner.run(args);

await flushEventQueue();

expect(events, hasLength(1));
expect(events.first, equals(MockCommand.commandName));
});
Expand All @@ -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<String> 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();
});
});
}
5 changes: 5 additions & 0 deletions test/test_utils.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/// Test helper to flush the event queue.
/// Useful for waiting for async events to complete before continuing the test.
Future<void> flushEventQueue() {
return Future.delayed(Duration.zero);
}