Skip to content

Commit b10462e

Browse files
wraithgarlukekarrys
wraithgar
authored and
lukekarrys
committed
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.
1 parent d75ed47 commit b10462e

File tree

4 files changed

+131
-107
lines changed

4 files changed

+131
-107
lines changed

‎lib/commands/deprecate.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class Deprecate extends BaseCommand {
2626
const packages = await libaccess.lsPackages(username, this.npm.flatOptions)
2727
return Object.keys(packages)
2828
.filter((name) =>
29-
packages[name] === 'write' &&
29+
packages[name] === 'read-write' &&
3030
(opts.conf.argv.remain.length === 0 ||
3131
name.startsWith(opts.conf.argv.remain[0])))
3232
}

‎test/fixtures/mock-registry.js‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,12 @@ class MockRegistry {
4040
this.#nock = nock
4141
}
4242

43-
whoami ({ username }) {
44-
this.nock = this.nock.get('/-/whoami').reply(200, { username })
43+
whoami ({ username, body, responseCode = 200, times = 1 }) {
44+
if (username) {
45+
this.nock = this.nock.get('/-/whoami').times(times).reply(responseCode, { username })
46+
} else {
47+
this.nock = this.nock.get('/-/whoami').times(times).reply(responseCode, body)
48+
}
4549
}
4650

4751
access ({ spec, access, publishRequires2fa }) {
@@ -108,7 +112,7 @@ class MockRegistry {
108112
}
109113

110114
// team can be a team or a username
111-
lsPackages ({ team, packages = {} }) {
115+
lsPackages ({ team, packages = {}, times = 1 }) {
112116
if (team.startsWith('@')) {
113117
team = team.slice(1)
114118
}
@@ -119,7 +123,7 @@ class MockRegistry {
119123
} else {
120124
uri = `/-/org/${encodeURIComponent(scope)}/package`
121125
}
122-
this.nock = this.nock.get(uri).query({ format: 'cli' }).reply(200, packages)
126+
this.nock = this.nock.get(uri).query({ format: 'cli' }).times(times).reply(200, packages)
123127
}
124128

125129
lsCollaborators ({ spec, user, collaborators = {} }) {
@@ -169,8 +173,10 @@ class MockRegistry {
169173
this.nock = nock
170174
}
171175

172-
// the last packument in the packuments array will be tagged as latest
173-
manifest ({ name = 'test-package', packuments } = {}) {
176+
// either pass in packuments if you need to set specific attributes besides version,
177+
// or an array of versions
178+
// the last packument in the packuments or versions array will be tagged latest
179+
manifest ({ name = 'test-package', packuments, versions } = {}) {
174180
packuments = this.packuments(packuments, name)
175181
const latest = packuments.slice(-1)[0]
176182
const manifest = {
@@ -184,6 +190,9 @@ class MockRegistry {
184190
'dist-tags': { latest: latest.version },
185191
...latest,
186192
}
193+
if (versions) {
194+
packuments = versions.map(version => ({ version }))
195+
}
187196

188197
for (const packument of packuments) {
189198
manifest.versions[packument.version] = {

‎test/lib/commands/deprecate.js‎

Lines changed: 96 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -1,137 +1,152 @@
11
const t = require('tap')
2+
const { load: loadMockNpm } = require('../../fixtures/mock-npm')
23

3-
let getIdentityImpl = () => 'someperson'
4-
let npmFetchBody = null
4+
const MockRegistry = require('../../fixtures/mock-registry.js')
55

6-
const npmFetch = async (uri, opts) => {
7-
npmFetchBody = opts.body
8-
}
6+
const user = 'test-user'
7+
const token = 'test-auth-token'
8+
const auth = { '//registry.npmjs.org/:_authToken': token }
9+
const versions = ['1.0.0', '1.0.1', '1.0.1-pre']
910

10-
npmFetch.json = async (uri, opts) => {
11-
return {
12-
versions: {
13-
'1.0.0': {},
14-
'1.0.1': {},
15-
'1.0.1-pre': {},
16-
},
17-
}
18-
}
19-
20-
const Deprecate = t.mock('../../../lib/commands/deprecate.js', {
21-
'../../../lib/utils/get-identity.js': async () => getIdentityImpl(),
22-
libnpmaccess: {
23-
lsPackages: async () => ({ foo: 'write', bar: 'write', baz: 'write', buzz: 'read' }),
24-
},
25-
'npm-registry-fetch': npmFetch,
26-
})
27-
28-
const deprecate = new Deprecate({
29-
flatOptions: { registry: 'https://registry.npmjs.org' },
30-
})
11+
// libnpmaccess maps these to read-write and read-only
12+
const packages = { foo: 'write', bar: 'write', baz: 'write', buzz: 'read' }
3113

3214
t.test('completion', async t => {
33-
const defaultIdentityImpl = getIdentityImpl
34-
t.teardown(() => {
35-
getIdentityImpl = defaultIdentityImpl
15+
const { npm } = await loadMockNpm(t, {
16+
config: {
17+
...auth,
18+
},
3619
})
3720

21+
const deprecate = await npm.cmd('deprecate')
3822
const testComp = async (argv, expect) => {
3923
const res =
4024
await deprecate.completion({ conf: { argv: { remain: argv } } })
4125
t.strictSame(res, expect, `completion: ${argv}`)
4226
}
4327

28+
const registry = new MockRegistry({
29+
tap: t,
30+
registry: npm.config.get('registry'),
31+
authorization: token,
32+
})
33+
34+
registry.whoami({ username: user, times: 4 })
35+
registry.lsPackages({ team: user, packages, times: 4 })
4436
await Promise.all([
4537
testComp([], ['foo', 'bar', 'baz']),
4638
testComp(['b'], ['bar', 'baz']),
4739
testComp(['fo'], ['foo']),
4840
testComp(['g'], []),
49-
testComp(['foo', 'something'], []),
5041
])
5142

52-
getIdentityImpl = () => {
53-
throw new Error('deprecate test failure')
54-
}
43+
await testComp(['foo', 'something'], [])
44+
45+
registry.whoami({ statusCode: 404, body: {} })
5546

56-
t.rejects(testComp([], []), { message: 'deprecate test failure' })
47+
t.rejects(testComp([], []), { code: 'ENEEDAUTH' })
5748
})
5849

5950
t.test('no args', async t => {
51+
const { npm } = await loadMockNpm(t)
6052
await t.rejects(
61-
deprecate.exec([]),
62-
/Usage:/,
53+
npm.exec('deprecate', []),
54+
{ code: 'EUSAGE' },
6355
'logs usage'
6456
)
6557
})
6658

6759
t.test('only one arg', async t => {
60+
const { npm } = await loadMockNpm(t)
6861
await t.rejects(
69-
deprecate.exec(['foo']),
70-
/Usage:/,
62+
npm.exec('deprecate', ['foo']),
63+
{ code: 'EUSAGE' },
7164
'logs usage'
7265
)
7366
})
7467

7568
t.test('invalid semver range', async t => {
69+
const { npm } = await loadMockNpm(t)
7670
await t.rejects(
77-
deprecate.exec(['foo@notaversion', 'this will fail']),
71+
npm.exec('deprecate', ['foo@notaversion', 'this will fail']),
7872
/invalid version range/,
7973
'logs semver error'
8074
)
8175
})
8276

8377
t.test('undeprecate', async t => {
84-
t.teardown(() => {
85-
npmFetchBody = null
78+
const { npm, joinedOutput } = await loadMockNpm(t, { config: { ...auth } })
79+
const registry = new MockRegistry({
80+
tap: t,
81+
registry: npm.config.get('registry'),
82+
authorization: token,
8683
})
87-
await deprecate.exec(['foo', ''])
88-
t.match(npmFetchBody, {
89-
versions: {
90-
'1.0.0': { deprecated: '' },
91-
'1.0.1': { deprecated: '' },
92-
'1.0.1-pre': { deprecated: '' },
93-
},
94-
}, 'undeprecates everything')
84+
const manifest = registry.manifest({
85+
name: 'foo',
86+
versions,
87+
})
88+
registry.package({ manifest, query: { write: true } })
89+
registry.nock.put('/foo', body => {
90+
for (const version of versions) {
91+
if (body.versions[version].deprecated !== '') {
92+
return false
93+
}
94+
}
95+
return true
96+
}).reply(200, {})
97+
98+
await npm.exec('deprecate', ['foo', ''])
99+
t.match(joinedOutput(), '')
95100
})
96101

97102
t.test('deprecates given range', async t => {
98-
t.teardown(() => {
99-
npmFetchBody = null
103+
const { npm, joinedOutput } = await loadMockNpm(t, { config: { ...auth } })
104+
const registry = new MockRegistry({
105+
tap: t,
106+
registry: npm.config.get('registry'),
107+
authorization: token,
100108
})
101-
102-
await deprecate.exec(['[email protected]', 'this version is deprecated'])
103-
t.match(npmFetchBody, {
104-
versions: {
105-
'1.0.0': {
106-
deprecated: 'this version is deprecated',
107-
},
108-
'1.0.1': {
109-
// the undefined here is necessary to ensure that we absolutely
110-
// did not assign this property
111-
deprecated: undefined,
112-
},
113-
},
109+
const manifest = registry.manifest({
110+
name: 'foo',
111+
versions,
114112
})
113+
registry.package({ manifest, query: { write: true } })
114+
const message = 'test deprecation message'
115+
registry.nock.put('/foo', body => {
116+
if (body.versions['1.0.1'].deprecated) {
117+
return false
118+
}
119+
if (body.versions['1.0.1-pre'].deprecated) {
120+
return false
121+
}
122+
return body.versions['1.0.0'].deprecated === message
123+
}).reply(200, {})
124+
await npm.exec('deprecate', ['[email protected]', message])
125+
t.match(joinedOutput(), '')
115126
})
116127

117128
t.test('deprecates all versions when no range is specified', async t => {
118-
t.teardown(() => {
119-
npmFetchBody = null
129+
const { npm, joinedOutput } = await loadMockNpm(t, { config: { ...auth } })
130+
const registry = new MockRegistry({
131+
tap: t,
132+
registry: npm.config.get('registry'),
133+
authorization: token,
120134
})
121-
122-
await deprecate.exec(['foo', 'this version is deprecated'])
123-
124-
t.match(npmFetchBody, {
125-
versions: {
126-
'1.0.0': {
127-
deprecated: 'this version is deprecated',
128-
},
129-
'1.0.1': {
130-
deprecated: 'this version is deprecated',
131-
},
132-
'1.0.1-pre': {
133-
deprecated: 'this version is deprecated',
134-
},
135-
},
135+
const manifest = registry.manifest({
136+
name: 'foo',
137+
versions,
136138
})
139+
registry.package({ manifest, query: { write: true } })
140+
const message = 'test deprecation message'
141+
registry.nock.put('/foo', body => {
142+
for (const version of versions) {
143+
if (body.versions[version].deprecated !== message) {
144+
return false
145+
}
146+
}
147+
return true
148+
}).reply(200, {})
149+
150+
await npm.exec('deprecate', ['foo', message])
151+
t.match(joinedOutput(), '')
137152
})

‎test/lib/commands/unpublish.js‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -423,8 +423,8 @@ t.test('completion', async t => {
423423
packuments: ['1.0.0', '1.0.1'],
424424
})
425425
await registry.package({ manifest, query: { write: true } })
426-
registry.nock.get('/-/whoami').reply(200, { username: user })
427-
.get('/-/org/test-user/package?format=cli').reply(200, { [pkg]: 'write' })
426+
registry.whoami({ username: user })
427+
registry.nock.get('/-/org/test-user/package?format=cli').reply(200, { [pkg]: 'write' })
428428

429429
await testComp(t, {
430430
argv: ['npm', 'unpublish'],
@@ -445,8 +445,8 @@ t.test('completion', async t => {
445445
const manifest = registry.manifest({ name: pkg })
446446
manifest.versions = {}
447447
await registry.package({ manifest, query: { write: true } })
448-
registry.nock.get('/-/whoami').reply(200, { username: user })
449-
.get('/-/org/test-user/package?format=cli').reply(200, { [pkg]: 'write' })
448+
registry.whoami({ username: user })
449+
registry.nock.get('/-/org/test-user/package?format=cli').reply(200, { [pkg]: 'write' })
450450

451451
await testComp(t, {
452452
argv: ['npm', 'unpublish'],
@@ -464,12 +464,12 @@ t.test('completion', async t => {
464464
registry: npm.config.get('registry'),
465465
authorization: 'test-auth-token',
466466
})
467-
registry.nock.get('/-/whoami').reply(200, { username: user })
468-
.get('/-/org/test-user/package?format=cli').reply(200, {
469-
[pkg]: 'write',
470-
[`${pkg}a`]: 'write',
471-
[`${pkg}b`]: 'write',
472-
})
467+
registry.whoami({ username: user })
468+
registry.nock.get('/-/org/test-user/package?format=cli').reply(200, {
469+
[pkg]: 'write',
470+
[`${pkg}a`]: 'write',
471+
[`${pkg}b`]: 'write',
472+
})
473473

474474
await testComp(t, {
475475
argv: ['npm', 'unpublish'],
@@ -488,7 +488,7 @@ t.test('completion', async t => {
488488
registry: npm.config.get('registry'),
489489
authorization: 'test-auth-token',
490490
})
491-
registry.nock.get('/-/whoami').reply(200, { username: user })
491+
registry.whoami({ username: user })
492492
registry.nock.get('/-/org/test-user/package?format=cli').reply(200, {})
493493

494494
await testComp(t, {
@@ -505,11 +505,11 @@ t.test('completion', async t => {
505505
registry: npm.config.get('registry'),
506506
authorization: 'test-auth-token',
507507
})
508-
registry.nock.get('/-/whoami').reply(200, { username: user })
509-
.get('/-/org/test-user/package?format=cli').reply(200, {
510-
[pkg]: 'write',
511-
[`${pkg}a`]: 'write',
512-
})
508+
registry.whoami({ username: user })
509+
registry.nock.get('/-/org/test-user/package?format=cli').reply(200, {
510+
[pkg]: 'write',
511+
[`${pkg}a`]: 'write',
512+
})
513513

514514
await testComp(t, {
515515
argv: ['npm', 'unpublish'],
@@ -525,8 +525,8 @@ t.test('completion', async t => {
525525
registry: npm.config.get('registry'),
526526
authorization: 'test-auth-token',
527527
})
528-
registry.nock.get('/-/whoami').reply(200, { username: user })
529-
.get('/-/org/test-user/package?format=cli').reply(200, null)
528+
registry.whoami({ username: user })
529+
registry.nock.get('/-/org/test-user/package?format=cli').reply(200, null)
530530

531531
await testComp(t, {
532532
argv: ['npm', 'unpublish'],
@@ -542,7 +542,7 @@ t.test('completion', async t => {
542542
registry: npm.config.get('registry'),
543543
authorization: 'test-auth-token',
544544
})
545-
registry.nock.get('/-/whoami').reply(404)
545+
registry.whoami({ responseCode: 404 })
546546

547547
await testComp(t, {
548548
argv: ['npm', 'unpublish'],

0 commit comments

Comments
 (0)