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: https://github.com/npm/cli/pull/2601
Credit: @nlf
Close: #2601
Reviewed-by: @ruyadorno, @wraithgar

11 files changed, 573 insertions, 186 deletions

diff --git a/lib/help.js b/lib/help.js
index 171c52704..f69961665 100644
--- a/lib/help.js
+++ b/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)
diff --git a/lib/utils/no-progress-while-running.js b/lib/utils/no-progress-while-running.js
deleted file mode 100644
index c2e6a01b2..000000000
--- a/lib/utils/no-progress-while-running.js
+++ /dev/null
@@ -1,25 +0,0 @@
-var log = require('npmlog')
-var progressEnabled
-var running = 0
-
-var startRunning = exports.startRunning = function () {
-  if (progressEnabled == null)
-    progressEnabled = log.progressEnabled
-  if (progressEnabled)
-    log.disableProgress()
-  ++running
-}
-
-var stopRunning = exports.stopRunning = function () {
-  --running
-  if (progressEnabled && running === 0)
-    log.enableProgress()
-}
-
-exports.tillDone = function noProgressTillDone (cb) {
-  startRunning()
-  return function () {
-    stopRunning()
-    cb.apply(this, arguments)
-  }
-}
diff --git a/lib/utils/pulse-till-done.js b/lib/utils/pulse-till-done.js
index 13147bae1..a88b8aacd 100644
--- a/lib/utils/pulse-till-done.js
+++ b/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
diff --git a/lib/utils/read-user-info.js b/lib/utils/read-user-info.js
index b0166e18c..e3c4a9fbe 100644
--- a/lib/utils/read-user-info.js
+++ b/lib/utils/read-user-info.js
@@ -8,21 +8,21 @@ 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, '')
 
@@ -30,9 +30,7 @@ function readOTP (msg, otp, isRetry) {
     .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
 
@@ -40,9 +38,7 @@ function readPassword (msg, password, isRetry) {
     .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)
diff --git a/lib/utils/spawn.js b/lib/utils/spawn.js
deleted file mode 100644
index 3bbe18384..000000000
--- a/lib/utils/spawn.js
+++ /dev/null
@@ -1,58 +0,0 @@
-module.exports = spawn
-
-var _spawn = require('child_process').spawn
-var EventEmitter = require('events').EventEmitter
-var npwr = require('./no-progress-while-running.js')
-
-function willCmdOutput (stdio) {
-  if (stdio === 'inherit')
-    return true
-  if (!Array.isArray(stdio))
-    return false
-  for (var fh = 1; fh <= 2; ++fh) {
-    if (stdio[fh] === 'inherit')
-      return true
-    if (stdio[fh] === 1 || stdio[fh] === 2)
-      return true
-  }
-  return false
-}
-
-function spawn (cmd, args, options) {
-  var cmdWillOutput = willCmdOutput(options && options.stdio)
-
-  if (cmdWillOutput)
-    npwr.startRunning()
-  var raw = _spawn(cmd, args, options)
-  var cooked = new EventEmitter()
-
-  raw.on('error', function (er) {
-    if (cmdWillOutput)
-      npwr.stopRunning()
-    er.file = cmd
-    cooked.emit('error', er)
-  }).on('close', function (code, signal) {
-    if (cmdWillOutput)
-      npwr.stopRunning()
-    // Create ENOENT error because Node.js v0.8 will not emit
-    // an `error` event if the command could not be found.
-    if (code === 127) {
-      var er = new Error('spawn ENOENT')
-      er.code = 'ENOENT'
-      er.errno = 'ENOENT'
-      er.syscall = 'spawn'
-      er.file = cmd
-      cooked.emit('error', er)
-    } else
-      cooked.emit('close', code, signal)
-  })
-
-  cooked.stdin = raw.stdin
-  cooked.stdout = raw.stdout
-  cooked.stderr = raw.stderr
-  cooked.kill = function (sig) {
-    return raw.kill(sig)
-  }
-
-  return cooked
-}
diff --git a/tap-snapshots/test-lib-utils-open-url.js-TAP.test.js b/tap-snapshots/test-lib-utils-open-url.js-TAP.test.js
new file mode 100644
index 000000000..8c8159ebc
--- /dev/null
+++ b/tap-snapshots/test-lib-utils-open-url.js-TAP.test.js
@@ -0,0 +1,25 @@
+/* IMPORTANT
+ * This snapshot file is auto-generated, but designed for humans.
+ * It should be checked into source control and tracked carefully.
+ * Re-generate by setting TAP_SNAPSHOT=1 and running tests.
+ * Make sure to inspect the output below.  Do not ignore changes!
+ */
+'use strict'
+exports[`test/lib/utils/open-url.js TAP prints where to go when browser is disabled > printed expected message 1`] = `
+npm home:
+  https://www.npmjs.com
+
+`
+
+exports[`test/lib/utils/open-url.js TAP prints where to go when browser is disabled and json is enabled > printed expected message 1`] = `
+{
+  "title": "npm home",
+  "url": "https://www.npmjs.com"
+}
+`
+
+exports[`test/lib/utils/open-url.js TAP prints where to go when given browser does not exist > printed expected message 1`] = `
+npm home:
+  https://www.npmjs.com
+
+`
diff --git a/test/lib/help.js b/test/lib/help.js
index 17018acc6..40a035421 100644
--- a/test/lib/help.js
+++ b/test/lib/help.js
@@ -55,12 +55,13 @@ const glob = (p, cb) => {
 
 let spawnBin = null
 let spawnArgs = null
+let spawnCode = 0
 const spawn = (bin, args) => {
   spawnBin = bin
   spawnArgs = args
   const spawnEmitter = new EventEmitter()
   process.nextTick(() => {
-    spawnEmitter.emit('close', 0)
+    spawnEmitter.emit('exit', spawnCode)
   })
   return spawnEmitter
 }
@@ -76,7 +77,9 @@ const help = requireInject('../../lib/help.js', {
   '../../lib/utils/npm-usage.js': npmUsage,
   '../../lib/utils/open-url.js': openUrl,
   '../../lib/utils/output.js': output,
-  '../../lib/utils/spawn.js': spawn,
+  child_process: {
+    spawn,
+  },
   glob,
 })
 
@@ -339,6 +342,29 @@ test('npm help ?(un)star', t => {
   })
 })
 
+test('npm help - woman viewer propagates errors', t => {
+  npmConfig.viewer = 'woman'
+  spawnCode = 1
+  globResult = [
+    '/root/man/man1/npm-star.1',
+    '/root/man/man1/npm-unstar.1',
+  ]
+  t.teardown(() => {
+    npmConfig.viewer = undefined
+    spawnCode = 0
+    globResult = globDefaults
+    spawnBin = null
+    spawnArgs = null
+  })
+
+  return help(['?(un)star'], (err) => {
+    t.match(err, /help process exited with code: 1/, 'received the correct error')
+    t.equal(spawnBin, 'emacsclient', 'maps woman to emacs correctly')
+    t.strictSame(spawnArgs, ['-e', `(woman-find-file '/root/man/man1/npm-unstar.1')`], 'passes the correct arguments')
+    t.end()
+  })
+})
+
 test('npm help un*', t => {
   globResult = [
     '/root/man/man1/npm-unstar.1',
@@ -360,3 +386,25 @@ test('npm help un*', t => {
     t.end()
   })
 })
+
+test('npm help - man viewer propagates errors', t => {
+  spawnCode = 1
+  globResult = [
+    '/root/man/man1/npm-unstar.1',
+    '/root/man/man1/npm-uninstall.1',
+    '/root/man/man1/npm-unpublish.1',
+  ]
+  t.teardown(() => {
+    spawnCode = 0
+    globResult = globDefaults
+    spawnBin = null
+    spawnArgs = null
+  })
+
+  return help(['un*'], (err) => {
+    t.match(err, /help process exited with code: 1/, 'received correct error')
+    t.equal(spawnBin, 'man', 'calls man by default')
+    t.strictSame(spawnArgs, ['1', 'npm-unstar'], 'passes the correct arguments')
+    t.end()
+  })
+})
diff --git a/test/lib/utils/open-url.js b/test/lib/utils/open-url.js
new file mode 100644
index 000000000..ce1783dad
--- /dev/null
+++ b/test/lib/utils/open-url.js
@@ -0,0 +1,165 @@
+const { test } = require('tap')
+const requireInject = require('require-inject')
+
+const npm = {
+  _config: {
+    json: false,
+    browser: true,
+  },
+  config: {
+    get: (k) => npm._config[k],
+    set: (k, v) => {
+      npm._config[k] = v
+    },
+  },
+}
+
+const OUTPUT = []
+const output = (...args) => OUTPUT.push(args)
+
+let openerUrl = null
+let openerOpts = null
+let openerResult = null
+const opener = (url, opts, cb) => {
+  openerUrl = url
+  openerOpts = opts
+  return cb(openerResult)
+}
+
+const openUrl = requireInject('../../../lib/utils/open-url.js', {
+  '../../../lib/npm.js': npm,
+  '../../../lib/utils/output.js': output,
+  opener,
+})
+
+test('opens a url', (t) => {
+  t.teardown(() => {
+    openerUrl = null
+    openerOpts = null
+    OUTPUT.length = 0
+  })
+  openUrl('https://www.npmjs.com', 'npm home', (err) => {
+    if (err)
+      throw err
+
+    t.equal(openerUrl, 'https://www.npmjs.com', 'opened the given url')
+    t.same(openerOpts, { command: null }, 'passed command as null (the default)')
+    t.same(OUTPUT, [], 'printed no output')
+    t.done()
+  })
+})
+
+test('returns error for non-https and non-file url', (t) => {
+  t.teardown(() => {
+    openerUrl = null
+    openerOpts = null
+    OUTPUT.length = 0
+  })
+  openUrl('ftp://www.npmjs.com', 'npm home', (err) => {
+    t.match(err, /Invalid URL/, 'got the correct error')
+    t.equal(openerUrl, null, 'did not open')
+    t.same(openerOpts, null, 'did not open')
+    t.same(OUTPUT, [], 'printed no output')
+    t.done()
+  })
+})
+
+test('returns error for non-parseable url', (t) => {
+  t.teardown(() => {
+    openerUrl = null
+    openerOpts = null
+    OUTPUT.length = 0
+  })
+  openUrl('git+ssh://user@host:repo.git', 'npm home', (err) => {
+    t.match(err, /Invalid URL/, 'got the correct error')
+    t.equal(openerUrl, null, 'did not open')
+    t.same(openerOpts, null, 'did not open')
+    t.same(OUTPUT, [], 'printed no output')
+    t.done()
+  })
+})
+
+test('opens a url with the given browser', (t) => {
+  npm.config.set('browser', 'chrome')
+  t.teardown(() => {
+    openerUrl = null
+    openerOpts = null
+    OUTPUT.length = 0
+    npm.config.set('browser', true)
+  })
+  openUrl('https://www.npmjs.com', 'npm home', (err) => {
+    if (err)
+      throw err
+
+    t.equal(openerUrl, 'https://www.npmjs.com', 'opened the given url')
+    t.same(openerOpts, { command: 'chrome' }, 'passed the given browser as command')
+    t.same(OUTPUT, [], 'printed no output')
+    t.done()
+  })
+})
+
+test('prints where to go when browser is disabled', (t) => {
+  npm.config.set('browser', false)
+  t.teardown(() => {
+    openerUrl = null
+    openerOpts = null
+    OUTPUT.length = 0
+    npm.config.set('browser', true)
+  })
+  openUrl('https://www.npmjs.com', 'npm home', (err) => {
+    if (err)
+      throw err
+
+    t.equal(openerUrl, null, 'did not open')
+    t.same(openerOpts, null, 'did not open')
+    t.equal(OUTPUT.length, 1, 'got one logged message')
+    t.equal(OUTPUT[0].length, 1, 'logged message had one value')
+    t.matchSnapshot(OUTPUT[0][0], 'printed expected message')
+    t.done()
+  })
+})
+
+test('prints where to go when browser is disabled and json is enabled', (t) => {
+  npm.config.set('browser', false)
+  npm.config.set('json', true)
+  t.teardown(() => {
+    openerUrl = null
+    openerOpts = null
+    OUTPUT.length = 0
+    npm.config.set('browser', true)
+    npm.config.set('json', false)
+  })
+  openUrl('https://www.npmjs.com', 'npm home', (err) => {
+    if (err)
+      throw err
+
+    t.equal(openerUrl, null, 'did not open')
+    t.same(openerOpts, null, 'did not open')
+    t.equal(OUTPUT.length, 1, 'got one logged message')
+    t.equal(OUTPUT[0].length, 1, 'logged message had one value')
+    t.matchSnapshot(OUTPUT[0][0], 'printed expected message')
+    t.done()
+  })
+})
+
+test('prints where to go when given browser does not exist', (t) => {
+  npm.config.set('browser', 'firefox')
+  openerResult = Object.assign(new Error('failed'), { code: 'ENOENT' })
+  t.teardown(() => {
+    openerUrl = null
+    openerOpts = null
+    OUTPUT.length = 0
+    npm.config.set('browser', true)
+  })
+  openUrl('https://www.npmjs.com', 'npm home', (err) => {
+    if (err)
+      throw err
+
+    t.equal(openerUrl, 'https://www.npmjs.com', 'tried to open the correct url')
+    t.same(openerOpts, { command: 'firefox' }, 'tried to use the correct browser')
+    t.equal(OUTPUT.length, 1, 'got one logged message')
+    t.equal(OUTPUT[0].length, 1, 'logged message had one value')
+    t.matchSnapshot(OUTPUT[0][0], 'printed expected message')
+    t.done()
+  })
+})
diff --git a/test/lib/utils/otplease.js b/test/lib/utils/otplease.js
new file mode 100644
index 000000000..048856b48
--- /dev/null
+++ b/test/lib/utils/otplease.js
@@ -0,0 +1,94 @@
+const { test } = require('tap')
+const requireInject = require('require-inject')
+
+const readUserInfo = {
+  otp: async () => '1234',
+}
+
+const otplease = requireInject('../../../lib/utils/otplease.js', {
+  '../../../lib/utils/read-user-info.js': readUserInfo,
+})
+
+test('prompts for otp for EOTP', async (t) => {
+  const stdinTTY = process.stdin.isTTY
+  const stdoutTTY = process.stdout.isTTY
+  process.stdin.isTTY = true
+  process.stdout.isTTY = true
+  t.teardown(() => {
+    process.stdin.isTTY = stdinTTY
+    process.stdout.isTTY = stdoutTTY
+  })
+
+  let runs = 0
+  const fn = async (opts) => {
+    if (++runs === 1)
+      throw Object.assign(new Error('nope'), { code: 'EOTP' })
+
+    t.equal(opts.some, 'prop', 'carried original options')
+    t.equal(opts.otp, '1234', 'received the otp')
+    t.done()
+  }
+
+  await otplease({ some: 'prop' }, fn)
+})
+
+test('prompts for otp for 401', async (t) => {
+  const stdinTTY = process.stdin.isTTY
+  const stdoutTTY = process.stdout.isTTY
+  process.stdin.isTTY = true
+  process.stdout.isTTY = true
+  t.teardown(() => {
+    process.stdin.isTTY = stdinTTY
+    process.stdout.isTTY = stdoutTTY
+  })
+
+  let runs = 0
+  const fn = async (opts) => {
+    if (++runs === 1) {
+      throw Object.assign(new Error('nope'), {
+        code: 'E401',
+        body: 'one-time pass required',
+      })
+    }
+
+    t.equal(opts.some, 'prop', 'carried original options')
+    t.equal(opts.otp, '1234', 'received the otp')
+    t.done()
+  }
+
+  await otplease({ some: 'prop' }, fn)
+})
+
+test('does not prompt for non-otp errors', async (t) => {
+  const stdinTTY = process.stdin.isTTY
+  const stdoutTTY = process.stdout.isTTY
+  process.stdin.isTTY = true
+  process.stdout.isTTY = true
+  t.teardown(() => {
+    process.stdin.isTTY = stdinTTY
+    process.stdout.isTTY = stdoutTTY
+  })
+
+  const fn = async (opts) => {
+    throw new Error('nope')
+  }
+
+  t.rejects(otplease({ some: 'prop' }, fn), { message: 'nope' }, 'rejects with the original error')
+})
+
+test('does not prompt if stdin or stdout is not a tty', async (t) => {
+  const stdinTTY = process.stdin.isTTY
+  const stdoutTTY = process.stdout.isTTY
+  process.stdin.isTTY = false
+  process.stdout.isTTY = false
+  t.teardown(() => {
+    process.stdin.isTTY = stdinTTY
+    process.stdout.isTTY = stdoutTTY
+  })
+
+  const fn = async (opts) => {
+    throw Object.assign(new Error('nope'), { code: 'EOTP' })
+  }
+
+  t.rejects(otplease({ some: 'prop' }, fn), { message: 'nope' }, 'rejects with the original error')
+})
diff --git a/test/lib/utils/pulse-till-done.js b/test/lib/utils/pulse-till-done.js
new file mode 100644
index 000000000..16c2d521d
--- /dev/null
+++ b/test/lib/utils/pulse-till-done.js
@@ -0,0 +1,35 @@
+const { test } = require('tap')
+const requireInject = require('require-inject')
+
+let pulseStarted = null
+const npmlog = {
+  gauge: {
+    pulse: () => {
+      if (pulseStarted)
+        pulseStarted()
+    },
+  },
+}
+
+const pulseTillDone = requireInject('../../../lib/utils/pulse-till-done.js', {
+  npmlog,
+})
+
+test('pulses (with promise)', async (t) => {
+  t.teardown(() => {
+    pulseStarted = null
+  })
+
+  let resolver
+  const promise = new Promise(resolve => {
+    resolver = resolve
+  })
+
+  const result = pulseTillDone.withPromise(promise)
+  // wait until the gauge has fired at least once
+  await new Promise(resolve => {
+    pulseStarted = resolve
+  })
+  resolver('value')
+  t.resolveMatch(result, 'value', 'returned the resolved promise')
+})
diff --git a/test/lib/utils/read-user-info.js b/test/lib/utils/read-user-info.js
new file mode 100644
index 000000000..99d85d66c
--- /dev/null
+++ b/test/lib/utils/read-user-info.js
@@ -0,0 +1,116 @@
+const { test } = require('tap')
+const requireInject = require('require-inject')
+
+let readOpts = null
+let readResult = null
+const read = (opts, cb) => {
+  readOpts = opts
+  return cb(null, readResult)
+}
+
+const npmlog = {
+  clearProgress: () => {},
+  showProgress: () => {},
+}
+
+const npmUserValidate = {
+  username: (username) => {
+    if (username === 'invalid')
+      return new Error('invalid username')
+
+    return null
+  },
+  email: (email) => {
+    if (email.startsWith('invalid'))
+      return new Error('invalid email')
+
+    return null
+  },
+}
+
+const readUserInfo = requireInject('../../../lib/utils/read-user-info.js', {
+  read,
+  npmlog,
+  'npm-user-validate': npmUserValidate,
+})
+
+test('otp', async (t) => {
+  readResult = '1234'
+  t.teardown(() => {
+    readResult = null
+    readOpts = null
+  })
+  const result = await readUserInfo.otp()
+  t.equal(result, '1234', 'received the otp')
+})
+
+test('password', async (t) => {
+  readResult = 'password'
+  t.teardown(() => {
+    readResult = null
+    readOpts = null
+  })
+  const result = await readUserInfo.password()
+  t.equal(result, 'password', 'received the password')
+  t.match(readOpts, {
+    silent: true,
+  }, 'got the correct options')
+})
+
+test('username', async (t) => {
+  readResult = 'username'
+  t.teardown(() => {
+    readResult = null
+    readOpts = null
+  })
+  const result = await readUserInfo.username()
+  t.equal(result, 'username', 'received the username')
+})
+
+test('username - invalid warns and retries', async (t) => {
+  readResult = 'invalid'
+  t.teardown(() => {
+    readResult = null
+    readOpts = null
+  })
+
+  let logMsg
+  const log = {
+    warn: (msg) => logMsg = msg,
+  }
+  const pResult = readUserInfo.username(null, null, { log })
+  // have to swap it to a valid username after execution starts
+  // or it will loop forever
+  readResult = 'valid'
+  const result = await pResult
+  t.equal(result, 'valid', 'received the username')
+  t.equal(logMsg, 'invalid username')
+})
+
+test('email', async (t) => {
+  readResult = 'foo@bar.baz'
+  t.teardown(() => {
+    readResult = null
+    readOpts = null
+  })
+  const result = await readUserInfo.email()
+  t.equal(result, 'foo@bar.baz', 'received the email')
+})
+
+test('email - invalid warns and retries', async (t) => {
+  readResult = 'invalid@bar.baz'
+  t.teardown(() => {
+    readResult = null
+    readOpts = null
+  })
+
+  let logMsg
+  const log = {
+    warn: (msg) => logMsg = msg,
+  }
+  const pResult = readUserInfo.email(null, null, { log })
+  readResult = 'foo@bar.baz'
+  const result = await pResult
+  t.equal(result, 'foo@bar.baz', 'received the email')
+  t.equal(logMsg, 'invalid email')
+})