Skip to content

Commit

Permalink
Reduce complexity of src/cli/index.js (#2887)
Browse files Browse the repository at this point in the history 
* clarify initial part of src/cli/index.js when we extracting arguments; flags and arguments after --

* change test if we do not provide any command name and specify a flag because the previous one was flaky

* fix silent error in test when we expect an error from a yarn command

* enrich commands with aliases

* simplify if statement: if no command we set install as default

* simplify if statement: we always have command with value undefined

* put every logic related to help in help command

* use deconstructuring instead of concat and some shift/unshit command

* remove useless invariant on commandName: we are sure that is always defined

* if command is not recognized set default to run; if command is run we set commandName as the first arg for npm_config_argv

* add some tests cases when we do not recognize command

* we use commander.js only to parse flags, remove every logic to put commandName and place only a placeholder

* implement hasWrapper function for help command

* display correct help link for aliases

* display documentation link correctly in every case

* add some console.asserts to ensure that we early crash if something is not expected

* fix erronous case: yarn constructor
  • Loading branch information
@voxsim @arcanis
voxsim authored and arcanis committed Apr 10, 2017
1 parent cd58df8 commit 45af656
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 112 deletions.
79 changes: 69 additions & 10 deletions __tests__/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ Promise<Array<?string>> {
}

return new Promise((resolve, reject) => {
exec(`node "${yarnBin}" ${cmd} ${args.join(' ')}`, {cwd:workingDir, env:process.env}, (err, stdout) => {
if (err) {
reject(err);
exec(`node "${yarnBin}" ${cmd} ${args.join(' ')}`, {cwd:workingDir, env:process.env}, (error, stdout) => {
if (error) {
reject({error, stdout});
} else {
const stdoutLines = stdout.toString()
.split('\n')
Expand Down Expand Up @@ -76,13 +76,23 @@ function expectHelpOutputAsSubcommand(stdout) {
}

function expectAnErrorMessage(command: Promise<Array<?string>>, error: string) : Promise<void> {
return command.catch((reason) =>
expect(reason.message).toContain(error),
return command
.then(function() {
throw new Error('the command did not fail');
})
.catch((reason) =>
expect(reason.error.message).toContain(error),
);
}

function expectInstallOutput(stdout) {
expect(stdout[0]).toEqual(`yarn install v${pkg.version}`);
function expectAnInfoMessageAfterError(command: Promise<Array<?string>>, info: string) : Promise<void> {
return command
.then(function() {
throw new Error('the command did not fail');
})
.catch((reason) =>
expect(reason.stdout).toContain(info),
);
}

test.concurrent('should add package', async () => {
Expand Down Expand Up @@ -182,12 +192,12 @@ test.concurrent('should run --version command', async () => {

test.concurrent('should install if no args', async () => {
const stdout = await execCommand('', [], 'run-add', true);
expectInstallOutput(stdout);
expect(stdout[0]).toEqual(`yarn install v${pkg.version}`);
});

test.concurrent('should install if first arg looks like a flag', async () => {
const stdout = await execCommand('--offline', [], 'run-add', true);
expectInstallOutput(stdout);
const stdout = await execCommand('--json', [], 'run-add', true);
expect(stdout[stdout.length - 1]).toEqual('{"type":"success","data":"Saved lockfile."}');
});

test.concurrent('should interpolate aliases', async () => {
Expand All @@ -197,6 +207,13 @@ test.concurrent('should interpolate aliases', async () => {
);
});

test.concurrent('should display correct documentation link for aliases', async () => {
await expectAnInfoMessageAfterError(
execCommand('i', [], 'run-add', true),
'Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.',
);
});

test.concurrent('should run help of run command if --help is before --', async () => {
const stdout = await execCommand('run', ['custom-script', '--help', '--'], 'run-custom-script-with-arguments');
expect(stdout[0]).toEqual('Usage: yarn [command] [flags]');
Expand All @@ -216,3 +233,45 @@ test.concurrent('should run bin command', async () => {
expect(stdout[0]).toEqual(path.join(fixturesLoc, 'node_modules', '.bin'));
expect(stdout.length).toEqual(1);
});

test.concurrent('should throws missing command for not camelised command', async () => {
await expectAnErrorMessage(
execCommand('HelP', [], 'run-add', true),
'Command \"HelP\" not found',
);
});

test.concurrent('should throws missing command for not alphabetic command', async () => {
await expectAnErrorMessage(
execCommand('123', [], 'run-add', true),
'Command \"123\" not found',
);
});

test.concurrent('should throws missing command for unknown command', async () => {
await expectAnErrorMessage(
execCommand('unknown', [], 'run-add', true),
'Command \"unknown\" not found',
);
});

test.concurrent('should not display documentation link for unknown command', async () => {
await expectAnInfoMessageAfterError(
execCommand('unknown', [], 'run-add', true),
'',
);
});

test.concurrent('should display documentation link for known command', async () => {
await expectAnInfoMessageAfterError(
execCommand('add', [], 'run-add', true),
'Visit https://yarnpkg.com/en/docs/cli/add for documentation about this command.',
);
});

test.concurrent('should throws missing command for constructor command', async () => {
await expectAnErrorMessage(
execCommand('constructor', [], 'run-add', true),
'Command \"constructor\" not found',
);
});
3 changes: 3 additions & 0 deletions __tests__/lifecycle-scripts.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ async () => {
stdout = await execCommand('', 'npm_config_argv_env_vars', env);
expect(stdout).toContain('##install##');

stdout = await execCommand('run test', 'npm_config_argv_env_vars', env);
expect(stdout).toContain('##test##');

stdout = await execCommand('test', 'npm_config_argv_env_vars', env);
expect(stdout).toContain('##test##');
});
Expand Down
57 changes: 42 additions & 15 deletions src/cli/commands/help.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,13 @@ import * as commands from './index.js';
import * as constants from '../../constants.js';
import type {Reporter} from '../../reporters/index.js';
import type Config from '../../config.js';
import {sortAlpha, hyphenate} from '../../util/misc.js';
import {sortAlpha, hyphenate, camelCase} from '../../util/misc.js';
const chalk = require('chalk');

export function hasWrapper(): boolean {
return false;
}

export function run(
config: Config,
reporter: Reporter,
Expand All @@ -17,24 +21,47 @@ export function run(
const getDocsInfo = (name) => 'Visit ' + chalk.bold(getDocsLink(name)) + ' for documentation about this command.';

if (args.length) {
const helpCommand = hyphenate(args[0]);
if (commands[helpCommand]) {
commander.on('--help', () => console.log(' ' + getDocsInfo(helpCommand) + '\n'));
}
} else {
commander.on('--help', () => {
console.log(' Commands:\n');
for (const name of Object.keys(commands).sort(sortAlpha)) {
if (commands[name].useless) {
continue;
const commandName = camelCase(args.shift());

if (commandName) {
const command = commands[commandName];

if (command) {
if (typeof command.setFlags === 'function') {
command.setFlags(commander);
}

const examples: Array<string> = (command && command.examples) || [];
if (examples.length) {
commander.on('--help', () => {
console.log(' Examples:\n');
for (const example of examples) {
console.log(` $ yarn ${example}`);
}
console.log();
});
}
commander.on('--help', () => console.log(' ' + getDocsInfo(commandName) + '\n'));

console.log(` - ${hyphenate(name)}`);
commander.help();
return Promise.resolve();
}
console.log('\n Run `' + chalk.bold('yarn help COMMAND') + '` for more information on specific commands.');
console.log(' Visit ' + chalk.bold(getDocsLink()) + ' to learn more about Yarn.\n');
});
}
}

commander.on('--help', () => {
console.log(' Commands:\n');
for (const name of Object.keys(commands).sort(sortAlpha)) {
if (commands[name].useless) {
continue;
}

console.log(` - ${hyphenate(name)}`);
}
console.log('\n Run `' + chalk.bold('yarn help COMMAND') + '` for more information on specific commands.');
console.log(' Visit ' + chalk.bold(getDocsLink()) + ' to learn more about Yarn.\n');
});

commander.help();
return Promise.resolve();
}
Loading

0 comments on commit 45af656

Please sign in to comment.