From 9e973af5aa50c540f66dc22b117606b43f70fdcd Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 26 May 2021 15:39:59 -0700 Subject: [PATCH] fix: refactor get package name renames read-local-package to read-package-name The global check needed to be moved outside this function, because it was handled differently (and will be even moreso when we implement diff workspaces) in each function. This allowed us to now pass in the prefix itself instead of the npm object, so we can reuse this function to look up package names when we refactor npm diff for workspaces. PR-URL: https://github.com/npm/cli/pull/3331 Credit: @wraithgar Close: #3331 Reviewed-by: @ruyadorno --- lib/diff.js | 8 +- lib/dist-tag.js | 14 +-- lib/owner.js | 19 +++- ...-local-package.js => read-package-name.js} | 7 +- tap-snapshots/test/lib/dist-tag.js.test.cjs | 57 ++++++++-- test/lib/diff.js | 62 ++++++----- test/lib/dist-tag.js | 14 +++ test/lib/owner.js | 100 ++++++++++++++---- ...-local-package.js => read-package-name.js} | 25 +---- 9 files changed, 203 insertions(+), 103 deletions(-) rename lib/utils/{read-local-package.js => read-package-name.js} (56%) rename test/lib/utils/{read-local-package.js => read-package-name.js} (55%) diff --git a/lib/diff.js b/lib/diff.js index 1eaceb4f2f61f..58834ca9c2674 100644 --- a/lib/diff.js +++ b/lib/diff.js @@ -8,7 +8,7 @@ const npmlog = require('npmlog') const pacote = require('pacote') const pickManifest = require('npm-pick-manifest') -const readLocalPkg = require('./utils/read-local-package.js') +const readPackageName = require('./utils/read-package-name.js') const BaseCommand = require('./base-command.js') class Diff extends BaseCommand { @@ -97,7 +97,7 @@ class Diff extends BaseCommand { let noPackageJson let pkgName try { - pkgName = await readLocalPkg(this.npm) + pkgName = await readPackageName(this.npm.prefix) } catch (e) { npmlog.verbose('diff', 'could not read project dir package.json') noPackageJson = true @@ -120,7 +120,7 @@ class Diff extends BaseCommand { let noPackageJson let pkgName try { - pkgName = await readLocalPkg(this.npm) + pkgName = await readPackageName(this.npm.prefix) } catch (e) { npmlog.verbose('diff', 'could not read project dir package.json') noPackageJson = true @@ -238,7 +238,7 @@ class Diff extends BaseCommand { if (semverA && semverB) { let pkgName try { - pkgName = await readLocalPkg(this.npm) + pkgName = await readPackageName(this.npm.prefix) } catch (e) { npmlog.verbose('diff', 'could not read project dir package.json') } diff --git a/lib/dist-tag.js b/lib/dist-tag.js index 64e8abc013745..11b1ad931e18b 100644 --- a/lib/dist-tag.js +++ b/lib/dist-tag.js @@ -4,7 +4,7 @@ const regFetch = require('npm-registry-fetch') const semver = require('semver') const otplease = require('./utils/otplease.js') -const readLocalPkgName = require('./utils/read-local-package.js') +const readPackageName = require('./utils/read-package-name.js') const getWorkspaces = require('./workspaces/get-workspaces.js') const BaseCommand = require('./base-command.js') @@ -64,7 +64,7 @@ class DistTag extends BaseCommand { // should be listing the existing tags return this.list(cmdName, opts) } else - throw this.usage + throw this.usageError() } execWorkspaces (args, filters, cb) { @@ -102,7 +102,7 @@ class DistTag extends BaseCommand { log.verbose('dist-tag add', defaultTag, 'to', spec.name + '@' + version) if (!spec.name || !version || !defaultTag) - throw this.usage + throw this.usageError() const t = defaultTag.trim() @@ -135,7 +135,7 @@ class DistTag extends BaseCommand { log.verbose('dist-tag del', tag, 'from', spec.name) if (!spec.name) - throw this.usage + throw this.usageError() const tags = await this.fetchTags(spec, opts) if (!tags[tag]) { @@ -157,9 +157,11 @@ class DistTag extends BaseCommand { async list (spec, opts) { if (!spec) { - const pkg = await readLocalPkgName(this.npm) + if (this.npm.config.get('global')) + throw this.usageError() + const pkg = await readPackageName(this.npm.prefix) if (!pkg) - throw this.usage + throw this.usageError() return this.list(pkg, opts) } diff --git a/lib/owner.js b/lib/owner.js index e57d2268d2619..311b25064e638 100644 --- a/lib/owner.js +++ b/lib/owner.js @@ -4,7 +4,7 @@ const npmFetch = require('npm-registry-fetch') const pacote = require('pacote') const otplease = require('./utils/otplease.js') -const readLocalPkg = require('./utils/read-local-package.js') +const readLocalPkgName = require('./utils/read-package-name.js') const BaseCommand = require('./base-command.js') class Owner extends BaseCommand { @@ -47,7 +47,9 @@ class Owner extends BaseCommand { // reaches registry in order to autocomplete rm if (argv[2] === 'rm') { - const pkgName = await readLocalPkg(this.npm) + if (this.npm.config.get('global')) + return [] + const pkgName = await readLocalPkgName(this.npm.prefix) if (!pkgName) return [] @@ -84,7 +86,10 @@ class Owner extends BaseCommand { async ls (pkg, opts) { if (!pkg) { - const pkgName = await readLocalPkg(this.npm) + if (this.npm.config.get('global')) + throw this.usageError() + + const pkgName = await readLocalPkgName(this.npm.prefix) if (!pkgName) throw this.usageError() @@ -113,7 +118,9 @@ class Owner extends BaseCommand { throw this.usageError() if (!pkg) { - const pkgName = await readLocalPkg(this.npm) + if (this.npm.config.get('global')) + throw this.usageError() + const pkgName = await readLocalPkgName(this.npm.prefix) if (!pkgName) throw this.usageError() @@ -131,7 +138,9 @@ class Owner extends BaseCommand { throw this.usageError() if (!pkg) { - const pkgName = await readLocalPkg(this.npm) + if (this.npm.config.get('global')) + throw this.usageError() + const pkgName = await readLocalPkgName(this.npm.prefix) if (!pkgName) throw this.usageError() diff --git a/lib/utils/read-local-package.js b/lib/utils/read-package-name.js similarity index 56% rename from lib/utils/read-local-package.js rename to lib/utils/read-package-name.js index 21506ca180a0f..7ed15987767bb 100644 --- a/lib/utils/read-local-package.js +++ b/lib/utils/read-package-name.js @@ -1,10 +1,7 @@ const { resolve } = require('path') const readJson = require('read-package-json-fast') -async function readLocalPackageName (npm) { - if (npm.config.get('global')) - return - - const filepath = resolve(npm.prefix, 'package.json') +async function readLocalPackageName (prefix) { + const filepath = resolve(prefix, 'package.json') const json = await readJson(filepath) return json.name } diff --git a/tap-snapshots/test/lib/dist-tag.js.test.cjs b/tap-snapshots/test/lib/dist-tag.js.test.cjs index 86a9c84eb1eb6..21d9331db1299 100644 --- a/tap-snapshots/test/lib/dist-tag.js.test.cjs +++ b/tap-snapshots/test/lib/dist-tag.js.test.cjs @@ -6,7 +6,8 @@ */ 'use strict' exports[`test/lib/dist-tag.js TAP add missing args > should exit usage error message 1`] = ` -npm dist-tag +Error: +Usage: npm dist-tag Modify package distribution tags @@ -21,11 +22,14 @@ Options: alias: dist-tags -Run "npm help dist-tag" for more info +Run "npm help dist-tag" for more info { + "code": "EUSAGE", +} ` exports[`test/lib/dist-tag.js TAP add missing pkg name > should exit usage error message 1`] = ` -npm dist-tag +Error: +Usage: npm dist-tag Modify package distribution tags @@ -40,7 +44,9 @@ Options: alias: dist-tags -Run "npm help dist-tag" for more info +Run "npm help dist-tag" for more info { + "code": "EUSAGE", +} ` exports[`test/lib/dist-tag.js TAP add new tag > should return success msg 1`] = ` @@ -53,7 +59,8 @@ dist-tag add 1.0.0 to @scoped/another@7.7.7 ` exports[`test/lib/dist-tag.js TAP borked cmd usage > should show usage error 1`] = ` -npm dist-tag +Error: +Usage: npm dist-tag Modify package distribution tags @@ -68,7 +75,31 @@ Options: alias: dist-tags -Run "npm help dist-tag" for more info +Run "npm help dist-tag" for more info { + "code": "EUSAGE", +} +` + +exports[`test/lib/dist-tag.js TAP ls global > should throw basic usage 1`] = ` +Error: +Usage: npm dist-tag + +Modify package distribution tags + +Usage: +npm dist-tag add @ [] +npm dist-tag rm +npm dist-tag ls [] + +Options: +[-w|--workspace [-w|--workspace ...]] +[-ws|--workspaces] + +alias: dist-tags + +Run "npm help dist-tag" for more info { + "code": "EUSAGE", +} ` exports[`test/lib/dist-tag.js TAP ls in current package > should list available tags for current package 1`] = ` @@ -78,7 +109,8 @@ latest: 1.0.0 ` exports[`test/lib/dist-tag.js TAP ls on missing name in current package > should throw usage error message 1`] = ` -npm dist-tag +Error: +Usage: npm dist-tag Modify package distribution tags @@ -93,7 +125,9 @@ Options: alias: dist-tags -Run "npm help dist-tag" for more info +Run "npm help dist-tag" for more info { + "code": "EUSAGE", +} ` exports[`test/lib/dist-tag.js TAP ls on missing package > should log no dist-tag found msg 1`] = ` @@ -133,7 +167,8 @@ exports[`test/lib/dist-tag.js TAP remove existing tag > should return success ms ` exports[`test/lib/dist-tag.js TAP remove missing pkg name > should exit usage error message 1`] = ` -npm dist-tag +Error: +Usage: npm dist-tag Modify package distribution tags @@ -148,7 +183,9 @@ Options: alias: dist-tags -Run "npm help dist-tag" for more info +Run "npm help dist-tag" for more info { + "code": "EUSAGE", +} ` exports[`test/lib/dist-tag.js TAP remove non-existing tag > should log error msg 1`] = ` diff --git a/test/lib/diff.js b/test/lib/diff.js index 355095c95786e..7a52ea5ee0ae1 100644 --- a/test/lib/diff.js +++ b/test/lib/diff.js @@ -4,7 +4,7 @@ const mockNpm = require('../fixtures/mock-npm') const noop = () => null let libnpmdiff = noop -let rlp = () => 'foo' +let rpn = () => 'foo' const config = { global: false, @@ -33,7 +33,7 @@ const mocks = { npmlog: { info: noop, verbose: noop }, libnpmdiff: (...args) => libnpmdiff(...args), 'npm-registry-fetch': async () => ({}), - '../../lib/utils/read-local-package.js': async () => rlp(), + '../../lib/utils/read-package-name.js': async (prefix) => rpn(prefix), '../../lib/utils/usage.js': () => 'usage instructions', } @@ -52,7 +52,7 @@ t.afterEach(() => { npm.globalDir = __dirname npm.prefix = '..' libnpmdiff = noop - rlp = () => 'foo' + rpn = () => 'foo' }) const Diff = t.mock('../../lib/diff.js', mocks) @@ -77,7 +77,7 @@ t.test('no args', t => { }) t.test('no args, missing package.json name in cwd', t => { - rlp = () => undefined + rpn = () => undefined diff.exec([], err => { t.match( @@ -90,7 +90,7 @@ t.test('no args', t => { }) t.test('no args, missing package.json in cwd', t => { - rlp = () => { + rpn = () => { throw new Error('ERR') } @@ -109,14 +109,17 @@ t.test('no args', t => { t.test('single arg', t => { t.test('spec using cwd package name', t => { - t.plan(3) + t.plan(4) + rpn = (prefix) => { + t.equal(prefix, path, 'read-package-name gets proper prefix') + return 'foo' + } const path = t.testdir({}) libnpmdiff = async ([a, b], opts) => { t.equal(a, 'foo@1.0.0', 'should forward single spec') t.equal(b, `file:${path}`, 'should compare to cwd') t.match(opts, npm.flatOptions, 'should forward flat options') - t.end() } config.diff = ['foo@1.0.0'] @@ -124,12 +127,13 @@ t.test('single arg', t => { diff.exec([], err => { if (err) throw err + t.end() }) }) t.test('unknown spec, no package.json', t => { const path = t.testdir({}) - rlp = () => { + rpn = () => { throw new Error('ERR') } @@ -182,7 +186,7 @@ t.test('single arg', t => { }) t.test('version, no package.json', t => { - rlp = () => { + rpn = () => { throw new Error('ERR') } @@ -273,7 +277,7 @@ t.test('single arg', t => { t.test('unknown package name, no package.json', t => { const path = t.testdir({}) - rlp = () => { + rpn = () => { throw new Error('ERR') } @@ -465,7 +469,7 @@ t.test('single arg', t => { const Diff = t.mock('../../lib/diff.js', { ...mocks, - '../../lib/utils/read-local-package.js': async () => 'my-project', + '../../lib/utils/read-package-name.js': async () => 'my-project', pacote: { packument: (spec) => { t.equal(spec.name, 'lorem', 'should have expected spec name') @@ -502,7 +506,7 @@ t.test('single arg', t => { const Diff = t.mock('../../lib/diff.js', { ...mocks, - '../../lib/utils/read-local-package.js': async () => 'my-project', + '../../lib/utils/read-package-name.js': async () => 'my-project', '@npmcli/arborist': class { constructor () { throw new Error('ERR') @@ -528,7 +532,7 @@ t.test('single arg', t => { t.plan(2) const path = t.testdir({}) - rlp = async () => undefined + rpn = async () => undefined libnpmdiff = async ([a, b], opts) => { t.equal(a, 'bar@latest', 'should target latest tag of name') t.equal(b, `file:${path}`, 'should compare to cwd') @@ -547,7 +551,7 @@ t.test('single arg', t => { t.plan(2) const path = t.testdir({}) - rlp = async () => 'my-project' + rpn = async () => 'my-project' libnpmdiff = async ([a, b], opts) => { t.equal(a, 'my-project@latest', 'should target latest tag of name') t.equal(b, `file:${path}`, 'should compare to cwd') @@ -565,7 +569,7 @@ t.test('single arg', t => { t.plan(2) const path = t.testdir({}) - rlp = async () => 'my-project' + rpn = async () => 'my-project' libnpmdiff = async ([a, b], opts) => { t.equal(a, 'file:/path/to/other-dir', 'should target dir') t.equal(b, `file:${path}`, 'should compare to cwd') @@ -580,7 +584,7 @@ t.test('single arg', t => { }) t.test('unsupported spec type', t => { - rlp = async () => 'my-project' + rpn = async () => 'my-project' config.diff = ['git+https://github.com/user/foo'] @@ -634,7 +638,7 @@ t.test('first arg is a qualified spec', t => { }), }) - rlp = async () => 'my-project' + rpn = async () => 'my-project' libnpmdiff = async ([a, b], opts) => { t.equal(a, 'bar@2.0.0', 'should set expected first spec') t.equal(b, `bar@file:${resolve(path, 'node_modules/bar')}`, 'should target local node_modules pkg') @@ -703,7 +707,7 @@ t.test('first arg is a known dependency name', t => { }), }) - rlp = async () => 'my-project' + rpn = async () => 'my-project' libnpmdiff = async ([a, b], opts) => { t.equal(a, `bar@file:${resolve(path, 'node_modules/bar')}`, 'should target local node_modules pkg') t.equal(b, 'bar@2.0.0', 'should set expected second spec') @@ -743,7 +747,7 @@ t.test('first arg is a known dependency name', t => { }), }) - rlp = async () => 'my-project' + rpn = async () => 'my-project' libnpmdiff = async ([a, b], opts) => { t.equal(a, `bar@file:${resolve(path, 'node_modules/bar')}`, 'should target local node_modules pkg') t.equal(b, `bar-fork@file:${resolve(path, 'node_modules/bar-fork')}`, 'should target fork local node_modules pkg') @@ -777,7 +781,7 @@ t.test('first arg is a known dependency name', t => { }), }) - rlp = async () => 'my-project' + rpn = async () => 'my-project' libnpmdiff = async ([a, b], opts) => { t.equal(a, `bar@file:${resolve(path, 'node_modules/bar')}`, 'should target local node_modules pkg') t.equal(b, 'bar@2.0.0', 'should use package name from first arg') @@ -811,7 +815,7 @@ t.test('first arg is a known dependency name', t => { }), }) - rlp = async () => 'my-project' + rpn = async () => 'my-project' libnpmdiff = async ([a, b], opts) => { t.equal(a, `bar@file:${resolve(path, 'node_modules/bar')}`, 'should target local node_modules pkg') t.equal(b, 'bar-fork@latest', 'should set expected second spec') @@ -865,7 +869,7 @@ t.test('first arg is a valid semver range', t => { }), }) - rlp = async () => 'my-project' + rpn = async () => 'my-project' libnpmdiff = async ([a, b], opts) => { t.equal(a, 'bar@1.0.0', 'should use name from second arg') t.equal(b, `bar@file:${resolve(path, 'node_modules/bar')}`, 'should set expected second spec from nm') @@ -882,7 +886,7 @@ t.test('first arg is a valid semver range', t => { t.test('second arg is ALSO a semver version', t => { t.plan(2) - rlp = async () => 'my-project' + rpn = async () => 'my-project' libnpmdiff = async ([a, b], opts) => { t.equal(a, 'my-project@1.0.0', 'should use name from project dir') t.equal(b, 'my-project@2.0.0', 'should use name from project dir') @@ -897,7 +901,7 @@ t.test('first arg is a valid semver range', t => { t.test('second arg is ALSO a semver version BUT cwd not a project dir', t => { const path = t.testdir({}) - rlp = () => { + rpn = () => { throw new Error('ERR') } @@ -916,7 +920,7 @@ t.test('first arg is a valid semver range', t => { t.test('second arg is an unknown dependency name', t => { t.plan(2) - rlp = async () => 'my-project' + rpn = async () => 'my-project' libnpmdiff = async ([a, b], opts) => { t.equal(a, 'bar@1.0.0', 'should use name from second arg') t.equal(b, 'bar@latest', 'should compare against latest tag') @@ -940,7 +944,7 @@ t.test('first arg is a valid semver range', t => { const Diff = t.mock('../../lib/diff.js', { ...mocks, - '../../lib/utils/read-local-package.js': async () => 'my-project', + '../../lib/utils/read-package-name.js': async () => 'my-project', '@npmcli/arborist': class { constructor () { throw new Error('ERR') @@ -1003,7 +1007,7 @@ t.test('first arg is an unknown dependency name', t => { }), }) - rlp = async () => 'my-project' + rpn = async () => 'my-project' libnpmdiff = async ([a, b], opts) => { t.equal(a, 'bar-fork@latest', 'should use latest tag') t.equal(b, `bar@file:${resolve(path, 'node_modules/bar')}`, 'should target local node_modules pkg') @@ -1051,7 +1055,7 @@ t.test('first arg is an unknown dependency name', t => { t.plan(2) const path = t.testdir({}) - rlp = () => { + rpn = () => { throw new Error('ERR') } libnpmdiff = async ([a, b], opts) => { @@ -1117,7 +1121,7 @@ t.test('various options', t => { t.plan(3) const path = t.testdir({}) - rlp = async () => 'my-project' + rpn = async () => 'my-project' libnpmdiff = async ([a, b], opts) => { t.equal(a, 'my-project@latest', 'should have default spec') t.equal(b, `file:${path}`, 'should compare to cwd') diff --git a/test/lib/dist-tag.js b/test/lib/dist-tag.js index 6bc17168cdce0..9af90c309c77c 100644 --- a/test/lib/dist-tag.js +++ b/test/lib/dist-tag.js @@ -93,6 +93,20 @@ t.test('ls in current package', (t) => { }) }) +t.test('ls global', (t) => { + t.teardown(() => { + config.global = false + }) + config.global = true + distTag.exec(['ls'], (err) => { + t.matchSnapshot( + err, + 'should throw basic usage' + ) + t.end() + }) +}) + t.test('no args in current package', (t) => { npm.prefix = t.testdir({ 'package.json': JSON.stringify({ diff --git a/test/lib/owner.js b/test/lib/owner.js index 4af8d1ebbb8fa..10ceb03030a5a 100644 --- a/test/lib/owner.js +++ b/test/lib/owner.js @@ -1,16 +1,18 @@ const t = require('tap') +const mockNpm = require('../fixtures/mock-npm.js') let result = '' -let readLocalPkgResponse = null +let readPackageNamePrefix = null +let readPackageNameResponse = null const noop = () => null -const npm = { - flatOptions: {}, +const npm = mockNpm({ output: (msg) => { result = result ? `${result}\n${msg}` : msg }, -} +}) + const npmFetch = { json: noop } const npmlog = { error: noop, info: noop, verbose: noop } const pacote = { packument: noop } @@ -20,7 +22,10 @@ const mocks = { 'npm-registry-fetch': npmFetch, pacote, '../../lib/utils/otplease.js': async (opts, fn) => fn({ otp: '123456', opts }), - '../../lib/utils/read-local-package.js': async () => readLocalPkgResponse, + '../../lib/utils/read-package-name.js': async (prefix) => { + readPackageNamePrefix = prefix + return readPackageNameResponse + }, '../../lib/utils/usage.js': () => 'usage instructions', } @@ -47,11 +52,11 @@ t.test('owner no args', t => { }) t.test('owner ls no args', t => { - t.plan(4) + t.plan(5) result = '' - readLocalPkgResponse = '@npmcli/map-workspaces' + readPackageNameResponse = '@npmcli/map-workspaces' pacote.packument = async (spec, opts) => { t.equal(spec.name, '@npmcli/map-workspaces', 'should use expect pkg name') t.match( @@ -65,14 +70,29 @@ t.test('owner ls no args', t => { return { maintainers: npmcliMaintainers } } t.teardown(() => { + npm.prefix = null result = '' pacote.packument = noop - readLocalPkgResponse = null + readPackageNameResponse = null }) + npm.prefix = 'test-npm-prefix' owner.exec(['ls'], err => { t.error(err, 'npm owner ls no args') t.matchSnapshot(result, 'should output owners of cwd package') + t.equal(readPackageNamePrefix, 'test-npm-prefix', 'read-package-name gets npm.prefix') + }) +}) + +t.test('owner ls global', t => { + t.teardown(() => { + npm.config.set('global', false) + }) + npm.config.set('global', true) + + owner.exec(['ls'], err => { + t.match(err, /usage instructions/, 'should throw usage instructions if no cwd package available') + t.end() }) }) @@ -93,7 +113,7 @@ t.test('owner ls fails to retrieve packument', t => { t.plan(4) result = '' - readLocalPkgResponse = '@npmcli/map-workspaces' + readPackageNameResponse = '@npmcli/map-workspaces' pacote.packument = () => { throw new Error('ERR') } @@ -233,7 +253,7 @@ t.test('owner add ', t => { t.test('owner add cwd package', t => { result = '' - readLocalPkgResponse = '@npmcli/map-workspaces' + readPackageNameResponse = '@npmcli/map-workspaces' npmFetch.json = async (uri, opts) => { // retrieve user info from couchdb request if (uri === '/-/user/org.couchdb.user:foo') { @@ -253,7 +273,7 @@ t.test('owner add cwd package', t => { }) t.teardown(() => { result = '' - readLocalPkgResponse = null + readPackageNameResponse = null npmFetch.json = noop pacote.packument = noop }) @@ -308,7 +328,7 @@ t.test('owner add already an owner', t => { t.test('owner add fails to retrieve user', t => { result = '' - readLocalPkgResponse = + readPackageNameResponse = npmFetch.json = async (uri, opts) => { // retrieve borked user info from couchdb request if (uri === '/-/user/org.couchdb.user:foo') @@ -324,7 +344,7 @@ t.test('owner add fails to retrieve user', t => { }) t.teardown(() => { result = '' - readLocalPkgResponse = null + readPackageNameResponse = null npmFetch.json = noop pacote.packument = noop }) @@ -465,6 +485,18 @@ t.test('owner add no user', t => { }) }) +t.test('owner add no pkg global', t => { + t.teardown(() => { + npm.config.set('global', false) + }) + npm.config.set('global', true) + + owner.exec(['add', 'gar'], err => { + t.match(err, /usage instructions/, 'should throw usage instructions if user provided') + t.end() + }) +}) + t.test('owner add no cwd package', t => { result = '' t.teardown(() => { @@ -581,7 +613,7 @@ t.test('owner rm not a current owner', t => { t.test('owner rm cwd package', t => { result = '' - readLocalPkgResponse = '@npmcli/map-workspaces' + readPackageNameResponse = '@npmcli/map-workspaces' npmFetch.json = async (uri, opts) => { // retrieve user info from couchdb request if (uri === '/-/user/org.couchdb.user:ruyadorno') { @@ -601,7 +633,7 @@ t.test('owner rm cwd package', t => { }) t.teardown(() => { result = '' - readLocalPkgResponse = null + readPackageNameResponse = null npmFetch.json = noop pacote.packument = noop }) @@ -615,7 +647,7 @@ t.test('owner rm cwd package', t => { t.test('owner rm only user', t => { result = '' - readLocalPkgResponse = 'ipt' + readPackageNameResponse = 'ipt' npmFetch.json = async (uri, opts) => { // retrieve user info from couchdb request if (uri === '/-/user/org.couchdb.user:ruyadorno') { @@ -636,7 +668,7 @@ t.test('owner rm only user', t => { }) t.teardown(() => { result = '' - readLocalPkgResponse = null + readPackageNameResponse = null npmFetch.json = noop pacote.packument = noop }) @@ -664,6 +696,18 @@ t.test('owner rm no user', t => { }) }) +t.test('owner rm no pkg global', t => { + t.teardown(() => { + npm.config.set('global', false) + }) + npm.config.set('global', true) + + owner.exec(['rm', 'gar'], err => { + t.match(err, /usage instructions/, 'should throw usage instructions if user provided') + t.end() + }) +}) + t.test('owner rm no cwd package', t => { result = '' t.teardown(() => { @@ -693,15 +737,15 @@ t.test('completion', async t => { // npm owner rm completion is async t.test('completion npm owner rm', async t => { t.plan(2) - readLocalPkgResponse = '@npmcli/map-workspaces' + readPackageNameResponse = '@npmcli/map-workspaces' pacote.packument = async spec => { - t.equal(spec.name, readLocalPkgResponse, 'should use package spec') + t.equal(spec.name, readPackageNameResponse, 'should use package spec') return { maintainers: npmcliMaintainers, } } t.teardown(() => { - readLocalPkgResponse = null + readPackageNameResponse = null pacote.packument = noop }) @@ -718,17 +762,27 @@ t.test('completion', async t => { t.end() }) + t.test('completion npm owner rm global', async t => { + t.teardown(() => { + npm.config.set('global', false) + }) + npm.config.set('global', true) + const res = await owner.completion({ conf: { argv: { remain: ['npm', 'owner', 'rm'] } } }) + t.strictSame(res, [], 'should have no owners to autocomplete if global') + t.end() + }) + t.test('completion npm owner rm no owners found', async t => { t.plan(2) - readLocalPkgResponse = '@npmcli/map-workspaces' + readPackageNameResponse = '@npmcli/map-workspaces' pacote.packument = async spec => { - t.equal(spec.name, readLocalPkgResponse, 'should use package spec') + t.equal(spec.name, readPackageNameResponse, 'should use package spec') return { maintainers: [], } } t.teardown(() => { - readLocalPkgResponse = null + readPackageNameResponse = null pacote.packument = noop }) diff --git a/test/lib/utils/read-local-package.js b/test/lib/utils/read-package-name.js similarity index 55% rename from test/lib/utils/read-local-package.js rename to test/lib/utils/read-package-name.js index 966e74a7ab7f4..c8f88bacd4b84 100644 --- a/test/lib/utils/read-local-package.js +++ b/test/lib/utils/read-package-name.js @@ -1,13 +1,8 @@ const t = require('tap') const mockNpm = require('../../fixtures/mock-npm') +const npm = mockNpm() -const config = { - json: false, - global: false, -} -const npm = mockNpm({ config }) - -const readLocalPackageName = require('../../../lib/utils/read-local-package.js') +const readPackageName = require('../../../lib/utils/read-package-name.js') t.test('read local package.json', async (t) => { npm.prefix = t.testdir({ @@ -16,7 +11,7 @@ t.test('read local package.json', async (t) => { version: '1.0.0', }), }) - const packageName = await readLocalPackageName(npm) + const packageName = await readPackageName(npm.prefix) t.equal( packageName, 'my-local-package', @@ -31,22 +26,10 @@ t.test('read local scoped-package.json', async (t) => { version: '1.0.0', }), }) - const packageName = await readLocalPackageName(npm) + const packageName = await readPackageName(npm.prefix) t.equal( packageName, '@my-scope/my-local-package', 'should retrieve scoped package name' ) }) - -t.test('read using --global', async (t) => { - npm.prefix = t.testdir({}) - config.global = true - const packageName = await readLocalPackageName(npm) - t.equal( - packageName, - undefined, - 'should not retrieve a package name' - ) - config.global = false -})