From 390a47b5e85ff1f790274b28004913fbf6f23f21 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 10 Dec 2025 15:48:25 -0800 Subject: [PATCH 1/2] feat: swap from tap to built-in node:test --- .github/workflows/audit.yml | 4 +- .github/workflows/ci-release.yml | 16 ++- .github/workflows/ci.yml | 16 ++- .github/workflows/post-dependabot.yml | 4 +- .github/workflows/pull-request.yml | 4 +- .github/workflows/release-integration.yml | 4 +- .github/workflows/release.yml | 8 +- lib/index.js | 7 +- package.json | 19 ++-- test/bin.js | 105 +++++++++-------- test/index.js | 131 ++++++++++++++++------ 11 files changed, 193 insertions(+), 125 deletions(-) diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index 85282bd..2543d79 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -30,8 +30,8 @@ jobs: uses: actions/setup-node@v4 id: node with: - node-version: 22.x - check-latest: contains('22.x', '.x') + node-version: 24.x + check-latest: contains('24.x', '.x') - name: Install Latest npm uses: ./.github/actions/install-latest-npm with: diff --git a/.github/workflows/ci-release.yml b/.github/workflows/ci-release.yml index e9ab5ff..94bcb30 100644 --- a/.github/workflows/ci-release.yml +++ b/.github/workflows/ci-release.yml @@ -51,8 +51,8 @@ jobs: uses: actions/setup-node@v4 id: node with: - node-version: 22.x - check-latest: contains('22.x', '.x') + node-version: 24.x + check-latest: contains('24.x', '.x') - name: Install Latest npm uses: ./.github/actions/install-latest-npm with: @@ -95,6 +95,7 @@ jobs: - 20.x - 22.9.0 - 22.x + - 24.x exclude: - platform: { name: macOS, os: macos-13, shell: bash } node-version: 20.17.0 @@ -104,6 +105,8 @@ jobs: node-version: 22.9.0 - platform: { name: macOS, os: macos-13, shell: bash } node-version: 22.x + - platform: { name: macOS, os: macos-13, shell: bash } + node-version: 24.x runs-on: ${{ matrix.platform.os }} defaults: run: @@ -137,9 +140,14 @@ jobs: node: ${{ steps.node.outputs.node-version }} - name: Install Dependencies run: npm i --ignore-scripts --no-audit --no-fund - - name: Add Problem Matcher - run: echo "::add-matcher::.github/matchers/tap.json" + - name: Test (with coverage on Node >= 24) + if: ${{ startsWith(matrix.node-version, '24') }} + run: npm run test:cover --ignore-scripts + - name: Test (on Node 20 with globbing workaround) + if: ${{ startsWith(matrix.node-version, '20') }} + run: npm run test:node20 --ignore-scripts - name: Test + if: ${{ !startsWith(matrix.node-version, '24') && !startsWith(matrix.node-version, '20') }} run: npm test --ignore-scripts - name: Conclude Check uses: LouisBrunner/checks-action@v1.6.0 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 92a33b5..ecfdefb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,8 +34,8 @@ jobs: uses: actions/setup-node@v4 id: node with: - node-version: 22.x - check-latest: contains('22.x', '.x') + node-version: 24.x + check-latest: contains('24.x', '.x') - name: Install Latest npm uses: ./.github/actions/install-latest-npm with: @@ -71,6 +71,7 @@ jobs: - 20.x - 22.9.0 - 22.x + - 24.x exclude: - platform: { name: macOS, os: macos-13, shell: bash } node-version: 20.17.0 @@ -80,6 +81,8 @@ jobs: node-version: 22.9.0 - platform: { name: macOS, os: macos-13, shell: bash } node-version: 22.x + - platform: { name: macOS, os: macos-13, shell: bash } + node-version: 24.x runs-on: ${{ matrix.platform.os }} defaults: run: @@ -103,7 +106,12 @@ jobs: node: ${{ steps.node.outputs.node-version }} - name: Install Dependencies run: npm i --ignore-scripts --no-audit --no-fund - - name: Add Problem Matcher - run: echo "::add-matcher::.github/matchers/tap.json" + - name: Test (with coverage on Node >= 24) + if: ${{ startsWith(matrix.node-version, '24') }} + run: npm run test:cover --ignore-scripts + - name: Test (on Node 20 with globbing workaround) + if: ${{ startsWith(matrix.node-version, '20') }} + run: npm run test:node20 --ignore-scripts - name: Test + if: ${{ !startsWith(matrix.node-version, '24') && !startsWith(matrix.node-version, '20') }} run: npm test --ignore-scripts diff --git a/.github/workflows/post-dependabot.yml b/.github/workflows/post-dependabot.yml index 3a91911..8e80d10 100644 --- a/.github/workflows/post-dependabot.yml +++ b/.github/workflows/post-dependabot.yml @@ -28,8 +28,8 @@ jobs: uses: actions/setup-node@v4 id: node with: - node-version: 22.x - check-latest: contains('22.x', '.x') + node-version: 24.x + check-latest: contains('24.x', '.x') - name: Install Latest npm uses: ./.github/actions/install-latest-npm with: diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index c69932d..9ecf311 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -34,8 +34,8 @@ jobs: uses: actions/setup-node@v4 id: node with: - node-version: 22.x - check-latest: contains('22.x', '.x') + node-version: 24.x + check-latest: contains('24.x', '.x') - name: Install Latest npm uses: ./.github/actions/install-latest-npm with: diff --git a/.github/workflows/release-integration.yml b/.github/workflows/release-integration.yml index 9ca9a2b..195f50a 100644 --- a/.github/workflows/release-integration.yml +++ b/.github/workflows/release-integration.yml @@ -45,8 +45,8 @@ jobs: uses: actions/setup-node@v4 id: node with: - node-version: 22.x - check-latest: contains('22.x', '.x') + node-version: 24.x + check-latest: contains('24.x', '.x') - name: Install Latest npm uses: ./.github/actions/install-latest-npm with: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 53ff3c2..863f9eb 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -39,8 +39,8 @@ jobs: uses: actions/setup-node@v4 id: node with: - node-version: 22.x - check-latest: contains('22.x', '.x') + node-version: 24.x + check-latest: contains('24.x', '.x') - name: Install Latest npm uses: ./.github/actions/install-latest-npm with: @@ -119,8 +119,8 @@ jobs: uses: actions/setup-node@v4 id: node with: - node-version: 22.x - check-latest: contains('22.x', '.x') + node-version: 24.x + check-latest: contains('24.x', '.x') - name: Install Latest npm uses: ./.github/actions/install-latest-npm with: diff --git a/lib/index.js b/lib/index.js index 2fd358b..da6415d 100644 --- a/lib/index.js +++ b/lib/index.js @@ -8,8 +8,9 @@ const isWindows = process.platform === 'win32' // a posix platform. don't use the isWindows check for this since that is mocked // in tests but we still need the code to actually work when called. that is also // why it is ignored from coverage. -/* istanbul ignore next */ +/* node:coverage disable */ const rSlash = new RegExp(`[${posix.sep}${sep === posix.sep ? '' : sep}]`.replace(/(\\)/g, '\\$1')) +/* node:coverage enable */ const rRel = new RegExp(`^\\.${rSlash.source}`) const getNotFoundError = (cmd) => @@ -22,11 +23,13 @@ const getPathInfo = (cmd, { }) => { // If it has a slash, then we don't bother searching the pathenv. // just check the file itself, and that's it. + /* node:coverage disable */ const pathEnv = cmd.match(rSlash) ? [''] : [ // windows always checks the cwd first ...(isWindows ? [process.cwd()] : []), - ...(optPath || /* istanbul ignore next: very unusual */ '').split(optDelimiter), + ...(optPath || '').split(optDelimiter), ] + /* node:coverage enable */ if (isWindows) { const pathExtExe = optPathExt || diff --git a/package.json b/package.json index 201007d..a19f057 100644 --- a/package.json +++ b/package.json @@ -21,32 +21,29 @@ "tap": "^16.3.0" }, "scripts": { - "test": "tap", + "test": "node --test './test/**/*.js'", "lint": "npm run eslint", "postlint": "template-oss-check", "template-oss-apply": "template-oss-apply --force", "lintfix": "npm run eslint -- --fix", - "snap": "tap", + "snap": "node --test --test-update-snapshots './test/**/*.js'", "posttest": "npm run lint", - "eslint": "eslint \"**/*.{js,cjs,ts,mjs,jsx,tsx}\"" + "eslint": "eslint \"**/*.{js,cjs,ts,mjs,jsx,tsx}\"", + "test:node20": "node --test test", + "test:cover": "node --test --experimental-test-coverage --test-timeout=3000 --test-coverage-lines=100 --test-coverage-functions=100 --test-coverage-branches=100 './test/**/*.js'" }, "files": [ "bin/", "lib/" ], - "tap": { - "check-coverage": true, - "nyc-arg": [ - "--exclude", - "tap-snapshots/**" - ] - }, "engines": { "node": "^20.17.0 || >=22.9.0" }, "templateOSS": { "//@npmcli/template-oss": "This file is partially managed by @npmcli/template-oss. Edits may be overwritten.", "version": "4.28.1", - "publish": "true" + "publish": "true", + "testRunner": "node:test", + "latestCiVersion": 24 } } diff --git a/test/bin.js b/test/bin.js index b8b9a34..4c34795 100644 --- a/test/bin.js +++ b/test/bin.js @@ -1,4 +1,5 @@ -const t = require('tap') +const { test } = require('node:test') +const assert = require('node:assert') const spawn = require('child_process').spawn const node = process.execPath @@ -32,87 +33,81 @@ function which (args, extraPath) { }) } -t.test('finds node', async (t) => { +test('finds node', async () => { const { code, signal, out, err } = await which('node') - t.equal(signal, null) - t.equal(code, 0) - t.equal(err, '') - t.match(out, /[\\/]node(\.exe)?$/i) + assert.strictEqual(signal, null) + assert.strictEqual(code, 0) + assert.strictEqual(err, '') + assert.match(out, /[\\/]node(\.exe)?$/i) }) -t.test('does not find flergyderp', async (t) => { +test('does not find flergyderp', async () => { const { code, signal, out, err } = await which('flergyderp') - t.equal(signal, null) - t.equal(code, 1) - t.equal(err, '') - t.match(out, '') + assert.strictEqual(signal, null) + assert.strictEqual(code, 1) + assert.strictEqual(err, '') + assert.strictEqual(out, '') }) -t.test('finds node and tap', async (t) => { - const { code, signal, out, err } = await which(['node', 'tap']) - t.equal(signal, null) - t.equal(code, 0) - t.equal(err, '') - t.match(out.split(/[\r\n]+/), [ - /[\\/]node(\.exe)?$/i, - /[\\/]tap(\.cmd)?$/i, - ]) +test('finds node and eslint', async () => { + const { code, signal, out, err } = await which(['node', 'eslint']) + assert.strictEqual(signal, null) + assert.strictEqual(code, 0) + assert.strictEqual(err, '') + const lines = out.split(/[\r\n]+/) + assert.match(lines[0], /[\\/]node(\.exe)?$/i) + assert.match(lines[1], /[\\/]eslint(\.cmd)?$/i) }) -t.test('finds node and tap, but not flergyderp', async (t) => { - const { code, signal, out, err } = await which(['node', 'flergyderp', 'tap']) - t.equal(signal, null) - t.equal(code, 1) - t.equal(err, '') - t.match(out.split(/[\r\n]+/), [ - /[\\/]node(\.exe)?$/i, - /[\\/]tap(\.cmd)?$/i, - ]) +test('finds node and eslint, but not flergyderp', async () => { + const { code, signal, out, err } = await which(['node', 'flergyderp', 'eslint']) + assert.strictEqual(signal, null) + assert.strictEqual(code, 1) + assert.strictEqual(err, '') + const lines = out.split(/[\r\n]+/) + assert.match(lines[0], /[\\/]node(\.exe)?$/i) + assert.match(lines[1], /[\\/]eslint(\.cmd)?$/i) }) -t.test('cli flags', async (t) => { +test('cli flags', async (t) => { const p = require('path').dirname(bin) for (const c of ['-a', '-s', '-as', '-sa']) { - t.test(c, async (t) => { + await t.test(c, { skip: process.platform === 'win32' && /a/.test(c) ? 'windows does not have builtin "which"' : false }, async () => { let { code, signal, out, err } = await which(['which', c], p) - t.equal(signal, null) - t.equal(code, 0) - t.equal(err, '') + assert.strictEqual(signal, null) + assert.strictEqual(code, 0) + assert.strictEqual(err, '') if (/s/.test(c)) { - t.equal(out, '', 'should be silent') + assert.strictEqual(out, '', 'should be silent') } else if (/a/.test(c)) { out = out.split(/[\r\n]+/) - const opt = { actual: out } - if (process.platform === 'win32') { - opt.skip = 'windows does not have builtin "which"' - } - t.ok(out.length > 0, 'should have a result', opt) + assert.ok(out.length > 0, 'should have a result') } }) } }) -t.test('shows usage', async (t) => { +test('shows usage', async () => { const { code, signal, out, err } = await which() - t.equal(signal, null) - t.equal(code, 1) - t.equal(err, 'usage: which [-as] program ...') - t.equal(out, '') + assert.strictEqual(signal, null) + assert.strictEqual(code, 1) + assert.strictEqual(err, 'usage: which [-as] program ...') + assert.strictEqual(out, '') }) -t.test('complains about unknown flag', async (t) => { +test('complains about unknown flag', async () => { const { code, signal, out, err } = await which(['node', '-sax']) - t.equal(signal, null) - t.equal(code, 1) - t.equal(out, '') - t.equal(err, 'which: illegal option -- x\nusage: which [-as] program ...') + assert.strictEqual(signal, null) + assert.strictEqual(code, 1) + assert.strictEqual(out, '') + assert.strictEqual(err, 'which: illegal option -- x\nusage: which [-as] program ...') }) -t.test('anything after -- is ignored', async (t) => { +test('anything after -- is ignored', async () => { const { code, signal, out, err } = await which(['node', '--', '--anything-goes-here']) - t.equal(signal, null) - t.equal(code, 0) - t.equal(err, '') - t.match(out, /[\\/]node(\.exe)?$/i) + assert.strictEqual(signal, null) + assert.strictEqual(code, 0) + assert.strictEqual(err, '') + assert.match(out, /[\\/]node(\.exe)?$/i) }) diff --git a/test/index.js b/test/index.js index c77fed5..15c814a 100644 --- a/test/index.js +++ b/test/index.js @@ -1,14 +1,56 @@ -const t = require('tap') +const { test, mock } = require('node:test') +const assert = require('node:assert') const fs = require('fs') +const os = require('os') const { basename, join, relative, sep, delimiter } = require('path') const isWindows = process.platform === 'win32' +// Helper to create test directories with automatic cleanup +// Call this within a test context to get automatic cleanup via t.after() +const testdirRegistry = new Set() + +function testdir (structure = {}, t = null) { + const dir = fs.mkdtempSync(join(os.tmpdir(), 'which-test-')) + function createStructure (base, struct) { + for (const [name, content] of Object.entries(struct)) { + const path = join(base, name) + if (typeof content === 'string') { + fs.writeFileSync(path, content) + } else { + fs.mkdirSync(path, { recursive: true }) + createStructure(path, content) + } + } + } + createStructure(dir, structure) + + // Register for cleanup + if (t && t.after) { + t.after(() => fs.rmSync(dir, { recursive: true, force: true })) + } else { + testdirRegistry.add(dir) + } + + return dir +} + +// Cleanup any remaining test directories on process exit +process.on('exit', () => { + for (const dir of testdirRegistry) { + try { + fs.rmSync(dir, { recursive: true, force: true }) + } catch { + // Ignore cleanup errors on exit + } + } +}) + const ENV_VARS = { PATH: process.env.PATH, PATHEXT: process.env.PATHEXT } const PLATFORM = Object.getOwnPropertyDescriptor(process, 'platform') const runTest = async (t, exec, expect, { platforms = ['posix', 'win32'], ..._opt } = {}) => { - t.teardown(() => { + t.after(() => { for (const [k, v] of Object.entries(ENV_VARS)) { if (v) { process.env[k] = v @@ -19,10 +61,10 @@ const runTest = async (t, exec, expect, { platforms = ['posix', 'win32'], ..._op }) for (const platform of platforms) { - t.test(`${t.name} - ${platform}`, async t => { + await t.test(`${t.name} - ${platform}`, async t => { Object.defineProperty(process, 'platform', { ...PLATFORM, value: platform }) - t.teardown(() => { + t.after(() => { Object.defineProperty(process, 'platform', PLATFORM) }) @@ -32,96 +74,111 @@ const runTest = async (t, exec, expect, { platforms = ['posix', 'win32'], ..._op // if we are actually on windows but testing posix we have to // mock isexe since that has special windows detection inside // of it. this is mostly to get 100% coverage on windows - const mocks = {} + let mockContext if (isWindows && platform === 'posix') { const isexe = async (p) => [].concat(expect).includes(p) isexe.sync = (p) => [].concat(expect).includes(p) - mocks.isexe = { isexe, sync: isexe.sync } + + // Use node:test mock functionality + mockContext = mock.module('isexe', { + namedExports: { + isexe, + sync: isexe.sync, + }, + }) } - const which = t.mock('..', mocks) + // Clear cache to get fresh module with mocks + delete require.cache[require.resolve('..')] + const which = require('..') + if (expect?.code) { - await t.rejects(() => which(exec, opt), expect, 'async rejects') - t.throws(() => which.sync(exec, opt), expect, 'sync throws') + await assert.rejects(() => which(exec, opt), expect, 'async rejects') + assert.throws(() => which.sync(exec, opt), expect, 'sync throws') } else { - t.strictSame(await which(exec, opt), expect, 'async') - t.strictSame(which.sync(exec, opt), expect, 'sync') + assert.deepStrictEqual(await which(exec, opt), expect, 'async') + assert.deepStrictEqual(which.sync(exec, opt), expect, 'sync') + } + + // Restore mocks + if (mockContext) { + mockContext.restore() } }) } } -t.test('does not find missed', async (t) => { - const fixture = t.testdir() +test('does not find missed', async (t) => { + const fixture = testdir({}, t) const cmd = join(fixture, 'foobar.sh') - t.test('throw', async t => { + await t.test('throw', async t => { await runTest(t, cmd, { code: 'ENOENT' }) }) - t.test('nothrow', async t => { + await t.test('nothrow', async t => { await runTest(t, cmd, null, { nothrow: true }) }) }) -t.test('does not find non-executable', async (t) => { - const dir = t.testdir({ 'foo.sh': 'echo foo\n' }) +test('does not find non-executable', async (t) => { + const dir = testdir({ 'foo.sh': 'echo foo\n' }, t) const foo = join(dir, 'foo.sh') - t.test('absolute', async (t) => { + await t.test('absolute', async (t) => { await runTest(t, foo, { code: 'ENOENT' }) }) - t.test('with path', async (t) => { + await t.test('with path', async (t) => { await runTest(t, basename(foo), { code: 'ENOENT' }, { path: dir }) }) }) -t.test('find when executable', async t => { - const fixture = t.testdir({ 'foo.sh': 'echo foo\n' }) +test('find when executable', async t => { + const fixture = testdir({ 'foo.sh': 'echo foo\n' }, t) const foo = join(fixture, 'foo.sh') fs.chmodSync(foo, '0755') // windows needs to explicitly look for .sh files by default const opts = isWindows ? { pathExt: '.sh' } : {} - t.test('absolute', async (t) => { + await t.test('absolute', async (t) => { await runTest(t, foo, foo, opts) }) - t.test('with process.env.PATH', async (t) => { + await t.test('with process.env.PATH', async (t) => { process.env.PATH = fixture await runTest(t, basename(foo), foo, opts) }) - t.test('with path opt', async (t) => { + await t.test('with path opt', async (t) => { await runTest(t, basename(foo), foo, { ...opts, path: fixture }) }) - t.test('no ./', async (t) => { + await t.test('no ./', async (t) => { const rel = relative(process.cwd(), foo) await runTest(t, rel, rel, opts) }) - t.test('with ./', async (t) => { + await t.test('with ./', async (t) => { const rel = `.${sep}${relative(process.cwd(), foo)}` await runTest(t, rel, rel, opts) }) - t.test('with ../', async (t) => { + await t.test('with ../', async (t) => { const dir = basename(process.cwd()) const rel = join('..', dir, relative(process.cwd(), foo)) await runTest(t, rel, rel, opts) }) }) -t.test('find all', async t => { +test('find all', async t => { const cmdName = 'x.cmd' - const fixture = t.testdir({ + const fixture = testdir({ all: { a: { [cmdName]: 'exec me' }, b: { [cmdName]: 'exec me' }, }, - }) + }, t) const dirs = [ join(fixture, 'all', 'a'), join(fixture, 'all', 'b'), @@ -137,35 +194,35 @@ t.test('find all', async t => { }) }) -t.test('pathExt', async (t) => { - const fixture = t.testdir({ 'foo.sh': 'echo foo\n' }) +test('pathExt', async (t) => { + const fixture = testdir({ 'foo.sh': 'echo foo\n' }, t) const foo = join(fixture, 'foo.sh') fs.chmodSync(foo, '0755') const pathExt = '.sh' const opts = { platforms: ['win32'] } - t.test('foo.sh - env vars', async (t) => { + await t.test('foo.sh - env vars', async (t) => { process.env.PATHEXT = pathExt process.env.PATH = fixture await runTest(t, basename(foo), foo, opts) }) - t.test('foo.sh - opts', async (t) => { + await t.test('foo.sh - opts', async (t) => { await runTest(t, basename(foo), foo, { ...opts, path: fixture, pathExt }) }) - t.test('foo - env vars', async (t) => { + await t.test('foo - env vars', async (t) => { process.env.PATHEXT = pathExt process.env.PATH = fixture await runTest(t, basename(foo, '.sh'), foo, opts) }) - t.test('foo - opts', async (t) => { + await t.test('foo - opts', async (t) => { await runTest(t, basename(foo, '.sh'), foo, { ...opts, path: fixture, pathExt }) }) - t.test('foo - no pathext', async (t) => { + await t.test('foo - no pathext', async (t) => { await runTest(t, basename(foo, '.sh'), { code: 'ENOENT' }, { ...opts, path: fixture, From 0018b40ec0006eb55e4049299a2c59e71f32fb6b Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 10 Dec 2025 15:48:59 -0800 Subject: [PATCH 2/2] chore: remove tap --- package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package.json b/package.json index a19f057..d6a47e1 100644 --- a/package.json +++ b/package.json @@ -17,8 +17,7 @@ }, "devDependencies": { "@npmcli/eslint-config": "^6.0.0", - "@npmcli/template-oss": "4.28.1", - "tap": "^16.3.0" + "@npmcli/template-oss": "4.28.1" }, "scripts": { "test": "node --test './test/**/*.js'",