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

4 files changed, 410 insertions, 0 deletions

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')
+})