From 53bd968cec303bbbd056b3f2fd7a135995b03a03 Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Wed, 1 Feb 2017 23:31:25 +0100 Subject: [PATCH 1/2] add tests to cover almost every cli parsing case and add docs info for `command --help` --- .../package.json | 8 + .../fixtures/index/run-version/package.json | 5 + __tests__/index.js | 150 ++++++++++++++---- src/cli/index.js | 3 + 4 files changed, 139 insertions(+), 27 deletions(-) create mode 100644 __tests__/fixtures/index/run-custom-script-with-arguments/package.json create mode 100644 __tests__/fixtures/index/run-version/package.json diff --git a/__tests__/fixtures/index/run-custom-script-with-arguments/package.json b/__tests__/fixtures/index/run-custom-script-with-arguments/package.json new file mode 100644 index 0000000000..b0d3da8bb5 --- /dev/null +++ b/__tests__/fixtures/index/run-custom-script-with-arguments/package.json @@ -0,0 +1,8 @@ +{ + "name": "test_run_custom_script", + "version": "1.0.0", + "license": "UNLICENSED", + "scripts": { + "custom-script": "print() { echo \"A message from custom script with args \"$@\"\"; }; print " + } +} diff --git a/__tests__/fixtures/index/run-version/package.json b/__tests__/fixtures/index/run-version/package.json new file mode 100644 index 0000000000..501b0f2ddf --- /dev/null +++ b/__tests__/fixtures/index/run-version/package.json @@ -0,0 +1,5 @@ +{ + "name": "test_add", + "version": "1.0.0", + "license": "UNLICENSED" +} diff --git a/__tests__/index.js b/__tests__/index.js index e2e96907d8..37177494a4 100644 --- a/__tests__/index.js +++ b/__tests__/index.js @@ -3,6 +3,7 @@ import NoopReporter from '../src/reporters/base-reporter.js'; import makeTemp from './_temp'; import * as fs from '../src/util/fs.js'; +const pkg = require('../package.json'); const path = require('path'); const exec = require('child_process').exec; @@ -37,40 +38,68 @@ Promise> { }); } -test.concurrent('should add package', async () => { - const stdout = await execCommand('add', ['left-pad'], 'run-add', true); +function expectAddSuccessfullOutput(stdout, pkg) { const lastLines = stdout.slice(stdout.length - 4); expect(lastLines[0]).toEqual('success Saved lockfile.'); expect(lastLines[1]).toEqual('success Saved 1 new dependency.'); - expect(lastLines[2]).toMatch(/left-pad/); - expect(lastLines[3]).toMatch(/^Done/); + expect(lastLines[2]).toContain(pkg); + expect(lastLines[3]).toContain('Done'); +} + +function expectAddSuccessfullOutputWithNoLockFile(stdout, pkg) { + const lastLines = stdout.slice(stdout.length - 4); + expect(lastLines[1]).toEqual('success Saved 1 new dependency.'); + expect(lastLines[2]).toContain(pkg); + expect(lastLines[3]).toContain('Done'); +} + +function expectRunOutput(stdout) { + const lastLines = stdout.slice(stdout.length - 2); + expect(lastLines[0]).toMatch(/A message from custom script/); + expect(lastLines[1]).toMatch(/^Done/); +} + +function expectHelpOutput(stdout) { + expect(stdout[0]).toEqual('Usage: yarn [command] [flags]'); + const lastLines = stdout.slice(stdout.length - 2); + expect(lastLines[0]).toEqual('Run `yarn help COMMAND` for more information on specific commands.'); + expect(lastLines[1]).toEqual('Visit https://yarnpkg.com/en/docs/cli/ to learn more about Yarn.'); +} + +function expectHelpOutputAsSubcommand(stdout) { + expect(stdout[0]).toEqual('Usage: yarn add [packages ...] [flags]'); + expect(stdout[stdout.length - 1]) + .toEqual('Visit https://yarnpkg.com/en/docs/cli/add for documentation about this command.'); +} + +function expectAnErrorMessage(command: Promise>, error: string) : Promise { + return command.catch((reason) => + expect(reason.message).toContain(error), + ); +} + +function expectInstallOutput(stdout) { + expect(stdout[0]).toEqual(`yarn install v${pkg.version}`); +} + +test.concurrent('should add package', async () => { + const stdout = await execCommand('add', ['left-pad'], 'run-add', true); + expectAddSuccessfullOutput(stdout, 'left-pad'); }); test.concurrent('should add package with no-lockfile option', async () => { const stdout = await execCommand('add', ['repeating', '--no-lockfile'], 'run-add-option', true); - const lastLines = stdout.slice(stdout.length - 4); - expect(lastLines[0]).not.toMatch(/Saved lockfile/); - expect(lastLines[1]).toEqual('success Saved 1 new dependency.'); - expect(lastLines[2]).toMatch(/repeating/); - expect(lastLines[3]).toMatch(/^Done/); + expectAddSuccessfullOutputWithNoLockFile(stdout, 'repeating'); }); test.concurrent('should add package with no-lockfile option in front', async () => { const stdout = await execCommand('add', ['--no-lockfile', 'split-lines'], 'run-add-option-in-front', true); - const lastLines = stdout.slice(stdout.length - 4); - expect(lastLines[0]).not.toMatch(/Saved lockfile/); - expect(lastLines[1]).toEqual('success Saved 1 new dependency.'); - expect(lastLines[2]).toMatch(/split-lines/); - expect(lastLines[3]).toMatch(/^Done/); + expectAddSuccessfullOutputWithNoLockFile(stdout, 'split-lines'); }); test.concurrent('should add lockfile package', async () => { const stdout = await execCommand('add', ['lockfile'], 'run-add-lockfile', true); - const lastLines = stdout.slice(stdout.length - 4); - expect(lastLines[0]).toEqual('success Saved lockfile.'); - expect(lastLines[1]).toEqual('success Saved 1 new dependency.'); - expect(lastLines[2]).toMatch(/lockfile/); - expect(lastLines[3]).toMatch(/^Done/); + expectAddSuccessfullOutput(stdout, 'lockfile'); }); test.concurrent('should add progress package globally', async () => { @@ -85,24 +114,91 @@ test.concurrent('should add progress package globally', async () => { test.concurrent('should run custom script', async () => { const stdout = await execCommand('run', ['custom-script'], 'run-custom-script'); - const lastLines = stdout.slice(stdout.length - 2); - expect(lastLines[0]).toMatch(/A message from custom script/); - expect(lastLines[1]).toMatch(/^Done/); + expectRunOutput(stdout); }); test.concurrent('should run custom script without run command', async () => { const stdout = await execCommand('custom-script', [], 'run-custom-script'); - const lastLines = stdout.slice(stdout.length - 2); - expect(lastLines[0]).toMatch(/A message from custom script/); - expect(lastLines[1]).toMatch(/^Done/); + expectRunOutput(stdout); }); test.concurrent('should run help command', async () => { const stdout = await execCommand('help', [], 'run-help'); - expect(stdout[0]).toEqual('Usage: yarn [command] [flags]'); + expectHelpOutput(stdout); +}); + +test.concurrent('should run help command with --help', async () => { + const stdout = await execCommand('--help', [], 'run-help'); + expectHelpOutput(stdout); +}); + +test.concurrent('should run help command with -h', async () => { + const stdout = await execCommand('-h', [], 'run-help'); + expectHelpOutput(stdout); }); test.concurrent('should run add command with help option', async () => { const stdout = await execCommand('add', ['--help'], 'run-help'); - expect(stdout[0]).toEqual('Usage: yarn add [packages ...] [flags]'); + expectHelpOutputAsSubcommand(stdout); +}); + +test.concurrent('should run add command with h option', async () => { + const stdout = await execCommand('add', ['-h'], 'run-help'); + expectHelpOutputAsSubcommand(stdout); +}); + +test.concurrent('should run help command with add option', async () => { + const stdout = await execCommand('help', ['add'], 'run-help'); + expectHelpOutputAsSubcommand(stdout); +}); + +test.concurrent('should run --help command with add option', async () => { + const stdout = await execCommand('--help', ['add'], 'run-help'); + expectHelpOutputAsSubcommand(stdout); +}); + +test.concurrent('should run -h command with add option', async () => { + const stdout = await execCommand('-h', ['add'], 'run-help'); + expectHelpOutputAsSubcommand(stdout); +}); + +test.concurrent('should run version command', async () => { + await expectAnErrorMessage( + execCommand('version', [], 'run-version'), + 'Can\'t answer a question unless a user TTY', + ); +}); + +test.concurrent('should run --version command', async () => { + const stdout = await execCommand('--version', [], 'run-version'); + expect(stdout[0]).toEqual(pkg.version); +}); + +test.concurrent('should install if no args', async () => { + const stdout = await execCommand('', [], 'run-add', true); + expectInstallOutput(stdout); +}); + +test.concurrent('should install if first arg looks like a flag', async () => { + const stdout = await execCommand('--offline', [], 'run-add', true); + expectInstallOutput(stdout); +}); + +test.concurrent('should interpolate aliases', async () => { + await expectAnErrorMessage( + execCommand('i', [], 'run-add', true), + 'Did you mean `yarn install`?', + ); +}); + +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]'); + expect(stdout[stdout.length - 1]) + .toEqual('Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.'); +}); + +test.concurrent('should run help of custom-script if --help is after --', async () => { + const stdout = await execCommand('run', ['custom-script', '--', '--help'], 'run-custom-script-with-arguments'); + expect(stdout[stdout.length - 2]).toEqual('A message from custom script with args --help'); }); diff --git a/src/cli/index.js b/src/cli/index.js index da28db6325..103f304438 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -171,6 +171,9 @@ if (commandName === 'help' || args.indexOf('--help') >= 0 || args.indexOf('-h') console.log(); }); } + if (commandName !== 'help') { + commander.on('--help', () => console.log(' ' + getDocsInfo(commandName) + '\n')); + } commander.parse(startArgs.concat(args)); commander.help(); From 671ab12899e8ce4121c2805f327f61510e47167c Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Wed, 1 Feb 2017 23:37:59 +0100 Subject: [PATCH 2/2] extract help command --- src/cli/commands/help.js | 40 +++++++++++++++++++++++++++++++++++++++ src/cli/commands/index.js | 1 + src/cli/index.js | 32 +++++-------------------------- 3 files changed, 46 insertions(+), 27 deletions(-) create mode 100644 src/cli/commands/help.js diff --git a/src/cli/commands/help.js b/src/cli/commands/help.js new file mode 100644 index 0000000000..c2398cf712 --- /dev/null +++ b/src/cli/commands/help.js @@ -0,0 +1,40 @@ +/* @flow */ + +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'; +const chalk = require('chalk'); + +export function run( + config: Config, + reporter: Reporter, + commander: Object, + args: Array, +): Promise { + const getDocsLink = (name) => `${constants.YARN_DOCS}${name || ''}`; + 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; + } + + 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(); +} diff --git a/src/cli/commands/index.js b/src/cli/commands/index.js index 3b303d7479..44669bdb57 100644 --- a/src/cli/commands/index.js +++ b/src/cli/commands/index.js @@ -9,6 +9,7 @@ import * as clean from './clean.js'; export {clean}; import * as config from './config.js'; export {config}; import * as generateLockEntry from './generate-lock-entry.js'; export {generateLockEntry}; import * as global from './global.js'; export {global}; +import * as help from './help.js'; export {help}; import * as import_ from './import.js'; export {import_ as import}; import * as info from './info.js'; export {info}; import * as init from './init.js'; export {init}; diff --git a/src/cli/index.js b/src/cli/index.js index 103f304438..505afe325f 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -1,7 +1,6 @@ /* @flow */ import {ConsoleReporter, JSONReporter} from '../reporters/index.js'; -import {sortAlpha} from '../util/misc.js'; import {registries, registryNames} from '../registries/index.js'; import * as commands from './commands/index.js'; import * as constants from '../constants.js'; @@ -9,7 +8,7 @@ import * as network from '../util/network.js'; import {MessageError} from '../errors.js'; import aliases from './aliases.js'; import Config from '../config.js'; -import {hyphenate, camelCase} from '../util/misc.js'; +import {camelCase} from '../util/misc.js'; const chalk = require('chalk'); const commander = require('commander'); @@ -99,27 +98,8 @@ const getDocsLink = (name) => `${constants.YARN_DOCS}${name || ''}`; const getDocsInfo = (name) => 'Visit ' + chalk.bold(getDocsLink(name)) + ' for documentation about this command.'; // -if (commandName === 'help' || commandName === '--help' || commandName === '-h') { +if (commandName === '--help' || commandName === '-h') { commandName = 'help'; - 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; - } - - 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'); - }); - } } // if no args or command name looks like a flag then default to `install` @@ -160,7 +140,7 @@ if (command && typeof command.setFlags === 'function') { command.setFlags(commander); } -if (commandName === 'help' || args.indexOf('--help') >= 0 || args.indexOf('-h') >= 0) { +if (args.indexOf('--help') >= 0 || args.indexOf('-h') >= 0) { const examples: Array = (command && command.examples) || []; if (examples.length) { commander.on('--help', () => { @@ -171,9 +151,7 @@ if (commandName === 'help' || args.indexOf('--help') >= 0 || args.indexOf('-h') console.log(); }); } - if (commandName !== 'help') { - commander.on('--help', () => console.log(' ' + getDocsInfo(commandName) + '\n')); - } + commander.on('--help', () => console.log(' ' + getDocsInfo(commandName) + '\n')); commander.parse(startArgs.concat(args)); commander.help(); @@ -220,7 +198,7 @@ if (typeof command.hasWrapper === 'function') { if (commander.json) { outputWrapper = false; } -if (outputWrapper) { +if (outputWrapper && commandName !== 'help') { reporter.header(commandName, pkg); }