Skip to content

Commit

Permalink
feat: basic implementation of command line config option (#1080)
Browse files Browse the repository at this point in the history
  • Loading branch information
saintsebastian authored and kumar303 committed Dec 13, 2017
1 parent 92ef317 commit 8a8ee94
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 39 deletions.
3 changes: 2 additions & 1 deletion src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export function applyConfigToArgv({
`an unknown option: "${option}"`);
}
if (options[decamelizedOptName].type === undefined) {
// This means yargs option type wasn't not defined correctly
throw new WebExtError(
`Option: ${option} was defined without a type.`);
}
Expand Down Expand Up @@ -109,4 +110,4 @@ export function loadJSConfigFile(filePath: string): Object {
'Did you set module.exports = {...}?');
}
return configObject;
}
}
35 changes: 32 additions & 3 deletions src/program.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import {UsageError} from './errors';
import {createLogger, consoleStream as defaultLogStream} from './util/logger';
import {coerceCLICustomPreference} from './firefox/preferences';
import {checkForUpdates as defaultUpdateChecker} from './util/updates';
import {
loadJSConfigFile as defaultLoadJSConfigFile,
applyConfigToArgv as defaultApplyConfigToArgv,
} from './config';

const log = createLogger(__filename);
const envPrefix = 'WEB_EXT';
Expand All @@ -20,13 +24,17 @@ type ProgramOptions = {|
absolutePackageDir?: string,
|}

export type VersionGetterFn = (absolutePackageDir: string) => string;

// TODO: add pipes to Flow type after https://github.com/facebook/flow/issues/2405 is fixed

type ExecuteOptions = {
checkForUpdates?: Function,
systemProcess?: typeof process,
logStream?: typeof defaultLogStream,
getVersion?: Function,
getVersion?: VersionGetterFn,
applyConfigToArgv?: typeof defaultApplyConfigToArgv,
loadJSConfigFile?: typeof defaultLoadJSConfigFile,
shouldExitProgram?: boolean,
globalEnv?: string,
}
Expand Down Expand Up @@ -115,6 +123,8 @@ export class Program {
{
checkForUpdates = defaultUpdateChecker, systemProcess = process,
logStream = defaultLogStream, getVersion = defaultVersionGetter,
applyConfigToArgv = defaultApplyConfigToArgv,
loadJSConfigFile = defaultLoadJSConfigFile,
shouldExitProgram = true, globalEnv = WEBEXT_BUILD_ENV,
}: ExecuteOptions = {}
): Promise<void> {
Expand Down Expand Up @@ -145,7 +155,19 @@ export class Program {
});
}

await runCommand(argv, {shouldExitProgram});
let argvFromConfig = { ...argv };
if (argv.config) {
const configFileName = path.resolve(argv.config);
const configObject = loadJSConfigFile(configFileName);
argvFromConfig = applyConfigToArgv({
argv,
configFileName,
configObject,
options: this.options,
});
}

await runCommand(argvFromConfig, {shouldExitProgram});

} catch (error) {
if (!(error instanceof UsageError) || argv.verbose) {
Expand Down Expand Up @@ -194,7 +216,7 @@ export function defaultVersionGetter(
// TODO: add pipes to Flow type after https://github.com/facebook/flow/issues/2405 is fixed

type MainParams = {
getVersion?: Function,
getVersion?: VersionGetterFn,
commands?: Object,
argv: Array<any>,
runOptions?: Object,
Expand Down Expand Up @@ -265,6 +287,13 @@ Example: $0 --help run.
describe: 'Disable all features that require standard input',
type: 'boolean',
},
'config': {
alias: 'c',
describe: 'Path to the config file',
default: undefined,
requiresArg: true,
type: 'string',
},
});

program
Expand Down
31 changes: 0 additions & 31 deletions tests/unit/test.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -661,37 +661,6 @@ describe('config', () => {
'WebExtError: Option: apiUrl was defined without a type.');
});

it('throws an error when the type of one of them is in config' +
' missing', () => {
const params = makeArgv({
userCmd: ['sign'],
command: 'sign',
commandOpt: {
'api-url': {
requiresArg: true,
demand: false,
default: 'pretend-default-value-of-apiKey',
},
'api-key': {
requiresArg: true,
demand: false,
type: 'string',
default: 'pretend-default-value-of-apiKey',
},
},
});
const configObject = {
sign: {
apiUrl: 2,
apiKey: 'fake-api-key',
},
};
assert.throws(() => {
applyConf({...params, configObject});
}, WebExtError,
'WebExtError: Option: apiUrl was defined without a type.');
});

it('throws an error when type of unrelated sub option is invalid', () => {
const program = new Program(['run']);

Expand Down
87 changes: 83 additions & 4 deletions tests/unit/test.program.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,20 @@ import {assert} from 'chai';

import {defaultVersionGetter, main, Program} from '../../src/program';
import commands from '../../src/cmd';
import {onlyInstancesOf, UsageError} from '../../src/errors';
import {
onlyInstancesOf,
UsageError,
} from '../../src/errors';
import {
createFakeProcess,
fake,
makeSureItFails,
ErrorWithCode,
} from './helpers';
import {ConsoleStream} from '../../src/util/logger';

import {
consoleStream, // instance is imported to inspect logged messages
ConsoleStream,
} from '../../src/util/logger';

describe('program.Program', () => {

Expand All @@ -26,7 +31,7 @@ describe('program.Program', () => {
const absolutePackageDir = path.join(__dirname, '..', '..');
return program.execute(
absolutePackageDir, {
getVersion: () => spy(),
getVersion: () => 'not-a-real-version',
checkForUpdates: spy(),
systemProcess: fakeProcess,
shouldExitProgram: false,
Expand Down Expand Up @@ -218,6 +223,33 @@ describe('program.Program', () => {
});
});

it('logs UsageErrors into console', () => {
// Clear console stream from previous messages and start recording
consoleStream.stopCapturing();
consoleStream.flushCapturedLogs();
consoleStream.startCapturing();

const program = new Program(['thing']).command('thing', '', () => {
throw new UsageError('some error');
});
program.setGlobalOptions({
verbose: {
type: 'boolean',
},
});
return execProgram(program)
.then(makeSureItFails())
.catch(onlyInstancesOf(UsageError, (error) => {
const {capturedMessages} = consoleStream;
// Stop recording
consoleStream.stopCapturing();
assert.match(error.message, /some error/);
assert.ok(
capturedMessages.some((message) => message.match(/some error/)
));
}));
});

it('throws an error about unknown commands', () => {
return execProgram(new Program(['nope']))
.then(makeSureItFails())
Expand Down Expand Up @@ -435,6 +467,53 @@ describe('program.main', () => {
assert.strictEqual(options.shouldExitProgram, false);
});
});

it('applies options from the specified config file', () => {
const fakeCommands = fake(commands, {
lint: () => Promise.resolve(),
});
const fakePath = 'path/to/web-ext-config.js';
const configObject = {
prop: 'prop',
};
const resolvedFakePath = path.resolve(fakePath);
const expectedArgv = {
_: ['lint'],
config: fakePath,
};
const fakeLoadJSConfigFile = sinon.spy(() => {
return configObject;
});
const fakeApplyConfigToArgv = sinon.spy(() => {
return expectedArgv;
});

return execProgram(
['lint', '--config', fakePath],
{
commands: fakeCommands,
runOptions: {
applyConfigToArgv: fakeApplyConfigToArgv,
loadJSConfigFile: fakeLoadJSConfigFile,
},
})
.then(() => {
const options = fakeCommands.lint.firstCall.args[0];
assert.strictEqual(options.config, fakePath);
sinon.assert.calledOnce(fakeLoadJSConfigFile);
sinon.assert.calledWith(fakeLoadJSConfigFile,
sinon.match(resolvedFakePath));
sinon.assert.calledOnce(fakeApplyConfigToArgv);
sinon.assert.calledWith(fakeApplyConfigToArgv, sinon.match({
configFileName: resolvedFakePath,
configObject,
argv: {
_: ['lint'],
config: fakePath,
},
}));
});
});
});

describe('program.defaultVersionGetter', () => {
Expand Down

0 comments on commit 8a8ee94

Please sign in to comment.