Skip to content

Commit

Permalink
fix(#524): Don't display usage() information for commands which run s…
Browse files Browse the repository at this point in the history 
…uccessfully

fix(#524): Don't display usage() information for commands which run successfully
  • Loading branch information
@kennsippell
kennsippell authored Mar 18, 2023
1 parent 4070185 commit 5b265e7
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 81 deletions.
14 changes: 6 additions & 8 deletions src/lib/get-api-url.js
View file
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
const userPrompt = require('./user-prompt');
const url = require('url');
const usage = require('../cli/usage');

const emoji = require('./emoji');
const { error, info } = require('./log');
const { info } = require('./log');
const usage = require('../cli/usage');

const getApiUrl = (cmdArgs, env = {}) => {
const specifiedModes = [cmdArgs.local, cmdArgs.instance, cmdArgs.url, cmdArgs.archive].filter(mode => mode);
if (specifiedModes.length !== 1) {
error('Require exactly one of these parameter: --local --instance --url --archive');
usage();
return false;
throw Error('One of these parameters is required: --local --instance --url --archive');
}

if (cmdArgs.user && !cmdArgs.instance) {
error('The --user switch can only be used if followed by --instance');
return false;
usage();
throw Error('The --user switch must be accompanied with --instance');
}

let instanceUrl;
Expand All @@ -26,8 +25,7 @@ const getApiUrl = (cmdArgs, env = {}) => {
// See ./archiving-db.js
instanceUrl = parseLocalUrl(env.COUCH_URL);
if (instanceUrl.hostname !== 'localhost') {
error(`You asked to configure localhost, but the COUCH_URL env var is set to '${instanceUrl.hostname}'. This may be a remote server.`);
return false;
throw Error(`--local was specified but COUCH_URL env var is set to '${instanceUrl.hostname}'. Please use --url for remote servers.`);
}
} else if (cmdArgs.instance) {
const password = userPrompt.question(`${emoji.key} Password: `, { hideEchoBack: true });
Expand Down
114 changes: 57 additions & 57 deletions src/lib/main.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,71 +134,16 @@ module.exports = async (argv, env) => {
//
// Build up actions
//
let actions = cmdArgs._;
if (actions.length) {
const unsupported = actions.filter(a => !supportedActions.includes(a));
if(unsupported.length) {
throw new Error(`Unsupported action(s): ${unsupported.join(' ')}`);
}
} else {
actions = !cmdArgs.archive ? defaultActions : defaultArchiveActions;
}

if (cmdArgs['skip-git-check']) {
actions = actions.filter(a => a !== 'check-git');
}

const skipValidate = cmdArgs['skip-validate'];
if(skipValidate) {
warn('Skipping all form validation.');
const validateActions = [
'validate-app-forms',
'validate-collect-forms',
'validate-contact-forms',
'validate-training-forms',
];
actions = actions.filter(action => !validateActions.includes(action));
} else {
const addFormValidationIfNecessary = (formType) => {
const updateFormsIndex = actions.indexOf(`upload-${formType}-forms`);
if (updateFormsIndex >= 0 && actions.indexOf(`validate-${formType}-forms`) < 0) {
actions.splice(updateFormsIndex, 0, `validate-${formType}-forms`);
}
};
addFormValidationIfNecessary('app');
addFormValidationIfNecessary('collect');
addFormValidationIfNecessary('contact');
addFormValidationIfNecessary('training');
}

actions = actions.map(actionName => {
const action = require(`../fn/${actionName}`);

if (typeof action.execute !== 'function') {
throw new Error(`${actionName} has not been implemented correctly: no 'execute' function`);
}

if (!Object.hasOwnProperty.call(action, 'requiresInstance')) {
action.requiresInstance = true;
}

action.name = actionName;

return action;
});
const actions = buildActions(cmdArgs, skipValidate);

//
// Initialize the environment
//
const projectName = fs.path.basename(pathToProject);

const apiUrl = getApiUrl(cmdArgs, env);
const requiresInstance = actions.some(action => action.requiresInstance);
if (requiresInstance) {
if (!apiUrl) {
throw new Error('Failed to obtain a url to the API');
}
}
const apiUrl = requiresInstance && getApiUrl(cmdArgs, env);

let extraArgs = cmdArgs['--'];
if (!extraArgs.length) {
Expand Down Expand Up @@ -252,5 +197,60 @@ module.exports = async (argv, env) => {
}
};

function buildActions(cmdArgs, skipValidate) {
let actions = cmdArgs._;
if (actions.length) {
const unsupported = actions.filter(a => !supportedActions.includes(a));
if (unsupported.length) {
usage();
throw new Error(`Unsupported action(s): ${unsupported.join(' ')}`);
}
} else {
actions = !cmdArgs.archive ? defaultActions : defaultArchiveActions;
}

if (cmdArgs['skip-git-check']) {
actions = actions.filter(a => a !== 'check-git');
}

if(skipValidate) {
warn('Skipping all form validation.');
const validateActions = [
'validate-app-forms',
'validate-collect-forms',
'validate-contact-forms',
'validate-training-forms',
];
actions = actions.filter(action => !validateActions.includes(action));
} else {
const addFormValidationIfNecessary = (formType) => {
const updateFormsIndex = actions.indexOf(`upload-${formType}-forms`);
if (updateFormsIndex >= 0 && actions.indexOf(`validate-${formType}-forms`) < 0) {
actions.splice(updateFormsIndex, 0, `validate-${formType}-forms`);
}
};
addFormValidationIfNecessary('app');
addFormValidationIfNecessary('collect');
addFormValidationIfNecessary('contact');
addFormValidationIfNecessary('training');
}

return actions.map(actionName => {
const action = require(`../fn/${actionName}`);

if (typeof action.execute !== 'function') {
throw new Error(`${actionName} has not been implemented correctly: no 'execute' function`);
}

if (!Object.hasOwnProperty.call(action, 'requiresInstance')) {
action.requiresInstance = true;
}

action.name = actionName;

return action;
});
}

// Exists for generic mocking purposes
const executeAction = action => action.execute();
19 changes: 7 additions & 12 deletions test/lib/get-api-url.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,24 @@ const getApiUrl = rewire('../../src/lib/get-api-url');
const userPrompt = rewire('../../src/lib/user-prompt');

describe('get-api-url', () => {
let error, readline;
let readline;
beforeEach(() => {
error = sinon.stub();
readline = {
question: sinon.stub().throws('unexpected'),
keyInYN: sinon.stub().throws('unexpected'),
};
userPrompt.__set__('readline', readline);
getApiUrl.__set__('error', error);
getApiUrl.__set__('userPrompt', userPrompt);
});

it('multiple destinations yields error', () => {
const actual = getApiUrl({ local: true, instance: 'demo' });
expect(error.args[0][0]).to.include('one of these');
expect(actual).to.eq(false);
const actual = () => getApiUrl({ local: true, instance: 'demo' });
expect(actual).to.throw('One of these');
});

it('no destination yields error', () => {
const actual = getApiUrl({});
expect(error.args[0][0]).to.include('one of these');
expect(actual).to.eq(false);
const actual = () => getApiUrl({});
expect(actual).to.throw('One of these');
});

describe('--local', () => {
Expand All @@ -42,9 +38,8 @@ describe('get-api-url', () => {
});

it('warn if environment variable targets remote', () => {
const actual = getApiUrl({ local: true }, { COUCH_URL: 'http://user:pwd@remote:5984/db' });
expect(error.args[0][0]).to.include('remote');
expect(actual).to.eq(false);
const actual = () => getApiUrl({ local: true }, { COUCH_URL: 'http://user:pwd@remote:5984/db' });
expect(actual).to.throw('remote');
});
});

Expand Down
14 changes: 10 additions & 4 deletions test/lib/main.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,10 @@ describe('main', () => {
await main([...normalArgv, 'initialise-project-layout'], {});
expect(mocks.executeAction.callCount).to.deep.eq(1);
expect(mocks.executeAction.args[0][0].name).to.eq('initialise-project-layout');
expect(mocks.usage.called).to.be.false;
});

const expectExecuteActionBehavior = (expectedActions, expectedExtraParams) => {
const expectExecuteActionBehavior = (expectedActions, expectedExtraParams, expectRequireUrl = true) => {
if (Array.isArray(expectedActions)) {
expect(mocks.executeAction.args.map(args => args[0].name)).to.deep.eq(expectedActions);
} else {
Expand All @@ -117,12 +118,15 @@ describe('main', () => {

expect(mocks.environment.initialize.args[0][3]).to.deep.eq(expectedExtraParams);

expect(mocks.environment.initialize.args[0][4]).to.eq('http://api');
if (expectRequireUrl) {
expect(mocks.environment.initialize.args[0][4]).to.eq('http://api');
}
};

it('--local no COUCH_URL', async () => {
await main([...normalArgv, '--local'], {});
expectExecuteActionBehavior(defaultActions, undefined);
expect(mocks.usage.called).to.be.false;
});

it('--local with COUCH_URL to localhost', async () => {
Expand All @@ -134,15 +138,16 @@ describe('main', () => {
it('--instance + 2 ordered actions', async () => {
await main([...normalArgv, '--instance=test.app', 'convert-app-forms', 'compile-app-settings'], {});
expect(mocks.executeAction.callCount).to.deep.eq(2);
expectExecuteActionBehavior('convert-app-forms', undefined);
expectExecuteActionBehavior('convert-app-forms', undefined, false);
expect(mocks.executeAction.args[1][0].name).to.eq('compile-app-settings');
});

it('convert one form', async () => {
const formName = 'form-name';
await main([...normalArgv, '--local', 'convert-app-forms', '--', formName], {});
expect(mocks.executeAction.callCount).to.deep.eq(1);
expectExecuteActionBehavior('convert-app-forms', [formName]);
expectExecuteActionBehavior('convert-app-forms', [formName], false);
expect(mocks.usage.called).to.be.false;
});

it('unsupported action', async () => {
Expand All @@ -151,6 +156,7 @@ describe('main', () => {
expect.fail('Expected error to be thrown');
} catch(e) {
expect(mocks.executeAction.called).to.be.false;
expect(mocks.usage.calledOnce).to.be.true;
expect(e.message).to.equal('Unsupported action(s): not-an-action');
}
});
Expand Down

0 comments on commit 5b265e7

Please sign in to comment.