From a2b0c94127c99f803c0c7e9ed0583d6177071c6c Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 30 Mar 2022 07:09:57 -0700 Subject: [PATCH] fix: remove lib/utils/read-package-name.js This code wasn't doing anything special, just dereferencing `name` from a packument. There is no need for this to exist. Most of the tests were able to handle having this go away, except for `npm owner` which had to have its tests rewritten to be real, which of course surfaced bugs along the way of behavior that was incorrectly being tested. `npm owner` needs some love to clean up its UX, it throws or returns inconsistently. I did fix it so that if there is no package.json in cwd it errored as expected instead of throwing `ENOENT` which is what it did before. --- lib/commands/diff.js | 11 +- lib/commands/dist-tag.js | 9 +- lib/commands/owner.js | 64 +- .../test/lib/commands/owner.js.test.cjs | 20 - test/fixtures/mock-registry.js | 15 +- test/lib/commands/diff.js | 2 - test/lib/commands/owner.js | 994 +++++++----------- test/lib/utils/read-package-name.js | 33 - 8 files changed, 426 insertions(+), 722 deletions(-) delete mode 100644 tap-snapshots/test/lib/commands/owner.js.test.cjs delete mode 100644 test/lib/utils/read-package-name.js diff --git a/lib/commands/diff.js b/lib/commands/diff.js index ff942cc44e946..11ee78265e62a 100644 --- a/lib/commands/diff.js +++ b/lib/commands/diff.js @@ -6,7 +6,7 @@ const Arborist = require('@npmcli/arborist') const pacote = require('pacote') const pickManifest = require('npm-pick-manifest') const log = require('../utils/log-shim') -const readPackageName = require('../utils/read-package-name.js') +const readPackage = require('read-package-json-fast') const BaseCommand = require('../base-command.js') class Diff extends BaseCommand { @@ -81,7 +81,8 @@ class Diff extends BaseCommand { async packageName (path) { let name try { - name = await readPackageName(this.prefix) + const pkg = await readPackage(resolve(this.prefix, 'package.json')) + name = pkg.name } catch (e) { log.verbose('diff', 'could not read project dir package.json') } @@ -114,7 +115,8 @@ class Diff extends BaseCommand { let noPackageJson let pkgName try { - pkgName = await readPackageName(this.prefix) + const pkg = await readPackage(resolve(this.prefix, 'package.json')) + pkgName = pkg.name } catch (e) { log.verbose('diff', 'could not read project dir package.json') noPackageJson = true @@ -225,7 +227,8 @@ class Diff extends BaseCommand { if (semverA && semverB) { let pkgName try { - pkgName = await readPackageName(this.prefix) + const pkg = await readPackage(resolve(this.prefix, 'package.json')) + pkgName = pkg.name } catch (e) { log.verbose('diff', 'could not read project dir package.json') } diff --git a/lib/commands/dist-tag.js b/lib/commands/dist-tag.js index 3b82c5194cca8..42cad80df0073 100644 --- a/lib/commands/dist-tag.js +++ b/lib/commands/dist-tag.js @@ -1,9 +1,10 @@ const npa = require('npm-package-arg') +const path = require('path') const regFetch = require('npm-registry-fetch') const semver = require('semver') const log = require('../utils/log-shim') const otplease = require('../utils/otplease.js') -const readPackageName = require('../utils/read-package-name.js') +const readPackage = require('read-package-json-fast') const BaseCommand = require('../base-command.js') class DistTag extends BaseCommand { @@ -150,12 +151,12 @@ class DistTag extends BaseCommand { if (this.npm.config.get('global')) { throw this.usageError() } - const pkg = await readPackageName(this.npm.prefix) - if (!pkg) { + const { name } = await readPackage(path.resolve(this.npm.prefix, 'package.json')) + if (!name) { throw this.usageError() } - return this.list(pkg, opts) + return this.list(name, opts) } spec = npa(spec) diff --git a/lib/commands/owner.js b/lib/commands/owner.js index 07f71c5974768..285b06be8e5fe 100644 --- a/lib/commands/owner.js +++ b/lib/commands/owner.js @@ -3,8 +3,18 @@ const npmFetch = require('npm-registry-fetch') const pacote = require('pacote') const log = require('../utils/log-shim') const otplease = require('../utils/otplease.js') -const readLocalPkgName = require('../utils/read-package-name.js') +const readPackageJsonFast = require('read-package-json-fast') const BaseCommand = require('../base-command.js') +const { resolve } = require('path') + +const readJson = async (pkg) => { + try { + const json = await readPackageJsonFast(pkg) + return json + } catch { + return {} + } +} class Owner extends BaseCommand { static description = 'Manage package owners' @@ -41,12 +51,12 @@ class Owner extends BaseCommand { if (this.npm.config.get('global')) { return [] } - const pkgName = await readLocalPkgName(this.npm.prefix) - if (!pkgName) { + const { name } = await readJson(resolve(this.npm.prefix, 'package.json')) + if (!name) { return [] } - const spec = npa(pkgName) + const spec = npa(name) const data = await pacote.packument(spec, { ...this.npm.flatOptions, fullMetadata: true, @@ -96,12 +106,12 @@ class Owner extends BaseCommand { if (this.npm.config.get('global')) { throw this.usageError() } - const pkgName = await readLocalPkgName(this.npm.prefix) - if (!pkgName) { + const { name } = await readJson(resolve(this.npm.prefix, 'package.json')) + if (!name) { throw this.usageError() } - return pkgName + return name } return pkg } @@ -125,15 +135,6 @@ class Owner extends BaseCommand { throw err } - if (!u || !u.name || u.error) { - throw Object.assign( - new Error( - "Couldn't get user data for " + user + ': ' + JSON.stringify(u) - ), - { code: 'EOWNERUSER' } - ) - } - // normalize user data u = { name: u.name, email: u.email } @@ -177,32 +178,31 @@ class Owner extends BaseCommand { } const dataPath = `/${spec.escapedName}/-rev/${encodeURIComponent(data._rev)}` - const res = await otplease(this.npm.flatOptions, opts => { - return npmFetch.json(dataPath, { - ...opts, - method: 'PUT', - body: { - _id: data._id, - _rev: data._rev, - maintainers, - }, - spec, + try { + const res = await otplease(this.npm.flatOptions, opts => { + return npmFetch.json(dataPath, { + ...opts, + method: 'PUT', + body: { + _id: data._id, + _rev: data._rev, + maintainers, + }, + spec, + }) }) - }) - - if (!res.error) { if (addOrRm === 'add') { this.npm.output(`+ ${user} (${spec.name})`) } else { this.npm.output(`- ${user} (${spec.name})`) } - } else { + return res + } catch (err) { throw Object.assign( - new Error('Failed to update package: ' + JSON.stringify(res)), + new Error('Failed to update package: ' + JSON.stringify(err.message)), { code: 'EOWNERMUTATE' } ) } - return res } } diff --git a/tap-snapshots/test/lib/commands/owner.js.test.cjs b/tap-snapshots/test/lib/commands/owner.js.test.cjs deleted file mode 100644 index f3d7335e47307..0000000000000 --- a/tap-snapshots/test/lib/commands/owner.js.test.cjs +++ /dev/null @@ -1,20 +0,0 @@ -/* IMPORTANT - * This snapshot file is auto-generated, but designed for humans. - * It should be checked into source control and tracked carefully. - * Re-generate by setting TAP_SNAPSHOT=1 and running tests. - * Make sure to inspect the output below. Do not ignore changes! - */ -'use strict' -exports[`test/lib/commands/owner.js TAP owner ls > should output owners of 1`] = ` -nlf -ruyadorno -darcyclarke -isaacs -` - -exports[`test/lib/commands/owner.js TAP owner ls no args > should output owners of cwd package 1`] = ` -nlf -ruyadorno -darcyclarke -isaacs -` diff --git a/test/fixtures/mock-registry.js b/test/fixtures/mock-registry.js index 8b847213f33a7..5e39dcf48315a 100644 --- a/test/fixtures/mock-registry.js +++ b/test/fixtures/mock-registry.js @@ -5,6 +5,7 @@ * for tests against any registry data. */ const pacote = require('pacote') +const npa = require('npm-package-arg') class MockRegistry { #tap #nock @@ -84,6 +85,16 @@ class MockRegistry { ).reply(200) } + couchuser ({ username, body, responseCode = 200 }) { + if (body) { + this.nock = this.nock.get(`/-/user/org.couchdb.user:${encodeURIComponent(username)}`) + .reply(responseCode, body) + } else { + this.nock = this.nock.get(`/-/user/org.couchdb.user:${encodeURIComponent(username)}`) + .reply(responseCode, { _id: `org.couchdb.user:${username}`, email: '', name: username }) + } + } + couchlogin ({ username, password, email, otp, token = 'npm_default-test-token' }) { this.nock = this.nock .post('/-/v1/login').reply(401, { error: 'You must be logged in to publish packages.' }) @@ -156,7 +167,8 @@ class MockRegistry { async package ({ manifest, times = 1, query, tarballs }) { let nock = this.nock - nock = nock.get(`/${manifest.name}`).times(times) + const spec = npa(manifest.name) + nock = nock.get(`/${spec.escapedName}`).times(times) if (query) { nock = nock.query(query) } @@ -203,6 +215,7 @@ class MockRegistry { dist: { tarball: `${this.#registry}/${name}/-/${name}-${packument.version}.tgz`, }, + maintainers: [], ...packument, } manifest.time[packument.version] = new Date() diff --git a/test/lib/commands/diff.js b/test/lib/commands/diff.js index c2b1a935da874..0adaa6568d8f7 100644 --- a/test/lib/commands/diff.js +++ b/test/lib/commands/diff.js @@ -417,7 +417,6 @@ t.test('single arg', t => { const Diff = t.mock('../../../lib/commands/diff.js', { ...mocks, - '../../../lib/utils/read-package-name.js': async () => 'my-project', pacote: { packument: spec => { t.equal(spec.name, 'lorem', 'should have expected spec name') @@ -455,7 +454,6 @@ t.test('single arg', t => { const Diff = t.mock('../../../lib/commands/diff.js', { ...mocks, - '../../../lib/utils/read-package-name.js': async () => 'my-project', '@npmcli/arborist': class { constructor () { throw new Error('ERR') diff --git a/test/lib/commands/owner.js b/test/lib/commands/owner.js index eadfa2bf08b56..d80ce36fece98 100644 --- a/test/lib/commands/owner.js +++ b/test/lib/commands/owner.js @@ -1,760 +1,502 @@ const t = require('tap') -const { fake: mockNpm } = require('../../fixtures/mock-npm.js') +const { load: loadMockNpm } = require('../../fixtures/mock-npm.js') +const MockRegistry = require('../../fixtures/mock-registry.js') -let result = '' -let readPackageNamePrefix = null -let readPackageNameResponse = null +const npa = require('npm-package-arg') +const packageName = '@npmcli/test-package' +const spec = npa(packageName) +const auth = { '//registry.npmjs.org/:_authToken': 'test-auth-token' } -const noop = () => null +const maintainers = [ + { email: 'test-user-a@npmjs.org', name: 'test-user-a' }, + { email: 'test-user-b@npmjs.org', name: 'test-user-b' }, +] -const npm = mockNpm({ - output: (msg) => { - result = result ? `${result}\n${msg}` : msg - }, +t.test('owner no args', async t => { + const { npm } = await loadMockNpm(t) + await t.rejects( + npm.exec('owner', []), + { code: 'EUSAGE' }, + 'rejects with usage' + ) }) -const npmFetch = { json: noop } -const log = { error: noop, info: noop, verbose: noop } -const pacote = { packument: noop } - -const mocks = { - 'proc-log': log, - 'npm-registry-fetch': npmFetch, - pacote, - '../../../lib/utils/read-package-name.js': async (prefix) => { - readPackageNamePrefix = prefix - return readPackageNameResponse - }, -} - -const npmcliMaintainers = [ - { email: 'quitlahok@gmail.com', name: 'nlf' }, - { email: 'ruyadorno@hotmail.com', name: 'ruyadorno' }, - { email: 'darcy@darcyclarke.me', name: 'darcyclarke' }, - { email: 'i@izs.me', name: 'isaacs' }, -] - -const Owner = t.mock('../../../lib/commands/owner.js', mocks) -const owner = new Owner(npm) +t.test('owner ls no args', async t => { + const { npm, joinedOutput } = await loadMockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ name: packageName }), + }, + }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) -t.test('owner no args', async t => { - result = '' - t.teardown(() => { - result = '' + const manifest = registry.manifest({ + name: packageName, + packuments: [{ maintainers, version: '1.0.0' }], }) + registry.package({ manifest }) - await t.rejects( - owner.exec([]), - owner.usage) + await npm.exec('owner', ['ls']) + t.match(joinedOutput(), maintainers.map(m => `${m.name} <${m.email}>`).join('\n')) }) -t.test('owner ls no args', async t => { - t.plan(4) - - result = '' - - readPackageNameResponse = '@npmcli/map-workspaces' - pacote.packument = async (spec, opts) => { - t.equal(spec.name, '@npmcli/map-workspaces', 'should use expect pkg name') - t.match( - opts, - { - ...npm.flatOptions, - fullMetadata: true, - }, - 'should forward expected options to pacote.packument' - ) - return { maintainers: npmcliMaintainers } - } - t.teardown(() => { - npm.prefix = null - result = '' - pacote.packument = noop - readPackageNameResponse = null - }) - npm.prefix = 'test-npm-prefix' - - await owner.exec(['ls']) - t.matchSnapshot(result, 'should output owners of cwd package') - t.equal(readPackageNamePrefix, 'test-npm-prefix', 'read-package-name gets npm.prefix') +t.test('local package.json has no name', async t => { + const { npm } = await loadMockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ hello: 'world' }), + }, + }) + await t.rejects( + npm.exec('owner', ['ls']), + { code: 'EUSAGE' } + ) }) t.test('owner ls global', async t => { - t.teardown(() => { - npm.config.set('global', false) + const { npm } = await loadMockNpm(t, { + config: { global: true }, }) - npm.config.set('global', true) await t.rejects( - owner.exec(['ls']), - owner.usage + npm.exec('owner', ['ls']), + { code: 'EUSAGE' }, + 'rejects with usage' ) }) t.test('owner ls no args no cwd package', async t => { - result = '' - t.teardown(() => { - result = '' - log.error = noop - }) + const { npm } = await loadMockNpm(t) await t.rejects( - owner.exec(['ls']), - owner.usage + npm.exec('owner', ['ls']) ) }) t.test('owner ls fails to retrieve packument', async t => { - t.plan(4) - - result = '' - readPackageNameResponse = '@npmcli/map-workspaces' - pacote.packument = () => { - throw new Error('ERR') - } - log.error = (title, msg, pkgName) => { - t.equal(title, 'owner ls', 'should list npm owner ls title') - t.equal(msg, "Couldn't get owner data", 'should use expected msg') - t.equal(pkgName, '@npmcli/map-workspaces', 'should use pkg name') - } - t.teardown(() => { - result = '' - log.error = noop - pacote.packument = noop - }) - - await t.rejects( - owner.exec(['ls']), - /ERR/, - 'should throw unknown error' - ) + const { npm, logs } = await loadMockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ name: packageName }), + }, + }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + registry.nock.get(`/${spec.escapedName}`).reply(404) + await t.rejects(npm.exec('owner', ['ls'])) + t.match(logs.error, [['owner ls', "Couldn't get owner data", '@npmcli/test-package']]) }) t.test('owner ls ', async t => { - t.plan(3) - - result = '' - pacote.packument = async (spec, opts) => { - t.equal(spec.name, '@npmcli/map-workspaces', 'should use expect pkg name') - t.match( - opts, - { - ...npm.flatOptions, - fullMetadata: true, - }, - 'should forward expected options to pacote.packument' - ) - return { maintainers: npmcliMaintainers } - } - t.teardown(() => { - result = '' - pacote.packument = noop - }) - - await owner.exec(['ls', '@npmcli/map-workspaces']) - t.matchSnapshot(result, 'should output owners of ') + const { npm, joinedOutput } = await loadMockNpm(t) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + + const manifest = registry.manifest({ + name: packageName, + packuments: [{ maintainers, version: '1.0.0' }], + }) + registry.package({ manifest }) + + await npm.exec('owner', ['ls', packageName]) + t.match(joinedOutput(), maintainers.map(m => `${m.name} <${m.email}>`).join('\n')) }) t.test('owner ls no maintainers', async t => { - result = '' - pacote.packument = async (spec, opts) => { - return { maintainers: [] } - } - t.teardown(() => { - result = '' - pacote.packument = noop + const { npm, joinedOutput } = await loadMockNpm(t) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), }) + const manifest = registry.manifest({ + name: packageName, + versions: ['1.0.0'], + }) + registry.package({ manifest }) - await owner.exec(['ls', '@npmcli/map-workspaces']) - t.equal(result, 'no admin found', 'should output no admint found msg') + await npm.exec('owner', ['ls', packageName]) + t.equal(joinedOutput(), 'no admin found') }) t.test('owner add ', async t => { - t.plan(8) - - result = '' - npmFetch.json = async (uri, opts) => { - // retrieve user info from couchdb request - if (uri === '/-/user/org.couchdb.user:foo') { - t.ok('should request user info') - t.match(opts, { ...npm.flatOptions }, 'should use expected opts') - return { - _id: 'org.couchdb.user:foo', - email: 'foo@github.com', - name: 'foo', - } - } else if (uri === '/@npmcli%2fmap-workspaces/-rev/1-foobaaa1') { - t.ok('should put changed owner') - t.match(opts, { - ...npm.flatOptions, - method: 'PUT', - body: { - _rev: '1-foobaaa1', - maintainers: npmcliMaintainers, - }, - spec: { - name: '@npmcli/map-workspaces', - }, - }, 'should use expected opts') - t.same( - opts.body.maintainers, - [ - ...npmcliMaintainers, - { - name: 'foo', - email: 'foo@github.com', - }, - ], - 'should contain expected new owners, adding requested user' - ) - return {} - } else { - t.fail(`unexpected fetch json call to uri: ${uri}`) - } - } - pacote.packument = async (spec, opts) => { - t.equal(spec.name, '@npmcli/map-workspaces', 'should use expect pkg name') - t.match( - opts, - { - ...npm.flatOptions, - fullMetadata: true, - }, - 'should forward expected options to pacote.packument' - ) - return { - _rev: '1-foobaaa1', - maintainers: npmcliMaintainers, - } - } - t.teardown(() => { - result = '' - npmFetch.json = noop - pacote.packument = noop - }) - - await owner.exec(['add', 'foo', '@npmcli/map-workspaces']) - t.equal(result, '+ foo (@npmcli/map-workspaces)', 'should output add result') + const { npm, joinedOutput } = await loadMockNpm(t, { + config: { ...auth }, + }) + const username = 'foo' + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + const manifest = registry.manifest({ + name: packageName, + packuments: [{ maintainers, version: '1.0.0' }], + }) + registry.couchuser({ username }) + registry.package({ manifest }) + registry.nock.put(`/${spec.escapedName}/-rev/${manifest._rev}`, body => { + t.match(body, { + _id: manifest._id, + _rev: manifest._rev, + maintainers: [ + ...manifest.maintainers, + { name: username, email: '' }, + ], + }) + return true + }).reply(200, {}) + await npm.exec('owner', ['add', username, packageName]) + t.equal(joinedOutput(), `+ ${username} (${packageName})`) }) t.test('owner add cwd package', async t => { - result = '' - readPackageNameResponse = '@npmcli/map-workspaces' - npmFetch.json = async (uri, opts) => { - // retrieve user info from couchdb request - if (uri === '/-/user/org.couchdb.user:foo') { - return { - _id: 'org.couchdb.user:foo', - email: 'foo@github.com', - name: 'foo', - } - } else if (uri === '/@npmcli%2fmap-workspaces/-rev/1-foobaaa1') { - return {} - } else { - t.fail(`unexpected fetch json call to uri: ${uri}`) - } - } - pacote.packument = async (spec, opts) => ({ - _rev: '1-foobaaa1', - maintainers: npmcliMaintainers, - }) - t.teardown(() => { - result = '' - readPackageNameResponse = null - npmFetch.json = noop - pacote.packument = noop - }) - - await owner.exec(['add', 'foo']) - t.equal(result, '+ foo (@npmcli/map-workspaces)', 'should output add result') + const { npm, joinedOutput } = await loadMockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ name: packageName }), + }, + config: { ...auth }, + }) + const username = 'foo' + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + const manifest = registry.manifest({ + name: packageName, + packuments: [{ maintainers, version: '1.0.0' }], + }) + registry.couchuser({ username }) + registry.package({ manifest }) + registry.nock.put(`/${spec.escapedName}/-rev/${manifest._rev}`, body => { + t.match(body, { + _id: manifest._id, + _rev: manifest._rev, + maintainers: [ + ...manifest.maintainers, + { name: username, email: '' }, + ], + }) + return true + }).reply(200, {}) + await npm.exec('owner', ['add', username]) + t.equal(joinedOutput(), `+ ${username} (${packageName})`) }) t.test('owner add already an owner', async t => { - t.plan(2) - - result = '' - log.info = (title, msg) => { - t.equal(title, 'owner add', 'should use expected title') - t.equal( - msg, - 'Already a package owner: ruyadorno ', - 'should log already package owner info message' - ) - } - npmFetch.json = async (uri, opts) => { - // retrieve user info from couchdb request - if (uri === '/-/user/org.couchdb.user:ruyadorno') { - return { - _id: 'org.couchdb.user:ruyadorno', - email: 'ruyadorno@hotmail.com', - name: 'ruyadorno', - } - } else { - t.fail(`unexpected fetch json call to uri: ${uri}`) - } - } - pacote.packument = async (spec, opts) => { - return { - _rev: '1-foobaaa1', - maintainers: npmcliMaintainers, - } - } - t.teardown(() => { - result = '' - log.info = noop - npmFetch.json = noop - pacote.packument = noop - }) - - await owner.exec(['add', 'ruyadorno', '@npmcli/map-workspaces']) + const { npm, joinedOutput, logs } = await loadMockNpm(t, { + config: { ...auth }, + }) + const username = maintainers[0].name + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + const manifest = registry.manifest({ + name: packageName, + packuments: [{ maintainers, version: '1.0.0' }], + }) + registry.couchuser({ username }) + registry.package({ manifest }) + await npm.exec('owner', ['add', username, packageName]) + t.equal(joinedOutput(), '') + t.match( + logs.info, + [['owner add', 'Already a package owner: test-user-a ']] + ) }) t.test('owner add fails to retrieve user', async t => { - result = '' - readPackageNameResponse = - npmFetch.json = async (uri, opts) => { - // retrieve borked user info from couchdb request - if (uri === '/-/user/org.couchdb.user:foo') { - return { ok: false } - } else if (uri === '/@npmcli%2fmap-workspaces/-rev/1-foobaaa1') { - return {} - } else { - t.fail(`unexpected fetch json call to uri: ${uri}`) - } - } - pacote.packument = async (spec, opts) => ({ - _rev: '1-foobaaa1', - maintainers: npmcliMaintainers, + const { npm, logs } = await loadMockNpm(t, { + config: { ...auth }, }) - t.teardown(() => { - result = '' - readPackageNameResponse = null - npmFetch.json = noop - pacote.packument = noop + const username = 'foo' + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), }) - - await t.rejects( - owner.exec(['add', 'foo', '@npmcli/map-workspaces']), - { code: 'EOWNERUSER', message: /Couldn't get user data for foo: {"ok":false}/ }, - 'should throw user data error' - ) + registry.couchuser({ username, responseCode: 404, body: {} }) + await t.rejects(npm.exec('owner', ['add', username, packageName])) + t.match(logs.error, [['owner mutate', `Error getting user data for ${username}`]]) }) t.test('owner add fails to PUT updates', async t => { - result = '' - npmFetch.json = async (uri, opts) => { - // retrieve user info from couchdb request - if (uri === '/-/user/org.couchdb.user:foo') { - return { - _id: 'org.couchdb.user:foo', - email: 'foo@github.com', - name: 'foo', - } - } else if (uri === '/@npmcli%2fmap-workspaces/-rev/1-foobaaa1') { - return { - error: { - status: '418', - message: "I'm a teapot", - }, - } - } else { - t.fail(`unexpected fetch json call to uri: ${uri}`) - } - } - pacote.packument = async (spec, opts) => ({ - _rev: '1-foobaaa1', - maintainers: npmcliMaintainers, + const { npm } = await loadMockNpm(t, { + config: { ...auth }, }) - t.teardown(() => { - result = '' - npmFetch.json = noop - pacote.packument = noop - }) - - await t.rejects( - owner.exec(['add', 'foo', '@npmcli/map-workspaces']), - { code: 'EOWNERMUTATE', message: /Failed to update package/ }, - 'should throw failed to update package error' - ) -}) - -t.test('owner add fails to retrieve user info', async t => { - t.plan(3) - - result = '' - log.error = (title, msg) => { - t.equal(title, 'owner mutate', 'should use expected title') - t.equal(msg, 'Error getting user data for foo') - } - npmFetch.json = async (uri, opts) => { - // retrieve user info from couchdb request - if (uri === '/-/user/org.couchdb.user:foo') { - throw Object.assign( - new Error("I'm a teapot"), - { status: 418 } - ) - } else { - t.fail(`unexpected fetch json call to uri: ${uri}`) - } - } - pacote.packument = async (spec, opts) => ({ - _rev: '1-foobaaa1', - maintainers: npmcliMaintainers, + const username = 'foo' + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), }) - t.teardown(() => { - result = '' - log.error = noop - npmFetch.json = noop - pacote.packument = noop + const manifest = registry.manifest({ + name: packageName, + packuments: [{ maintainers, version: '1.0.0' }], }) - + registry.couchuser({ username }) + registry.package({ manifest }) + registry.nock.put(`/${spec.escapedName}/-rev/${manifest._rev}`).reply(404, {}) await t.rejects( - owner.exec(['add', 'foo', '@npmcli/map-workspaces']), - "I'm a teapot", - 'should throw server error response' + npm.exec('owner', ['add', username, packageName]), + { code: 'EOWNERMUTATE' } ) }) t.test('owner add no previous maintainers property from server', async t => { - result = '' - npmFetch.json = async (uri, opts) => { - // retrieve user info from couchdb request - if (uri === '/-/user/org.couchdb.user:foo') { - return { - _id: 'org.couchdb.user:foo', - email: 'foo@github.com', - name: 'foo', - } - } else if (uri === '/@npmcli%2fno-owners-pkg/-rev/1-foobaaa1') { - return {} - } else { - t.fail(`unexpected fetch json call to uri: ${uri}`) - } - } - pacote.packument = async (spec, opts) => { - return { - _rev: '1-foobaaa1', - maintainers: null, - } - } - t.teardown(() => { - result = '' - npmFetch.json = noop - pacote.packument = noop - }) - - await owner.exec(['add', 'foo', '@npmcli/no-owners-pkg']) - t.equal(result, '+ foo (@npmcli/no-owners-pkg)', 'should output add result') + const { npm, joinedOutput } = await loadMockNpm(t, { + config: { ...auth }, + }) + const username = 'foo' + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + const manifest = registry.manifest({ + name: packageName, + packuments: [{ maintainers: undefined, version: '1.0.0' }], + }) + registry.couchuser({ username }) + registry.package({ manifest }) + registry.nock.put(`/${spec.escapedName}/-rev/${manifest._rev}`, body => { + t.match(body, { + _id: manifest._id, + _rev: manifest._rev, + maintainers: [{ name: username, email: '' }], + }) + return true + }).reply(200, {}) + await npm.exec('owner', ['add', username, packageName]) + t.equal(joinedOutput(), `+ ${username} (${packageName})`) }) t.test('owner add no user', async t => { - result = '' - t.teardown(() => { - result = '' - }) + const { npm } = await loadMockNpm(t) await t.rejects( - owner.exec(['add']), - owner.usage + npm.exec('owner', ['add']), + { code: 'EUSAGE' } ) }) -t.test('owner add no pkg global', async t => { - t.teardown(() => { - npm.config.set('global', false) +t.test('owner add no pkg global', async t => { + const { npm } = await loadMockNpm(t, { + config: { global: true }, }) - npm.config.set('global', true) await t.rejects( - owner.exec(['add', 'gar']), - owner.usage + npm.exec('owner', ['add', 'foo']), + { code: 'EUSAGE' } ) }) t.test('owner add no cwd package', async t => { - result = '' - t.teardown(() => { - result = '' - }) + const { npm } = await loadMockNpm(t) await t.rejects( - owner.exec(['add', 'foo']), - owner.usage + npm.exec('owner', ['add', 'foo']), + { code: 'EUSAGE' } ) }) t.test('owner rm ', async t => { - t.plan(8) - - result = '' - npmFetch.json = async (uri, opts) => { - // retrieve user info from couchdb request - if (uri === '/-/user/org.couchdb.user:ruyadorno') { - t.ok('should request user info') - t.match(opts, { ...npm.flatOptions }, 'should use expected opts') - return { - _id: 'org.couchdb.user:ruyadorno', - email: 'ruyadorno@hotmail.com', - name: 'ruyadorno', - } - } else if (uri === '/@npmcli%2fmap-workspaces/-rev/1-foobaaa1') { - t.ok('should put changed owner') - t.match(opts, { - ...npm.flatOptions, - method: 'PUT', - body: { - _rev: '1-foobaaa1', - }, - spec: { - name: '@npmcli/map-workspaces', - }, - }, 'should use expected opts') - t.same( - opts.body.maintainers, - npmcliMaintainers.filter(m => m.name !== 'ruyadorno'), - 'should contain expected new owners, removing requested user' - ) - return {} - } else { - t.fail(`unexpected fetch json call to: ${uri}`) - } - } - pacote.packument = async (spec, opts) => { - t.equal(spec.name, '@npmcli/map-workspaces', 'should use expect pkg name') - t.match( - opts, - { - ...npm.flatOptions, - fullMetadata: true, - }, - 'should forward expected options to pacote.packument' - ) - return { - _rev: '1-foobaaa1', - maintainers: npmcliMaintainers, - } - } - t.teardown(() => { - result = '' - npmFetch.json = noop - pacote.packument = noop - }) - - await owner.exec(['rm', 'ruyadorno', '@npmcli/map-workspaces']) - t.equal(result, '- ruyadorno (@npmcli/map-workspaces)', 'should output rm result') + const { npm, joinedOutput } = await loadMockNpm(t, { + config: { ...auth }, + }) + const username = maintainers[0].name + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + const manifest = registry.manifest({ + name: packageName, + packuments: [{ maintainers, version: '1.0.0' }], + }) + registry.couchuser({ username }) + registry.package({ manifest }) + registry.nock.put(`/${spec.escapedName}/-rev/${manifest._rev}`, body => { + t.match(body, { + _id: manifest._id, + _rev: manifest._rev, + maintainers: maintainers.slice(1), + }) + return true + }).reply(200, {}) + await npm.exec('owner', ['rm', username, packageName]) + t.equal(joinedOutput(), `- ${username} (${packageName})`) }) t.test('owner rm not a current owner', async t => { - t.plan(2) - - result = '' - log.info = (title, msg) => { - t.equal(title, 'owner rm', 'should log expected title') - t.equal(msg, 'Not a package owner: foo', 'should log.info not a package owner msg') - } - npmFetch.json = async (uri, opts) => { - // retrieve user info from couchdb request - if (uri === '/-/user/org.couchdb.user:foo') { - return { - _id: 'org.couchdb.user:foo', - email: 'foo@github.com', - name: 'foo', - } - } else if (uri === '/@npmcli%2fmap-workspaces/-rev/1-foobaaa1') { - return {} - } else { - t.fail(`unexpected fetch json call to: ${uri}`) - } - } - pacote.packument = async (spec, opts) => { - return { - _rev: '1-foobaaa1', - maintainers: npmcliMaintainers, - } - } - t.teardown(() => { - result = '' - log.info = noop - npmFetch.json = noop - pacote.packument = noop - }) - - await owner.exec(['rm', 'foo', '@npmcli/map-workspaces']) + const { npm, logs } = await loadMockNpm(t, { + config: { ...auth }, + }) + const username = 'foo' + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + const manifest = registry.manifest({ + name: packageName, + packuments: [{ maintainers, version: '1.0.0' }], + }) + registry.couchuser({ username }) + registry.package({ manifest }) + await npm.exec('owner', ['rm', username, packageName]) + t.match(logs.info, [['owner rm', `Not a package owner: ${username}`]]) }) t.test('owner rm cwd package', async t => { - result = '' - readPackageNameResponse = '@npmcli/map-workspaces' - npmFetch.json = async (uri, opts) => { - // retrieve user info from couchdb request - if (uri === '/-/user/org.couchdb.user:ruyadorno') { - return { - _id: 'org.couchdb.user:ruyadorno', - email: 'ruyadorno@hotmail.com', - name: 'ruyadorno', - } - } else if (uri === '/@npmcli%2fmap-workspaces/-rev/1-foobaaa1') { - return {} - } else { - t.fail(`unexpected fetch json call to uri: ${uri}`) - } - } - pacote.packument = async (spec, opts) => ({ - _rev: '1-foobaaa1', - maintainers: npmcliMaintainers, - }) - t.teardown(() => { - result = '' - readPackageNameResponse = null - npmFetch.json = noop - pacote.packument = noop - }) - - await owner.exec(['rm', 'ruyadorno']) - t.equal(result, '- ruyadorno (@npmcli/map-workspaces)', 'should output rm result') + const { npm, joinedOutput } = await loadMockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ name: packageName }), + }, + config: { ...auth }, + }) + const username = maintainers[0].name + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + const manifest = registry.manifest({ + name: packageName, + packuments: [{ maintainers, version: '1.0.0' }], + }) + registry.couchuser({ username }) + registry.package({ manifest }) + registry.nock.put(`/${spec.escapedName}/-rev/${manifest._rev}`, body => { + t.match(body, { + _id: manifest._id, + _rev: manifest._rev, + maintainers: maintainers.slice(1), + }) + return true + }).reply(200, {}) + await npm.exec('owner', ['rm', username]) + t.equal(joinedOutput(), `- ${username} (${packageName})`) }) t.test('owner rm only user', async t => { - result = '' - readPackageNameResponse = 'ipt' - npmFetch.json = async (uri, opts) => { - // retrieve user info from couchdb request - if (uri === '/-/user/org.couchdb.user:ruyadorno') { - return { - _id: 'org.couchdb.user:ruyadorno', - email: 'ruyadorno@hotmail.com', - name: 'ruyadorno', - } - } else { - t.fail(`unexpected fetch json call to uri: ${uri}`) - } - } - pacote.packument = async (spec, opts) => ({ - _rev: '1-foobaaa1', - maintainers: [{ - name: 'ruyadorno', - email: 'ruyadorno@hotmail.com', - }], - }) - t.teardown(() => { - result = '' - readPackageNameResponse = null - npmFetch.json = noop - pacote.packument = noop - }) - + const { npm } = await loadMockNpm(t, { + prefixDir: { + 'package.json': JSON.stringify({ name: packageName }), + }, + config: { ...auth }, + }) + const username = maintainers[0].name + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + const manifest = registry.manifest({ + name: packageName, + packuments: [{ maintainers: maintainers.slice(0, 1), version: '1.0.0' }], + }) + registry.couchuser({ username }) + registry.package({ manifest }) await t.rejects( - owner.exec(['rm', 'ruyadorno']), - { code: 'EOWNERRM', message: 'Cannot remove all owners of a package. Add someone else first.' }, - 'should throw unable to remove unique owner message' + npm.exec('owner', ['rm', username]), + { + code: 'EOWNERRM', + message: 'Cannot remove all owners of a package. Add someone else first.', + } ) }) t.test('owner rm no user', async t => { - result = '' - t.teardown(() => { - result = '' - }) - + const { npm } = await loadMockNpm(t) await t.rejects( - owner.exec(['rm']), - owner.usage + npm.exec('owner', ['rm']), + { code: 'EUSAGE' } ) }) t.test('owner rm no pkg global', async t => { - t.teardown(() => { - npm.config.set('global', false) + const { npm } = await loadMockNpm(t, { + config: { global: true }, }) - npm.config.set('global', true) - await t.rejects( - owner.exec(['rm', 'foo']), - owner.usage + npm.exec('owner', ['rm', 'foo']), + { code: 'EUSAGE' } ) }) t.test('owner rm no cwd package', async t => { - result = '' - t.teardown(() => { - result = '' - }) - + const { npm } = await loadMockNpm(t) await t.rejects( - owner.exec(['rm', 'foo']), - owner.usage + npm.exec('owner', ['rm', 'foo']), + { code: 'EUSAGE' } ) }) t.test('completion', async t => { - const testComp = async (argv, expect) => { - const res = await owner.completion({ conf: { argv: { remain: argv } } }) - t.strictSame(res, expect, argv.join(' ')) - } - - await Promise.all([ - testComp(['npm', 'foo'], []), - testComp(['npm', 'owner'], ['add', 'rm', 'ls']), - testComp(['npm', 'owner', 'add'], []), - testComp(['npm', 'owner', 'ls'], []), - testComp(['npm', 'owner', 'rm', 'foo'], []), - ]) - - // npm owner rm completion is async - t.test('completion npm owner rm', async t => { - t.plan(2) - readPackageNameResponse = '@npmcli/map-workspaces' - pacote.packument = async spec => { - t.equal(spec.name, readPackageNameResponse, 'should use package spec') - return { - maintainers: npmcliMaintainers, - } + t.test('basic commands', async t => { + const { npm } = await loadMockNpm(t) + const owner = await npm.cmd('owner') + const testComp = async (argv, expect) => { + const res = await owner.completion({ conf: { argv: { remain: argv } } }) + t.strictSame(res, expect, argv.join(' ')) } - t.teardown(() => { - readPackageNameResponse = null - pacote.packument = noop - }) + await Promise.all([ + testComp(['npm', 'foo'], []), + testComp(['npm', 'owner'], ['add', 'rm', 'ls']), + testComp(['npm', 'owner', 'add'], []), + testComp(['npm', 'owner', 'ls'], []), + testComp(['npm', 'owner', 'rm', 'foo'], []), + ]) + }) + + t.test('completion npm owner rm', async t => { + const { npm } = await loadMockNpm(t, { + prefixDir: { 'package.json': JSON.stringify({ name: packageName }) }, + }) + const owner = await npm.cmd('owner') + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + const manifest = registry.manifest({ + name: packageName, + packuments: [{ maintainers, version: '1.0.0' }], + }) + registry.package({ manifest }) const res = await owner.completion({ conf: { argv: { remain: ['npm', 'owner', 'rm'] } } }) - t.strictSame(res, - ['nlf', 'ruyadorno', 'darcyclarke', 'isaacs'], - 'should return list of current owners' - ) + t.strictSame(res, maintainers.map(m => m.name), 'should return list of current owners') }) t.test('completion npm owner rm no cwd package', async t => { + const { npm } = await loadMockNpm(t) + const owner = await npm.cmd('owner') const res = await owner.completion({ conf: { argv: { remain: ['npm', 'owner', 'rm'] } } }) t.strictSame(res, [], 'should have no owners to autocomplete if not cwd package') - t.end() }) t.test('completion npm owner rm global', async t => { - t.teardown(() => { - npm.config.set('global', false) + const { npm } = await loadMockNpm(t, { + config: { global: true }, }) - npm.config.set('global', true) + const owner = await npm.cmd('owner') 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) - readPackageNameResponse = '@npmcli/map-workspaces' - pacote.packument = async spec => { - t.equal(spec.name, readPackageNameResponse, 'should use package spec') - return { - maintainers: [], - } - } - t.teardown(() => { - readPackageNameResponse = null - pacote.packument = noop + const { npm } = await loadMockNpm(t, { + prefixDir: { 'package.json': JSON.stringify({ name: packageName }) }, }) + const owner = await npm.cmd('owner') + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + }) + const manifest = registry.manifest({ + name: packageName, + packuments: [{ maintainers: [], version: '1.0.0' }], + }) + registry.package({ manifest }) const res = await owner.completion({ conf: { argv: { remain: ['npm', 'owner', 'rm'] } } }) t.strictSame(res, [], 'should return no owners if not found') }) - - t.end() }) diff --git a/test/lib/utils/read-package-name.js b/test/lib/utils/read-package-name.js deleted file mode 100644 index a1a1b4a1504dc..0000000000000 --- a/test/lib/utils/read-package-name.js +++ /dev/null @@ -1,33 +0,0 @@ -const t = require('tap') - -const readPackageName = require('../../../lib/utils/read-package-name.js') - -t.test('read local package.json', async (t) => { - const prefix = t.testdir({ - 'package.json': JSON.stringify({ - name: 'my-local-package', - version: '1.0.0', - }), - }) - const packageName = await readPackageName(prefix) - t.equal( - packageName, - 'my-local-package', - 'should retrieve current package name' - ) -}) - -t.test('read local scoped-package.json', async (t) => { - const prefix = t.testdir({ - 'package.json': JSON.stringify({ - name: '@my-scope/my-local-package', - version: '1.0.0', - }), - }) - const packageName = await readPackageName(prefix) - t.equal( - packageName, - '@my-scope/my-local-package', - 'should retrieve scoped package name' - ) -})