Skip to content

Commit

Permalink
feat: execute is now async
Browse files Browse the repository at this point in the history
BREAKING CHANGE:

The execute function is now async and will resolve/reject
once the command finishes executing or errors.

This allows for performing global cleanup in your
application's entry point.
  • Loading branch information
codeandcats committed Nov 12, 2023
1 parent b916474 commit dcf648c
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 69 deletions.
6 changes: 3 additions & 3 deletions example/calc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ async function run() {
.ioc(container)
.commandsFromDirectory(path.join(__dirname, '/commands'));

cli.execute();
await cli.execute();
}

// tslint:disable-next-line:no-console
run().catch(console.error);
// tslint:disable-next-line:no-empty
run().catch(() => {});
28 changes: 21 additions & 7 deletions src/classy.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// tslint:disable:no-console
import chalk = require('chalk');
import * as cli from 'commander';
import * as fs from 'fs-extra';
import * as glob from 'glob';
Expand Down Expand Up @@ -46,28 +47,41 @@ export class Commander {
return this;
}

public execute(argv?: string[]) {
public async execute(argv?: string[]) {
if (this._version) {
cli.version(this._version);
cli.program.version(this._version);
}

const args = argv || process.argv;

cli.program.addHelpText('after', '\n');

if (args.length <= 2) {
cli.help((text) => `${text}\n`);
cli.program.outputHelp();
return;
}

cli.on('command:*', () => {
cli.program.on('command:*', () => {
console.error();
console.error(
'Invalid command: %s\nSee --help for a list of available commands.',
cli.args.join(' ')
cli.program.args.join(' ')
);
console.error();
process.exit(1);
process.exitCode = 1;
});

cli.parse(args);
try {
await cli.program.parseAsync(args);
} catch (err: any) {
process.exitCode = 1;
console.error();
// istanbul ignore next
const errorMessage = err.message ?? err;
console.error(chalk.red(errorMessage));
console.error();
throw err;
}
}

private getPackageVersion(packageFileName: string): string | undefined {
Expand Down
21 changes: 9 additions & 12 deletions src/commander.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { getCommandUsage, getIocContainer, registerCommand, setIocContainer } fr
import { option, value } from './decorators';
import { Command, CommandDefinition, IocContainer } from './types';

type CommanderCommand = typeof commander.program.command;

jest.mock('commander');

const noop = () => {
Expand Down Expand Up @@ -77,7 +79,7 @@ describe('src/commander', () => {

describe('registerCommand', () => {
let command: CommandDefinition<any>;
let commanderCommandMock: ReturnType<typeof commander['command']>;
let commanderCommandMock: ReturnType<typeof commander.program.command>;
let action: (...args: any[]) => void | undefined;

beforeEach(() => {
Expand All @@ -88,19 +90,19 @@ describe('src/commander', () => {
type: LoginCommand
};

commanderCommandMock = partialOf<ReturnType<typeof commander['command']>>({
commanderCommandMock = partialOf<ReturnType<CommanderCommand>>({
action: jest.fn().mockImplementation((a) => action = a),
description: jest.fn(),
option: jest.fn()
});

jest.spyOn(commander, 'command').mockReturnValue(commanderCommandMock);
jest.spyOn(commander.program, 'command').mockReturnValue(commanderCommandMock);

registerCommand(command);
});

it('should register a command with commander', () => {
expect(commander.command).toHaveBeenCalledWith('login <username> [password]');
expect(commander.program.command).toHaveBeenCalledWith('login <username> [password]');

expect(commanderCommandMock.description).toHaveBeenCalledWith('Logs user in');

Expand All @@ -115,7 +117,7 @@ describe('src/commander', () => {

expect(commanderCommandMock.option).toHaveBeenCalledWith(
'--mfaCode <code>',
undefined,
'',
expect.any(Function),
undefined
);
Expand All @@ -136,19 +138,14 @@ describe('src/commander', () => {
expect(loginHandler).toHaveBeenCalledWith({ username: 'john', password: undefined, pin: 8008, rememberMeFor: 3, mfaCode: 'abcdef' });
});

it('should register an action that executes the command and handles errors', async () => {
it('should register an action that executes the command and bubbles errors up', async () => {
jest.spyOn(console, 'error').mockImplementation(noop);
(loginHandler as unknown as jest.Mock<{}>).mockImplementation(() => {
throw new Error('Computer says no');
});
expect(action).toBeDefined();
await expect(action('john', undefined, {})).resolves.toBeUndefined();
await expect(action('john', undefined, {})).rejects.toEqual(new Error('Computer says no'));
expect(loginHandler).toHaveBeenCalledWith({ username: 'john', rememberMeFor: 7 });
// tslint:disable-next-line:no-console
expect(console.error).toHaveBeenCalledWith(
expect.stringMatching(/Computer says no/gi)
);
expect(process.exit).toHaveBeenCalledWith(1);
});
});

Expand Down
22 changes: 6 additions & 16 deletions src/commander.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import chalk = require('chalk');
import * as cli from 'commander';
import * as _ from 'lodash';
import { coerceValue } from './coercion';
Expand Down Expand Up @@ -84,7 +83,7 @@ function registerCommandOption(
option: CommandOptionDefinition
) {
const optionUsage = getOptionUsage(option);
const coercedValue = !option.valueName ? undefined : ((value: string) => {
const coercedValue = ((value: string) => {
if (option.valueName && option.type === Number) {
return +value;
} else {
Expand All @@ -97,7 +96,7 @@ function registerCommandOption(

cliCommand.option(
optionUsage,
option.description,
option.description ?? '',
coercedValue,
defaultValue
);
Expand All @@ -107,7 +106,7 @@ export function registerCommand(commandDefinition: CommandDefinition<any>) {
commandDefinitions.push(commandDefinition);

const usage = getCommandDefinitionUsage(commandDefinition);
const cliCommand = cli.command(usage);
const cliCommand = cli.program.command(usage);

if (commandDefinition.description) {
cliCommand.description(commandDefinition.description);
Expand All @@ -119,18 +118,9 @@ export function registerCommand(commandDefinition: CommandDefinition<any>) {
}

cliCommand.action(async (...args: any[]) => {
try {
const commandInstance = instantiateCommand(commandDefinition);
const params = getParams(commandDefinition, args);
await commandInstance.execute(params);
} catch (err) {
// tslint:disable:no-console
console.error();
console.error(chalk.red(errorToString(err)));
console.error();
process.exit(1);
// tslint:enable:no-console
}
const commandInstance = instantiateCommand(commandDefinition);
const params = getParams(commandDefinition, args);
await commandInstance.execute(params);
});
}

Expand Down
58 changes: 31 additions & 27 deletions src/functional.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,20 @@ import * as cli from './index';
import { AuthService } from './testFixtures/services/auth';
import { Logger } from './testFixtures/services/logger';

const noop = () => {
//
};

describe('Functional Tests', () => {
let auth: AuthService;
let logger: Logger;

beforeEach(async () => {
jest.spyOn(console, 'error').mockImplementation(noop);
jest.spyOn(process, 'exit').mockImplementation(noop as () => never);
jest.spyOn(console, 'error').mockImplementation();
jest.spyOn(process, 'exit').mockImplementation();
jest.spyOn(commander.program, 'outputHelp').mockImplementation();

logger = new Logger();
jest.spyOn(logger, 'log');
jest.spyOn(logger, 'error');

auth = new AuthService(new Logger());
auth = new AuthService(logger);
jest.spyOn(auth, 'login');
jest.spyOn(auth, 'logout');

Expand All @@ -29,12 +31,24 @@ describe('Functional Tests', () => {
await cli.commandsFromDirectory(path.join(__dirname, 'testFixtures', 'commands'));
});

afterEach(() => jest.restoreAllMocks());
afterEach(() => {
process.exitCode = 0;
jest.restoreAllMocks();
});

it('should execute a command', async () => {
await cli.execute(['', '', 'login', 'john', 'swordfish', '--rememberMeFor', '30']);
let resolved = false;

const promise = cli
.execute(['', '', 'login', 'john', 'swordfish', '--rememberMeFor', '30'])
.then(() => resolved = true);

expect(auth.login).toHaveBeenCalledWith('john', 'swordfish', 30);
expect(resolved).toBe(false);

await promise;

expect(resolved).toBe(true);
});

it('should execute another command', async () => {
Expand Down Expand Up @@ -76,32 +90,22 @@ describe('Functional Tests', () => {
});

it('should show usage if not supplied any args', async () => {
let modifiedHelp: string | undefined;

const helpCallbackMock = ((cb: (output: string) => string) => {
modifiedHelp = cb('some help');
}) as unknown as ((cb?: ((output: string) => string) | undefined) => never);

jest.spyOn(commander, 'help').mockImplementation(helpCallbackMock);
jest.spyOn(commander.program, 'outputHelp').mockImplementation();

await cli.execute(['', '']);

expect(commander.help).toHaveBeenCalled();

expect(modifiedHelp).toEqual('some help\n');
expect(commander.program.outputHelp).toHaveBeenCalled();
});

it('should show usage if supplied a command that does not exist', () => {
jest.spyOn(console, 'error').mockImplementation(noop);

cli.execute(['', '', 'find-prime-factors', '1290833']);
it('should show usage if supplied a command that does not exist', async () => {
await cli.execute(['', '', 'find-prime-factors', '1290833']);

// tslint:disable-next-line:no-console
expect(console.error).toHaveBeenCalledWith(
'Invalid command: %s\nSee --help for a list of available commands.',
'find-prime-factors 1290833'
);
expect(process.exit).toHaveBeenCalled();
expect(process.exitCode).toBe(1);
});

describe('when not explicitly providing args', () => {
Expand All @@ -122,11 +126,11 @@ describe('Functional Tests', () => {
});
});

it('should log errors and exit', async () => {
await cli.execute(['', '', 'admin']);
it('should log errors and set exit code', async () => {
await expect(() => cli.execute(['', '', 'admin'])).rejects.toThrow('Not authorised');

// tslint:disable-next-line:no-console
expect(console.error).toHaveBeenCalledWith(expect.stringMatching(/Not authorised/i));
expect(process.exit).toHaveBeenCalledWith(1);
expect(process.exitCode).toBe(1);
});
});
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export async function commandsFromDirectory(directoryPath: string): Promise<Comm
return commander.commandsFromDirectory(directoryPath);
}

export function execute(argv?: string[]) {
export async function execute(argv?: string[]) {
return commander.execute(argv);
}

Expand Down
4 changes: 2 additions & 2 deletions src/testFixtures/commands/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export class LoginCommand implements Command<LoginCommandParams> {
constructor(private auth: AuthService) {
}

execute(params: LoginCommandParams) {
this.auth.login(params.username, params.password, params.rememberMeFor);
async execute(params: LoginCommandParams) {
await this.auth.login(params.username, params.password, params.rememberMeFor);
}
}
5 changes: 4 additions & 1 deletion src/testFixtures/services/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ export class AuthService {
constructor(private logger: Logger) {
}

login(username: string, password: string | undefined, keepAliveDurationDays: number) {
async login(username: string, password: string | undefined, keepAliveDurationDays: number) {
// Artificial delay to simulate async operation
await new Promise((resolve) => setTimeout(resolve, 1000));

if (password === 'swordfish' || (username === 'guest' && !password)) {
this.logger.log(`User authenticated`);
return;
Expand Down

0 comments on commit dcf648c

Please sign in to comment.