diff --git a/src/commands/validate.ts b/src/commands/validate.ts index 6519ce6c..2bc6768c 100644 --- a/src/commands/validate.ts +++ b/src/commands/validate.ts @@ -55,7 +55,7 @@ export function getValidationErrors(commandKey: string, commandConfig: any, comm return errors; } -function createValidationCommandSet(commandMaps: Map>) { +function getValdatableCommands(commandMaps: Map>) { let toValidate = new Set(); commandMaps.forEach((commandMap) => { [...commandMap.values()].forEach((command) => { @@ -64,7 +64,7 @@ function createValidationCommandSet(commandMaps: Map { @@ -94,44 +94,47 @@ export function builtInCommandValidation(validation: ValidationWrapper): Promise }); } -function validateCommands(commands: Map>, helper: HelperFactory) { +async function validateCommands( + commands: Map>, + helper: HelperFactory +): Promise { 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; + 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 { +function run(helper: Helper, args: ValidateArgs): Promise { return loadExternalCommands().then((commands) => { const helperContext = {}; const commandHelper = new CommandHelper(commands, helperContext, configurationHelperFactory); @@ -144,7 +147,7 @@ function run(helper: Helper, args: ValidateArgs): Promise { validateHelper ); - validateCommands(commands, helperFactory); + return validateCommands(commands, helperFactory); }); } diff --git a/src/interfaces.d.ts b/src/interfaces.d.ts index 4e3f33f3..748feafc 100644 --- a/src/interfaces.d.ts +++ b/src/interfaces.d.ts @@ -42,7 +42,7 @@ export type ValidationWrapper = { }; export interface ValidateHelper { - validate(validateOpts: ValidationWrapper): Promise; + validate(validateOpts: ValidationWrapper): Promise; } export interface CommandHelper { run(group: string, commandName?: string, args?: Argv): Promise; @@ -103,7 +103,7 @@ export interface Command { register(options: OptionsHelper, helper: Helper): void; run(helper: Helper, args?: T): Promise; eject?(helper: Helper): EjectOutput; - validate?: (helper: Helper) => boolean; + validate?: (helper: Helper) => Promise; name?: string; group?: string; alias?: Alias[] | Alias; diff --git a/src/registerCommands.ts b/src/registerCommands.ts index 93f86edd..454b83fb 100644 --- a/src/registerCommands.ts +++ b/src/registerCommands.ts @@ -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)); @@ -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; } @@ -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({}); @@ -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; } diff --git a/tests/support/testHelper.ts b/tests/support/testHelper.ts index c31f80d3..8f1fec3e 100644 --- a/tests/support/testHelper.ts +++ b/tests/support/testHelper.ts @@ -19,7 +19,7 @@ export interface CommandWrapperConfig { eject?: boolean; installed?: boolean; global?: boolean; - validate?: () => boolean; + validate?: () => Promise; } export function getGroupMap(groupDef: GroupDef, registerMock?: Function, validate?: boolean) { diff --git a/tests/unit/commands/validate.ts b/tests/unit/commands/validate.ts index 6c1ad9ad..a777e135 100644 --- a/tests/unit/commands/validate.ts +++ b/tests/unit/commands/validate.ts @@ -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' @@ -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([ + ['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'); } ); }); @@ -324,7 +362,7 @@ describe('validate', () => { getCommandWrapperWithConfiguration({ group: 'command', name: 'test', - validate: sinon.stub().returns(true) + validate: sinon.stub().resolves(true) }) ], [ @@ -332,7 +370,7 @@ describe('validate', () => { getCommandWrapperWithConfiguration({ group: 'command1', name: 'test1', - validate: sinon.stub().returns(true) + validate: sinon.stub().resolves(true) }) ] ]); diff --git a/tests/unit/registerCommands.ts b/tests/unit/registerCommands.ts index 95c51c06..d7eb3fe1 100644 --- a/tests/unit/registerCommands.ts +++ b/tests/unit/registerCommands.ts @@ -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); } @@ -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'); } } },