chore: fix failing tests in windows + node 18lk/enginges

Some of our tests were failing in windows after testing on node 18. The
reason was the inability to clean up the logs dir. This changes forces
a few tests to run in order and also cleans up any use of multiple
`t.testdir` calls in a single child test which can cause problems.

4 files changed, 247 insertions, 275 deletions

diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js
index adf6c7432..330c34147 100644
--- a/test/fixtures/mock-npm.js
+++ b/test/fixtures/mock-npm.js
@@ -66,6 +66,7 @@ const LoadMockNpm = async (t, {
   globalPrefixDir = { lib: {} },
   config = {},
   mocks = {},
+  otherDirs = {},
   globals = null,
 } = {}) => {
   // Mock some globals with their original values so they get torn down
@@ -107,6 +108,7 @@ const LoadMockNpm = async (t, {
     prefix: prefixDir,
     cache: cacheDir,
     global: globalPrefixDir,
+    other: otherDirs,
   })
   const dirs = {
     testdir: dir,
@@ -114,6 +116,7 @@ const LoadMockNpm = async (t, {
     cache: path.join(dir, 'cache'),
     globalPrefix: path.join(dir, 'global'),
     home: path.join(dir, 'home'),
+    other: path.join(dir, 'other'),
   }
 
   // Set cache to testdir via env var so it is available when load is run
diff --git a/test/lib/cli.js b/test/lib/cli.js
index d36048cd9..5a75aa7fa 100644
--- a/test/lib/cli.js
+++ b/test/lib/cli.js
@@ -172,37 +172,15 @@ t.test('load error calls error handler', async t => {
   t.strictSame(exitHandlerCalled(), [err])
 })
 
-t.test('known broken node version', async t => {
-  const errors = []
-  let exitCode
-  const { cli } = await cliMock(t, {
-    globals: {
-      'console.error': (msg) => errors.push(msg),
-      'process.version': '6.0.0',
-      'process.exit': e => exitCode = e,
-    },
-  })
-  await cli(process)
-  t.match(errors, [
-    'ERROR: npm is known not to run on Node.js 6.0.0',
-    'You\'ll need to upgrade to a newer Node.js version in order to use this',
-    'version of npm. You can find the latest version at https://nodejs.org/',
-  ])
-  t.match(exitCode, 1)
-})
-
 t.test('unsupported node version', async t => {
-  const errors = []
-  const { cli } = await cliMock(t, {
+  const { cli, logs } = await cliMock(t, {
     globals: {
-      'console.error': (msg) => errors.push(msg),
       'process.version': '12.6.0',
     },
   })
   await cli(process)
-  t.match(errors, [
-    'npm does not support Node.js 12.6.0',
-    'You should probably upgrade to a newer version of node as we',
-    'can\'t make any promises that npm will work with this version.',
-  ])
+  t.match(
+    logs.warn[0][1],
+    /npm v.* does not support Node\.js 12\.6\.0\./
+  )
 })
diff --git a/test/lib/commands/install.js b/test/lib/commands/install.js
index 9b2d52f6e..632ca22be 100644
--- a/test/lib/commands/install.js
+++ b/test/lib/commands/install.js
@@ -1,273 +1,262 @@
 const t = require('tap')
-const path = require('path')
 
 const { load: _loadMockNpm } = require('../../fixtures/mock-npm')
 
 // Make less churn in the test to pass in mocks only signature
 const loadMockNpm = (t, mocks) => _loadMockNpm(t, { mocks })
 
-t.test('with args, dev=true', async t => {
-  const SCRIPTS = []
-  let ARB_ARGS = null
-  let REIFY_CALLED = false
-  let ARB_OBJ = null
+t.test('exec commands', async t => {
+  await t.test('with args, dev=true', async t => {
+    const SCRIPTS = []
+    let ARB_ARGS = null
+    let REIFY_CALLED = false
+    let ARB_OBJ = null
 
-  const { npm } = await loadMockNpm(t, {
-    '@npmcli/run-script': ({ event }) => {
-      SCRIPTS.push(event)
-    },
-    '@npmcli/arborist': function (args) {
-      ARB_ARGS = args
-      ARB_OBJ = this
-      this.reify = () => {
-        REIFY_CALLED = true
-      }
-    },
-    '../../lib/utils/reify-finish.js': (npm, arb) => {
-      if (arb !== ARB_OBJ) {
-        throw new Error('got wrong object passed to reify-finish')
-      }
-    },
-  })
-
-  // This is here because CI calls tests with `--ignore-scripts`, which config
-  // picks up from argv
-  npm.config.set('ignore-scripts', false)
-  npm.config.set('audit-level', 'low')
-  npm.config.set('dev', true)
-  // tap sets this to
-  // D:\\a\\cli\\cli\\test\\lib\\commands/tap-testdir-install-should-install-globally-using-Arborist
-  // which gets normalized elsewhere so comparative tests fail
-  npm.prefix = path.resolve(t.testdir({}))
-
-  await npm.exec('install', ['fizzbuzz'])
+    const { npm } = await loadMockNpm(t, {
+      '@npmcli/run-script': ({ event }) => {
+        SCRIPTS.push(event)
+      },
+      '@npmcli/arborist': function (args) {
+        ARB_ARGS = args
+        ARB_OBJ = this
+        this.reify = () => {
+          REIFY_CALLED = true
+        }
+      },
+      '../../lib/utils/reify-finish.js': (npm, arb) => {
+        if (arb !== ARB_OBJ) {
+          throw new Error('got wrong object passed to reify-finish')
+        }
+      },
+    })
 
-  t.match(
-    ARB_ARGS,
-    { global: false, path: npm.prefix, auditLevel: null },
-    'Arborist gets correct args and ignores auditLevel'
-  )
-  t.equal(REIFY_CALLED, true, 'called reify')
-  t.strictSame(SCRIPTS, [], 'no scripts when adding dep')
-})
+    // This is here because CI calls tests with `--ignore-scripts`, which config
+    // picks up from argv
+    npm.config.set('ignore-scripts', false)
+    npm.config.set('audit-level', 'low')
+    npm.config.set('dev', true)
 
-t.test('without args', async t => {
-  const SCRIPTS = []
-  let ARB_ARGS = null
-  let REIFY_CALLED = false
-  let ARB_OBJ = null
+    await npm.exec('install', ['fizzbuzz'])
 
-  const { npm } = await loadMockNpm(t, {
-    '@npmcli/run-script': ({ event }) => {
-      SCRIPTS.push(event)
-    },
-    '@npmcli/arborist': function (args) {
-      ARB_ARGS = args
-      ARB_OBJ = this
-      this.reify = () => {
-        REIFY_CALLED = true
-      }
-    },
-    '../../lib/utils/reify-finish.js': (npm, arb) => {
-      if (arb !== ARB_OBJ) {
-        throw new Error('got wrong object passed to reify-finish')
-      }
-    },
+    t.match(
+      ARB_ARGS,
+      { global: false, path: npm.prefix, auditLevel: null },
+      'Arborist gets correct args and ignores auditLevel'
+    )
+    t.equal(REIFY_CALLED, true, 'called reify')
+    t.strictSame(SCRIPTS, [], 'no scripts when adding dep')
   })
 
-  npm.prefix = path.resolve(t.testdir({}))
-  npm.config.set('ignore-scripts', false)
-  await npm.exec('install', [])
-  t.match(ARB_ARGS, { global: false, path: npm.prefix })
-  t.equal(REIFY_CALLED, true, 'called reify')
-  t.strictSame(SCRIPTS, [
-    'preinstall',
-    'install',
-    'postinstall',
-    'prepublish',
-    'preprepare',
-    'prepare',
-    'postprepare',
-  ], 'exec scripts when doing local build')
-})
+  await t.test('without args', async t => {
+    const SCRIPTS = []
+    let ARB_ARGS = null
+    let REIFY_CALLED = false
+    let ARB_OBJ = null
 
-t.test('should ignore scripts with --ignore-scripts', async t => {
-  const SCRIPTS = []
-  let REIFY_CALLED = false
-  const { npm } = await loadMockNpm(t, {
-    '../../lib/utils/reify-finish.js': async () => {},
-    '@npmcli/run-script': ({ event }) => {
-      SCRIPTS.push(event)
-    },
-    '@npmcli/arborist': function () {
-      this.reify = () => {
-        REIFY_CALLED = true
-      }
-    },
-  })
-  npm.config.set('ignore-scripts', true)
-  npm.prefix = path.resolve(t.testdir({}))
-  await npm.exec('install', [])
-  t.equal(REIFY_CALLED, true, 'called reify')
-  t.strictSame(SCRIPTS, [], 'no scripts when adding dep')
-})
+    const { npm } = await loadMockNpm(t, {
+      '@npmcli/run-script': ({ event }) => {
+        SCRIPTS.push(event)
+      },
+      '@npmcli/arborist': function (args) {
+        ARB_ARGS = args
+        ARB_OBJ = this
+        this.reify = () => {
+          REIFY_CALLED = true
+        }
+      },
+      '../../lib/utils/reify-finish.js': (npm, arb) => {
+        if (arb !== ARB_OBJ) {
+          throw new Error('got wrong object passed to reify-finish')
+        }
+      },
+    })
 
-t.test('should install globally using Arborist', async t => {
-  const SCRIPTS = []
-  let ARB_ARGS = null
-  let REIFY_CALLED
-  const { npm } = await loadMockNpm(t, {
-    '@npmcli/run-script': ({ event }) => {
-      SCRIPTS.push(event)
-    },
-    '../../lib/utils/reify-finish.js': async () => {},
-    '@npmcli/arborist': function (args) {
-      ARB_ARGS = args
-      this.reify = () => {
-        REIFY_CALLED = true
-      }
-    },
+    npm.config.set('ignore-scripts', false)
+    await npm.exec('install', [])
+    t.match(ARB_ARGS, { global: false, path: npm.prefix })
+    t.equal(REIFY_CALLED, true, 'called reify')
+    t.strictSame(SCRIPTS, [
+      'preinstall',
+      'install',
+      'postinstall',
+      'prepublish',
+      'preprepare',
+      'prepare',
+      'postprepare',
+    ], 'exec scripts when doing local build')
   })
-  npm.config.set('global', true)
-  npm.globalPrefix = path.resolve(t.testdir({}))
-  await npm.exec('install', [])
-  t.match(
-    ARB_ARGS,
-    { global: true, path: npm.globalPrefix }
-  )
-  t.equal(REIFY_CALLED, true, 'called reify')
-  t.strictSame(SCRIPTS, [], 'no scripts when installing globally')
-})
 
-t.test('should not install invalid global package name', async t => {
-  const { npm } = await loadMockNpm(t, {
-    '@npmcli/run-script': () => {},
-    '../../lib/utils/reify-finish.js': async () => {},
-    '@npmcli/arborist': function (args) {
-      throw new Error('should not reify')
-    },
+  await t.test('should ignore scripts with --ignore-scripts', async t => {
+    const SCRIPTS = []
+    let REIFY_CALLED = false
+    const { npm } = await loadMockNpm(t, {
+      '../../lib/utils/reify-finish.js': async () => {},
+      '@npmcli/run-script': ({ event }) => {
+        SCRIPTS.push(event)
+      },
+      '@npmcli/arborist': function () {
+        this.reify = () => {
+          REIFY_CALLED = true
+        }
+      },
+    })
+    npm.config.set('ignore-scripts', true)
+    await npm.exec('install', [])
+    t.equal(REIFY_CALLED, true, 'called reify')
+    t.strictSame(SCRIPTS, [], 'no scripts when adding dep')
   })
-  npm.config.set('global', true)
-  npm.globalPrefix = path.resolve(t.testdir({}))
-  await t.rejects(
-    npm.exec('install', ['']),
-    /Usage:/,
-    'should not install invalid package name'
-  )
-})
 
-t.test('npm i -g npm engines check success', async t => {
-  const { npm } = await loadMockNpm(t, {
-    '../../lib/utils/reify-finish.js': async () => {},
-    '@npmcli/arborist': function () {
-      this.reify = () => {}
-    },
-    pacote: {
-      manifest: () => {
-        return {
-          version: '100.100.100',
-          engines: {
-            node: '>1',
-          },
+  await t.test('should install globally using Arborist', async t => {
+    const SCRIPTS = []
+    let ARB_ARGS = null
+    let REIFY_CALLED
+    const { npm } = await loadMockNpm(t, {
+      '@npmcli/run-script': ({ event }) => {
+        SCRIPTS.push(event)
+      },
+      '../../lib/utils/reify-finish.js': async () => {},
+      '@npmcli/arborist': function (args) {
+        ARB_ARGS = args
+        this.reify = () => {
+          REIFY_CALLED = true
         }
       },
-    },
+    })
+    npm.config.set('global', true)
+    await npm.exec('install', [])
+    t.match(
+      ARB_ARGS,
+      { global: true, path: npm.globalPrefix }
+    )
+    t.equal(REIFY_CALLED, true, 'called reify')
+    t.strictSame(SCRIPTS, [], 'no scripts when installing globally')
   })
-  npm.globalDir = t.testdir({})
-  npm.config.set('global', true)
-  await npm.exec('install', ['npm'])
-  t.ok('No exceptions happen')
-})
 
-t.test('npm i -g npm engines check failure', async t => {
-  const { npm } = await loadMockNpm(t, {
-    pacote: {
-      manifest: () => {
-        return {
-          _id: 'npm@1.2.3',
-          version: '100.100.100',
-          engines: {
-            node: '>1000',
-          },
-        }
+  await t.test('should not install invalid global package name', async t => {
+    const { npm } = await loadMockNpm(t, {
+      '@npmcli/run-script': () => {},
+      '../../lib/utils/reify-finish.js': async () => {},
+      '@npmcli/arborist': function (args) {
+        throw new Error('should not reify')
       },
-    },
+    })
+    npm.config.set('global', true)
+    await t.rejects(
+      npm.exec('install', ['']),
+      /Usage:/,
+      'should not install invalid package name'
+    )
   })
-  npm.globalDir = t.testdir({})
-  npm.config.set('global', true)
-  await t.rejects(
-    npm.exec('install', ['npm']),
-    {
-      message: 'Unsupported engine',
-      pkgid: 'npm@1.2.3',
-      current: {
-        node: process.version,
-        npm: '100.100.100',
+
+  await t.test('npm i -g npm engines check success', async t => {
+    const { npm } = await loadMockNpm(t, {
+      '../../lib/utils/reify-finish.js': async () => {},
+      '@npmcli/arborist': function () {
+        this.reify = () => {}
       },
-      required: {
-        node: '>1000',
+      pacote: {
+        manifest: () => {
+          return {
+            version: '100.100.100',
+            engines: {
+              node: '>1',
+            },
+          }
+        },
       },
-      code: 'EBADENGINE',
-    }
-  )
-})
+    })
+    npm.config.set('global', true)
+    await npm.exec('install', ['npm'])
+    t.ok('No exceptions happen')
+  })
 
-t.test('npm i -g npm engines check failure forced override', async t => {
-  const { npm } = await loadMockNpm(t, {
-    '../../lib/utils/reify-finish.js': async () => {},
-    '@npmcli/arborist': function () {
-      this.reify = () => {}
-    },
-    pacote: {
-      manifest: () => {
-        return {
-          _id: 'npm@1.2.3',
-          version: '100.100.100',
-          engines: {
-            node: '>1000',
-          },
-        }
+  await t.test('npm i -g npm engines check failure', async t => {
+    const { npm } = await loadMockNpm(t, {
+      pacote: {
+        manifest: () => {
+          return {
+            _id: 'npm@1.2.3',
+            version: '100.100.100',
+            engines: {
+              node: '>1000',
+            },
+          }
+        },
       },
-    },
+    })
+    npm.config.set('global', true)
+    await t.rejects(
+      npm.exec('install', ['npm']),
+      {
+        message: 'Unsupported engine',
+        pkgid: 'npm@1.2.3',
+        current: {
+          node: process.version,
+          npm: '100.100.100',
+        },
+        required: {
+          node: '>1000',
+        },
+        code: 'EBADENGINE',
+      }
+    )
   })
-  npm.globalDir = t.testdir({})
-  npm.config.set('global', true)
-  npm.config.set('force', true)
-  await npm.exec('install', ['npm'])
-  t.ok('Does not throw')
-})
 
-t.test('npm i -g npm@version engines check failure', async t => {
-  const { npm } = await loadMockNpm(t, {
-    pacote: {
-      manifest: () => {
-        return {
-          _id: 'npm@1.2.3',
-          version: '100.100.100',
-          engines: {
-            node: '>1000',
-          },
-        }
+  await t.test('npm i -g npm engines check failure forced override', async t => {
+    const { npm } = await loadMockNpm(t, {
+      '../../lib/utils/reify-finish.js': async () => {},
+      '@npmcli/arborist': function () {
+        this.reify = () => {}
       },
-    },
-  })
-  npm.globalDir = t.testdir({})
-  npm.config.set('global', true)
-  await t.rejects(
-    npm.exec('install', ['npm@100']),
-    {
-      message: 'Unsupported engine',
-      pkgid: 'npm@1.2.3',
-      current: {
-        node: process.version,
-        npm: '100.100.100',
+      pacote: {
+        manifest: () => {
+          return {
+            _id: 'npm@1.2.3',
+            version: '100.100.100',
+            engines: {
+              node: '>1000',
+            },
+          }
+        },
       },
-      required: {
-        node: '>1000',
+    })
+    npm.config.set('global', true)
+    npm.config.set('force', true)
+    await npm.exec('install', ['npm'])
+    t.ok('Does not throw')
+  })
+
+  await t.test('npm i -g npm@version engines check failure', async t => {
+    const { npm } = await loadMockNpm(t, {
+      pacote: {
+        manifest: () => {
+          return {
+            _id: 'npm@1.2.3',
+            version: '100.100.100',
+            engines: {
+              node: '>1000',
+            },
+          }
+        },
       },
-      code: 'EBADENGINE',
-    }
-  )
+    })
+    npm.config.set('global', true)
+    await t.rejects(
+      npm.exec('install', ['npm@100']),
+      {
+        message: 'Unsupported engine',
+        pkgid: 'npm@1.2.3',
+        current: {
+          node: process.version,
+          npm: '100.100.100',
+        },
+        required: {
+          node: '>1000',
+        },
+        code: 'EBADENGINE',
+      }
+    )
+  })
 })
 
 t.test('completion', async t => {
diff --git a/test/lib/npm.js b/test/lib/npm.js
index 62e48ce60..d9201c8a6 100644
--- a/test/lib/npm.js
+++ b/test/lib/npm.js
@@ -52,7 +52,7 @@ t.test('not yet loaded', async t => {
 })
 
 t.test('npm.load', async t => {
-  t.test('load error', async t => {
+  await t.test('load error', async t => {
     const { npm } = await loadMockNpm(t, { load: false })
     const loadError = new Error('load error')
     npm.config.load = async () => {
@@ -75,9 +75,12 @@ t.test('npm.load', async t => {
     t.equal(npm.loadErr, loadError)
   })
 
-  t.test('basic loading', async t => {
-    const { npm, logs, prefix: dir, cache } = await loadMockNpm(t, {
+  await t.test('basic loading', async t => {
+    const { npm, logs, prefix: dir, cache, other } = await loadMockNpm(t, {
       prefixDir: { node_modules: {} },
+      otherDirs: {
+        newCache: {},
+      },
     })
 
     t.equal(npm.loaded, true)
@@ -94,10 +97,9 @@ t.test('npm.load', async t => {
 
     mockGlobals(t, { process: { platform: 'posix' } })
     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')
+    npm.cache = other.newCache
+    t.equal(npm.config.get('cache'), other.newCache, 'cache setter sets config')
+    t.equal(npm.cache, other.newCache, 'cache getter gets new config')
     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')
@@ -138,7 +140,7 @@ t.test('npm.load', async t => {
     t.equal(tmp, npm.tmp, 'getter only generates it once')
   })
 
-  t.test('forceful loading', async t => {
+  await t.test('forceful loading', async t => {
     const { logs } = await loadMockNpm(t, {
       globals: {
         'process.argv': [...process.argv, '--force', '--color', 'always'],
@@ -152,7 +154,7 @@ t.test('npm.load', async t => {
     ])
   })
 
-  t.test('node is a symlink', async t => {
+  await t.test('node is a symlink', async t => {
     const node = process.platform === 'win32' ? 'node.exe' : 'node'
     const { npm, logs, outputs, prefix } = await loadMockNpm(t, {
       prefixDir: {
@@ -227,7 +229,7 @@ t.test('npm.load', async t => {
     t.same(outputs, [['scope=@foo\n\u2010not-a-dash=undefined']])
   })
 
-  t.test('--no-workspaces with --workspace', async t => {
+  await t.test('--no-workspaces with --workspace', async t => {
     const { npm } = await loadMockNpm(t, {
       load: false,
       prefixDir: {
@@ -262,7 +264,7 @@ t.test('npm.load', async t => {
     )
   })
 
-  t.test('workspace-aware configs and commands', async t => {
+  await t.test('workspace-aware configs and commands', async t => {
     const { npm, outputs } = await loadMockNpm(t, {
       prefixDir: {
         packages: {
@@ -318,7 +320,7 @@ t.test('npm.load', async t => {
     )
   })
 
-  t.test('workspaces in global mode', async t => {
+  await t.test('workspaces in global mode', async t => {
     const { npm } = await loadMockNpm(t, {
       prefixDir: {
         packages: {