diff options
author | Ruy Adorno <ruyadorno@hotmail.com> | 2020-12-08 22:10:20 +0300 |
---|---|---|
committer | isaacs <i@izs.me> | 2020-12-09 02:58:51 +0300 |
commit | b7d74b627859f08fca23209d6e0d3ec6657a4489 (patch) | |
tree | be0f9ad88faa5a47d2150ebfcc0446f4b4ef014d | |
parent | 3db90d94474f673591811fdab5eb6a5bfdeba261 (diff) |
fix: minor tweaks to lib/unpublish.js
- Fix handling missing files error on reading package.json
- Fixes autocompletion
- Fixes printing name and version when using no args
- Adds `test/lib/unpublish.js` tests
Fixes: https://github.com/npm/statusboard/issues/180
PR-URL: https://github.com/npm/cli/pull/2304
Credit: @ruyadorno
Close: #2304
Reviewed-by: @isaacs
-rw-r--r-- | lib/unpublish.js | 55 | ||||
-rw-r--r-- | test/lib/unpublish.js | 506 |
2 files changed, 537 insertions, 24 deletions
diff --git a/lib/unpublish.js b/lib/unpublish.js index 4d05627d2..d6dbc8d6e 100644 --- a/lib/unpublish.js +++ b/lib/unpublish.js @@ -1,3 +1,5 @@ +'use strict' + const path = require('path') const util = require('util') const log = require('npmlog') @@ -11,7 +13,7 @@ const npm = require('./npm.js') const usageUtil = require('./utils/usage.js') const output = require('./utils/output.js') const otplease = require('./utils/otplease.js') -const whoami = util.promisify(require('./whoami.js')) +const getIdentity = require('./utils/get-identity.js') const usage = usageUtil('unpublish', 'npm unpublish [<@scope>/]<pkg>[@<version>]') @@ -25,18 +27,18 @@ const completionFn = async (args) => { const { partialWord, conf } = args if (conf.argv.remain.length >= 3) - return + return [] - const username = await whoami([], true) + const opts = npm.flatOptions + const username = await getIdentity({ ...opts }).catch(() => null) if (!username) return [] - const opts = npm.flatOptions const access = await libaccess.lsPackages(username, opts) // do a bit of filtering at this point, so that we don't need // to fetch versions for more than one thing, but also don't // accidentally a whole project - let pkgs = Object.keys(access) + let pkgs = Object.keys(access || {}) if (!partialWord || !pkgs.length) return pkgs @@ -55,18 +57,20 @@ const completionFn = async (args) => { async function unpublish (args) { if (args.length > 1) - throw usage + throw new Error(usage) const spec = args.length && npa(args[0]) const opts = npm.flatOptions const { force, silent, loglevel } = opts - let ret + let res + let pkgName + let pkgVersion log.silly('unpublish', 'args[0]', args[0]) log.silly('unpublish', 'spec', spec) if (!spec.rawSpec && !force) { - throw ( + throw new Error( 'Refusing to delete entire project.\n' + 'Run with --force to do this.\n' + usage @@ -77,31 +81,34 @@ async function unpublish (args) { // if there's a package.json in the current folder, then // read the package name and version out of that. const pkgJson = path.join(npm.localPrefix, 'package.json') - const manifest = await readJson(pkgJson) - - log.verbose('unpublish', manifest) - - const { name, version, publishConfig } = manifest - const pkgJsonSpec = npa.resolve(name, version) - + let manifest try { - ret = await otplease(opts, opts => libunpub(pkgJsonSpec, { ...opts, publishConfig })) + manifest = await readJson(pkgJson) } catch (err) { if (err && err.code !== 'ENOENT' && err.code !== 'ENOTDIR') throw err else - throw `Usage: ${usage}` + throw new Error(`Usage: ${usage}`) } - } else - ret = await otplease(opts, opts => libunpub(spec, opts)) - if (!silent && loglevel !== 'silent') { - output(`- ${spec.name}${ - spec.type === 'version' ? `@${spec.rawSpec}` : '' - }`) + log.verbose('unpublish', manifest) + + const { name, version, publishConfig } = manifest + const pkgJsonSpec = npa.resolve(name, version) + + res = await otplease(opts, opts => libunpub(pkgJsonSpec, { ...opts, publishConfig })) + pkgName = name + pkgVersion = version ? `@${version}` : '' + } else { + res = await otplease(opts, opts => libunpub(spec, opts)) + pkgName = spec.name + pkgVersion = spec.type === 'version' ? `@${spec.rawSpec}` : '' } - return ret + if (!silent && loglevel !== 'silent') + output(`- ${pkgName}${pkgVersion}`) + + return res } module.exports = Object.assign(cmd, { completion, usage }) diff --git a/test/lib/unpublish.js b/test/lib/unpublish.js new file mode 100644 index 000000000..11e24714d --- /dev/null +++ b/test/lib/unpublish.js @@ -0,0 +1,506 @@ +const t = require('tap') +const requireInject = require('require-inject') + +let result = '' +const noop = () => null +const npm = { + localPrefix: '', + flatOptions: { + force: false, + silent: false, + loglevel: 'silly', + }, +} +const mocks = { + npmlog: { silly () {}, verbose () {} }, + libnpmaccess: { lsPackages: noop }, + libnpmpublish: { unpublish: noop }, + 'npm-package-arg': noop, + 'npm-registry-fetch': { json: noop }, + 'read-package-json': cb => cb(), + '../../lib/npm.js': npm, + '../../lib/utils/output.js': (...msg) => { + result += msg.join('\n') + }, + '../../lib/utils/otplease.js': async (opts, fn) => fn(opts), + '../../lib/utils/usage.js': () => 'usage instructions', + '../../lib/utils/get-identity.js': async () => 'foo', +} + +t.afterEach(cb => { + result = '' + npm.flatOptions.force = false + npm.flatOptions.loglevel = 'silly' + npm.flatOptions.silent = false + cb() +}) + +t.test('no args --force', t => { + t.plan(9) + + npm.flatOptions.force = true + + const npmlog = { + silly (title) { + t.equal(title, 'unpublish', 'should silly log args') + }, + verbose (title, msg) { + t.equal(title, 'unpublish', 'should have expected title') + t.match( + msg, + { name: 'pkg', version: '1.0.0' }, + 'should have msg printing package.json contents' + ) + }, + } + + const npa = { + resolve (name, version) { + t.equal(name, 'pkg', 'should npa.resolve package name') + t.equal(version, '1.0.0', 'should npa.resolve package version') + return 'pkg@1.0.0' + }, + } + + const libnpmpublish = { + unpublish (spec, opts) { + t.equal(spec, 'pkg@1.0.0', 'should unpublish expected spec') + t.deepEqual( + opts, + { + force: true, + silent: false, + loglevel: 'silly', + publishConfig: undefined, + }, + 'should unpublish with expected opts' + ) + }, + } + + const unpublish = requireInject('../../lib/unpublish.js', { + ...mocks, + npmlog, + libnpmpublish, + 'npm-package-arg': npa, + 'read-package-json': (path, cb) => cb(null, { + name: 'pkg', + version: '1.0.0', + }), + }) + + unpublish([], err => { + if (err) + throw err + + t.equal( + result, + '- pkg@1.0.0', + 'should output removed pkg@version on success' + ) + }) +}) + +t.test('no args --force missing package.json', t => { + npm.flatOptions.force = true + + const unpublish = requireInject('../../lib/unpublish.js', { + ...mocks, + 'read-package-json': (path, cb) => cb(Object.assign( + new Error('ENOENT'), + { code: 'ENOENT' } + )), + }) + + unpublish([], err => { + t.match( + err, + /usage instructions/, + 'should throw usage instructions on missing package.json' + ) + t.end() + }) +}) + +t.test('no args --force unknown error reading package.json', t => { + npm.flatOptions.force = true + + const unpublish = requireInject('../../lib/unpublish.js', { + ...mocks, + 'read-package-json': (path, cb) => cb(new Error('ERR')), + }) + + unpublish([], err => { + t.match( + err, + /ERR/, + 'should throw unknown error from reading package.json' + ) + t.end() + }) +}) + +t.test('no args', t => { + const unpublish = requireInject('../../lib/unpublish.js', { + ...mocks, + }) + + unpublish([], err => { + t.match( + err, + /Refusing to delete entire project/, + 'should throw --force required error on no args' + ) + t.end() + }) +}) + +t.test('too many args', t => { + const unpublish = requireInject('../../lib/unpublish.js', { + ...mocks, + }) + + unpublish(['a', 'b'], err => { + t.match( + err, + /usage instructions/, + 'should throw usage instructions if too many args' + ) + t.end() + }) +}) + +t.test('unpublish <pkg>@version', t => { + t.plan(7) + + const pa = { + name: 'pkg', + rawSpec: '1.0.0', + type: 'version', + } + + const npmlog = { + silly (title, key, value) { + t.equal(title, 'unpublish', 'should silly log args') + if (key === 'spec') + t.equal(value, pa, 'should log parsed npa object') + else + t.equal(value, 'pkg@1.0.0', 'should log originally passed arg') + }, + } + + const npa = () => pa + + const libnpmpublish = { + unpublish (spec, opts) { + t.equal(spec, pa, 'should unpublish expected parsed spec') + t.deepEqual( + opts, + { + force: false, + silent: false, + loglevel: 'silly', + }, + 'should unpublish with expected opts' + ) + }, + } + + const unpublish = requireInject('../../lib/unpublish.js', { + ...mocks, + npmlog, + libnpmpublish, + 'npm-package-arg': npa, + }) + + unpublish(['pkg@1.0.0'], err => { + if (err) + throw err + + t.equal( + result, + '- pkg@1.0.0', + 'should output removed pkg@version on success' + ) + }) +}) + +t.test('no version found in package.json', t => { + npm.flatOptions.force = true + + const npa = () => ({ + name: 'pkg', + type: 'version', + }) + + npa.resolve = () => '' + + const unpublish = requireInject('../../lib/unpublish.js', { + ...mocks, + 'npm-package-arg': npa, + 'read-package-json': (path, cb) => cb(null, { + name: 'pkg', + }), + }) + + unpublish([], err => { + if (err) + throw err + + t.equal( + result, + '- pkg', + 'should output removed pkg on success' + ) + t.end() + }) +}) + +t.test('unpublish <pkg> --force no version set', t => { + npm.flatOptions.force = true + + const unpublish = requireInject('../../lib/unpublish.js', { + ...mocks, + 'npm-package-arg': () => ({ + name: 'pkg', + rawSpec: '', + type: 'tag', + }), + }) + + unpublish(['pkg'], err => { + if (err) + throw err + + t.equal( + result, + '- pkg', + 'should output pkg removed' + ) + t.end() + }) +}) + +t.test('silent', t => { + npm.flatOptions.loglevel = 'silent' + + const npa = () => ({ + name: 'pkg', + rawSpec: '1.0.0', + type: 'version', + }) + + npa.resolve = () => '' + + const unpublish = requireInject('../../lib/unpublish.js', { + ...mocks, + 'npm-package-arg': npa, + }) + + unpublish(['pkg@1.0.0'], err => { + if (err) + throw err + + t.equal( + result, + '', + 'should have no output' + ) + t.end() + }) +}) + +t.test('completion', t => { + const testComp = (t, { completion, argv, partialWord, expect, title }) => + new Promise((resolve, rej) => { + completion({conf: {argv: {remain: argv}}, partialWord}, (er, res) => { + if (er) + rej(er) + t.strictSame(res, expect, title || argv.join(' ')) + resolve() + }) + }) + + t.test('completing with multiple versions from the registry', async t => { + const { completion } = requireInject('../../lib/unpublish.js', { + ...mocks, + libnpmaccess: { + async lsPackages () { + return { + pkg: 'write', + bar: 'write', + } + }, + }, + 'npm-package-arg': require('npm-package-arg'), + 'npm-registry-fetch': { + async json () { + return { + versions: { + '1.0.0': {}, + '1.0.1': {}, + '2.0.0': {}, + }, + } + }, + }, + }) + + await testComp(t, { + completion, + argv: ['npm', 'unpublish'], + partialWord: 'pkg', + expect: [ + 'pkg@1.0.0', + 'pkg@1.0.1', + 'pkg@2.0.0', + ], + }) + }) + + t.test('no versions retrieved', async t => { + const { completion } = requireInject('../../lib/unpublish.js', { + ...mocks, + libnpmaccess: { + async lsPackages () { + return { + pkg: 'write', + bar: 'write', + } + }, + }, + 'npm-package-arg': require('npm-package-arg'), + 'npm-registry-fetch': { + async json () { + return { + versions: {}, + } + }, + }, + }) + + await testComp(t, { + completion, + argv: ['npm', 'unpublish'], + partialWord: 'pkg', + expect: [ + 'pkg', + ], + title: 'should autocomplete package name only', + }) + }) + + t.test('packages starting with same letters', async t => { + const { completion } = requireInject('../../lib/unpublish.js', { + ...mocks, + libnpmaccess: { + async lsPackages () { + return { + pkg: 'write', + pkga: 'write', + pkgb: 'write', + } + }, + }, + 'npm-package-arg': require('npm-package-arg'), + }) + + await testComp(t, { + completion, + argv: ['npm', 'unpublish'], + partialWord: 'pkg', + expect: [ + 'pkg', + 'pkga', + 'pkgb', + ], + }) + }) + + t.test('no packages retrieved', async t => { + const { completion } = requireInject('../../lib/unpublish.js', { + ...mocks, + libnpmaccess: { + async lsPackages () { + return {} + }, + }, + }) + + await testComp(t, { + completion, + argv: ['npm', 'unpublish'], + partialWord: 'pkg', + expect: [], + title: 'should have no autocompletion', + }) + }) + + t.test('no pkg name to complete', async t => { + const { completion } = requireInject('../../lib/unpublish.js', { + ...mocks, + libnpmaccess: { + async lsPackages () { + return { + pkg: {}, + bar: {}, + } + }, + }, + }) + + await testComp(t, { + completion, + argv: ['npm', 'unpublish'], + partialWord: undefined, + expect: ['pkg', 'bar'], + title: 'should autocomplete with available package names from user', + }) + }) + + t.test('no pkg names retrieved from user account', async t => { + const { completion } = requireInject('../../lib/unpublish.js', { + ...mocks, + libnpmaccess: { + async lsPackages () { + return null + }, + }, + }) + + await testComp(t, { + completion, + argv: ['npm', 'unpublish'], + partialWord: 'pkg', + expect: [], + title: 'should have no autocomplete', + }) + }) + + t.test('logged out user', async t => { + const { completion } = requireInject('../../lib/unpublish.js', { + ...mocks, + '../../lib/utils/get-identity.js': () => Promise.reject(new Error('ERR')), + }) + + await testComp(t, { + completion, + argv: ['npm', 'unpublish'], + partialWord: 'pkg', + expect: [], + }) + }) + + t.test('too many args', async t => { + const { completion } = requireInject('../../lib/unpublish.js', mocks) + + await testComp(t, { + completion, + argv: ['npm', 'unpublish', 'foo'], + partialWord: undefined, + expect: [], + }) + }) + + t.end() +}) |