chore: utils cleanup and tests

- remove spawn util, refactor help command
- add tests for read-user-info, minor refactor
- add tests for pulse-till-done util, refactor
- add tests for otplease util
- add tests for open-url util
- remove unused no-progress-while-running util

PR-URL: npm#2601
Credit: @nlf
Close: npm#2601
Reviewed-by: @ruyadorno, @wraithgar

lib/help.js

@@ -8,22 +8,22 @@ help.completion = function (opts, cb) {
 }
 
 const npmUsage = require('./utils/npm-usage.js')
-var path = require('path')
-var spawn = require('./utils/spawn')
-var npm = require('./npm.js')
-var log = require('npmlog')
-var openUrl = require('./utils/open-url')
-var glob = require('glob')
-var output = require('./utils/output.js')
+const { spawn } = require('child_process')
+const path = require('path')
+const npm = require('./npm.js')
+const log = require('npmlog')
+const openUrl = require('./utils/open-url')
+const glob = require('glob')
+const output = require('./utils/output.js')
 
 const usage = require('./utils/usage.js')
 
 help.usage = usage('help', 'npm help <term> [<terms..>]')
 
 function help (args, cb) {
-  var argv = npm.config.parsedArgv.cooked
+  const argv = npm.config.parsedArgv.cooked
 
-  var argnum = 0
+  let argnum = 0
   if (args.length === 2 && ~~args[0])
     argnum = ~~args.shift()
 
@@ -34,7 +34,7 @@ function help (args, cb) {
   const affordances = {
     'find-dupes': 'dedupe',
   }
-  var section = affordances[args[0]] || npm.deref(args[0]) || args[0]
+  let section = affordances[args[0]] || npm.deref(args[0]) || args[0]
 
   // npm help <noargs>:  show basic usage
   if (!section) {
@@ -52,15 +52,12 @@ function help (args, cb) {
     return cb()
   }
 
-  var pref = [1, 5, 7]
-  if (argnum) {
-    pref = [argnum].concat(pref.filter(function (n) {
-      return n !== argnum
-    }))
-  }
+  let pref = [1, 5, 7]
+  if (argnum)
+    pref = [argnum].concat(pref.filter(n => n !== argnum))
 
   // npm help <section>: Try to find the path
-  var manroot = path.resolve(__dirname, '..', 'man')
+  const manroot = path.resolve(__dirname, '..', 'man')
 
   // legacy
   if (section === 'global')
@@ -71,18 +68,18 @@ function help (args, cb) {
   // find either /section.n or /npm-section.n
   // The glob is used in the glob.  The regexp is used much
   // further down.  Globs and regexps are different
-  var compextglob = '.+(gz|bz2|lzma|[FYzZ]|xz)'
-  var compextre = '\\.(gz|bz2|lzma|[FYzZ]|xz)$'
-  var f = '+(npm-' + section + '|' + section + ').[0-9]?(' + compextglob + ')'
-  return glob(manroot + '/*/' + f, function (er, mans) {
+  const compextglob = '.+(gz|bz2|lzma|[FYzZ]|xz)'
+  const compextre = '\\.(gz|bz2|lzma|[FYzZ]|xz)$'
+  const f = '+(npm-' + section + '|' + section + ').[0-9]?(' + compextglob + ')'
+  return glob(manroot + '/*/' + f, (er, mans) => {
     if (er)
       return cb(er)
 
     if (!mans.length)
       return npm.commands['help-search'](args, cb)
 
-    mans = mans.map(function (man) {
-      var ext = path.extname(man)
+    mans = mans.map((man) => {
+      const ext = path.extname(man)
       if (man.match(new RegExp(compextre)))
         man = path.basename(man, ext)
 
@@ -94,14 +91,12 @@ function help (args, cb) {
 }
 
 function pickMan (mans, pref_) {
-  var nre = /([0-9]+)$/
-  var pref = {}
-  pref_.forEach(function (sect, i) {
-    pref[sect] = i
-  })
-  mans = mans.sort(function (a, b) {
-    var an = a.match(nre)[1]
-    var bn = b.match(nre)[1]
+  const nre = /([0-9]+)$/
+  const pref = {}
+  pref_.forEach((sect, i) => pref[sect] = i)
+  mans = mans.sort((a, b) => {
+    const an = a.match(nre)[1]
+    const bn = b.match(nre)[1]
     return an === bn ? (a > b ? -1 : 1)
       : pref[an] < pref[bn] ? -1
       : 1
@@ -110,48 +105,61 @@ function pickMan (mans, pref_) {
 }
 
 function viewMan (man, cb) {
-  var nre = /([0-9]+)$/
-  var num = man.match(nre)[1]
-  var section = path.basename(man, '.' + num)
+  const nre = /([0-9]+)$/
+  const num = man.match(nre)[1]
+  const section = path.basename(man, '.' + num)
 
   // at this point, we know that the specified man page exists
-  var manpath = path.join(__dirname, '..', 'man')
-  var env = {}
+  const manpath = path.join(__dirname, '..', 'man')
+  const env = {}
   Object.keys(process.env).forEach(function (i) {
     env[i] = process.env[i]
   })
   env.MANPATH = manpath
-  var viewer = npm.config.get('viewer')
+  const viewer = npm.config.get('viewer')
+
+  const opts = {
+    env,
+    stdio: 'inherit',
+  }
 
-  var conf
+  let bin = 'man'
+  const args = []
   switch (viewer) {
     case 'woman':
-      var a = ['-e', '(woman-find-file \'' + man + '\')']
-      conf = { env: env, stdio: 'inherit' }
-      var woman = spawn('emacsclient', a, conf)
-      woman.on('close', cb)
+      bin = 'emacsclient'
+      args.push('-e', `(woman-find-file '${man}')`)
       break
 
     case 'browser':
+      bin = false
       try {
-        var url = htmlMan(man)
+        const url = htmlMan(man)
+        openUrl(url, 'help available at the following URL', cb)
       } catch (err) {
         return cb(err)
       }
-      openUrl(url, 'help available at the following URL', cb)
       break
 
     default:
-      conf = { env: env, stdio: 'inherit' }
-      var manProcess = spawn('man', [num, section], conf)
-      manProcess.on('close', cb)
+      args.push(num, section)
       break
   }
+
+  if (bin) {
+    const proc = spawn(bin, args, opts)
+    proc.on('exit', (code) => {
+      if (code)
+        return cb(new Error(`help process exited with code: ${code}`))
+
+      return cb()
+    })
+  }
 }
 
 function htmlMan (man) {
-  var sect = +man.match(/([0-9]+)$/)[1]
-  var f = path.basename(man).replace(/[.]([0-9]+)$/, '')
+  let sect = +man.match(/([0-9]+)$/)[1]
+  const f = path.basename(man).replace(/[.]([0-9]+)$/, '')
   switch (sect) {
     case 1:
       sect = 'commands'
@@ -169,7 +177,7 @@ function htmlMan (man) {
 }
 
 function getSections (cb) {
-  var g = path.resolve(__dirname, '../man/man[0-9]/*.[0-9]')
+  const g = path.resolve(__dirname, '../man/man[0-9]/*.[0-9]')
   glob(g, function (er, files) {
     if (er)
       return cb(er)

lib/utils/pulse-till-done.js

@@ -1,41 +1,26 @@
 const log = require('npmlog')
 
-let pulsers = 0
-let pulse
+let pulseTimer = null
+const withPromise = async (promise) => {
+  pulseStart()
+  try {
+    return await promise
+  } finally {
+    pulseStop()
+  }
+}
 
-function pulseStart (prefix) {
-  if (++pulsers > 1)
-    return
-  pulse = setInterval(function () {
-    log.gauge.pulse(prefix)
+const pulseStart = () => {
+  pulseTimer = pulseTimer || setInterval(() => {
+    log.gauge.pulse('')
   }, 150)
 }
-function pulseStop () {
-  if (--pulsers > 0)
-    return
-  clearInterval(pulse)
-}
 
-module.exports = function (prefix, cb) {
-  if (!prefix)
-    prefix = 'network'
-  pulseStart(prefix)
-  return (er, ...args) => {
-    pulseStop()
-    cb(er, ...args)
-  }
+const pulseStop = () => {
+  clearInterval(pulseTimer)
+  pulseTimer = null
 }
 
-const pulseWhile = async (prefix, promise) => {
-  if (!promise) {
-    promise = prefix
-    prefix = ''
-  }
-  pulseStart(prefix)
-  try {
-    return await promise
-  } finally {
-    pulseStop()
-  }
+module.exports = {
+  withPromise,
 }
-module.exports.withPromise = pulseWhile

lib/utils/read-user-info.js

@@ -8,41 +8,37 @@ exports.password = readPassword
 exports.username = readUsername
 exports.email = readEmail
 
+const otpPrompt = `This command requires a one-time password (OTP) from your authenticator app.
+Enter one below. You can also pass one on the command line by appending --otp=123456.
+For more information, see:
+https://docs.npmjs.com/getting-started/using-two-factor-authentication
+Enter OTP: `
+const passwordPrompt = 'npm password: '
+const usernamePrompt = 'npm username: '
+const emailPrompt = 'email (this IS public): '
+
 function read (opts) {
   log.clearProgress()
   return readAsync(opts).finally(() => log.showProgress())
 }
 
-function readOTP (msg, otp, isRetry) {
-  if (!msg) {
-    msg = [
-      'This command requires a one-time password (OTP) from your authenticator app.',
-      'Enter one below. You can also pass one on the command line by appending --otp=123456.',
-      'For more information, see:',
-      'https://docs.npmjs.com/getting-started/using-two-factor-authentication',
-      'Enter OTP: ',
-    ].join('\n')
-  }
+function readOTP (msg = otpPrompt, otp, isRetry) {
   if (isRetry && otp && /^[\d ]+$|^[A-Fa-f0-9]{64,64}$/.test(otp))
     return otp.replace(/\s+/g, '')
 
   return read({ prompt: msg, default: otp || '' })
     .then((otp) => readOTP(msg, otp, true))
 }
 
-function readPassword (msg, password, isRetry) {
-  if (!msg)
-    msg = 'npm password: '
+function readPassword (msg = passwordPrompt, password, isRetry) {
   if (isRetry && password)
     return password
 
   return read({ prompt: msg, silent: true, default: password || '' })
     .then((password) => readPassword(msg, password, true))
 }
 
-function readUsername (msg, username, opts, isRetry) {
-  if (!msg)
-    msg = 'npm username: '
+function readUsername (msg = usernamePrompt, username, opts = {}, isRetry) {
   if (isRetry && username) {
     const error = userValidate.username(username)
     if (error)
@@ -55,9 +51,7 @@ function readUsername (msg, username, opts, isRetry) {
     .then((username) => readUsername(msg, username, opts, true))
 }
 
-function readEmail (msg, email, opts, isRetry) {
-  if (!msg)
-    msg = 'email (this IS public): '
+function readEmail (msg = emailPrompt, email, opts = {}, isRetry) {
   if (isRetry && email) {
     const error = userValidate.email(email)
     if (error)