Skip to content

Commit

Permalink
Make validate asynchronous (#271)
Browse files Browse the repository at this point in the history 
* Make validate asynchronous

* Add test case for succesful and uncessful validate runs

* Bring test helper in line with other validate interfaces

* Tidy up code, make return value of validate run Promise<boolean>

* Remove impossible branch
  • Loading branch information
@JamesLMilner
JamesLMilner committed Dec 14, 2018
1 parent d1bcd78 commit b8a6866
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 58 deletions.
49 changes: 26 additions & 23 deletions src/commands/validate.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export function getValidationErrors(commandKey: string, commandConfig: any, comm
return errors;
}

function createValidationCommandSet(commandMaps: Map<string, Map<string, CommandWrapper>>) {
function getValdatableCommands(commandMaps: Map<string, Map<string, CommandWrapper>>) {
let toValidate = new Set<CommandWrapper>();
commandMaps.forEach((commandMap) => {
[...commandMap.values()].forEach((command) => {
Expand All @@ -64,7 +64,7 @@ function createValidationCommandSet(commandMaps: Map<string, Map<string, Command
}
});
});
return toValidate;
return [...toValidate];
}

export function builtInCommandValidation(validation: ValidationWrapper): Promise<any> {
Expand Down Expand Up @@ -94,44 +94,47 @@ export function builtInCommandValidation(validation: ValidationWrapper): Promise
});
}

function validateCommands(commands: Map<string, Map<string, CommandWrapper>>, helper: HelperFactory) {
async function validateCommands(
commands: Map<string, Map<string, CommandWrapper>>,
helper: HelperFactory
): Promise<boolean> {
const config = getConfig();

const noConfig = config === undefined;
const emptyConfig = typeof config === 'object' && Object.keys(config).length === 0;
if (noConfig) {
logNoConfig();
return;
return true;
} else if (emptyConfig) {
logEmptyConfig();
return;
return true;
}

const toValidate = createValidationCommandSet(commands);

if (toValidate.size === 0) {
const commandsToValidate = getValdatableCommands(commands);
if (commandsToValidate.length === 0) {
logNoValidatableCommands();
return;
return true;
}

let noMismatches = true;

[...toValidate].forEach((command) => {
try {
const valid = !!command.validate && command.validate(helper.sandbox(command.group, command.name));
noMismatches = valid && noMismatches;
} catch (error) {
const commandValidations = commandsToValidate.map((command) => {
const validate = command.validate as (helper: Helper) => Promise<boolean>;
return validate(helper.sandbox(command.group, command.name)).catch((error) => {
logValidateFunctionFailed(error);
noMismatches = false;
}
return false;
});
});

if (noMismatches) {
logConfigValidateSuccess();
}
// Wait for all validations to resolve and check if all commands are valid
return Promise.all(commandValidations).then((validations) => {
const allValid = validations.every((validation) => validation);
if (allValid) {
logConfigValidateSuccess();
}
return allValid;
});
}

function run(helper: Helper, args: ValidateArgs): Promise<any> {
function run(helper: Helper, args: ValidateArgs): Promise<boolean> {
return loadExternalCommands().then((commands) => {
const helperContext = {};
const commandHelper = new CommandHelper(commands, helperContext, configurationHelperFactory);
Expand All @@ -144,7 +147,7 @@ function run(helper: Helper, args: ValidateArgs): Promise<any> {
validateHelper
);

validateCommands(commands, helperFactory);
return validateCommands(commands, helperFactory);
});
}

Expand Down
4 changes: 2 additions & 2 deletions src/interfaces.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export type ValidationWrapper = {
};

export interface ValidateHelper {
validate(validateOpts: ValidationWrapper): Promise<any>;
validate(validateOpts: ValidationWrapper): Promise<boolean>;
}
export interface CommandHelper {
run(group: string, commandName?: string, args?: Argv): Promise<any>;
Expand Down Expand Up @@ -103,7 +103,7 @@ export interface Command<T = any> {
register(options: OptionsHelper, helper: Helper): void;
run(helper: Helper, args?: T): Promise<any>;
eject?(helper: Helper): EjectOutput;
validate?: (helper: Helper) => boolean;
validate?: (helper: Helper) => Promise<boolean>;
name?: string;
group?: string;
alias?: Alias[] | Alias;
Expand Down
8 changes: 4 additions & 4 deletions src/registerCommands.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ function registerGroups(yargs: Argv, helper: HelperFactory, groupName: string, c
.showHelpOnFail(false, formatHelp({ _: [groupName] }, groupMap))
.strict();
},
(argv: any) => {
async (argv: any) => {
if (defaultCommand && argv._.length === 1) {
if (argv.h || argv.help) {
console.log(formatHelp(argv, groupMap));
Expand All @@ -141,7 +141,7 @@ function registerGroups(yargs: Argv, helper: HelperFactory, groupName: string, c
const args = getOptions(aliases, config, argv);

if (typeof defaultCommand.validate === 'function') {
const valid = defaultCommand.validate(helper.sandbox(groupName, defaultCommand.name));
const valid = await defaultCommand.validate(helper.sandbox(groupName, defaultCommand.name));
if (!valid) {
return;
}
Expand All @@ -168,7 +168,7 @@ function registerCommands(yargs: Argv, helper: HelperFactory, groupName: string,

return optionsYargs.showHelpOnFail(false, formatHelp({ _: [groupName, name] }, groupMap)).strict();
},
(argv: any) => {
async (argv: any) => {
if (argv.h || argv.help) {
console.log(formatHelp(argv, groupMap));
return Promise.resolve({});
Expand All @@ -178,7 +178,7 @@ function registerCommands(yargs: Argv, helper: HelperFactory, groupName: string,
const args = getOptions(aliases, config, argv);

if (typeof command.validate === 'function') {
const valid = command.validate(helper.sandbox(groupName, command.name));
const valid = await command.validate(helper.sandbox(groupName, command.name));
if (!valid) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/support/testHelper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface CommandWrapperConfig {
eject?: boolean;
installed?: boolean;
global?: boolean;
validate?: () => boolean;
validate?: () => Promise<boolean>;
}

export function getGroupMap(groupDef: GroupDef, registerMock?: Function, validate?: boolean) {
Expand Down
50 changes: 44 additions & 6 deletions tests/unit/commands/validate.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ describe('validate', () => {
const installedCommandWrapper = getCommandWrapperWithConfiguration({
group: 'command',
name: 'test',
validate: sinon.stub().throws('A test error')
validate: sinon.stub().rejects('A test error')
});
mockConfigurationHelper.getConfig = sandbox.stub().returns({
foo: 'bar'
Expand All @@ -303,15 +303,53 @@ describe('validate', () => {
mockAllExternalCommands.loadExternalCommands = sandbox.stub().resolves(groupMap);
return moduleUnderTest.run(helper, {}).then(
() => {
assert.isTrue((installedCommandWrapper.validate as sinon.SinonStub).called);
assert.isTrue((installedCommandWrapper.validate as sinon.SinonStub).threw());
assert.isTrue(
(installedCommandWrapper.validate as sinon.SinonStub).called,
'validate should be called'
);
assert.isTrue(consoleLogStub.called, 'error log should be called');
assert.equal(
consoleLogStub.getCall(0).args[0],
red(`The validation function for this command threw an error: A test error`)
);
},
(error: { message: string }) => {
assert.fail(null, null, 'validate should handle error throws gracefully, got error:' + error);
}
);
});

it(`should handle case with successful and failure validate functions gracefully`, () => {
mockConfigurationHelper.getConfig = sandbox.stub().returns({ foo: 'bar' });
const command1 = getCommandWrapperWithConfiguration({
group: 'command1',
name: 'test',
validate: sinon.stub().resolves(true)
});
const command2 = getCommandWrapperWithConfiguration({
group: 'command2',
name: 'test',
validate: sinon.stub().rejects('A test error')
});
const commandMap: CommandMap = new Map<string, CommandWrapper>([
['command1', command1],
['command2', command2]
]);
const groupMap = new Map([['test', commandMap]]);
const helper = getHelper();
mockAllExternalCommands.loadExternalCommands = sandbox.stub().resolves(groupMap);
return moduleUnderTest.run(helper, {}).then(
() => {
assert.isTrue((command1.validate as sinon.SinonStub).called, 'validate should be called');
assert.isTrue((command2.validate as sinon.SinonStub).called, 'validate should be called');
assert.equal(consoleLogStub.callCount, 1, 'error log should be called');
assert.equal(
consoleLogStub.getCall(0).args[0],
red(`The validation function for this command threw an error: A test error`)
);
},
(error: { message: string }) => {
assert.fail(null, null, 'validate should handle error throws gracefully');
assert.fail(null, null, 'no config route should be taken which should be error free');
}
);
});
Expand All @@ -324,15 +362,15 @@ describe('validate', () => {
getCommandWrapperWithConfiguration({
group: 'command',
name: 'test',
validate: sinon.stub().returns(true)
validate: sinon.stub().resolves(true)
})
],
[
'command1',
getCommandWrapperWithConfiguration({
group: 'command1',
name: 'test1',
validate: sinon.stub().returns(true)
validate: sinon.stub().resolves(true)
})
]
]);
Expand Down
42 changes: 20 additions & 22 deletions tests/unit/registerCommands.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,27 +263,27 @@ registerSuite('registerCommands', {
const { register } = groupMap.get('group1').get('command1');
assert.isTrue(register.calledTwice);
},
'Should run default command when yargs called with only group name'() {
async 'Should run default command when yargs called with only group name'() {
const { run } = groupMap.get('group1').get('command1');
yargsStub.command.firstCall.args[3]({ _: ['group'] });
await yargsStub.command.firstCall.args[3]({ _: ['group'] });
assert.isTrue(run.calledOnce);
},
'Should not run default command when yargs called with group name and command'() {
async 'Should not run default command when yargs called with group name and command'() {
const { run } = groupMap.get('group1').get('command1');
yargsStub.command.firstCall.args[3]({ _: ['group', 'command'] });
await yargsStub.command.firstCall.args[3]({ _: ['group', 'command'] });
assert.isFalse(run.calledOnce);
},
'Should run validateable command when yargs called'() {
async 'Should run validateable command when yargs called'() {
const command = groupMap.get('group1').get('command1');
command.validate = sinon.stub().returns(true);
yargsStub.command.firstCall.args[3]({ _: ['group'] });
command.validate = sinon.stub().resolves(true);
await yargsStub.command.firstCall.args[3]({ _: ['group'] });
assert.isTrue(command.validate.calledOnce);
assert.isTrue(command.run.calledOnce);
},
'Should not run validateable command when yargs called with failing command'() {
async 'Should not run validateable command when yargs called with failing command'() {
const command = groupMap.get('group1').get('command1');
command.validate = sinon.stub().returns(false);
yargsStub.command.firstCall.args[3]({ _: ['group'] });
command.validate = sinon.stub().resolves(false);
await yargsStub.command.firstCall.args[3]({ _: ['group'] });
assert.isTrue(command.validate.calledOnce);
assert.isFalse(command.run.called);
}
Expand All @@ -300,27 +300,25 @@ registerSuite('registerCommands', {
},

tests: {
'Should run validateCommand and continue if valid'() {
async 'Should run validateCommand and continue if valid'() {
groupMap = getGroupMap(groupDef, () => () => {}, true);
const command = groupMap.get('group1').get('command1');
command.validate = sinon.stub().returns(true);
command.validate = sinon.stub().resolves(true);
const registerCommands = mockModule.getModuleUnderTest().default;
registerCommands(yargsStub, groupMap);
yargsStub.command.secondCall.args[3]({});
assert.isTrue(command.validate.called);
assert.isTrue(command.validate.returned(true));
assert.isTrue(command.run.calledOnce);
await yargsStub.command.secondCall.args[3]({});
assert.isTrue(command.validate.called, 'validate was not called');
assert.isTrue(command.run.calledOnce, 'run wasnt called');
},
'Should run validateCommand stop if invalid'() {
async 'Should run validateCommand stop if invalid'() {
groupMap = getGroupMap(groupDef, () => () => {}, true);
const command = groupMap.get('group1').get('command1');
command.validate = sinon.stub().returns(false);
command.validate = sinon.stub().resolves(false);
const registerCommands = mockModule.getModuleUnderTest().default;
registerCommands(yargsStub, groupMap);
yargsStub.command.secondCall.args[3]({});
assert.isTrue(command.validate.called);
assert.isTrue(command.validate.returned(false));
assert.isFalse(command.run.calledOnce);
await yargsStub.command.secondCall.args[3]({});
assert.isTrue(command.validate.called, 'validate was not called');
assert.isFalse(command.run.calledOnce, 'run was called when it shouldnt have been');
}
}
},
Expand Down

0 comments on commit b8a6866

Please sign in to comment.