From 9a834536e081fea94d7b99d20ac073e61d4fa8bf Mon Sep 17 00:00:00 2001
From: Gar <gar+gh@danger.computer>
Date: Mon, 21 Jun 2021 14:41:31 -0700
Subject: [PATCH] chore(refactor): async npm.load

Tests for cli now use the real npm
---
 lib/cli.js                     |  14 +-
 lib/npm.js                     |  71 ++---
 lib/utils/perf.js              |   7 +
 lib/utils/proc-log-listener.js |   6 +
 test/lib/cli.js                | 241 ++++++---------
 test/lib/npm.js                | 521 ++++++++++++++-------------------
 test/lib/utils/exit-handler.js |   2 +-
 7 files changed, 369 insertions(+), 493 deletions(-)

diff --git a/lib/cli.js b/lib/cli.js
index 106dd629d4e19..9544f8451f8ae 100644
--- a/lib/cli.js
+++ b/lib/cli.js
@@ -1,5 +1,5 @@
 // Separated out for easier unit testing
-module.exports = (process) => {
+module.exports = async (process) => {
   // set it here so that regardless of what happens later, we don't
   // leak any private CLI configs to other programs
   process.title = 'npm'
@@ -39,12 +39,8 @@ module.exports = (process) => {
 
   // now actually fire up npm and run the command.
   // this is how to use npm programmatically:
-  npm.load(async er => {
-    // Any exceptions here will be picked up by the uncaughtException handler
-    if (er)
-      return exitHandler(er)
-
-    // npm --version=cli
+  try {
+    await npm.load()
     if (npm.config.get('version', 'cli')) {
       npm.output(npm.version)
       return exitHandler()
@@ -75,5 +71,7 @@ module.exports = (process) => {
     }
 
     impl(npm.argv, exitHandler)
-  })
+  } catch (err) {
+    return exitHandler(err)
+  }
 }
diff --git a/lib/npm.js b/lib/npm.js
index 937459501c0a5..7046a84d0bcfa 100644
--- a/lib/npm.js
+++ b/lib/npm.js
@@ -11,6 +11,7 @@ const Config = require('@npmcli/config')
 // Patch the global fs module here at the app level
 require('graceful-fs').gracefulify(require('fs'))
 
+// TODO make this only ever load once (or unload) in tests
 const procLogListener = require('./utils/proc-log-listener.js')
 
 const proxyCmds = new Proxy({}, {
@@ -48,6 +49,7 @@ const _title = Symbol('_title')
 const npm = module.exports = new class extends EventEmitter {
   constructor () {
     super()
+    // TODO make this only ever load once (or unload) in tests
     require('./utils/perf.js')
     this.started = Date.now()
     this.command = null
@@ -77,8 +79,8 @@ const npm = module.exports = new class extends EventEmitter {
   [_runCmd] (cmd, impl, args, cb) {
     if (!this.loaded) {
       throw new Error(
-        'Call npm.load(cb) before using this command.\n' +
-        'See the README.md or bin/npm-cli.js for example usage.'
+        'Call npm.load() before using this command.\n' +
+        'See lib/cli.js for example usage.'
       )
     }
 
@@ -96,7 +98,7 @@ const npm = module.exports = new class extends EventEmitter {
       args.filter(arg => /^[\u2010-\u2015\u2212\uFE58\uFE63\uFF0D]/.test(arg))
         .forEach(arg => {
           warnedNonDashArg = true
-          log.error('arg', 'Argument starts with non-ascii dash, this is probably invalid:', arg)
+          this.log.error('arg', 'Argument starts with non-ascii dash, this is probably invalid:', arg)
         })
     }
 
@@ -123,33 +125,32 @@ const npm = module.exports = new class extends EventEmitter {
     }
   }
 
-  // call with parsed CLI options and a callback when done loading
-  // XXX promisify this and stop taking a callback
   load (cb) {
-    if (!cb || typeof cb !== 'function')
-      throw new TypeError('must call as: npm.load(callback)')
-
-    this.once('load', cb)
-    if (this.loaded || this.loadErr) {
-      this.emit('load', this.loadErr)
-      return
+    if (cb && typeof cb !== 'function')
+      throw new TypeError('callback must be a function if provided')
+
+    if (!this.loadPromise) {
+      process.emit('time', 'npm:load')
+      this.log.pause()
+      this.loadPromise = new Promise((resolve, reject) => {
+        this[_load]().catch(er => er).then((er) => {
+          this.loadErr = er
+          if (!er && this.config.get('force'))
+            this.log.warn('using --force', 'Recommended protections disabled.')
+
+          process.emit('timeEnd', 'npm:load')
+          if (er)
+            return reject(er)
+          resolve()
+        })
+      })
     }
-    if (this.loading)
-      return
-
-    this.loading = true
+    if (!cb)
+      return this.loadPromise
 
-    process.emit('time', 'npm:load')
-    this.log.pause()
-    return this[_load]().catch(er => er).then((er) => {
-      this.loading = false
-      this.loadErr = er
-      if (!er && this.config.get('force'))
-        this.log.warn('using --force', 'Recommended protections disabled.')
-
-      process.emit('timeEnd', 'npm:load')
-      this.emit('load', er)
-    })
+    // loadPromise is returned here for legacy purposes, old code was allowing
+    // the mixing of callback and promise here.
+    return this.loadPromise.then(cb, cb)
   }
 
   get loaded () {
@@ -167,10 +168,15 @@ const npm = module.exports = new class extends EventEmitter {
 
   async [_load] () {
     process.emit('time', 'npm:load:whichnode')
-    const node = await which(process.argv[0]).catch(er => null)
+    let node
+    try {
+      node = which.sync(process.argv[0])
+    } catch (_) {
+      // TODO should we throw here?
+    }
     process.emit('timeEnd', 'npm:load:whichnode')
     if (node && node.toUpperCase() !== process.execPath.toUpperCase()) {
-      log.verbose('node symlink', node)
+      this.log.verbose('node symlink', node)
       process.execPath = node
       this.config.execPath = node
     }
@@ -198,10 +204,10 @@ const npm = module.exports = new class extends EventEmitter {
     process.env.COLOR = this.color ? '1' : '0'
 
     process.emit('time', 'npm:load:cleanupLog')
-    cleanUpLogFiles(this.cache, this.config.get('logs-max'), log.warn)
+    cleanUpLogFiles(this.cache, this.config.get('logs-max'), this.log.warn)
     process.emit('timeEnd', 'npm:load:cleanupLog')
 
-    log.resume()
+    this.log.resume()
 
     process.emit('time', 'npm:load:configScope')
     const configScope = this.config.get('scope')
@@ -314,9 +320,8 @@ const npm = module.exports = new class extends EventEmitter {
 // now load everything required by the class methods
 
 const log = require('npmlog')
-const { promisify } = require('util')
 
-const which = promisify(require('which'))
+const which = require('which')
 
 const deref = require('./utils/deref-command.js')
 const setupLog = require('./utils/setup-log.js')
diff --git a/lib/utils/perf.js b/lib/utils/perf.js
index 3f81ee4b049e4..4961054d909ad 100644
--- a/lib/utils/perf.js
+++ b/lib/utils/perf.js
@@ -14,3 +14,10 @@ process.on('timeEnd', (name) => {
   } else
     log.silly('timing', "Tried to end timer that doesn't exist:", name)
 })
+
+// for tests
+/* istanbul ignore next */
+exports.reset = () => {
+  process.removeAllListeners('time')
+  process.removeAllListeners('timeEnd')
+}
diff --git a/lib/utils/proc-log-listener.js b/lib/utils/proc-log-listener.js
index 1dc4b4399eaea..2cfe94ecb0cf2 100644
--- a/lib/utils/proc-log-listener.js
+++ b/lib/utils/proc-log-listener.js
@@ -14,3 +14,9 @@ module.exports = () => {
     }
   })
 }
+
+// for tests
+/* istanbul ignore next */
+module.exports.reset = () => {
+  process.removeAllListeners('log')
+}
diff --git a/test/lib/cli.js b/test/lib/cli.js
index 132d4224b6b79..89c3736e886d2 100644
--- a/test/lib/cli.js
+++ b/test/lib/cli.js
@@ -1,23 +1,8 @@
 const t = require('tap')
 
-let LOAD_ERROR = null
-const npmOutputs = []
-const npmock = {
-  log: { level: 'silent' },
-  output: (...msg) => npmOutputs.push(msg),
-  usage: 'npm usage test example',
-  version: '99.99.99',
-  load: cb => cb(LOAD_ERROR),
-  argv: [],
-  config: {
-    settings: {},
-    get: (k) => npmock.config.settings[k],
-    set: (k, v) => {
-      npmock.config.settings[k] = v
-    },
-  },
-  commands: {},
-}
+// NOTE lib/npm.js is wrapped in t.mock() every time because it's currently a
+// singleton.  In the next semver major we will export the class and lib/cli.js
+// can call `new` on it and then we won't have to do that anymore
 
 const unsupportedMock = {
   checkForBrokenNode: () => {},
@@ -43,166 +28,130 @@ const npmlogMock = {
   info: (...msg) => logs.push(['info', ...msg]),
 }
 
-const cli = t.mock('../../lib/cli.js', {
-  '../../lib/npm.js': npmock,
+const cliMock = (npm) => t.mock('../../lib/cli.js', {
+  '../../lib/npm.js': npm,
   '../../lib/utils/update-notifier.js': async () => null,
-  '../../lib/utils/did-you-mean.js': () => '\ntest did you mean',
   '../../lib/utils/unsupported.js': unsupportedMock,
   '../../lib/utils/exit-handler.js': exitHandlerMock,
   npmlog: npmlogMock,
 })
 
-t.test('print the version, and treat npm_g to npm -g', t => {
-  t.teardown(() => {
-    delete npmock.config.settings.version
-    process.argv = argv
-    npmock.argv.length = 0
-    proc.argv.length = 0
-    logs.length = 0
-    npmOutputs.length = 0
-    exitHandlerCalled = null
-  })
+const npmOutputs = []
+const npmMock = () => {
+  const npm = t.mock('../../lib/npm.js')
+  npm.output = (...msg) => npmOutputs.push(msg)
+  return npm
+}
 
-  const { argv } = process
-  const proc = {
-    argv: ['node', 'npm_g', '-v'],
-    version: '420.69.lol',
+const processMock = (proc) => {
+  const mocked = {
+    ...process,
     on: () => {},
+    ...proc,
   }
-  process.argv = proc.argv
-  npmock.config.settings.version = true
+  // nopt looks at process directly
+  process.argv = mocked.argv
+  return mocked
+}
+
+const { argv } = process
 
-  cli(proc)
+t.afterEach(() => {
+  logs.length = 0
+  process.argv = argv
+  npmOutputs.length = 0
+  exitHandlerCalled = null
+  exitHandlerNpm = null
+})
 
-  t.strictSame(npmock.argv, [])
-  t.strictSame(proc.argv, ['node', 'npm', '-g', '-v'])
+t.test('print the version, and treat npm_g as npm -g', async t => {
+  const proc = processMock({
+    argv: ['node', 'npm_g', '-v'],
+    version: process.version,
+  })
+
+  const npm = npmMock()
+  const cli = cliMock(npm)
+  await cli(proc)
+
+  t.strictSame(proc.argv, ['node', 'npm', '-g', '-v'], 'npm process.argv was rewritten')
+  t.strictSame(process.argv, ['node', 'npm', '-g', '-v'], 'system process.argv was rewritten')
   t.strictSame(logs, [
     'pause',
-    ['verbose', 'cli', ['node', 'npm', '-g', '-v']],
-    ['info', 'using', 'npm@%s', '99.99.99'],
-    ['info', 'using', 'node@%s', '420.69.lol'],
+    ['verbose', 'cli', proc.argv],
+    ['info', 'using', 'npm@%s', npm.version],
+    ['info', 'using', 'node@%s', process.version],
   ])
-  t.strictSame(npmOutputs, [['99.99.99']])
+  t.strictSame(npmOutputs, [[npm.version]])
   t.strictSame(exitHandlerCalled, [])
-
-  t.end()
 })
 
-t.test('calling with --versions calls npm version with no args', t => {
-  const processArgv = process.argv
-  const proc = {
+t.test('calling with --versions calls npm version with no args', async t => {
+  const proc = processMock({
     argv: ['node', 'npm', 'install', 'or', 'whatever', '--versions'],
-    on: () => {},
-  }
-  process.argv = proc.argv
-  npmock.config.set('versions', true)
-
-  t.teardown(() => {
-    delete npmock.config.settings.versions
-    process.argv = processArgv
-    npmock.argv.length = 0
-    proc.argv.length = 0
-    logs.length = 0
-    npmOutputs.length = 0
-    exitHandlerCalled = null
-    delete npmock.commands.version
   })
+  const npm = npmMock()
+  const cli = cliMock(npm)
 
-  npmock.commands.version = (args, cb) => {
-    t.equal(proc.title, 'npm')
-    t.strictSame(npmock.argv, [])
-    t.strictSame(proc.argv, ['node', 'npm', 'install', 'or', 'whatever', '--versions'])
-    t.strictSame(logs, [
-      'pause',
-      ['verbose', 'cli', ['node', 'npm', 'install', 'or', 'whatever', '--versions']],
-      ['info', 'using', 'npm@%s', '99.99.99'],
-      ['info', 'using', 'node@%s', undefined],
-    ])
-
-    t.strictSame(npmOutputs, [])
-    t.strictSame(exitHandlerCalled, null)
-
-    t.strictSame(args, [])
-    t.end()
+  let versionArgs
+  npm.commands.version = (args, cb) => {
+    versionArgs = args
+    cb()
   }
 
-  cli(proc)
-})
+  await cli(proc)
+  t.strictSame(versionArgs, [])
+  t.equal(proc.title, 'npm')
+  t.strictSame(npm.argv, [])
+  t.strictSame(logs, [
+    'pause',
+    ['verbose', 'cli', proc.argv],
+    ['info', 'using', 'npm@%s', npm.version],
+    ['info', 'using', 'node@%s', process.version],
+  ])
 
-t.test('print usage if no params provided', t => {
-  const { output } = npmock
-  t.teardown(() => {
-    npmock.output = output
-  })
-  const proc = {
-    argv: ['node', 'npm'],
-    on: () => {},
-  }
-  npmock.argv = []
-  npmock.output = (msg) => {
-    if (msg) {
-      t.match(msg, 'npm usage test example', 'outputs npm usage')
-      t.end()
-    }
-  }
-  cli(proc)
+  t.strictSame(npmOutputs, [])
+  t.strictSame(exitHandlerCalled, [])
 })
 
-t.test('print usage if non-command param provided', t => {
-  const { output } = npmock
-  t.teardown(() => {
-    npmock.output = output
+t.test('print usage if no params provided', async t => {
+  const proc = processMock({
+    argv: ['node', 'npm'],
   })
-  const proc = {
-    argv: ['node', 'npm', 'asdf'],
-    on: () => {},
-  }
-  npmock.argv = ['asdf']
-  npmock.output = (msg) => {
-    if (msg) {
-      t.match(msg, 'Unknown command: "asdf"\ntest did you mean', 'outputs did you mean')
-      t.end()
-    }
-  }
-  cli(proc)
+
+  const npm = npmMock()
+  const cli = cliMock(npm)
+  await cli(proc)
+  t.match(npmOutputs[0][0], 'Usage:', 'outputs npm usage')
+  t.match(exitHandlerCalled, [], 'should call exitHandler with no args')
+  t.ok(exitHandlerNpm, 'exitHandler npm is set')
+  t.match(proc.exitCode, 1)
 })
 
-t.test('gracefully handles error printing usage', t => {
-  const { output } = npmock
-  t.teardown(() => {
-    npmock.output = output
-    exitHandlerCb = null
-    exitHandlerCalled = null
+t.test('print usage if non-command param provided', async t => {
+  const proc = processMock({
+    argv: ['node', 'npm', 'tset'],
   })
-  const proc = {
-    argv: ['node', 'npm'],
-    on: () => {},
-  }
-  npmock.argv = []
-  exitHandlerCb = () => {
-    t.match(exitHandlerCalled, [], 'should call exitHandler with no args')
-    t.match(exitHandlerNpm, npmock, 'exitHandler npm is set')
-    t.end()
-  }
-  cli(proc)
+
+  const npm = npmMock()
+  const cli = cliMock(npm)
+  await cli(proc)
+  t.match(npmOutputs[0][0], 'Unknown command: "tset"')
+  t.match(npmOutputs[0][0], 'Did you mean this?')
+  t.match(exitHandlerCalled, [], 'should call exitHandler with no args')
+  t.ok(exitHandlerNpm, 'exitHandler npm is set')
+  t.match(proc.exitCode, 1)
 })
 
-t.test('load error calls error handler', t => {
-  t.teardown(() => {
-    exitHandlerCb = null
-    exitHandlerCalled = null
-    LOAD_ERROR = null
+t.test('load error calls error handler', async t => {
+  const proc = processMock({
+    argv: ['node', 'npm', 'asdf'],
   })
 
+  const npm = npmMock()
+  const cli = cliMock(npm)
   const er = new Error('test load error')
-  LOAD_ERROR = er
-  const proc = {
-    argv: ['node', 'npm', 'asdf'],
-    on: () => {},
-  }
-  exitHandlerCb = () => {
-    t.strictSame(exitHandlerCalled, [er])
-    t.end()
-  }
-  cli(proc)
+  npm.load = () => Promise.reject(er)
+  await cli(proc)
+  t.strictSame(exitHandlerCalled, [er])
 })
diff --git a/test/lib/npm.js b/test/lib/npm.js
index 6909c43e4ff0e..1ccfbbd7ea507 100644
--- a/test/lib/npm.js
+++ b/test/lib/npm.js
@@ -1,8 +1,13 @@
 const t = require('tap')
 
+const npmlog = require('npmlog')
+const perf = require('../../lib/utils/perf.js')
+const procLog = require('../../lib/utils/proc-log-listener.js')
+
 // delete this so that we don't have configs from the fact that it
 // is being run by 'npm test'
 const event = process.env.npm_lifecycle_event
+
 for (const env of Object.keys(process.env).filter(e => /^npm_/.test(e))) {
   if (env === 'npm_command') {
     // should only be running this in the 'test' or 'run-script' command!
@@ -23,52 +28,57 @@ for (const env of Object.keys(process.env).filter(e => /^npm_/.test(e))) {
 const { resolve, dirname } = require('path')
 
 const actualPlatform = process.platform
-
 const beWindows = () => {
   Object.defineProperty(process, 'platform', {
     value: 'win32',
     configurable: true,
   })
 }
-
 const bePosix = () => {
   Object.defineProperty(process, 'platform', {
     value: 'posix',
     configurable: true,
   })
 }
+const argv = [...process.argv]
 
-const npmlog = require('npmlog')
+const realLog = {}
+for (const level of ['silly', 'verbose', 'timing', 'notice', 'warn', 'error'])
+  realLog[level] = npmlog[level]
 
-const npmPath = resolve(__dirname, '..', '..')
-const Config = require('@npmcli/config')
-const { definitions, shorthands, flatten } = require('../../lib/utils/config')
-const freshConfig = (opts = {}) => {
+t.afterEach(() => {
   for (const env of Object.keys(process.env).filter(e => /^npm_/.test(e)))
     delete process.env[env]
-
   process.env.npm_config_cache = CACHE
-
-  npm.config = new Config({
-    definitions,
-    shorthands,
-    npmPath,
-    log: npmlog,
-    ...opts,
-    flatten,
+  process.argv = argv
+  Object.defineProperty(process, 'platform', {
+    value: actualPlatform,
+    configurable: true,
   })
-}
-
-const logs = []
-for (const level of ['silly', 'verbose', 'timing', 'notice', 'warn', 'error'])
-  npmlog[level] = (...msg) => logs.push([level, ...msg])
+  for (const level of ['silly', 'verbose', 'timing', 'notice', 'warn', 'error'])
+    npmlog[level] = realLog[level]
+  perf.reset()
+  procLog.reset()
+})
 
-const npm = require('../../lib/npm.js')
+const npmMock = () => {
+  const logs = []
+  const outputs = []
+  const npm = t.mock('../../lib/npm.js')
+  for (const level of ['silly', 'verbose', 'timing', 'notice', 'warn', 'error']) {
+    npmlog[level] = (...msg) => {
+      logs.push([level, ...msg])
+    }
+  }
+  npm.output = (...msg) => outputs.push(msg)
+  return { npm, logs, outputs }
+}
 
 const CACHE = t.testdir()
 process.env.npm_config_cache = CACHE
 
 t.test('not yet loaded', t => {
+  const { npm, logs } = npmMock()
   t.match(npm, {
     started: Number,
     command: null,
@@ -89,160 +99,125 @@ t.test('not yet loaded', t => {
   t.equal(npm.commands.asdfasdf, undefined)
   t.equal(npm.deref('list'), 'ls')
   t.same(logs, [])
-  logs.length = 0
   t.end()
 })
 
 t.test('npm.load', t => {
-  t.test('must be called with proper args', t => {
-    const er = new TypeError('must call as: npm.load(callback)')
-    t.throws(() => npm.load(), er)
+  t.test('callback must be a function', t => {
+    const { npm, logs } = npmMock()
+    const er = new TypeError('callback must be a function if provided')
     t.throws(() => npm.load({}), er)
     t.same(logs, [])
-    logs.length = 0
     t.end()
   })
 
-  t.test('load error', t => {
-    const { load } = npm.config
+  t.test('callback style', t => {
+    const { npm } = npmMock()
+    npm.load((err) => {
+      if (err)
+        throw err
+      t.ok(npm.loaded)
+      t.end()
+    })
+  })
+
+  t.test('load error', async t => {
+    const { npm } = npmMock()
     const loadError = new Error('load error')
     npm.config.load = async () => {
       throw loadError
     }
-    npm.load(er => {
+    await npm.load().catch(er => {
       t.equal(er, loadError)
       t.equal(npm.loadErr, loadError)
-      npm.config.load = load
-      // loading again just returns the same error
-      npm.load(er => {
-        t.equal(er, loadError)
-        t.equal(npm.loadErr, loadError)
-        npm.loadErr = null
-        t.end()
-      })
+    })
+    npm.config.load = async () => {
+      throw new Error('new load error')
+    }
+    await npm.load().catch(er => {
+      t.equal(er, loadError, 'loading again returns the original error')
+      t.equal(npm.loadErr, loadError)
     })
   })
 
-  t.test('basic loading', t => {
+  t.test('basic loading', async t => {
+    const { npm, logs } = npmMock()
     const dir = t.testdir({
       node_modules: {},
     })
-    let firstCalled = false
-    const first = (er) => {
-      if (er)
-        throw er
-
-      firstCalled = true
-      t.equal(npm.loaded, true)
-      t.equal(npm.config.loaded, true)
-      t.equal(npm.config.get('force'), false)
-    }
-
-    let secondCalled = false
-    const second = () => {
-      secondCalled = true
-    }
-
-    t.equal(npm.loading, false, 'not loading yet')
-    const p = npm.load(first).then(() => {
-      t.ok(npm.usage, 'has usage')
-      npm.config.set('prefix', dir)
-      t.match(npm, {
-        loaded: true,
-        loading: false,
-        flatOptions: {},
-      })
-      t.equal(firstCalled, true, 'first callback got called')
-      t.equal(secondCalled, true, 'second callback got called')
-      let thirdCalled = false
-      const third = () => {
-        thirdCalled = true
-      }
-      npm.load(third)
-      t.equal(thirdCalled, true, 'third callbback got called')
-      t.match(logs, [
-        ['timing', 'npm:load', /Completed in [0-9.]+ms/],
-      ])
-      logs.length = 0
-
-      bePosix()
-      t.equal(resolve(npm.cache), resolve(CACHE), 'cache is cache')
-      const newCache = t.testdir()
-      npm.cache = newCache
-      t.equal(npm.config.get('cache'), newCache, 'cache setter sets config')
-      t.equal(npm.cache, newCache, 'cache getter gets new config')
-      t.equal(npm.log, npmlog, 'npmlog getter')
-      t.equal(npm.lockfileVersion, 2, 'lockfileVersion getter')
-      t.equal(npm.prefix, npm.localPrefix, 'prefix is local prefix')
-      t.not(npm.prefix, npm.globalPrefix, 'prefix is not global prefix')
-      npm.globalPrefix = npm.prefix
-      t.equal(npm.prefix, npm.globalPrefix, 'globalPrefix setter')
-      npm.localPrefix = dir + '/extra/prefix'
-      t.equal(npm.prefix, npm.localPrefix, 'prefix is local prefix after localPrefix setter')
-      t.not(npm.prefix, npm.globalPrefix, 'prefix is not global prefix after localPrefix setter')
-
-      npm.prefix = dir + '/some/prefix'
-      t.equal(npm.prefix, npm.localPrefix, 'prefix is local prefix after prefix setter')
-      t.not(npm.prefix, npm.globalPrefix, 'prefix is not global prefix after prefix setter')
-      t.equal(npm.bin, npm.localBin, 'bin is local bin after prefix setter')
-      t.not(npm.bin, npm.globalBin, 'bin is not global bin after prefix setter')
-      t.equal(npm.dir, npm.localDir, 'dir is local dir after prefix setter')
-      t.not(npm.dir, npm.globalDir, 'dir is not global dir after prefix setter')
-
-      npm.config.set('global', true)
-      t.equal(npm.prefix, npm.globalPrefix, 'prefix is global prefix after setting global')
-      t.not(npm.prefix, npm.localPrefix, 'prefix is not local prefix after setting global')
-      t.equal(npm.bin, npm.globalBin, 'bin is global bin after setting global')
-      t.not(npm.bin, npm.localBin, 'bin is not local bin after setting global')
-      t.equal(npm.dir, npm.globalDir, 'dir is global dir after setting global')
-      t.not(npm.dir, npm.localDir, 'dir is not local dir after setting global')
-
-      npm.prefix = dir + '/new/global/prefix'
-      t.equal(npm.prefix, npm.globalPrefix, 'prefix is global prefix after prefix setter')
-      t.not(npm.prefix, npm.localPrefix, 'prefix is not local prefix after prefix setter')
-      t.equal(npm.bin, npm.globalBin, 'bin is global bin after prefix setter')
-      t.not(npm.bin, npm.localBin, 'bin is not local bin after prefix setter')
-
-      beWindows()
-      t.equal(npm.bin, npm.globalBin, 'bin is global bin in windows mode')
-      t.equal(npm.dir, npm.globalDir, 'dir is global dir in windows mode')
-      bePosix()
-
-      const tmp = npm.tmp
-      t.match(tmp, String, 'npm.tmp is a string')
-      t.equal(tmp, npm.tmp, 'getter only generates it once')
+    await npm.load()
+    t.equal(npm.loaded, true)
+    t.equal(npm.config.loaded, true)
+    t.equal(npm.config.get('force'), false)
+    t.ok(npm.usage, 'has usage')
+    npm.config.set('prefix', dir)
+
+    t.match(npm, {
+      flatOptions: {},
     })
-
-    t.equal(npm.loaded, false, 'not loaded yet')
-    t.equal(npm.loading, true, 'working on it tho')
-    t.type(p, Promise, 'npm.load() returned a Promise first time')
-    t.equal(npm.load(second), undefined,
-      'npm.load() returns nothing second time')
-
-    return p
+    t.match(logs, [
+      ['timing', 'npm:load', /Completed in [0-9.]+ms/],
+    ])
+
+    bePosix()
+    t.equal(resolve(npm.cache), resolve(CACHE), 'cache is cache')
+    const newCache = t.testdir()
+    npm.cache = newCache
+    t.equal(npm.config.get('cache'), newCache, 'cache setter sets config')
+    t.equal(npm.cache, newCache, 'cache getter gets new config')
+    t.equal(npm.log, npmlog, 'npmlog getter')
+    t.equal(npm.lockfileVersion, 2, 'lockfileVersion getter')
+    t.equal(npm.prefix, npm.localPrefix, 'prefix is local prefix')
+    t.not(npm.prefix, npm.globalPrefix, 'prefix is not global prefix')
+    npm.globalPrefix = npm.prefix
+    t.equal(npm.prefix, npm.globalPrefix, 'globalPrefix setter')
+    npm.localPrefix = dir + '/extra/prefix'
+    t.equal(npm.prefix, npm.localPrefix, 'prefix is local prefix after localPrefix setter')
+    t.not(npm.prefix, npm.globalPrefix, 'prefix is not global prefix after localPrefix setter')
+
+    npm.prefix = dir + '/some/prefix'
+    t.equal(npm.prefix, npm.localPrefix, 'prefix is local prefix after prefix setter')
+    t.not(npm.prefix, npm.globalPrefix, 'prefix is not global prefix after prefix setter')
+    t.equal(npm.bin, npm.localBin, 'bin is local bin after prefix setter')
+    t.not(npm.bin, npm.globalBin, 'bin is not global bin after prefix setter')
+    t.equal(npm.dir, npm.localDir, 'dir is local dir after prefix setter')
+    t.not(npm.dir, npm.globalDir, 'dir is not global dir after prefix setter')
+
+    npm.config.set('global', true)
+    t.equal(npm.prefix, npm.globalPrefix, 'prefix is global prefix after setting global')
+    t.not(npm.prefix, npm.localPrefix, 'prefix is not local prefix after setting global')
+    t.equal(npm.bin, npm.globalBin, 'bin is global bin after setting global')
+    t.not(npm.bin, npm.localBin, 'bin is not local bin after setting global')
+    t.equal(npm.dir, npm.globalDir, 'dir is global dir after setting global')
+    t.not(npm.dir, npm.localDir, 'dir is not local dir after setting global')
+
+    npm.prefix = dir + '/new/global/prefix'
+    t.equal(npm.prefix, npm.globalPrefix, 'prefix is global prefix after prefix setter')
+    t.not(npm.prefix, npm.localPrefix, 'prefix is not local prefix after prefix setter')
+    t.equal(npm.bin, npm.globalBin, 'bin is global bin after prefix setter')
+    t.not(npm.bin, npm.localBin, 'bin is not local bin after prefix setter')
+
+    beWindows()
+    t.equal(npm.bin, npm.globalBin, 'bin is global bin in windows mode')
+    t.equal(npm.dir, npm.globalDir, 'dir is global dir in windows mode')
+    bePosix()
+
+    const tmp = npm.tmp
+    t.match(tmp, String, 'npm.tmp is a string')
+    t.equal(tmp, npm.tmp, 'getter only generates it once')
   })
 
-  t.test('forceful loading', t => {
-    // also, don't get thrown off if argv[0] isn't found for some reason
-    const [argv0] = process.argv
-    t.teardown(() => {
-      process.argv[0] = argv0
-    })
-    freshConfig({ argv: [...process.argv, '--force', '--color', 'always'] })
-    process.argv[0] = 'this exe does not exist or else this test will fail'
-    return npm.load(er => {
-      if (er)
-        throw er
-
-      t.match(logs.filter(l => l[0] !== 'timing'), [
-        [
-          'warn',
-          'using --force',
-          'Recommended protections disabled.',
-        ],
-      ])
-      logs.length = 0
-    })
+  t.test('forceful loading', async t => {
+    process.argv = [...process.argv, '--force', '--color', 'always']
+    const { npm, logs } = npmMock()
+    await npm.load()
+    t.match(logs.filter(l => l[0] !== 'timing'), [
+      [
+        'warn',
+        'using --force',
+        'Recommended protections disabled.',
+      ],
+    ])
   })
 
   t.test('node is a symlink', async t => {
@@ -254,7 +229,7 @@ t.test('npm.load', t => {
 
     const PATH = process.env.PATH || process.env.Path
     process.env.PATH = resolve(dir, 'bin')
-    const { execPath, argv: processArgv } = process
+    const { execPath } = process
     process.argv = [
       node,
       process.argv[1],
@@ -267,48 +242,34 @@ t.test('npm.load', t => {
       'blergggg',
     ]
 
-    freshConfig()
-    const { log } = console
-    const consoleLogs = []
-    console.log = (...msg) => consoleLogs.push(msg)
-
     t.teardown(() => {
-      console.log = log
       process.env.PATH = PATH
-      process.argv = processArgv
-      freshConfig()
-      logs.length = 0
       process.execPath = execPath
     })
 
-    logs.length = 0
-
-    await npm.load(er => {
-      if (er)
-        throw er
-
-      t.equal(npm.config.get('scope'), '@foo', 'added the @ sign to scope')
-      t.match(logs.filter(l => l[0] !== 'timing' || !/^config:/.test(l[1])), [
-        [
-          'timing',
-          'npm:load:whichnode',
-          /Completed in [0-9.]+ms/,
-        ],
-        [
-          'verbose',
-          'node symlink',
-          resolve(dir, 'bin', node),
-        ],
-        [
-          'timing',
-          'npm:load',
-          /Completed in [0-9.]+ms/,
-        ],
-      ])
-      logs.length = 0
-      t.equal(process.execPath, resolve(dir, 'bin', node))
-    })
+    const { npm, logs, outputs } = npmMock()
+    await npm.load()
+    t.equal(npm.config.get('scope'), '@foo', 'added the @ sign to scope')
+    t.match(logs.filter(l => l[0] !== 'timing' || !/^config:/.test(l[1])), [
+      [
+        'timing',
+        'npm:load:whichnode',
+        /Completed in [0-9.]+ms/,
+      ],
+      [
+        'verbose',
+        'node symlink',
+        resolve(dir, 'bin', node),
+      ],
+      [
+        'timing',
+        'npm:load',
+        /Completed in [0-9.]+ms/,
+      ],
+    ])
+    t.equal(process.execPath, resolve(dir, 'bin', node))
 
+    outputs.length = 0
     await npm.commands.ll([], (er) => {
       if (er)
         throw er
@@ -316,13 +277,13 @@ t.test('npm.load', t => {
       t.equal(npm.command, 'll', 'command set to first npm command')
       t.equal(npm.flatOptions.npmCommand, 'll', 'npmCommand flatOption set')
 
-      t.same(consoleLogs, [[npm.commands.ll.usage]], 'print usage')
-      consoleLogs.length = 0
+      t.same(outputs, [[npm.commands.ll.usage]], 'print usage')
       npm.config.set('usage', false)
       t.equal(npm.commands.ll, npm.commands.ll, 'same command, different name')
-      logs.length = 0
     })
 
+    outputs.length = 0
+    logs.length = 0
     await npm.commands.get(['scope', '\u2010not-a-dash'], (er) => {
       if (er)
         throw er
@@ -348,7 +309,7 @@ t.test('npm.load', t => {
           /Completed in [0-9.]+ms/,
         ],
       ])
-      t.same(consoleLogs, [['scope=@foo\n\u2010not-a-dash=undefined']])
+      t.same(outputs, [['scope=@foo\n\u2010not-a-dash=undefined']])
     })
 
     // need this here or node 10 will improperly end the promise ahead of time
@@ -381,33 +342,21 @@ t.test('npm.load', t => {
       '.npmrc': '',
     })
 
-    const { log } = console
-    const consoleLogs = []
-    console.log = (...msg) => consoleLogs.push(msg)
-
     const { execPath } = process
-    t.teardown(() => {
-      console.log = log
-    })
 
-    freshConfig({
-      argv: [
-        execPath,
-        process.argv[1],
-        '--userconfig',
-        resolve(dir, '.npmrc'),
-        '--color',
-        'false',
-        '--workspaces',
-        'true',
-      ],
-    })
-
-    await npm.load(er => {
-      if (er)
-        throw er
-    })
+    process.argv = [
+      execPath,
+      process.argv[1],
+      '--userconfig',
+      resolve(dir, '.npmrc'),
+      '--color',
+      'false',
+      '--workspaces',
+      'true',
+    ]
 
+    const { npm, outputs } = npmMock()
+    await npm.load()
     npm.localPrefix = dir
 
     await new Promise((res, rej) => {
@@ -421,7 +370,7 @@ t.test('npm.load', t => {
         t.equal(npm.command, 'run-script', 'npm.command set to canonical name')
 
         t.match(
-          consoleLogs,
+          outputs,
           [
             ['Lifecycle scripts included in a@1.0.0:'],
             ['  test\n    echo test a'],
@@ -463,23 +412,19 @@ t.test('npm.load', t => {
       }),
     })
     const { execPath } = process
-    freshConfig({
-      argv: [
-        execPath,
-        process.argv[1],
-        '--userconfig',
-        resolve(dir, '.npmrc'),
-        '--color',
-        'false',
-        '--workspaces',
-        '--global',
-        'true',
-      ],
-    })
-    await npm.load(er => {
-      if (er)
-        throw er
-    })
+    process.argv = [
+      execPath,
+      process.argv[1],
+      '--userconfig',
+      resolve(dir, '.npmrc'),
+      '--color',
+      'false',
+      '--workspaces',
+      '--global',
+      'true',
+    ]
+    const { npm } = npmMock()
+    await npm.load()
     npm.localPrefix = dir
     await new Promise((res, rej) => {
       // verify that calling the command with a short name still sets
@@ -491,7 +436,6 @@ t.test('npm.load', t => {
       })
     })
   })
-
   t.end()
 })
 
@@ -512,82 +456,49 @@ t.test('loading as main will load the cli', t => {
 })
 
 t.test('set process.title', t => {
-  const { argv: processArgv } = process
-  const { log } = console
-  const titleDesc = Object.getOwnPropertyDescriptor(process, 'title')
-  Object.defineProperty(process, 'title', {
-    value: '',
-    settable: true,
-    enumerable: true,
-    configurable: true,
-  })
-  const consoleLogs = []
-  console.log = (...msg) => consoleLogs.push(msg)
-
-  t.teardown(() => {
-    console.log = log
-    process.argv = processArgv
-    Object.defineProperty(process, 'title', titleDesc)
-    freshConfig()
-  })
-
-  t.afterEach(() => consoleLogs.length = 0)
-
   t.test('basic title setting', async t => {
-    freshConfig({
-      argv: [
-        process.execPath,
-        process.argv[1],
-        '--usage',
-        '--scope=foo',
-        'ls',
-      ],
-    })
-    await npm.load(er => {
-      if (er)
-        throw er
-      t.equal(npm.title, 'npm ls')
-      t.equal(process.title, 'npm ls')
-    })
+    process.argv = [
+      process.execPath,
+      process.argv[1],
+      '--usage',
+      '--scope=foo',
+      'ls',
+    ]
+    const { npm } = npmMock()
+    await npm.load()
+    t.equal(npm.title, 'npm ls')
+    t.equal(process.title, 'npm ls')
   })
 
   t.test('do not expose token being revoked', async t => {
-    freshConfig({
-      argv: [
-        process.execPath,
-        process.argv[1],
-        '--usage',
-        '--scope=foo',
-        'token',
-        'revoke',
-        'deadbeefcafebad',
-      ],
-    })
-    await npm.load(er => {
-      if (er)
-        throw er
-      t.equal(npm.title, 'npm token revoke ***')
-      t.equal(process.title, 'npm token revoke ***')
-    })
+    process.argv = [
+      process.execPath,
+      process.argv[1],
+      '--usage',
+      '--scope=foo',
+      'token',
+      'revoke',
+      'deadbeefcafebad',
+    ]
+    const { npm } = npmMock()
+    await npm.load()
+    t.equal(npm.title, 'npm token revoke ***')
+    t.equal(process.title, 'npm token revoke ***')
   })
 
   t.test('do show *** unless a token is actually being revoked', async t => {
-    freshConfig({
-      argv: [
-        process.execPath,
-        process.argv[1],
-        '--usage',
-        '--scope=foo',
-        'token',
-        'revoke',
-      ],
-    })
-    await npm.load(er => {
-      if (er)
-        throw er
-      t.equal(npm.title, 'npm token revoke')
-      t.equal(process.title, 'npm token revoke')
-    })
+    process.argv = [
+      process.execPath,
+      process.argv[1],
+      '--usage',
+      '--scope=foo',
+      'token',
+      'revoke',
+    ]
+    const { npm } = npmMock()
+    await npm.load()
+    t.equal(npm.title, 'npm token revoke')
+    t.equal(process.title, 'npm token revoke')
   })
 
   t.end()
diff --git a/test/lib/utils/exit-handler.js b/test/lib/utils/exit-handler.js
index beb1ef8c56690..72f94bfded7b0 100644
--- a/test/lib/utils/exit-handler.js
+++ b/test/lib/utils/exit-handler.js
@@ -355,7 +355,7 @@ t.test('it worked', (t) => {
   const _exit = process.exit
   process.exit = (code) => {
     process.exit = _exit
-    t.false(code, 'should exit with no code')
+    t.notOk(code, 'should exit with no code')
 
     const _info = npmlog.info
     npmlog.info = (msg) => {