From cde6a3a6fdf3373e170c3a72cb6257846f96d55b Mon Sep 17 00:00:00 2001 From: Gar Date: Thu, 14 Apr 2022 08:59:10 -0700 Subject: [PATCH] fix: deprecate completion Found a bug refactoring the tests. libnpmaccess mutates the response from the server, and the completion code was not looking for the right value. --- lib/commands/deprecate.js | 2 +- test/fixtures/mock-registry.js | 21 ++-- test/lib/commands/deprecate.js | 177 ++++++++++++++++++--------------- test/lib/commands/unpublish.js | 24 ++--- 4 files changed, 124 insertions(+), 100 deletions(-) diff --git a/lib/commands/deprecate.js b/lib/commands/deprecate.js index 88eb320c32a52..0ae88f1921f56 100644 --- a/lib/commands/deprecate.js +++ b/lib/commands/deprecate.js @@ -26,7 +26,7 @@ class Deprecate extends BaseCommand { const packages = await libaccess.lsPackages(username, this.npm.flatOptions) return Object.keys(packages) .filter((name) => - packages[name] === 'write' && + packages[name] === 'read-write' && (opts.conf.argv.remain.length === 0 || name.startsWith(opts.conf.argv.remain[0]))) } diff --git a/test/fixtures/mock-registry.js b/test/fixtures/mock-registry.js index 6b6722fcbbc3c..8b847213f33a7 100644 --- a/test/fixtures/mock-registry.js +++ b/test/fixtures/mock-registry.js @@ -40,8 +40,12 @@ class MockRegistry { this.#nock = nock } - whoami ({ username }) { - this.nock = this.nock.get('/-/whoami').reply(200, { username }) + whoami ({ username, body, responseCode = 200, times = 1 }) { + if (username) { + this.nock = this.nock.get('/-/whoami').times(times).reply(responseCode, { username }) + } else { + this.nock = this.nock.get('/-/whoami').times(times).reply(responseCode, body) + } } access ({ spec, access, publishRequires2fa }) { @@ -108,7 +112,7 @@ class MockRegistry { } // team can be a team or a username - lsPackages ({ team, packages = {} }) { + lsPackages ({ team, packages = {}, times = 1 }) { if (team.startsWith('@')) { team = team.slice(1) } @@ -119,7 +123,7 @@ class MockRegistry { } else { uri = `/-/org/${encodeURIComponent(scope)}/package` } - this.nock = this.nock.get(uri).query({ format: 'cli' }).reply(200, packages) + this.nock = this.nock.get(uri).query({ format: 'cli' }).times(times).reply(200, packages) } lsCollaborators ({ spec, user, collaborators = {} }) { @@ -169,8 +173,10 @@ class MockRegistry { this.nock = nock } - // the last packument in the packuments array will be tagged as latest - manifest ({ name = 'test-package', packuments } = {}) { + // either pass in packuments if you need to set specific attributes besides version, + // or an array of versions + // the last packument in the packuments or versions array will be tagged latest + manifest ({ name = 'test-package', packuments, versions } = {}) { packuments = this.packuments(packuments, name) const latest = packuments.slice(-1)[0] const manifest = { @@ -184,6 +190,9 @@ class MockRegistry { 'dist-tags': { latest: latest.version }, ...latest, } + if (versions) { + packuments = versions.map(version => ({ version })) + } for (const packument of packuments) { manifest.versions[packument.version] = { diff --git a/test/lib/commands/deprecate.js b/test/lib/commands/deprecate.js index 37a407c3b6a1a..996ce9cf4932f 100644 --- a/test/lib/commands/deprecate.js +++ b/test/lib/commands/deprecate.js @@ -1,137 +1,152 @@ const t = require('tap') +const { load: loadMockNpm } = require('../../fixtures/mock-npm') -let getIdentityImpl = () => 'someperson' -let npmFetchBody = null +const MockRegistry = require('../../fixtures/mock-registry.js') -const npmFetch = async (uri, opts) => { - npmFetchBody = opts.body -} +const user = 'test-user' +const token = 'test-auth-token' +const auth = { '//registry.npmjs.org/:_authToken': token } +const versions = ['1.0.0', '1.0.1', '1.0.1-pre'] -npmFetch.json = async (uri, opts) => { - return { - versions: { - '1.0.0': {}, - '1.0.1': {}, - '1.0.1-pre': {}, - }, - } -} - -const Deprecate = t.mock('../../../lib/commands/deprecate.js', { - '../../../lib/utils/get-identity.js': async () => getIdentityImpl(), - libnpmaccess: { - lsPackages: async () => ({ foo: 'write', bar: 'write', baz: 'write', buzz: 'read' }), - }, - 'npm-registry-fetch': npmFetch, -}) - -const deprecate = new Deprecate({ - flatOptions: { registry: 'https://registry.npmjs.org' }, -}) +// libnpmaccess maps these to read-write and read-only +const packages ={ foo: 'write', bar: 'write', baz: 'write', buzz: 'read' } t.test('completion', async t => { - const defaultIdentityImpl = getIdentityImpl - t.teardown(() => { - getIdentityImpl = defaultIdentityImpl + const { npm } = await loadMockNpm(t, { + config: { + ...auth, + }, }) + const deprecate = await npm.cmd('deprecate') const testComp = async (argv, expect) => { const res = await deprecate.completion({ conf: { argv: { remain: argv } } }) t.strictSame(res, expect, `completion: ${argv}`) } + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: token, + }) + + registry.whoami({ username: user, times: 4 }) + registry.lsPackages({ team: user, packages, times: 4 }) await Promise.all([ testComp([], ['foo', 'bar', 'baz']), testComp(['b'], ['bar', 'baz']), testComp(['fo'], ['foo']), testComp(['g'], []), - testComp(['foo', 'something'], []), ]) - getIdentityImpl = () => { - throw new Error('deprecate test failure') - } + await testComp(['foo', 'something'], []) + + registry.whoami({ statusCode: 404, body: {} }) - t.rejects(testComp([], []), { message: 'deprecate test failure' }) + t.rejects(testComp([], []), { code: 'ENEEDAUTH' }) }) t.test('no args', async t => { + const { npm } = await loadMockNpm(t) await t.rejects( - deprecate.exec([]), - /Usage:/, + npm.exec('deprecate', []), + { code: 'EUSAGE' }, 'logs usage' ) }) t.test('only one arg', async t => { + const { npm } = await loadMockNpm(t) await t.rejects( - deprecate.exec(['foo']), - /Usage:/, + npm.exec('deprecate', ['foo']), + { code: 'EUSAGE' }, 'logs usage' ) }) t.test('invalid semver range', async t => { + const { npm } = await loadMockNpm(t) await t.rejects( - deprecate.exec(['foo@notaversion', 'this will fail']), + npm.exec('deprecate', ['foo@notaversion', 'this will fail']), /invalid version range/, 'logs semver error' ) }) t.test('undeprecate', async t => { - t.teardown(() => { - npmFetchBody = null + const { npm, joinedOutput } = await loadMockNpm(t, { config: { ...auth } }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: token, }) - await deprecate.exec(['foo', '']) - t.match(npmFetchBody, { - versions: { - '1.0.0': { deprecated: '' }, - '1.0.1': { deprecated: '' }, - '1.0.1-pre': { deprecated: '' }, - }, - }, 'undeprecates everything') + const manifest = registry.manifest({ + name: 'foo', + versions + }) + registry.package({ manifest, query: { write: true } }) + registry.nock.put('/foo', body => { + for (const version of versions) { + if (body.versions[version].deprecated !== '') { + return false + } + } + return true + }).reply(200, {}) + + await npm.exec('deprecate', ['foo', '']) + t.match(joinedOutput(), '') }) t.test('deprecates given range', async t => { - t.teardown(() => { - npmFetchBody = null + const { npm, joinedOutput } = await loadMockNpm(t, { config: { ...auth } }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: token, }) - - await deprecate.exec(['foo@1.0.0', 'this version is deprecated']) - t.match(npmFetchBody, { - versions: { - '1.0.0': { - deprecated: 'this version is deprecated', - }, - '1.0.1': { - // the undefined here is necessary to ensure that we absolutely - // did not assign this property - deprecated: undefined, - }, - }, + const manifest = registry.manifest({ + name: 'foo', + versions }) + registry.package({ manifest, query: { write: true } }) + const message = 'test deprecation message' + registry.nock.put('/foo', body => { + if (body.versions['1.0.1'].deprecated) { + return false + } + if (body.versions['1.0.1-pre'].deprecated) { + return false + } + return body.versions['1.0.0'].deprecated === message + }).reply(200, {}) + await npm.exec('deprecate', ['foo@1.0.0', message]) + t.match(joinedOutput(), '') }) t.test('deprecates all versions when no range is specified', async t => { - t.teardown(() => { - npmFetchBody = null + const { npm, joinedOutput } = await loadMockNpm(t, { config: { ...auth } }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: token, }) - - await deprecate.exec(['foo', 'this version is deprecated']) - - t.match(npmFetchBody, { - versions: { - '1.0.0': { - deprecated: 'this version is deprecated', - }, - '1.0.1': { - deprecated: 'this version is deprecated', - }, - '1.0.1-pre': { - deprecated: 'this version is deprecated', - }, - }, + const manifest = registry.manifest({ + name: 'foo', + versions }) + registry.package({ manifest, query: { write: true } }) + const message = 'test deprecation message' + registry.nock.put('/foo', body => { + for (const version of versions) { + if (body.versions[version].deprecated !== message) { + return false + } + } + return true + }).reply(200, {}) + + await npm.exec('deprecate', ['foo', message]) + t.match(joinedOutput(), '') }) diff --git a/test/lib/commands/unpublish.js b/test/lib/commands/unpublish.js index 829d41c5bb875..075330473fbaf 100644 --- a/test/lib/commands/unpublish.js +++ b/test/lib/commands/unpublish.js @@ -423,8 +423,8 @@ t.test('completion', async t => { packuments: ['1.0.0', '1.0.1'], }) await registry.package({ manifest, query: { write: true } }) - registry.nock.get('/-/whoami').reply(200, { username: user }) - .get('/-/org/test-user/package?format=cli').reply(200, { [pkg]: 'write' }) + registry.whoami({ username: user }) + registry.nock.get('/-/org/test-user/package?format=cli').reply(200, { [pkg]: 'write' }) await testComp(t, { argv: ['npm', 'unpublish'], @@ -445,8 +445,8 @@ t.test('completion', async t => { const manifest = registry.manifest({ name: pkg }) manifest.versions = {} await registry.package({ manifest, query: { write: true } }) - registry.nock.get('/-/whoami').reply(200, { username: user }) - .get('/-/org/test-user/package?format=cli').reply(200, { [pkg]: 'write' }) + registry.whoami({ username: user }) + registry.nock.get('/-/org/test-user/package?format=cli').reply(200, { [pkg]: 'write' }) await testComp(t, { argv: ['npm', 'unpublish'], @@ -464,8 +464,8 @@ t.test('completion', async t => { registry: npm.config.get('registry'), authorization: 'test-auth-token', }) - registry.nock.get('/-/whoami').reply(200, { username: user }) - .get('/-/org/test-user/package?format=cli').reply(200, { + registry.whoami({ username: user }) + registry.nock.get('/-/org/test-user/package?format=cli').reply(200, { [pkg]: 'write', [`${pkg}a`]: 'write', [`${pkg}b`]: 'write', @@ -488,7 +488,7 @@ t.test('completion', async t => { registry: npm.config.get('registry'), authorization: 'test-auth-token', }) - registry.nock.get('/-/whoami').reply(200, { username: user }) + registry.whoami({ username: user }) registry.nock.get('/-/org/test-user/package?format=cli').reply(200, {}) await testComp(t, { @@ -505,8 +505,8 @@ t.test('completion', async t => { registry: npm.config.get('registry'), authorization: 'test-auth-token', }) - registry.nock.get('/-/whoami').reply(200, { username: user }) - .get('/-/org/test-user/package?format=cli').reply(200, { + registry.whoami({ username: user }) + registry.nock.get('/-/org/test-user/package?format=cli').reply(200, { [pkg]: 'write', [`${pkg}a`]: 'write', }) @@ -525,8 +525,8 @@ t.test('completion', async t => { registry: npm.config.get('registry'), authorization: 'test-auth-token', }) - registry.nock.get('/-/whoami').reply(200, { username: user }) - .get('/-/org/test-user/package?format=cli').reply(200, null) + registry.whoami({ username: user }) + registry.nock.get('/-/org/test-user/package?format=cli').reply(200, null) await testComp(t, { argv: ['npm', 'unpublish'], @@ -542,7 +542,7 @@ t.test('completion', async t => { registry: npm.config.get('registry'), authorization: 'test-auth-token', }) - registry.nock.get('/-/whoami').reply(404) + registry.whoami({ responseCode: 404 }) await testComp(t, { argv: ['npm', 'unpublish'],