chore: run all windows shims tests

.github/workflows/ci.yml

@@ -165,3 +165,40 @@ jobs:
         run: node . test -w smoke-tests --ignore-scripts
       - name: Check Git Status
         run: node scripts/git-dirty.js
+
+  windows-shims:
+    name: Windows Shims Tests
+    runs-on: windows-latest
+    defaults:
+      run:
+        shell: cmd
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v3
+      - name: Setup Git User
+        run: |
+          git config --global user.email "[email protected]"
+          git config --global user.name "npm CLI robot"
+      - name: Setup Node
+        uses: actions/setup-node@v3
+        with:
+          node-version: 18.x
+          cache: npm
+      - name: Check Git Status
+        run: node scripts/git-dirty.js
+      - name: Reset Deps
+        run: node scripts/resetdeps.js
+      - name: Check Git Status
+        run: node scripts/git-dirty.js
+      - name: Setup WSL
+        uses: Vampire/[email protected]
+      - name: Set up Cygwin
+        uses: egor-tensin/[email protected]
+        with:
+          install-dir: C:\cygwin64
+      - name: Run Windows Shims Tests
+        run: node . test --ignore-scripts -- test/bin/windows-shims.js --no-coverage
+        env:
+          WINDOWS_SHIMS_TEST: true
+      - name: Check Git Status
+        run: node scripts/git-dirty.js

scripts/template-oss/ci.yml

@@ -11,3 +11,24 @@
         run: {{rootNpmPath}} test -w smoke-tests --ignore-scripts
       - name: Check Git Status
         run: node scripts/git-dirty.js
+
+  windows-shims:
+    name: Windows Shims Tests
+    runs-on: windows-latest
+    defaults:
+      run:
+        shell: cmd
+    steps:
+      {{> stepsSetup }}
+      - name: Setup WSL
+        uses: Vampire/[email protected]
+      - name: Set up Cygwin
+        uses: egor-tensin/[email protected]
+        with:
+          install-dir: C:\cygwin64
+      - name: Run Windows Shims Tests
+        run: {{rootNpmPath}} test --ignore-scripts -- test/bin/windows-shims.js --no-coverage
+        env:
+          WINDOWS_SHIMS_TEST: true
+      - name: Check Git Status
+        run: node scripts/git-dirty.js

test/bin/windows-shims.js

@@ -1,68 +1,72 @@
 const t = require('tap')
-const spawn = require('@npmcli/promise-spawn')
 const { spawnSync } = require('child_process')
-const { resolve, join } = require('path')
-const { readFileSync, chmodSync } = require('fs')
+const { resolve, join, extname, basename } = require('path')
+const { readFileSync, chmodSync, readdirSync } = require('fs')
 const Diff = require('diff')
+const { sync: which } = require('which')
 const { version } = require('../../package.json')
 
-const root = resolve(__dirname, '../..')
-const npmShim = join(root, 'bin/npm')
-const npxShim = join(root, 'bin/npx')
+const ROOT = resolve(__dirname, '../..')
+const BIN = join(ROOT, 'bin')
+const SHIMS = readdirSync(BIN).reduce((acc, shim) => {
+  if (extname(shim) !== '.js') {
+    acc[shim] = readFileSync(join(BIN, shim), 'utf-8')
+  }
+  return acc
+}, {})
+
+const SHIM_EXTS = [...new Set(Object.keys(SHIMS).map(p => extname(p)))]
 
-t.test('npm vs npx', t => {
+t.test('shim contents', t => {
   // these scripts should be kept in sync so this tests the contents of each
   // and does a diff to ensure the only differences between them are necessary
-  const diffFiles = (ext = '') => Diff.diffChars(
-    readFileSync(`${npmShim}${ext}`, 'utf8'),
-    readFileSync(`${npxShim}${ext}`, 'utf8')
-  ).filter(v => v.added || v.removed).map((v, i) => i === 0 ? v.value : v.value.toUpperCase())
+  const diffFiles = (npm, npx) => Diff.diffChars(npm, npx)
+    .filter(v => v.added || v.removed)
+    .reduce((acc, v) => {
+      if (v.value.length === 1) {
+        acc.letters.add(v.value.toUpperCase())
+      } else {
+        acc.diff.push(v.value)
+      }
+      return acc
+    }, { diff: [], letters: new Set() })
+
+  t.plan(SHIM_EXTS.length)
 
   t.test('bash', t => {
-    const [npxCli, ...changes] = diffFiles()
-    const npxCliLine = npxCli.split('\n').reverse().join('')
-    t.match(npxCliLine, /^NPX_CLI_JS=/, 'has NPX_CLI')
-    t.equal(changes.length, 20)
-    t.strictSame([...new Set(changes)], ['M', 'X'], 'all other changes are m->x')
+    const { diff, letters } = diffFiles(SHIMS.npm, SHIMS.npx)
+    t.match(diff[0].split('\n').reverse().join(''), /^NPX_CLI_JS=/, 'has NPX_CLI')
+    t.equal(diff.length, 1)
+    t.strictSame([...letters], ['M', 'X'], 'all other changes are m->x')
     t.end()
   })
 
   t.test('cmd', t => {
-    const [npxCli, ...changes] = diffFiles('.cmd')
-    t.match(npxCli, /^SET "NPX_CLI_JS=/, 'has NPX_CLI')
-    t.equal(changes.length, 12)
-    t.strictSame([...new Set(changes)], ['M', 'X'], 'all other changes are m->x')
+    const { diff, letters } = diffFiles(SHIMS['npm.cmd'], SHIMS['npx.cmd'])
+    t.match(diff[0], /^SET "NPX_CLI_JS=/, 'has NPX_CLI')
+    t.equal(diff.length, 1)
+    t.strictSame([...letters], ['M', 'X'], 'all other changes are m->x')
     t.end()
   })
-
-  t.end()
 })
 
-t.test('basic', async t => {
-  if (process.platform !== 'win32') {
-    t.comment('test only relevant on windows')
-    return
-  }
-
+t.test('run shims', t => {
   const path = t.testdir({
+    ...SHIMS,
     'node.exe': readFileSync(process.execPath),
-    npm: readFileSync(npmShim),
-    npx: readFileSync(npxShim),
     // simulate the state where one version of npm is installed
     // with node, but we should load the globally installed one
     'global-prefix': {
       node_modules: {
-        npm: t.fixture('symlink', root),
+        npm: t.fixture('symlink', ROOT),
       },
     },
     // put in a shim that ONLY prints the intended global prefix,
     // and should not be used for anything else.
     node_modules: {
       npm: {
         bin: {
-          'npx-cli.js': `
-            throw new Error('this should not be called')
-          `,
+          'npx-cli.js': `throw new Error('this should not be called')`,
           'npm-cli.js': `
             const assert = require('assert')
             const args = process.argv.slice(2)
@@ -76,70 +80,106 @@ t.test('basic', async t => {
     },
   })
 
-  chmodSync(join(path, 'npm'), 0o755)
-  chmodSync(join(path, 'npx'), 0o755)
-
-  const { ProgramFiles, SystemRoot, NYC_CONFIG } = process.env
-  const gitBash = join(ProgramFiles, 'Git', 'bin', 'bash.exe')
-  const gitUsrBinBash = join(ProgramFiles, 'Git', 'usr', 'bin', 'bash.exe')
-  const wslBash = join(SystemRoot, 'System32', 'bash.exe')
-  const cygwinBash = join(SystemRoot, '/', 'cygwin64', 'bin', 'bash.exe')
-
-  const bashes = Object.entries({
-    'wsl bash': wslBash,
-    'git bash': gitBash,
-    'git internal bash': gitUsrBinBash,
-    'cygwin bash': cygwinBash,
-  }).map(([name, bash]) => {
-    let skip
-    if (bash === cygwinBash && NYC_CONFIG) {
-      skip = 'does not play nicely with NYC, run without coverage'
-    } else {
-      try {
-        // If WSL is installed, it *has* a bash.exe, but it fails if
-        // there is no distro installed, so we need to detect that.
-        if (spawnSync(bash, ['-l', '-c', 'exit 0']).status !== 0) {
-          throw new Error('not installed')
+  for (const shim of Object.keys(SHIMS)) {
+    chmodSync(join(path, shim), 0o755)
+  }
+
+  const { ProgramFiles = '/', SystemRoot = '/', NYC_CONFIG, WINDOWS_SHIMS_TEST } = process.env
+  const skipDefault = WINDOWS_SHIMS_TEST || process.platform === 'win32'
+    ? null : 'test not relevant on platform'
+
+  const shells = Object.entries({
+    cmd: 'cmd',
+    git: join(ProgramFiles, 'Git', 'bin', 'bash.exe'),
+    'user git': join(ProgramFiles, 'Git', 'usr', 'bin', 'bash.exe'),
+    wsl: join(SystemRoot, 'System32', 'bash.exe'),
+    cygwin: resolve(SystemRoot, '/', 'cygwin64', 'bin', 'bash.exe'),
+  }).map(([name, cmd]) => {
+    let skip = skipDefault
+    const isBash = cmd.endsWith('bash.exe')
+    const testName = `${name} ${isBash ? 'bash' : ''}`.trim()
+
+    if (!skip) {
+      if (isBash) {
+        try {
+          // If WSL is installed, it *has* a bash.exe, but it fails if
+          // there is no distro installed, so we need to detect that.
+          if (spawnSync(cmd, ['-l', '-c', 'exit 0']).status !== 0) {
+            throw new Error('not installed')
+          }
+          if (cmd.includes('cygwin') && NYC_CONFIG) {
+            throw new Error('does not play nicely with nyc')
+          }
+        } catch (err) {
+          skip = err.message
+        }
+      } else {
+        try {
+          cmd = which(cmd)
+        } catch {
+          skip = 'not installed'
         }
-      } catch {
-        skip = 'not installed'
       }
     }
-    return { name, bash, skip }
+
+    return {
+      cmd,
+      name: testName,
+      skip: skip ? `${testName} - ${skip}` : null,
+    }
   })
 
-  for (const { name, bash, skip } of bashes) {
-    if (skip) {
-      t.skip(name, { diagnostic: true, bin: bash, reason: skip })
-      continue
+  const matchCmd = (t, cmd, bin) => {
+    const args = []
+    const opts = {}
+
+    switch (basename(cmd).toLowerCase()) {
+      case 'cmd.exe':
+        cmd = `${bin}.cmd`
+        break
+      case 'bash.exe':
+        // only cygwin *requires* the -l, but the others are ok with it
+        args.push('-l', bin)
+        break
+      default:
+        throw new Error('unknown shell')
     }
 
-    await t.test(name, async t => {
-      const bins = Object.entries({
-        // should have loaded this instance of npm we symlinked in
-        npm: [['help'], `npm@${version} ${root}`],
-        npx: [['--version'], version],
-      })
-
-      for (const [binName, [cmdArgs, stdout]] of bins) {
-        await t.test(binName, async t => {
-          // only cygwin *requires* the -l, but the others are ok with it
-          const args = ['-l', binName, ...cmdArgs]
-          const result = await spawn(bash, args, {
-            // don't hit the registry for the update check
-            env: { PATH: path, npm_config_update_notifier: 'false' },
-            cwd: path,
-          })
-          t.match(result, {
-            cmd: bash,
-            args: args,
-            code: 0,
-            signal: null,
-            stderr: String,
-            stdout,
-          })
-        })
+    const isNpm = bin === 'npm'
+    const result = spawnSync(cmd, [...args, isNpm ? 'help' : '--version'], {
+      // don't hit the registry for the update check
+      env: { PATH: path, npm_config_update_notifier: 'false' },
+      cwd: path,
+      windowsHide: true,
+      ...opts,
+    })
+    result.stdout = result.stdout?.toString()?.trim()
+    result.stderr = result.stderr?.toString()?.trim()
+
+    t.match(result, {
+      status: 0,
+      signal: null,
+      stderr: '',
+      stdout: isNpm ? `npm@${version} ${ROOT}` : version,
+    }, 'command result')
+  }
+
+  // ensure that all tests are either run or skipped
+  t.plan(shells.length)
+
+  for (const { cmd, skip, name } of shells) {
+    t.test(name, t => {
+      if (skip) {
+        if (WINDOWS_SHIMS_TEST) {
+          t.fail(skip)
+        } else {
+          t.skip(skip)
+        }
+        return t.end()
       }
+      t.plan(2)
+      matchCmd(t, cmd, 'npm')
+      matchCmd(t, cmd, 'npx')
     })
   }
 })