Skip to content

Commit

Permalink
fix: deprecate completion
Browse files Browse the repository at this point in the history
Found a bug refactoring the tests.

libnpmaccess mutates the response from the server, and the completion
code was not looking for the right value.
  • Loading branch information
wraithgar authored and lukekarrys committed Apr 14, 2022
1 parent d75ed47 commit b10462e
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 107 deletions.
2 changes: 1 addition & 1 deletion lib/commands/deprecate.js
Original file line number Diff line number Diff line change
Expand Up @@ -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])))
}
Expand Down
21 changes: 15 additions & 6 deletions test/fixtures/mock-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) {
Expand Down Expand Up @@ -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)
}
Expand All @@ -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 = {} }) {
Expand Down Expand Up @@ -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 = {
Expand All @@ -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] = {
Expand Down
177 changes: 96 additions & 81 deletions test/lib/commands/deprecate.js
Original file line number Diff line number Diff line change
@@ -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(), '')
})
38 changes: 19 additions & 19 deletions test/lib/commands/unpublish.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand All @@ -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'],
Expand All @@ -464,12 +464,12 @@ 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, {
[pkg]: 'write',
[`${pkg}a`]: 'write',
[`${pkg}b`]: 'write',
})
registry.whoami({ username: user })
registry.nock.get('/-/org/test-user/package?format=cli').reply(200, {
[pkg]: 'write',
[`${pkg}a`]: 'write',
[`${pkg}b`]: 'write',
})

await testComp(t, {
argv: ['npm', 'unpublish'],
Expand All @@ -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, {
Expand All @@ -505,11 +505,11 @@ 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, {
[pkg]: 'write',
[`${pkg}a`]: 'write',
})
registry.whoami({ username: user })
registry.nock.get('/-/org/test-user/package?format=cli').reply(200, {
[pkg]: 'write',
[`${pkg}a`]: 'write',
})

await testComp(t, {
argv: ['npm', 'unpublish'],
Expand All @@ -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'],
Expand All @@ -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'],
Expand Down

0 comments on commit b10462e

Please sign in to comment.