From 8bcc5d73f35434e781ff56419dd7f0c380efd072 Mon Sep 17 00:00:00 2001 From: Gar Date: Thu, 25 Mar 2021 14:21:30 -0700 Subject: [PATCH] feat(workspaces): add repo and docs This adds workspaces support to `npm repo` and `npm docs`. It also updates the usage output to support the -w and -ws parameters output, and cleans up some unneccessary functions in `run-script` and `exec`. PR-URL: https://github.com/npm/cli/pull/2972 Credit: @wraithgar Close: #2972 Reviewed-by: @nlf --- lib/base-command.js | 1 + lib/docs.js | 16 + lib/exec.js | 12 +- lib/repo.js | 16 + lib/run-script.js | 15 +- lib/utils/config/definition.js | 48 ++- lib/utils/config/definitions.js | 3 +- ...b-utils-config-describe-all.js-TAP.test.js | 2 +- .../test-lib-utils-npm-usage.js-TAP.test.js | 12 + test/lib/docs.js | 148 ++++++-- test/lib/load-all-commands.js | 9 +- test/lib/repo.js | 345 ++++++++++++------ test/lib/utils/config/definition.js | 8 +- 13 files changed, 454 insertions(+), 181 deletions(-) diff --git a/lib/base-command.js b/lib/base-command.js index 7a9e4b8ee377e..91c7c5357c9a0 100644 --- a/lib/base-command.js +++ b/lib/base-command.js @@ -27,6 +27,7 @@ class BaseCommand { usage = `${usage}${this.constructor.usage.map(u => `npm ${this.constructor.name} ${u}`).join('\n')}` if (this.constructor.params) + // TODO word wrap this along params boundaries usage = `${usage}\n\nOptions:\n[${this.constructor.params.map(p => ConfigDefinitions[p].usage).join('] [')}]` // Mostly this just appends aliases, this could be more clear diff --git a/lib/docs.js b/lib/docs.js index 089d77eb046d9..8be16cd5ec806 100644 --- a/lib/docs.js +++ b/lib/docs.js @@ -2,6 +2,7 @@ const log = require('npmlog') const pacote = require('pacote') const openUrl = require('./utils/open-url.js') const hostedFromMani = require('./utils/hosted-git-info-from-manifest.js') +const getWorkspaces = require('./workspaces/get-workspaces.js') const BaseCommand = require('./base-command.js') class Docs extends BaseCommand { @@ -15,6 +16,11 @@ class Docs extends BaseCommand { return 'docs' } + /* istanbul ignore next - see test/lib/load-all-commands.js */ + static get params () { + return ['browser', 'workspace', 'workspaces'] + } + /* istanbul ignore next - see test/lib/load-all-commands.js */ static get usage () { return ['[ [ ...]]'] @@ -24,6 +30,10 @@ class Docs extends BaseCommand { this.docs(args).then(() => cb()).catch(cb) } + execWorkspaces (args, filters, cb) { + this.docsWorkspaces(args, filters).then(() => cb()).catch(cb) + } + async docs (args) { if (!args || !args.length) args = ['.'] @@ -31,6 +41,12 @@ class Docs extends BaseCommand { await Promise.all(args.map(pkg => this.getDocs(pkg))) } + async docsWorkspaces (args, filters) { + const workspaces = + await getWorkspaces(filters, { path: this.npm.localPrefix }) + return this.docs([...workspaces.values()]) + } + async getDocs (pkg) { const opts = { ...this.npm.flatOptions, fullMetadata: true } const mani = await pacote.manifest(pkg, opts) diff --git a/lib/exec.js b/lib/exec.js index 5967ee4234592..f8c76eeed4c51 100644 --- a/lib/exec.js +++ b/lib/exec.js @@ -53,6 +53,11 @@ class Exec extends BaseCommand { return 'Run a command from a local or remote npm package' } + /* istanbul ignore next - see test/lib/load-all-commands.js */ + static get params () { + return ['workspace', 'workspaces'] + } + /* istanbul ignore next - see test/lib/load-all-commands.js */ static get name () { return 'exec' @@ -339,12 +344,9 @@ class Exec extends BaseCommand { .slice(0, 16) } - async workspaces (filters) { - return getWorkspaces(filters, { path: this.npm.localPrefix }) - } - async _execWorkspaces (args, filters) { - const workspaces = await this.workspaces(filters) + const workspaces = + await getWorkspaces(filters, { path: this.npm.localPrefix }) const getLocationMsg = async path => { const color = this.npm.config.get('color') const colorize = color ? chalk : nocolor diff --git a/lib/repo.js b/lib/repo.js index 5ab136abd73d8..645c0eeae32fe 100644 --- a/lib/repo.js +++ b/lib/repo.js @@ -1,5 +1,6 @@ const log = require('npmlog') const pacote = require('pacote') +const getWorkspaces = require('./workspaces/get-workspaces.js') const { URL } = require('url') const hostedFromMani = require('./utils/hosted-git-info-from-manifest.js') @@ -17,6 +18,11 @@ class Repo extends BaseCommand { return 'repo' } + /* istanbul ignore next - see test/lib/load-all-commands.js */ + static get params () { + return ['browser', 'workspace', 'workspaces'] + } + /* istanbul ignore next - see test/lib/load-all-commands.js */ static get usage () { return ['[ [ ...]]'] @@ -26,6 +32,10 @@ class Repo extends BaseCommand { this.repo(args).then(() => cb()).catch(cb) } + execWorkspaces (args, filters, cb) { + this.repoWorkspaces(args, filters).then(() => cb()).catch(cb) + } + async repo (args) { if (!args || !args.length) args = ['.'] @@ -33,6 +43,12 @@ class Repo extends BaseCommand { await Promise.all(args.map(pkg => this.get(pkg))) } + async repoWorkspaces (args, filters) { + const workspaces = + await getWorkspaces(filters, { path: this.npm.localPrefix }) + return this.repo([...workspaces.values()]) + } + async get (pkg) { const opts = { ...this.npm.flatOptions, fullMetadata: true } const mani = await pacote.manifest(pkg, opts) diff --git a/lib/run-script.js b/lib/run-script.js index 054f0ae4a551f..f781f25c5d058 100644 --- a/lib/run-script.js +++ b/lib/run-script.js @@ -34,6 +34,11 @@ class RunScript extends BaseCommand { return 'Run arbitrary package scripts' } + /* istanbul ignore next - see test/lib/load-all-commands.js */ + static get params () { + return ['workspace', 'workspaces'] + } + /* istanbul ignore next - see test/lib/load-all-commands.js */ static get name () { return 'run-script' @@ -182,13 +187,10 @@ class RunScript extends BaseCommand { return allScripts } - async workspaces (filters) { - return getWorkspaces(filters, { path: this.npm.localPrefix }) - } - async runWorkspaces (args, filters) { const res = [] - const workspaces = await this.workspaces(filters) + const workspaces = + await getWorkspaces(filters, { path: this.npm.localPrefix }) for (const workspacePath of workspaces.values()) { const pkg = await rpj(`${workspacePath}/package.json`) @@ -219,7 +221,8 @@ class RunScript extends BaseCommand { } async listWorkspaces (args, filters) { - const workspaces = await this.workspaces(filters) + const workspaces = + await getWorkspaces(filters, { path: this.npm.localPrefix }) if (log.level === 'silent') return diff --git a/lib/utils/config/definition.js b/lib/utils/config/definition.js index cb4eb78210c6e..1d487f531243d 100644 --- a/lib/utils/config/definition.js +++ b/lib/utils/config/definition.js @@ -45,6 +45,7 @@ class Definition { this.defaultDescription = describeValue(this.default) if (!this.typeDescription) this.typeDescription = describeType(this.type) + // hint is only used for non-boolean values if (!this.hint) this.hint = `<${this.key}>` if (!this.usage) @@ -79,26 +80,43 @@ ${description} } } -// Usage for a single param, abstracted because we have arrays of types in -// config definition -const paramUsage = (type, def) => { +const describeUsage = (def) => { let key = `--${def.key}` if (def.short && typeof def.short === 'string') key = `-${def.short}|${key}` - if (type === Boolean) - return `${key}` - else - return `${key} ${def.hint}` -} -const describeUsage = (def) => { - if (Array.isArray(def.type)) { - if (!def.type.some(d => d !== null && typeof d !== 'string')) - return `--${def.key} <${def.type.filter(d => d).join('|')}>` - else - return def.type.filter(d => d).map((t) => paramUsage(t, def)).join('|') + // Single type + if (!Array.isArray(def.type)) + return `${key}${def.type === Boolean ? '' : ' ' + def.hint}` + + // Multiple types + let types = def.type + const multiple = types.includes(Array) + const bool = types.includes(Boolean) + + // null type means optional and doesn't currently affect usage output since + // all non-optional params have defaults so we render everything as optional + types = types.filter(t => t !== null && t !== Array && t !== Boolean) + + if (!types.length) + return key + + let description + if (!types.some(t => typeof t !== 'string')) + // Specific values, use specifics given + description = `<${types.filter(d => d).join('|')}>` + else { + // Generic values, use hint + description = def.hint } - return paramUsage(def.type, def) + + if (multiple) + description = `${description} [${description} ...]` + + if (bool) + key = `${key}|${key}` + + return `${key} ${description}` } const describeType = type => { diff --git a/lib/utils/config/definitions.js b/lib/utils/config/definitions.js index db66aa495ba0f..14d8eaa699435 100644 --- a/lib/utils/config/definitions.js +++ b/lib/utils/config/definitions.js @@ -223,7 +223,7 @@ define('audit', { define('audit-level', { default: null, - type: ['info', 'low', 'moderate', 'high', 'critical', 'none', null], + type: [null, 'info', 'low', 'moderate', 'high', 'critical', 'none'], description: ` The minimum level of vulnerability for \`npm audit\` to exit with a non-zero exit code. @@ -2042,6 +2042,7 @@ define('which', { define('workspace', { default: [], type: [String, Array], + hint: '', short: 'w', description: ` Enable running a command in the context of the configured workspaces of the diff --git a/tap-snapshots/test-lib-utils-config-describe-all.js-TAP.test.js b/tap-snapshots/test-lib-utils-config-describe-all.js-TAP.test.js index 2a3d0146b187d..d424508c308c4 100644 --- a/tap-snapshots/test-lib-utils-config-describe-all.js-TAP.test.js +++ b/tap-snapshots/test-lib-utils-config-describe-all.js-TAP.test.js @@ -64,7 +64,7 @@ registry and all registries configured for scopes. See the documentation for #### \`audit-level\` * Default: null -* Type: "info", "low", "moderate", "high", "critical", "none", or null +* Type: null, "info", "low", "moderate", "high", "critical", or "none" The minimum level of vulnerability for \`npm audit\` to exit with a non-zero exit code. diff --git a/tap-snapshots/test-lib-utils-npm-usage.js-TAP.test.js b/tap-snapshots/test-lib-utils-npm-usage.js-TAP.test.js index 5a860bd2ee554..2d229a2691f85 100644 --- a/tap-snapshots/test-lib-utils-npm-usage.js-TAP.test.js +++ b/tap-snapshots/test-lib-utils-npm-usage.js-TAP.test.js @@ -334,6 +334,9 @@ All commands: Usage: npm docs [ [ ...]] + Options: + [--browser|--browser ] [-w|--workspace [ ...]] [-ws|--workspaces] + alias: home Run "npm help docs" for more info @@ -366,6 +369,9 @@ All commands: npm exec -c ' [args...]' npm exec --package=foo -c ' [args...]' + Options: + [-w|--workspace [ ...]] [-ws|--workspaces] + alias: x Run "npm help exec" for more info @@ -695,6 +701,9 @@ All commands: Usage: npm repo [ [ ...]] + Options: + [--browser|--browser ] [-w|--workspace [ ...]] [-ws|--workspaces] + Run "npm help repo" for more info restart npm restart @@ -725,6 +734,9 @@ All commands: Usage: npm run-script [-- ] + Options: + [-w|--workspace [ ...]] [-ws|--workspaces] + aliases: run, rum, urn Run "npm help run-script" for more info diff --git a/test/lib/docs.js b/test/lib/docs.js index a7325738ba43e..e8176eb47bdf9 100644 --- a/test/lib/docs.js +++ b/test/lib/docs.js @@ -1,51 +1,92 @@ const t = require('tap') - const requireInject = require('require-inject') -const pacote = { - manifest: async (spec, options) => { - return spec === 'nodocs' ? { +const mockNpm = require('../fixtures/mock-npm.js') +const { join, sep } = require('path') + +const pkgDirs = t.testdir({ + 'package.json': JSON.stringify({ + name: 'thispkg', + version: '1.2.3', + homepage: 'https://example.com', + }), + nodocs: { + 'package.json': JSON.stringify({ name: 'nodocs', version: '1.2.3', - } - : spec === 'docsurl' ? { - name: 'docsurl', - version: '1.2.3', - homepage: 'https://bugzilla.localhost/docsurl', - } - : spec === 'repourl' ? { - name: 'repourl', - version: '1.2.3', - repository: 'https://github.com/foo/repourl', - } - : spec === 'repoobj' ? { - name: 'repoobj', - version: '1.2.3', - repository: { url: 'https://github.com/foo/repoobj' }, - } - : spec === '.' ? { - name: 'thispkg', - version: '1.2.3', - homepage: 'https://example.com', - } - : null + }), }, -} + docsurl: { + 'package.json': JSON.stringify({ + name: 'docsurl', + version: '1.2.3', + homepage: 'https://bugzilla.localhost/docsurl', + }), + }, + repourl: { + 'package.json': JSON.stringify({ + name: 'repourl', + version: '1.2.3', + repository: 'https://github.com/foo/repourl', + }), + }, + repoobj: { + 'package.json': JSON.stringify({ + name: 'repoobj', + version: '1.2.3', + repository: { url: 'https://github.com/foo/repoobj' }, + }), + }, + workspaces: { + 'package.json': JSON.stringify({ + name: 'workspaces-test', + version: '1.2.3-test', + workspaces: ['workspace-a', 'workspace-b', 'workspace-c'], + }), + 'workspace-a': { + 'package.json': JSON.stringify({ + name: 'workspace-a', + version: '1.2.3-a', + homepage: 'http://docs.workspace-a/', + }), + }, + 'workspace-b': { + 'package.json': JSON.stringify({ + name: 'workspace-b', + version: '1.2.3-n', + repository: 'https://github.com/npm/workspace-b', + }), + }, + 'workspace-c': JSON.stringify({ + 'package.json': { + name: 'workspace-n', + version: '1.2.3-n', + }, + }), + }, +}) // keep a tally of which urls got opened -const opened = {} +let opened = {} const openUrl = async (npm, url, errMsg) => { opened[url] = opened[url] || 0 opened[url]++ } const Docs = requireInject('../../lib/docs.js', { - pacote, '../../lib/utils/open-url.js': openUrl, }) +const flatOptions = {} +const npm = mockNpm({ flatOptions }) +const docs = new Docs(npm) -const docs = new Docs({ flatOptions: {} }) +t.afterEach(async () => { + opened = {} +}) t.test('open docs urls', t => { + // XXX It is very odd that `where` is how pacote knows to look anywhere other + // than the cwd. I would think npm.localPrefix would factor in somehow + flatOptions.where = pkgDirs const expect = { nodocs: 'https://www.npmjs.com/package/nodocs', docsurl: 'https://bugzilla.localhost/docsurl', @@ -57,11 +98,13 @@ t.test('open docs urls', t => { t.plan(keys.length) keys.forEach(pkg => { t.test(pkg, t => { - docs.exec([pkg], (er) => { - if (er) - throw er + docs.exec([['.', pkg].join(sep)], (err) => { + if (err) + throw err const url = expect[pkg] - t.equal(opened[url], 1, url, {opened}) + t.match({ + [url]: 1, + }, opened, `opened ${url}`, {opened}) t.end() }) }) @@ -72,7 +115,42 @@ t.test('open default package if none specified', t => { docs.exec([], (er) => { if (er) throw er - t.equal(opened['https://example.com'], 2, 'opened expected url', {opened}) + t.equal(opened['https://example.com'], 1, 'opened expected url', {opened}) t.end() }) }) + +t.test('workspaces', (t) => { + flatOptions.where = undefined + npm.localPrefix = join(pkgDirs, 'workspaces') + t.test('all workspaces', (t) => { + docs.execWorkspaces([], [], (err) => { + t.notOk(err) + t.match({ + 'http://docs.workspace-a/': 1, + 'https://github.com/npm/workspace-b#readme': 1, + }, opened, 'opened two valid docs urls') + t.end() + }) + }) + + t.test('one workspace', (t) => { + docs.execWorkspaces([], ['workspace-a'], (err) => { + t.notOk(err) + t.match({ + 'http://docs.workspace-a/': 1, + }, opened, 'opened one requested docs urls') + t.end() + }) + }) + + t.test('invalid workspace', (t) => { + docs.execWorkspaces([], ['workspace-x'], (err) => { + t.match(err, /No workspaces found/) + t.match(err, /workspace-x/) + t.match({}, opened, 'opened no docs urls') + t.end() + }) + }) + t.end() +}) diff --git a/test/lib/load-all-commands.js b/test/lib/load-all-commands.js index d7eb2eae0a8ad..cb3aefb1f8842 100644 --- a/test/lib/load-all-commands.js +++ b/test/lib/load-all-commands.js @@ -1,8 +1,7 @@ -// Thanks to nyc not working properly with proxies this doesn't affect -// coverage. but it does ensure that every command has a usage that renders, -// contains its name, a description, and if it has completion it is a function. -// That it renders also ensures that any params we've defined in our commands -// work. +// Our coverage mapping means that stuff like this doen't count for coverage. +// It does ensure that every command has a usage that renders, contains its +// name, a description, and if it has completion it is a function. That it +// renders also ensures that any params we've defined in our commands work. const requireInject = require('require-inject') const npm = requireInject('../../lib/npm.js') const t = require('tap') diff --git a/test/lib/repo.js b/test/lib/repo.js index 9c22bbaea340b..c05c02d80a885 100644 --- a/test/lib/repo.js +++ b/test/lib/repo.js @@ -1,125 +1,206 @@ const t = require('tap') - const requireInject = require('require-inject') -const pacote = { - manifest: async (spec, options) => { - return spec === 'norepo' ? { +const mockNpm = require('../fixtures/mock-npm.js') +const { join, sep } = require('path') + +const pkgDirs = t.testdir({ + 'package.json': JSON.stringify({ + name: 'thispkg', + version: '1.2.3', + repository: 'https://example.com/thispkg.git', + }), + norepo: { + 'package.json': JSON.stringify({ name: 'norepo', version: '1.2.3', - } - - : spec === 'repoobbj-nourl' ? { - name: 'repoobj-nourl', - repository: { no: 'url' }, - } - - : spec === 'hostedgit' ? { - repository: 'git://github.com/foo/hostedgit', - } - : spec === 'hostedgitat' ? { - repository: 'git@github.com:foo/hostedgitat', - } - : spec === 'hostedssh' ? { - repository: 'ssh://git@github.com/foo/hostedssh', - } - : spec === 'hostedgitssh' ? { - repository: 'git+ssh://git@github.com/foo/hostedgitssh', - } - : spec === 'hostedgithttp' ? { - repository: 'git+http://github.com/foo/hostedgithttp', - } - : spec === 'hostedgithttps' ? { - repository: 'git+https://github.com/foo/hostedgithttps', - } - - : spec === 'hostedgitobj' ? { - repository: { url: 'git://github.com/foo/hostedgitobj' }, - } - : spec === 'hostedgitatobj' ? { - repository: { url: 'git@github.com:foo/hostedgitatobj' }, - } - : spec === 'hostedsshobj' ? { - repository: { url: 'ssh://git@github.com/foo/hostedsshobj' }, - } - : spec === 'hostedgitsshobj' ? { - repository: { url: 'git+ssh://git@github.com/foo/hostedgitsshobj' }, - } - : spec === 'hostedgithttpobj' ? { - repository: { url: 'git+http://github.com/foo/hostedgithttpobj' }, - } - : spec === 'hostedgithttpsobj' ? { - repository: { url: 'git+https://github.com/foo/hostedgithttpsobj' }, - } - - : spec === 'unhostedgit' ? { - repository: 'git://gothib.com/foo/unhostedgit', - } - : spec === 'unhostedgitat' ? { - repository: 'git@gothib.com:foo/unhostedgitat', - } - : spec === 'unhostedssh' ? { - repository: 'ssh://git@gothib.com/foo/unhostedssh', - } - : spec === 'unhostedgitssh' ? { - repository: 'git+ssh://git@gothib.com/foo/unhostedgitssh', - } - : spec === 'unhostedgithttp' ? { - repository: 'git+http://gothib.com/foo/unhostedgithttp', - } - : spec === 'unhostedgithttps' ? { - repository: 'git+https://gothib.com/foo/unhostedgithttps', - } - - : spec === 'unhostedgitobj' ? { - repository: { url: 'git://gothib.com/foo/unhostedgitobj' }, - } - : spec === 'unhostedgitatobj' ? { - repository: { url: 'git@gothib.com:foo/unhostedgitatobj' }, - } - : spec === 'unhostedsshobj' ? { - repository: { url: 'ssh://git@gothib.com/foo/unhostedsshobj' }, - } - : spec === 'unhostedgitsshobj' ? { - repository: { url: 'git+ssh://git@gothib.com/foo/unhostedgitsshobj' }, - } - : spec === 'unhostedgithttpobj' ? { - repository: { url: 'git+http://gothib.com/foo/unhostedgithttpobj' }, - } - : spec === 'unhostedgithttpsobj' ? { - repository: { url: 'git+https://gothib.com/foo/unhostedgithttpsobj' }, - } - - : spec === 'directory' ? { - repository: { - type: 'git', - url: 'git+https://github.com/foo/test-repo-with-directory.git', - directory: 'some/directory', - }, - } - - : spec === '.' ? { - name: 'thispkg', - version: '1.2.3', - repository: 'https://example.com/thispkg.git', - } - : null + }), }, -} + 'repoobbj-nourl': { + 'package.json': JSON.stringify({ + name: 'repoobj-nourl', + repository: { no: 'url' }, + }), + }, + hostedgit: { + 'package.json': JSON.stringify({ + repository: 'git://github.com/foo/hostedgit', + }), + }, + hostedgitat: { + 'package.json': JSON.stringify({ + repository: 'git@github.com:foo/hostedgitat', + }), + }, + hostedssh: { + 'package.json': JSON.stringify({ + repository: 'ssh://git@github.com/foo/hostedssh', + }), + }, + hostedgitssh: { + 'package.json': JSON.stringify({ + repository: 'git+ssh://git@github.com/foo/hostedgitssh', + }), + }, + hostedgithttp: { + 'package.json': JSON.stringify({ + repository: 'git+http://github.com/foo/hostedgithttp', + }), + }, + hostedgithttps: { + 'package.json': JSON.stringify({ + repository: 'git+https://github.com/foo/hostedgithttps', + }), + }, + hostedgitobj: { + 'package.json': JSON.stringify({ + repository: { url: 'git://github.com/foo/hostedgitobj' }, + }), + }, + hostedgitatobj: { + 'package.json': JSON.stringify({ + repository: { url: 'git@github.com:foo/hostedgitatobj' }, + }), + }, + hostedsshobj: { + 'package.json': JSON.stringify({ + repository: { url: 'ssh://git@github.com/foo/hostedsshobj' }, + }), + }, + hostedgitsshobj: { + 'package.json': JSON.stringify({ + repository: { url: 'git+ssh://git@github.com/foo/hostedgitsshobj' }, + }), + }, + hostedgithttpobj: { + 'package.json': JSON.stringify({ + repository: { url: 'git+http://github.com/foo/hostedgithttpobj' }, + }), + }, + hostedgithttpsobj: { + 'package.json': JSON.stringify({ + repository: { url: 'git+https://github.com/foo/hostedgithttpsobj' }, + }), + }, + unhostedgit: { + 'package.json': JSON.stringify({ + repository: 'git://gothib.com/foo/unhostedgit', + }), + }, + unhostedgitat: { + 'package.json': JSON.stringify({ + repository: 'git@gothib.com:foo/unhostedgitat', + }), + }, + unhostedssh: { + 'package.json': JSON.stringify({ + repository: 'ssh://git@gothib.com/foo/unhostedssh', + }), + }, + unhostedgitssh: { + 'package.json': JSON.stringify({ + repository: 'git+ssh://git@gothib.com/foo/unhostedgitssh', + }), + }, + unhostedgithttp: { + 'package.json': JSON.stringify({ + repository: 'git+http://gothib.com/foo/unhostedgithttp', + }), + }, + unhostedgithttps: { + 'package.json': JSON.stringify({ + repository: 'git+https://gothib.com/foo/unhostedgithttps', + }), + }, + unhostedgitobj: { + 'package.json': JSON.stringify({ + repository: { url: 'git://gothib.com/foo/unhostedgitobj' }, + }), + }, + unhostedgitatobj: { + 'package.json': JSON.stringify({ + repository: { url: 'git@gothib.com:foo/unhostedgitatobj' }, + }), + }, + unhostedsshobj: { + 'package.json': JSON.stringify({ + repository: { url: 'ssh://git@gothib.com/foo/unhostedsshobj' }, + }), + }, + unhostedgitsshobj: { + 'package.json': JSON.stringify({ + repository: { url: 'git+ssh://git@gothib.com/foo/unhostedgitsshobj' }, + }), + }, + unhostedgithttpobj: { + 'package.json': JSON.stringify({ + repository: { url: 'git+http://gothib.com/foo/unhostedgithttpobj' }, + }), + }, + unhostedgithttpsobj: { + 'package.json': JSON.stringify({ + repository: { url: 'git+https://gothib.com/foo/unhostedgithttpsobj' }, + }), + }, + directory: { + 'package.json': JSON.stringify({ + repository: { + type: 'git', + url: 'git+https://github.com/foo/test-repo-with-directory.git', + directory: 'some/directory', + }, + }), + }, + workspaces: { + 'package.json': JSON.stringify({ + name: 'workspaces-test', + version: '1.2.3-test', + workspaces: ['workspace-a', 'workspace-b', 'workspace-c'], + }), + 'workspace-a': { + 'package.json': JSON.stringify({ + name: 'workspace-a', + version: '1.2.3-a', + repository: 'http://repo.workspace-a/', + }), + }, + 'workspace-b': { + 'package.json': JSON.stringify({ + name: 'workspace-b', + version: '1.2.3-n', + repository: 'https://github.com/npm/workspace-b', + }), + }, + 'workspace-c': JSON.stringify({ + 'package.json': { + name: 'workspace-n', + version: '1.2.3-n', + }, + }), + }, +}) // keep a tally of which urls got opened -const opened = {} +let opened = {} const openUrl = async (npm, url, errMsg) => { opened[url] = opened[url] || 0 opened[url]++ } const Repo = requireInject('../../lib/repo.js', { - pacote, '../../lib/utils/open-url.js': openUrl, }) -const repo = new Repo({ flatOptions: {} }) +const flatOptions = {} +const npm = mockNpm({ flatOptions }) +const repo = new Repo(npm) + +t.afterEach(async () => { + opened = {} +}) t.test('open repo urls', t => { + // XXX It is very odd that `where` is how pacote knows to look anywhere other + // than the cwd. I would think npm.localPrefix would factor in somehow + flatOptions.where = pkgDirs const expect = { hostedgit: 'https://github.com/foo/hostedgit', hostedgitat: 'https://github.com/foo/hostedgitat', @@ -150,11 +231,13 @@ t.test('open repo urls', t => { t.plan(keys.length) keys.forEach(pkg => { t.test(pkg, t => { - repo.exec([pkg], (er) => { - if (er) - throw er + repo.exec([['.', pkg].join(sep)], (err) => { + if (err) + throw err const url = expect[pkg] - t.equal(opened[url], 1, url, {opened}) + t.match({ + [url]: 1, + }, opened, `opened ${url}`, {opened}) t.end() }) }) @@ -162,6 +245,7 @@ t.test('open repo urls', t => { }) t.test('fail if cannot figure out repo url', t => { + flatOptions.where = pkgDirs const cases = [ 'norepo', 'repoobbj-nourl', @@ -173,8 +257,8 @@ t.test('fail if cannot figure out repo url', t => { cases.forEach(pkg => { t.test(pkg, t => { - repo.exec([pkg], er => { - t.match(er, { pkgid: pkg }) + repo.exec([['.', pkg].join(sep)], (err) => { + t.match(err, { pkgid: pkg }) t.end() }) }) @@ -182,10 +266,47 @@ t.test('fail if cannot figure out repo url', t => { }) t.test('open default package if none specified', t => { + flatOptions.where = pkgDirs repo.exec([], (er) => { if (er) throw er - t.equal(opened['https://example.com/thispkg'], 2, 'opened expected url', {opened}) + t.equal(opened['https://example.com/thispkg'], 1, 'opened expected url', {opened}) t.end() }) }) + +t.test('workspaces', t => { + flatOptions.where = undefined + npm.localPrefix = join(pkgDirs, 'workspaces') + + t.test('all workspaces', (t) => { + repo.execWorkspaces([], [], (err) => { + t.notOk(err) + t.match({ + 'https://repo.workspace-a/': 1, // Gets translated to https! + 'https://github.com/npm/workspace-b': 1, + }, opened, 'opened two valid repo urls') + t.end() + }) + }) + + t.test('one workspace', (t) => { + repo.execWorkspaces([], ['workspace-a'], (err) => { + t.notOk(err) + t.match({ + 'https://repo.workspace-a/': 1, + }, opened, 'opened one requested repo urls') + t.end() + }) + }) + + t.test('invalid workspace', (t) => { + repo.execWorkspaces([], ['workspace-x'], (err) => { + t.match(err, /No workspaces found/) + t.match(err, /workspace-x/) + t.match({}, opened, 'opened no repo urls') + t.end() + }) + }) + t.end() +}) diff --git a/test/lib/utils/config/definition.js b/test/lib/utils/config/definition.js index 56e10da0cbd7d..88a527db6949d 100644 --- a/test/lib/utils/config/definition.js +++ b/test/lib/utils/config/definition.js @@ -22,7 +22,7 @@ t.test('basic definition', async t => { defaultDescription: '"some default value"', type: [Number, String], hint: '', - usage: '--key |--key ', + usage: '--key ', typeDescription: 'Number or String', description: 'just a test thingie', }) @@ -113,6 +113,12 @@ t.test('basic definition', async t => { hint: '', }) t.equal(hasHint.usage, '--key ') + const optionalBool = new Definition('key', { + default: null, + type: [null, Boolean], + description: 'asdf', + }) + t.equal(optionalBool.usage, '--key') }) t.test('missing fields', async t => {