refactor deprecate command and add tests

PR-URL: #2302
Credit: @nlf
Close: #2302
Reviewed-by: @ruyadorno

lib/deprecate.js

@@ -5,68 +5,72 @@ const fetch = require('npm-registry-fetch')
 const otplease = require('./utils/otplease.js')
 const npa = require('npm-package-arg')
 const semver = require('semver')
-const getItentity = require('./utils/get-identity')
+const getIdentity = require('./utils/get-identity.js')
+const libaccess = require('libnpmaccess')
+const usageUtil = require('./utils/usage.js')
 
-module.exports = deprecate
+const UsageError = () =>
+  Object.assign(new Error(`\nUsage: ${usage}`), {
+    code: 'EUSAGE',
+  })
 
-deprecate.usage = 'npm deprecate <pkg>[@<version>] <message>'
+const usage = usageUtil(
+  'deprecate',
+  'npm deprecate <pkg>[@<version>] <message>'
+)
 
-deprecate.completion = function (opts, cb) {
-  return Promise.resolve().then(() => {
-    if (opts.conf.argv.remain.length > 2)
-      return
-    return getItentity(npm.flatOptions).then(username => {
-      if (username) {
-        // first, get a list of remote packages this user owns.
-        // once we have a user account, then don't complete anything.
-        // get the list of packages by user
-        return fetch(
-          `/-/by-user/${encodeURIComponent(username)}`,
-          npm.flatOptions
-        ).then(list => list[username])
-      }
+const completion = (opts, cb) => {
+  if (opts.conf.argv.remain.length > 1)
+    return cb(null, [])
+
+  return getIdentity(npm.flatOptions).then((username) => {
+    return libaccess.lsPackages(username, npm.flatOptions).then((packages) => {
+      return Object.keys(packages)
+        .filter((name) => packages[name] === 'write' &&
+          (opts.conf.argv.remain.length === 0 || name.startsWith(opts.conf.argv.remain[0]))
+        )
     })
-  }).then(() => cb(), er => cb(er))
+  }).then((list) => cb(null, list), (err) => cb(err))
 }
 
-function deprecate ([pkg, msg], opts, cb) {
-  if (typeof cb !== 'function') {
-    cb = opts
-    opts = null
-  }
-  opts = opts || npm.flatOptions
-  return Promise.resolve().then(() => {
-    if (msg == null)
-      throw new Error(`Usage: ${deprecate.usage}`)
-    // fetch the data and make sure it exists.
-    const p = npa(pkg)
+const cmd = (args, cb) =>
+  deprecate(args)
+    .then(() => cb())
+    .catch(err => cb(err.code === 'EUSAGE' ? err.message : err))
+
+const deprecate = async ([pkg, msg]) => {
+  if (!pkg || !msg)
+    throw UsageError()
+
+  // fetch the data and make sure it exists.
+  const p = npa(pkg)
+  // npa makes the default spec "latest", but for deprecation
+  // "*" is the appropriate default.
+  const spec = p.rawSpec === '' ? '*' : p.fetchSpec
 
-    // npa makes the default spec "latest", but for deprecation
-    // "*" is the appropriate default.
-    const spec = p.rawSpec === '' ? '*' : p.fetchSpec
+  if (semver.validRange(spec, true) === null)
+    throw new Error(`invalid version range: ${spec}`)
 
-    if (semver.validRange(spec, true) === null)
-      throw new Error('invalid version range: ' + spec)
+  const uri = '/' + p.escapedName
+  const packument = await fetch.json(uri, {
+    ...npm.flatOptions,
+    spec: p,
+    query: { write: true },
+  })
 
-    const uri = '/' + p.escapedName
-    return fetch.json(uri, {
-      ...opts,
-      spec: p,
-      query: { write: true },
-    }).then(packument => {
-      // filter all the versions that match
-      Object.keys(packument.versions)
-        .filter(v => semver.satisfies(v, spec))
-        .forEach(v => {
-          packument.versions[v].deprecated = msg
-        })
-      return otplease(opts, opts => fetch(uri, {
-        ...opts,
-        spec: p,
-        method: 'PUT',
-        body: packument,
-        ignoreBody: true,
-      }))
+  Object.keys(packument.versions)
+    .filter(v => semver.satisfies(v, spec))
+    .forEach(v => {
+      packument.versions[v].deprecated = msg
     })
-  }).then(() => cb(), cb)
+
+  return otplease(npm.flatOptions, opts => fetch(uri, {
+    ...opts,
+    spec: p,
+    method: 'PUT',
+    body: packument,
+    ignoreBody: true,
+  }))
 }
+
+module.exports = Object.assign(cmd, { completion, usage })

test/lib/deprecate.js

@@ -0,0 +1,134 @@
+const { test } = require('tap')
+const requireInject = require('require-inject')
+
+let getIdentityImpl = () => 'someperson'
+let npmFetchBody = null
+
+const npmFetch = async (uri, opts) => {
+  npmFetchBody = opts.body
+}
+
+npmFetch.json = async (uri, opts) => {
+  return {
+    versions: {
+      '1.0.0': {},
+      '1.0.1': {},
+    },
+  }
+}
+
+const deprecate = requireInject('../../lib/deprecate.js', {
+  '../../lib/npm.js': {
+    flatOptions: { registry: 'https://registry.npmjs.org' },
+  },
+  '../../lib/utils/get-identity.js': async () => getIdentityImpl(),
+  '../../lib/utils/otplease.js': async (opts, fn) => fn(opts),
+  libnpmaccess: {
+    lsPackages: async () => ({ foo: 'write', bar: 'write', baz: 'write', buzz: 'read' }),
+  },
+  'npm-registry-fetch': npmFetch,
+})
+
+test('completion', async t => {
+  const defaultIdentityImpl = getIdentityImpl
+  t.teardown(() => {
+    getIdentityImpl = defaultIdentityImpl
+  })
+
+  const { completion } = deprecate
+
+  const testComp = (argv, expect) => {
+    return new Promise((resolve, reject) => {
+      completion({ conf: { argv: { remain: argv } } }, (err, res) => {
+        if (err)
+          return reject(err)
+
+        t.strictSame(res, expect, `completion: ${argv}`)
+        resolve()
+      })
+    })
+  }
+
+  await testComp([], ['foo', 'bar', 'baz'])
+  await testComp(['b'], ['bar', 'baz'])
+  await testComp(['fo'], ['foo'])
+  await testComp(['g'], [])
+  await testComp(['foo', 'something'], [])
+
+  getIdentityImpl = () => {
+    throw new Error('unknown failure')
+  }
+
+  t.rejects(testComp([], []), /unknown failure/)
+})
+
+test('no args', t => {
+  deprecate([], (err) => {
+    t.match(err, /Usage: npm deprecate/, 'logs usage')
+    t.end()
+  })
+})
+
+test('only one arg', t => {
+  deprecate(['foo'], (err) => {
+    t.match(err, /Usage: npm deprecate/, 'logs usage')
+    t.end()
+  })
+})
+
+test('invalid semver range', t => {
+  deprecate(['foo@notaversion', 'this will fail'], (err) => {
+    t.match(err, /invalid version range/, 'logs semver error')
+    t.end()
+  })
+})
+
+test('deprecates given range', t => {
+  t.teardown(() => {
+    npmFetchBody = null
+  })
+
+  deprecate(['foo@1.0.0', 'this version is deprecated'], (err) => {
+    if (err)
+      throw err
+
+    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,
+        },
+      },
+    })
+
+    t.end()
+  })
+})
+
+test('deprecates all versions when no range is specified', t => {
+  t.teardown(() => {
+    npmFetchBody = null
+  })
+
+  deprecate(['foo', 'this version is deprecated'], (err) => {
+    if (err)
+      throw err
+
+    t.match(npmFetchBody, {
+      versions: {
+        '1.0.0': {
+          deprecated: 'this version is deprecated',
+        },
+        '1.0.1': {
+          deprecated: 'this version is deprecated',
+        },
+      },
+    })
+
+    t.end()
+  })
+})