feat: add logs-dir config to set custom logging location

This also allows logs-max to be set to 0 to disable log file writing.

Closes #4466
Closes #4206

9 files changed, 416 insertions, 305 deletions

diff --git a/test/lib/cli.js b/test/lib/cli.js
index f02c57d8c..b6606c69f 100644
--- a/test/lib/cli.js
+++ b/test/lib/cli.js
@@ -1,9 +1,8 @@
 const t = require('tap')
 
-const mockGlobals = require('../fixtures/mock-globals.js')
 const { load: loadMockNpm } = require('../fixtures/mock-npm.js')
 
-const cliMock = async (t, mocks) => {
+const cliMock = async (t, opts) => {
   let exitHandlerArgs = null
   let npm = null
   const exitHandlerMock = (...args) => {
@@ -12,10 +11,9 @@ const cliMock = async (t, mocks) => {
   }
   exitHandlerMock.setNpm = _npm => npm = _npm
 
-  const { Npm, outputs, logMocks, logs } = await loadMockNpm(t, { mocks, init: false })
+  const { Npm, outputs, logMocks, logs } = await loadMockNpm(t, { ...opts, init: false })
   const cli = t.mock('../../lib/cli.js', {
     '../../lib/npm.js': Npm,
-    '../../lib/utils/update-notifier.js': async () => null,
     '../../lib/utils/unsupported.js': {
       checkForBrokenNode: () => {},
       checkForUnsupportedNode: () => {},
@@ -31,6 +29,7 @@ const cliMock = async (t, mocks) => {
     exitHandlerCalled: () => exitHandlerArgs,
     exitHandlerNpm: () => npm,
     logs,
+    logsBy: (title) => logs.verbose.filter(([p]) => p === title).map(([p, ...rest]) => rest),
   }
 }
 
@@ -39,17 +38,15 @@ t.afterEach(() => {
 })
 
 t.test('print the version, and treat npm_g as npm -g', async t => {
-  mockGlobals(t, {
-    'process.argv': ['node', 'npm_g', '-v'],
+  const { logsBy, logs, cli, Npm, outputs, exitHandlerCalled } = await cliMock(t, {
+    globals: { 'process.argv': ['node', 'npm_g', '-v'] },
   })
-
-  const { logs, cli, Npm, outputs, exitHandlerCalled } = await cliMock(t)
   await cli(process)
 
   t.strictSame(process.argv, ['node', 'npm', '-g', '-v'], 'system process.argv was rewritten')
-  t.strictSame(logs.verbose.filter(([p]) => p !== 'logfile'), [
-    ['cli', process.argv],
-  ])
+  t.strictSame(logsBy('cli'), [['node npm']])
+  t.strictSame(logsBy('title'), [['npm']])
+  t.strictSame(logsBy('argv'), [['"--global" "--version"']])
   t.strictSame(logs.info, [
     ['using', 'npm@%s', Npm.version],
     ['using', 'node@%s', process.version],
@@ -59,68 +56,82 @@ t.test('print the version, and treat npm_g as npm -g', async t => {
 })
 
 t.test('calling with --versions calls npm version with no args', async t => {
-  t.plan(6)
-  mockGlobals(t, {
-    'process.argv': ['node', 'npm', 'install', 'or', 'whatever', '--versions'],
-  })
-  const { logs, cli, Npm, outputs, exitHandlerCalled } = await cliMock(t, {
-    '../../lib/commands/version.js': class Version {
-      async exec (args) {
-        t.strictSame(args, [])
-      }
+  const { logsBy, cli, outputs, exitHandlerCalled } = await cliMock(t, {
+    mocks: {
+      '../../lib/commands/version.js': class Version {
+        async exec (args) {
+          t.strictSame(args, [])
+        }
+      },
+    },
+    globals: {
+      'process.argv': ['node', 'npm', 'install', 'or', 'whatever', '--versions'],
     },
   })
-
   await cli(process)
-  t.equal(process.title, 'npm install or whatever')
-  t.strictSame(logs.verbose.filter(([p]) => p !== 'logfile'), [
-    ['cli', process.argv],
-  ])
-  t.strictSame(logs.info, [
-    ['using', 'npm@%s', Npm.version],
-    ['using', 'node@%s', process.version],
-  ])
 
+  t.equal(process.title, 'npm install or whatever')
+  t.strictSame(logsBy('cli'), [['node npm']])
+  t.strictSame(logsBy('title'), [['npm install or whatever']])
+  t.strictSame(logsBy('argv'), [['"install" "or" "whatever" "--versions"']])
   t.strictSame(outputs, [])
   t.strictSame(exitHandlerCalled(), [])
 })
 
 t.test('logged argv is sanitized', async t => {
-  mockGlobals(t, {
-    'process.argv': [
-      'node',
-      'npm',
-      'version',
-      'https://username:password@npmjs.org/test_url_with_a_password',
-    ],
-  })
-  const { logs, cli, Npm } = await cliMock(t, {
-    '../../lib/commands/version.js': class Version {
-      async exec (args) {}
+  const { logsBy, cli } = await cliMock(t, {
+    mocks: {
+      '../../lib/commands/version.js': class Version {
+        async exec () {}
+      },
+    },
+    globals: {
+      'process.argv': [
+        'node',
+        'npm',
+        'version',
+        '--registry',
+        'https://u:password@npmjs.org/password',
+      ],
     },
   })
 
   await cli(process)
-  t.ok(process.title.startsWith('npm version https://username:***@npmjs.org'))
-  t.strictSame(logs.verbose.filter(([p]) => p !== 'logfile'), [
-    [
-      'cli',
-      ['node', 'npm', 'version', 'https://username:***@npmjs.org/test_url_with_a_password'],
-    ],
-  ])
-  t.strictSame(logs.info, [
-    ['using', 'npm@%s', Npm.version],
-    ['using', 'node@%s', process.version],
-  ])
+  t.equal(process.title, 'npm version')
+  t.strictSame(logsBy('cli'), [['node npm']])
+  t.strictSame(logsBy('title'), [['npm version']])
+  t.strictSame(logsBy('argv'), [['"version" "--registry" "https://u:***@npmjs.org/password"']])
 })
 
-t.test('print usage if no params provided', async t => {
-  mockGlobals(t, {
-    'process.argv': ['node', 'npm'],
+t.test('logged argv is sanitized with equals', async t => {
+  const { logsBy, cli } = await cliMock(t, {
+    mocks: {
+      '../../lib/commands/version.js': class Version {
+        async exec () {}
+      },
+    },
+    globals: {
+      'process.argv': [
+        'node',
+        'npm',
+        'version',
+        '--registry=https://u:password@npmjs.org',
+      ],
+    },
   })
+  await cli(process)
+
+  t.strictSame(logsBy('argv'), [['"version" "--registry" "https://u:***@npmjs.org"']])
+})
 
-  const { cli, outputs, exitHandlerCalled, exitHandlerNpm } = await cliMock(t)
+t.test('print usage if no params provided', async t => {
+  const { cli, outputs, exitHandlerCalled, exitHandlerNpm } = await cliMock(t, {
+    globals: {
+      'process.argv': ['node', 'npm'],
+    },
+  })
   await cli(process)
+
   t.match(outputs[0][0], 'Usage:', 'outputs npm usage')
   t.match(exitHandlerCalled(), [], 'should call exitHandler with no args')
   t.ok(exitHandlerNpm(), 'exitHandler npm is set')
@@ -128,12 +139,13 @@ t.test('print usage if no params provided', async t => {
 })
 
 t.test('print usage if non-command param provided', async t => {
-  mockGlobals(t, {
-    'process.argv': ['node', 'npm', 'tset'],
+  const { cli, outputs, exitHandlerCalled, exitHandlerNpm } = await cliMock(t, {
+    globals: {
+      'process.argv': ['node', 'npm', 'tset'],
+    },
   })
-
-  const { cli, outputs, exitHandlerCalled, exitHandlerNpm } = await cliMock(t)
   await cli(process)
+
   t.match(outputs[0][0], 'Unknown command: "tset"')
   t.match(outputs[0][0], 'Did you mean this?')
   t.match(exitHandlerCalled(), [], 'should call exitHandler with no args')
@@ -142,21 +154,22 @@ t.test('print usage if non-command param provided', async t => {
 })
 
 t.test('load error calls error handler', async t => {
-  mockGlobals(t, {
-    'process.argv': ['node', 'npm', 'asdf'],
-  })
-
   const err = new Error('test load error')
   const { cli, exitHandlerCalled } = await cliMock(t, {
-    '../../lib/utils/config/index.js': {
-      definitions: null,
-      flatten: null,
-      shorthands: null,
+    mocks: {
+      '../../lib/utils/config/index.js': {
+        definitions: null,
+        flatten: null,
+        shorthands: null,
+      },
+      '@npmcli/config': class BadConfig {
+        async load () {
+          throw err
+        }
+      },
     },
-    '@npmcli/config': class BadConfig {
-      async load () {
-        throw err
-      }
+    globals: {
+      'process.argv': ['node', 'npm', 'asdf'],
     },
   })
   await cli(process)
diff --git a/test/lib/commands/bin.js b/test/lib/commands/bin.js
index 4de5a923b..a889b1336 100644
--- a/test/lib/commands/bin.js
+++ b/test/lib/commands/bin.js
@@ -1,76 +1,60 @@
 const t = require('tap')
-const { fake: mockNpm } = require('../../fixtures/mock-npm')
+const { relative, join } = require('path')
+const { load: loadMockNpm } = require('../../fixtures/mock-npm')
+const mockGlobals = require('../../fixtures/mock-globals')
 
-t.test('bin', async t => {
-  t.plan(2)
-  const dir = '/bin/dir'
-
-  const Bin = require('../../../lib/commands/bin.js')
+const mockBin = async (t, { args = [], config = {} } = {}) => {
+  const { npm, outputs, ...rest } = await loadMockNpm(t, {
+    config,
+  })
+  const cmd = await npm.cmd('bin')
+  await npm.exec('bin', args)
+
+  return {
+    npm,
+    cmd,
+    bin: outputs[0][0],
+    ...rest,
+  }
+}
 
-  const npm = mockNpm({
-    bin: dir,
+t.test('bin', async t => {
+  const { cmd, bin, prefix, outputErrors } = await mockBin(t, {
     config: { global: false },
-    output: (output) => {
-      t.equal(output, dir, 'prints the correct directory')
-    },
   })
-  const bin = new Bin(npm)
-  t.match(bin.usage, 'bin', 'usage has command name in it')
 
-  await bin.exec([])
+  t.match(cmd.usage, 'bin', 'usage has command name in it')
+  t.equal(relative(prefix, bin), join('node_modules/.bin'), 'prints the correct directory')
+  t.strictSame(outputErrors, [])
 })
 
 t.test('bin -g', async t => {
-  t.plan(1)
-  const consoleError = console.error
-  t.teardown(() => {
-    console.error = consoleError
+  mockGlobals(t, { 'process.platform': 'posix' })
+  const { globalPrefix, bin, outputErrors } = await mockBin(t, {
+    config: { global: true },
   })
 
-  console.error = (output) => {
-    t.fail('should not have printed to console.error')
-  }
-  const dir = '/bin/dir'
-
-  const Bin = t.mock('../../../lib/commands/bin.js', {
-    '../../../lib/utils/path.js': [dir],
-  })
+  t.equal(relative(globalPrefix, bin), 'bin', 'prints the correct directory')
+  t.strictSame(outputErrors, [])
+})
 
-  const npm = mockNpm({
-    bin: dir,
+t.test('bin -g win32', async t => {
+  mockGlobals(t, { 'process.platform': 'win32' })
+  const { globalPrefix, bin, outputErrors } = await mockBin(t, {
     config: { global: true },
-    output: (output) => {
-      t.equal(output, dir, 'prints the correct directory')
-    },
   })
-  const bin = new Bin(npm)
 
-  await bin.exec([])
+  t.equal(relative(globalPrefix, bin), '', 'prints the correct directory')
+  t.strictSame(outputErrors, [])
 })
 
 t.test('bin -g (not in path)', async t => {
-  t.plan(2)
-  const consoleError = console.error
-  t.teardown(() => {
-    console.error = consoleError
-  })
-
-  console.error = (output) => {
-    t.equal(output, '(not in PATH env variable)', 'prints env warning')
-  }
-  const dir = '/bin/dir'
-
-  const Bin = t.mock('../../../lib/commands/bin.js', {
-    '../../../lib/utils/path.js': ['/not/my/dir'],
-  })
-  const npm = mockNpm({
-    bin: dir,
+  const { logs } = await mockBin(t, {
     config: { global: true },
-    output: (output) => {
-      t.equal(output, dir, 'prints the correct directory')
+    globals: {
+      'process.env.PATH': 'emptypath',
     },
   })
-  const bin = new Bin(npm)
 
-  await bin.exec([])
+  t.strictSame(logs.error[0], ['bin', '(not in PATH env variable)'])
 })
diff --git a/test/lib/commands/doctor.js b/test/lib/commands/doctor.js
index 5badab99a..620d908d3 100644
--- a/test/lib/commands/doctor.js
+++ b/test/lib/commands/doctor.js
@@ -52,17 +52,7 @@ const dirs = {
   },
 }
 
-let consoleError = false
-t.afterEach(() => {
-  consoleError = false
-})
-
 const globals = {
-  console: {
-    error: () => {
-      consoleError = true
-    },
-  },
   process: {
     platform: 'test-not-windows',
     version: 'v1.0.0',
@@ -104,7 +94,6 @@ t.test('all clear', async t => {
     .get('/dist/index.json').reply(200, nodeVersions)
   await npm.exec('doctor', [])
   t.matchSnapshot(joinedOutput(), 'output')
-  t.notOk(consoleError, 'console.error not called')
   t.matchSnapshot({ info: logs.info, warn: logs.warn, error: logs.error }, 'logs')
 })
 
@@ -122,7 +111,6 @@ t.test('all clear in color', async t => {
   npm.config.set('color', 'always')
   await npm.exec('doctor', [])
   t.matchSnapshot(joinedOutput(), 'everything is ok in color')
-  t.notOk(consoleError, 'console.error not called')
   t.matchSnapshot({ info: logs.info, warn: logs.warn, error: logs.error }, 'logs')
 })
 
@@ -142,7 +130,6 @@ t.test('silent', async t => {
     .get('/dist/index.json').reply(200, nodeVersions)
   await npm.exec('doctor', [])
   t.matchSnapshot(joinedOutput(), 'output')
-  t.notOk(consoleError, 'console.error not called')
   t.matchSnapshot({ info: logs.info, warn: logs.warn, error: logs.error }, 'logs')
 })
 
@@ -159,7 +146,6 @@ t.test('ping 404', async t => {
     .get('/dist/index.json').reply(200, nodeVersions)
   await t.rejects(npm.exec('doctor', []))
   t.matchSnapshot(joinedOutput(), 'ping 404')
-  t.ok(consoleError, 'console.error called')
   t.matchSnapshot({ info: logs.info, warn: logs.warn, error: logs.error }, 'logs')
 })
 
diff --git a/test/lib/npm.js b/test/lib/npm.js
index 3ae2af35c..4302437a6 100644
--- a/test/lib/npm.js
+++ b/test/lib/npm.js
@@ -139,10 +139,11 @@ t.test('npm.load', async t => {
   })
 
   t.test('forceful loading', async t => {
-    mockGlobals(t, {
-      'process.argv': [...process.argv, '--force', '--color', 'always'],
+    const { logs } = await loadMockNpm(t, {
+      globals: {
+        'process.argv': [...process.argv, '--force', '--color', 'always'],
+      },
     })
-    const { logs } = await loadMockNpm(t)
     t.match(logs.warn, [
       [
         'using --force',
@@ -153,23 +154,21 @@ t.test('npm.load', async t => {
 
   t.test('node is a symlink', async t => {
     const node = process.platform === 'win32' ? 'node.exe' : 'node'
-    mockGlobals(t, {
-      'process.argv': [
-        node,
-        process.argv[1],
-        '--usage',
-        '--scope=foo',
-        'token',
-        'revoke',
-        'blergggg',
-      ],
-    })
     const { npm, logs, outputs, prefix } = await loadMockNpm(t, {
       prefixDir: {
         bin: t.fixture('symlink', dirname(process.execPath)),
       },
       globals: ({ prefix }) => ({
         'process.env.PATH': resolve(prefix, 'bin'),
+        'process.argv': [
+          node,
+          process.argv[1],
+          '--usage',
+          '--scope=foo',
+          'token',
+          'revoke',
+          'blergggg',
+        ],
       }),
     })
 
@@ -181,6 +180,9 @@ t.test('npm.load', async t => {
     ], [
       ['npm:load:whichnode', /Completed in [0-9.]+ms/],
       ['node symlink', resolve(prefix, 'bin', node)],
+      ['title', 'npm token revoke blergggg'],
+      ['argv', '"--usage" "--scope" "foo" "token" "revoke" "blergggg"'],
+      ['logfile', /logs-max:\d+ dir:.*/],
       ['logfile', /.*-debug-0.log/],
       ['npm:load', /Completed in [0-9.]+ms/],
     ])
@@ -226,15 +228,6 @@ t.test('npm.load', async t => {
   })
 
   t.test('--no-workspaces with --workspace', async t => {
-    mockGlobals(t, {
-      'process.argv': [
-        process.execPath,
-        process.argv[1],
-        '--color', 'false',
-        '--workspaces', 'false',
-        '--workspace', 'a',
-      ],
-    })
     const { npm } = await loadMockNpm(t, {
       load: false,
       prefixDir: {
@@ -253,6 +246,15 @@ t.test('npm.load', async t => {
           workspaces: ['./packages/*'],
         }),
       },
+      globals: {
+        'process.argv': [
+          process.execPath,
+          process.argv[1],
+          '--color', 'false',
+          '--workspaces', 'false',
+          '--workspace', 'a',
+        ],
+      },
     })
     await t.rejects(
       npm.exec('run', []),
@@ -261,14 +263,6 @@ t.test('npm.load', async t => {
   })
 
   t.test('workspace-aware configs and commands', async t => {
-    mockGlobals(t, {
-      'process.argv': [
-        process.execPath,
-        process.argv[1],
-        '--color', 'false',
-        '--workspaces', 'true',
-      ],
-    })
     const { npm, outputs } = await loadMockNpm(t, {
       prefixDir: {
         packages: {
@@ -293,6 +287,14 @@ t.test('npm.load', async t => {
           workspaces: ['./packages/*'],
         }),
       },
+      globals: {
+        'process.argv': [
+          process.execPath,
+          process.argv[1],
+          '--color', 'false',
+          '--workspaces', 'true',
+        ],
+      },
     })
 
     // verify that calling the command with a short name still sets
@@ -317,17 +319,6 @@ t.test('npm.load', async t => {
   })
 
   t.test('workspaces in global mode', async t => {
-    mockGlobals(t, {
-      'process.argv': [
-        process.execPath,
-        process.argv[1],
-        '--color',
-        'false',
-        '--workspaces',
-        '--global',
-        'true',
-      ],
-    })
     const { npm } = await loadMockNpm(t, {
       prefixDir: {
         packages: {
@@ -352,6 +343,17 @@ t.test('npm.load', async t => {
           workspaces: ['./packages/*'],
         }),
       },
+      globals: {
+        'process.argv': [
+          process.execPath,
+          process.argv[1],
+          '--color',
+          'false',
+          '--workspaces',
+          '--global',
+          'true',
+        ],
+      },
     })
     // verify that calling the command with a short name still sets
     // the npm.command property to the full canonical name of the cmd.
@@ -365,68 +367,93 @@ t.test('npm.load', async t => {
 
 t.test('set process.title', async t => {
   t.test('basic title setting', async t => {
-    mockGlobals(t, {
-      'process.argv': [
-        process.execPath,
-        process.argv[1],
-        '--usage',
-        '--scope=foo',
-        'ls',
-      ],
+    const { npm } = await loadMockNpm(t, {
+      globals: {
+        'process.argv': [
+          process.execPath,
+          process.argv[1],
+          '--usage',
+          '--scope=foo',
+          'ls',
+        ],
+      },
     })
-    const { npm } = await loadMockNpm(t)
     t.equal(npm.title, 'npm ls')
     t.equal(process.title, 'npm ls')
   })
 
   t.test('do not expose token being revoked', async t => {
-    mockGlobals(t, {
-      'process.argv': [
-        process.execPath,
-        process.argv[1],
-        '--usage',
-        '--scope=foo',
-        'token',
-        'revoke',
-        'deadbeefcafebad',
-      ],
+    const { npm } = await loadMockNpm(t, {
+      globals: {
+        'process.argv': [
+          process.execPath,
+          process.argv[1],
+          '--usage',
+          '--scope=foo',
+          'token',
+          'revoke',
+          `npm_${'a'.repeat(36)}`,
+        ],
+      },
     })
-    const { npm } = await loadMockNpm(t)
-    t.equal(npm.title, 'npm token revoke ***')
-    t.equal(process.title, 'npm token revoke ***')
+    t.equal(npm.title, 'npm token revoke npm_***')
+    t.equal(process.title, 'npm token revoke npm_***')
   })
 
   t.test('do show *** unless a token is actually being revoked', async t => {
-    mockGlobals(t, {
-      'process.argv': [
-        process.execPath,
-        process.argv[1],
-        '--usage',
-        '--scope=foo',
-        'token',
-        'revoke',
-      ],
+    const { npm } = await loadMockNpm(t, {
+      globals: {
+        'process.argv': [
+          process.execPath,
+          process.argv[1],
+          '--usage',
+          '--scope=foo',
+          'token',
+          'revoke',
+          'notatoken',
+        ],
+      },
     })
-    const { npm } = await loadMockNpm(t)
-    t.equal(npm.title, 'npm token revoke')
-    t.equal(process.title, 'npm token revoke')
+    t.equal(npm.title, 'npm token revoke notatoken')
+    t.equal(process.title, 'npm token revoke notatoken')
   })
 })
 
-t.test('debug-log', async t => {
-  const { npm, debugFile } = await loadMockNpm(t, { load: false })
+t.test('debug log', async t => {
+  t.test('writes log file', async t => {
+    const { npm, debugFile } = await loadMockNpm(t, { load: false })
+
+    const log1 = ['silly', 'test', 'before load']
+    const log2 = ['silly', 'test', 'after load']
 
-  const log1 = ['silly', 'test', 'before load']
-  const log2 = ['silly', 'test', 'after load']
+    process.emit('log', ...log1)
+    await npm.load()
+    process.emit('log', ...log2)
 
-  process.emit('log', ...log1)
-  await npm.load()
-  process.emit('log', ...log2)
+    const debug = await debugFile()
+    t.equal(npm.logFiles.length, 1, 'one debug file')
+    t.match(debug, log1.join(' '), 'before load appears')
+    t.match(debug, log2.join(' '), 'after load log appears')
+  })
+
+  t.test('with bad dir', async t => {
+    const { npm } = await loadMockNpm(t, {
+      config: {
+        'logs-dir': 'LOGS_DIR',
+      },
+      mocks: {
+        '@npmcli/fs': {
+          mkdir: async (dir) => {
+            if (dir.includes('LOGS_DIR')) {
+              throw new Error('err')
+            }
+          },
+        },
+      },
+    })
 
-  const debug = await debugFile()
-  t.equal(npm.logFiles.length, 1, 'one debug file')
-  t.match(debug, log1.join(' '), 'before load appears')
-  t.match(debug, log2.join(' '), 'after load log appears')
+    t.equal(npm.logFiles.length, 0, 'no log file')
+  })
 })
 
 t.test('timings', async t => {
@@ -458,13 +485,14 @@ t.test('timings', async t => {
   })
 
   t.test('writes timings file', async t => {
-    const { npm, timingFile } = await loadMockNpm(t, {
+    const { npm, cache, timingFile } = await loadMockNpm(t, {
       config: { timing: true },
     })
     process.emit('time', 'foo')
     process.emit('timeEnd', 'foo')
     process.emit('time', 'bar')
-    npm.unload()
+    npm.writeTimingFile()
+    t.equal(npm.timingFile, join(cache, '_timing.json'))
     const timings = await timingFile()
     t.match(timings, {
       command: [],
@@ -484,21 +512,16 @@ t.test('timings', async t => {
     const { npm, timingFile } = await loadMockNpm(t, {
       config: { false: true },
     })
-    npm.unload()
+    npm.writeTimingFile()
     await t.rejects(() => timingFile())
   })
 })
 
 t.test('output clears progress and console.logs the message', async t => {
-  t.plan(2)
+  t.plan(4)
   let showingProgress = true
   const logs = []
-  mockGlobals(t, {
-    'console.log': (...args) => {
-      t.equal(showingProgress, false, 'should not be showing progress right now')
-      logs.push(args)
-    },
-  })
+  const errors = []
   const { npm } = await loadMockNpm(t, {
     load: false,
     mocks: {
@@ -507,9 +530,22 @@ t.test('output clears progress and console.logs the message', async t => {
         showProgress: () => showingProgress = true,
       },
     },
+    globals: {
+      'console.log': (...args) => {
+        t.equal(showingProgress, false, 'should not be showing progress right now')
+        logs.push(args)
+      },
+      'console.error': (...args) => {
+        t.equal(showingProgress, false, 'should not be showing progress right now')
+        errors.push(args)
+      },
+    },
   })
   npm.originalOutput('hello')
+  npm.originalOutputError('error')
+
   t.match(logs, [['hello']])
+  t.match(errors, [['error']])
   t.end()
 })
 
@@ -522,14 +558,6 @@ t.test('unknown command', async t => {
 })
 
 t.test('explicit workspace rejection', async t => {
-  mockGlobals(t, {
-    'process.argv': [
-      process.execPath,
-      process.argv[1],
-      '--color', 'false',
-      '--workspace', './packages/a',
-    ],
-  })
   const mock = await loadMockNpm(t, {
     prefixDir: {
       packages: {
@@ -547,6 +575,14 @@ t.test('explicit workspace rejection', async t => {
         workspaces: ['./packages/a'],
       }),
     },
+    globals: {
+      'process.argv': [
+        process.execPath,
+        process.argv[1],
+        '--color', 'false',
+        '--workspace', './packages/a',
+      ],
+    },
   })
   await t.rejects(
     mock.npm.exec('ping', []),
@@ -572,15 +608,17 @@ t.test('implicit workspace rejection', async t => {
         workspaces: ['./packages/a'],
       }),
     },
-  })
-  const cwd = join(mock.npm.config.localPrefix, 'packages', 'a')
-  mock.npm.config.set('workspace', [cwd], 'default')
-  mockGlobals(t, {
-    'process.argv': [
-      process.execPath,
-      process.argv[1],
-      '--color', 'false',
-    ],
+    globals: {
+      'process.argv': [
+        process.execPath,
+        process.argv[1],
+        '--color', 'false',
+        '--workspace', './packages/a',
+      ],
+    },
+    config: ({ prefix }) => ({
+      workspace: { value: [join(prefix, 'packages', 'a')], where: 'default' },
+    }),
   })
   await t.rejects(
     mock.npm.exec('owner', []),
@@ -606,19 +644,17 @@ t.test('implicit workspace accept', async t => {
         workspaces: ['./packages/a'],
       }),
     },
+    globals: ({ prefix }) => ({
+      'process.cwd': () => prefix,
+      'process.argv': [
+        process.execPath,
+        process.argv[1],
+        '--color', 'false',
+      ],
+    }),
+    config: ({ prefix }) => ({
+      workspace: { value: [join(prefix, 'packages', 'a')], where: 'default' },
+    }),
   })
-  const cwd = join(mock.npm.config.localPrefix, 'packages', 'a')
-  mock.npm.config.set('workspace', [cwd], 'default')
-  mockGlobals(t, {
-    'process.cwd': () => mock.npm.config.cwd,
-    'process.argv': [
-      process.execPath,
-      process.argv[1],
-      '--color', 'false',
-    ],
-  })
-  await t.rejects(
-    mock.npm.exec('org', []),
-    /.*Usage/
-  )
+  await t.rejects(mock.npm.exec('org', []), /.*Usage/)
 })
diff --git a/test/lib/utils/exit-handler.js b/test/lib/utils/exit-handler.js
index 6a96d92dd..73bbf06fe 100644
--- a/test/lib/utils/exit-handler.js
+++ b/test/lib/utils/exit-handler.js
@@ -21,9 +21,10 @@ t.formatSnapshot = (obj) => {
 }
 
 t.cleanSnapshot = (path) => cleanDate(cleanCwd(path))
-// Config loading is dependent on env so strip those from snapshots
+  // Config loading is dependent on env so strip those from snapshots
   .replace(/.*timing config:load:.*\n/gm, '')
   .replace(/(Completed in )\d+(ms)/g, '$1{TIME}$2')
+  .replace(/(removing )\d+( files)/g, '$1${NUM}2')
 
 // cut off process from script so that it won't quit the test runner
 // while trying to run through the myriad of cases.  need to make it
@@ -44,9 +45,8 @@ mockGlobals(t, {
   }),
 }, { replace: true })
 
-const mockExitHandler = async (t, { init, load, testdir, config } = {}) => {
+const mockExitHandler = async (t, { init, load, testdir, config, globals, mocks } = {}) => {
   const errors = []
-  mockGlobals(t, { 'console.error': (err) => errors.push(err) })
 
   const { npm, logMocks, ...rest } = await loadMockNpm(t, {
     init,
@@ -56,11 +56,15 @@ const mockExitHandler = async (t, { init, load, testdir, config } = {}) => {
       '../../package.json': {
         version: '1.0.0',
       },
+      ...mocks,
     },
     config: {
       loglevel: 'notice',
       ...config,
     },
+    globals: {
+      'console.error': (err) => errors.push(err),
+    },
   })
 
   const exitHandler = t.mock('../../../lib/utils/exit-handler.js', {
@@ -74,6 +78,7 @@ const mockExitHandler = async (t, { init, load, testdir, config } = {}) => {
       release: () => '1.0.0',
     },
     ...logMocks,
+    ...mocks,
   })
 
   if (npm) {
@@ -89,13 +94,14 @@ const mockExitHandler = async (t, { init, load, testdir, config } = {}) => {
     ...rest,
     errors,
     npm,
-    // Make it async to make testing ergonomics a little
-    // easier so we dont need to t.plan() every test to
-    // make sure we get process.exit called
-    exitHandler: (...args) => new Promise(resolve => {
+    // Make it async to make testing ergonomics a little easier so we dont need
+    // to t.plan() every test to make sure we get process.exit called. Also
+    // introduce a small artificial delay so the logs are consistently finished
+    // by the time the exit handler forces process.exit
+    exitHandler: (...args) => new Promise(resolve => setTimeout(() => {
       process.once('exit', resolve)
       exitHandler(...args)
-    }),
+    }, 50)),
   }
 }
 
@@ -199,17 +205,15 @@ t.test('exit handler called - no npm with error without stack', async (t) => {
 })
 
 t.test('console.log output using --json', async (t) => {
-  const { exitHandler, errors } = await mockExitHandler(t, {
-    config: {
-      json: true,
-    },
+  const { exitHandler, outputErrors } = await mockExitHandler(t, {
+    config: { json: true },
   })
 
   await exitHandler(err('Error: EBADTHING Something happened'))
 
   t.equal(process.exitCode, 1)
   t.same(
-    JSON.parse(errors[0]),
+    JSON.parse(outputErrors[0]),
     {
       error: {
         code: 'EBADTHING', // should default error code to E[A-Z]+
@@ -273,11 +277,43 @@ t.test('npm.config not ready', async (t) => {
   ], 'should exit with config error msg')
 })
 
-t.test('timing with no error', async (t) => {
-  const { exitHandler, timingFile, npm, logs } = await mockExitHandler(t, {
+t.test('no logs dir', async (t) => {
+  const { exitHandler, logs } = await mockExitHandler(t, {
+    config: { 'logs-max': 0 },
+  })
+
+  await exitHandler(new Error())
+
+  t.match(logs.error.filter(([t]) => t === ''), [
+    ['', 'Log files were not written due to the config logs-max=0'],
+  ])
+})
+
+t.test('log file error', async (t) => {
+  const { exitHandler, logs } = await mockExitHandler(t, {
     config: {
+      'logs-dir': 'LOGS_DIR',
       timing: true,
     },
+    mocks: {
+      '@npmcli/fs': {
+        mkdir: async (dir) => {
+          if (dir.includes('LOGS_DIR')) {
+            throw new Error('err')
+          }
+        },
+      },
+    },
+  })
+
+  await exitHandler(new Error())
+
+  t.match(logs.error.filter(([t]) => t === ''), [['', `error writing to the directory`]])
+})
+
+t.test('timing with no error', async (t) => {
+  const { exitHandler, timingFile, npm, logs } = await mockExitHandler(t, {
+    config: { timing: true },
   })
 
   await exitHandler()
@@ -285,9 +321,9 @@ t.test('timing with no error', async (t) => {
 
   t.equal(process.exitCode, 0)
 
-  t.match(logs.error, [
-    ['', /A complete log of this run can be found in:[\s\S]*-debug-\d\.log/],
-  ])
+  const msg = logs.info.filter(([t]) => t === '')[0][1]
+  t.match(msg, /A complete log of this run can be found in:/)
+  t.match(msg, /Timing info written to:/)
 
   t.match(
     timingFileData,
@@ -308,9 +344,7 @@ t.test('timing with no error', async (t) => {
 
 t.test('unfinished timers', async (t) => {
   const { exitHandler, timingFile, npm } = await mockExitHandler(t, {
-    config: {
-      timing: true,
-    },
+    config: { timing: true },
   })
 
   process.emit('time', 'foo')
@@ -376,7 +410,7 @@ t.test('verbose logs replace info on err props', async t => {
   await exitHandler(err('Error with code type number', properties))
   t.equal(process.exitCode, 1)
   t.match(
-    logs.verbose.filter(([p]) => p !== 'logfile'),
+    logs.verbose.filter(([p]) => !['logfile', 'title', 'argv'].includes(p)),
     keys.map((k) => [k, `${k}-https://user:***@registry.npmjs.org/`]),
     'all special keys get replaced'
   )
diff --git a/test/lib/utils/log-file.js b/test/lib/utils/log-file.js
index 007ce221b..ce6f0bf4c 100644
--- a/test/lib/utils/log-file.js
+++ b/test/lib/utils/log-file.js
@@ -116,12 +116,12 @@ t.test('max files per process', async t => {
   }
 
   for (const i of range(5)) {
-    logFile.log('verbose', `log ${i}`)
+    logFile.log('verbose', `ignored after maxlogs hit ${i}`)
   }
 
   const logs = await readLogs()
   t.equal(logs.length, maxFilesPerProcess, 'total log files')
-  t.equal(last(last(logs).logs), '49 error log 49')
+  t.match(last(last(logs).logs), /49 error log \d+/)
 })
 
 t.test('stream error', async t => {
@@ -182,8 +182,7 @@ t.test('turns off', async t => {
   logFile.load()
 
   const logs = await readLogs()
-  t.equal(logs.length, 1)
-  t.equal(logs[0].logs[0], '0 error test')
+  t.match(last(last(logs).logs), /^\d+ error test$/)
 })
 
 t.test('cleans logs', async t => {
@@ -198,7 +197,7 @@ t.test('cleans logs', async t => {
 })
 
 t.test('doesnt clean current log by default', async t => {
-  const logsMax = 0
+  const logsMax = 1
   const { readLogs, logFile } = await loadLogFile(t, {
     logsMax,
     testdir: makeOldLogs(10),
@@ -207,7 +206,6 @@ t.test('doesnt clean current log by default', async t => {
   logFile.log('error', 'test')
 
   const logs = await readLogs()
-  t.equal(logs.length, 1)
   t.match(last(logs).content, /\d+ error test/)
 })
 
@@ -221,8 +219,7 @@ t.test('negative logs max', async t => {
   logFile.log('error', 'test')
 
   const logs = await readLogs()
-  t.equal(logs.length, 1)
-  t.match(last(logs).content, /\d+ error test/)
+  t.equal(logs.length, 0)
 })
 
 t.test('doesnt need to clean', async t => {
@@ -257,7 +254,7 @@ t.test('cleans old style logs too', async t => {
   const oldLogs = 10
   const { readLogs } = await loadLogFile(t, {
     logsMax,
-    testdir: makeOldLogs(oldLogs, false),
+    testdir: makeOldLogs(oldLogs, true),
   })
 
   const logs = await readLogs()
@@ -304,7 +301,7 @@ t.test('delete log file while open', async t => {
 })
 
 t.test('snapshot', async t => {
-  const { logFile, readLogs } = await loadLogFile(t)
+  const { logFile, readLogs } = await loadLogFile(t, { logsMax: 10 })
 
   logFile.log('error', '', 'no prefix')
   logFile.log('error', 'prefix', 'with prefix')
diff --git a/test/lib/utils/replace-info.js b/test/lib/utils/replace-info.js
index e4b83783a..c7fffdb54 100644
--- a/test/lib/utils/replace-info.js
+++ b/test/lib/utils/replace-info.js
@@ -20,12 +20,30 @@ t.equal(
 )
 
 t.equal(
+  replaceInfo('  ==  = = '),
+  '  ==  = = ',
+  'should return same string with only separators'
+)
+
+t.equal(
+  replaceInfo(''),
+  '',
+  'should return empty string'
+)
+
+t.equal(
   replaceInfo('https://user:pass@registry.npmjs.org/'),
   'https://user:***@registry.npmjs.org/',
   'should replace single item'
 )
 
 t.equal(
+  replaceInfo(`https://registry.npmjs.org/path/npm_${'a'.repeat('36')}`),
+  'https://registry.npmjs.org/path/npm_***',
+  'should replace single item token'
+)
+
+t.equal(
   replaceInfo('https://example.npmjs.org'),
   'https://example.npmjs.org',
   'should not replace single item with no password'
@@ -49,6 +67,12 @@ t.equal(
   'should replace single item within a phrase'
 )
 
+t.equal(
+  replaceInfo('Something --x=https://user:pass@registry.npmjs.org/ foo bar'),
+  'Something --x=https://user:***@registry.npmjs.org/ foo bar',
+  'should replace single item within a phrase separated by ='
+)
+
 t.same(
   replaceInfo([
     'Something https://user:pass@registry.npmjs.org/ foo bar',
@@ -60,7 +84,21 @@ t.same(
     'http://foo:***@registry.npmjs.org',
     'http://example.npmjs.org',
   ],
-  'should replace single item within a phrase'
+  'should replace items in an array'
+)
+
+t.same(
+  replaceInfo([
+    'Something --x=https://user:pass@registry.npmjs.org/ foo bar',
+    '--url=http://foo:bar@registry.npmjs.org',
+    '--url=http://example.npmjs.org',
+  ]),
+  [
+    'Something --x=https://user:***@registry.npmjs.org/ foo bar',
+    '--url=http://foo:***@registry.npmjs.org',
+    '--url=http://example.npmjs.org',
+  ],
+  'should replace items in an array with equals'
 )
 
 t.same(
diff --git a/test/lib/utils/timers.js b/test/lib/utils/timers.js
index 6127f346b..30e54700c 100644
--- a/test/lib/utils/timers.js
+++ b/test/lib/utils/timers.js
@@ -1,5 +1,5 @@
 const t = require('tap')
-const { resolve } = require('path')
+const { resolve, join } = require('path')
 const fs = require('graceful-fs')
 const mockLogs = require('../../fixtures/mock-logs')
 
@@ -31,6 +31,17 @@ t.test('listens/stops on process', async (t) => {
   t.notOk(timers.unfinished.get('baz'))
 })
 
+t.test('convenience time method', async (t) => {
+  const { timers } = mockTimers(t)
+
+  const end = timers.time('later')
+  timers.time('sync', () => {})
+  await timers.time('async', () => new Promise(r => setTimeout(r, 10)))
+  end()
+
+  t.match(timers.finished, { later: Number, sync: Number, async: Number })
+})
+
 t.test('initial timer', async (t) => {
   const { timers } = mockTimers(t, { start: 'foo' })
   process.emit('timeEnd', 'foo')
@@ -75,8 +86,21 @@ t.test('writes file', async (t) => {
 
 t.test('fails to write file', async (t) => {
   const { logs, timers } = mockTimers(t)
+  const dir = t.testdir()
+
+  timers.load({ dir: join(dir, 'does', 'not', 'exist') })
+  timers.writeFile()
+
+  t.match(logs.warn, [['timing', 'could not write timing file']])
+  t.equal(timers.file, null)
+})
+
+t.test('no dir and no file', async (t) => {
+  const { logs, timers } = mockTimers(t)
+
+  timers.load()
   timers.writeFile()
-  t.match(logs.warn, [
-    ['timing', 'could not write timing file', Error],
-  ])
+
+  t.strictSame(logs, [])
+  t.equal(timers.file, null)
 })
diff --git a/test/lib/utils/update-notifier.js b/test/lib/utils/update-notifier.js
index fa4af2947..a35886c6e 100644
--- a/test/lib/utils/update-notifier.js
+++ b/test/lib/utils/update-notifier.js
@@ -85,12 +85,11 @@ t.afterEach(() => {
 
 const runUpdateNotifier = async ({ color = true, ...npmOptions } = {}) => {
   const _npm = { ...defaultNpm, ...npmOptions, logColor: color }
-  await t.mock('../../../lib/utils/update-notifier.js', {
+  return t.mock('../../../lib/utils/update-notifier.js', {
     '@npmcli/ci-detect': () => ciMock,
     pacote,
     fs,
   })(_npm)
-  return _npm.updateNotification
 }
 
 t.test('situations in which we do not notify', t => {