From 410a1d64a1615395cdd56f1cae46685bfb9ea676 Mon Sep 17 00:00:00 2001 From: isaacs Date: Sat, 9 Sep 2023 22:45:48 -0700 Subject: [PATCH] run: use --import, do not intercept exitCode If plugins export an `importLoader`, and Module.register exists, then use that instead of their `loader` export. process.exitCode became {configurable:false} in https://github.com/nodejs/node/pull/44711 Can bring back exitCode interception when/if it becomes configurable again, re https://github.com/nodejs/node/pull/49579 For now, just set it, and then verify it's the expected value, and put it back to 0 if so. --- src/run/package.json | 5 ++- src/run/src/run.ts | 24 +----------- src/run/src/test-argv.ts | 31 +++++++++++++++ src/run/test/list.ts | 6 +-- src/run/test/plugin.ts | 15 ++++---- src/run/test/repl.ts | 4 +- src/run/test/report.ts | 14 +++---- src/run/test/test-argv.ts | 80 +++++++++++++++++++++++++++++++++++++++ 8 files changed, 135 insertions(+), 44 deletions(-) create mode 100644 src/run/src/test-argv.ts create mode 100644 src/run/test/test-argv.ts diff --git a/src/run/package.json b/src/run/package.json index cd50250ae..61e03aa55 100644 --- a/src/run/package.json +++ b/src/run/package.json @@ -44,7 +44,7 @@ "@tapjs/after": "0.0.0-20", "@tapjs/before": "0.0.0-20", "@tapjs/config": "2.0.0-21", - "@tapjs/processinfo": "^2.5.6", + "@tapjs/processinfo": "^3.0.2", "@tapjs/reporter": "0.0.0-22", "@tapjs/spawn": "0.0.0-20", "@tapjs/stdin": "0.0.0-20", @@ -58,7 +58,7 @@ "opener": "^1.5.2", "pacote": "^17.0.3", "path-scurry": "^1.9.2", - "resolve-import": "^1.0.4", + "resolve-import": "^1.2.1", "rimraf": "^5.0.0", "semver": "^7.5.4", "signal-exit": "^4.1.0", @@ -69,6 +69,7 @@ "which": "^4.0.0" }, "tap": { + "typecheck": false, "coverage-map": "map.js" }, "engines": { diff --git a/src/run/src/run.ts b/src/run/src/run.ts index 5ff477ebd..23c6e60a8 100644 --- a/src/run/src/run.ts +++ b/src/run/src/run.ts @@ -5,11 +5,9 @@ import { LoadedConfig } from '@tapjs/config' import { TapFile, TapFileOpts, type TAP } from '@tapjs/core' import { plugin as SpawnPlugin } from '@tapjs/spawn' import { plugin as StdinPlugin } from '@tapjs/stdin' -import { loaders } from '@tapjs/test' import { glob } from 'glob' import { stat } from 'node:fs/promises' import { relative, resolve } from 'node:path' -import { resolveImport } from 'resolve-import' import { rimraf } from 'rimraf' import { runAfter } from './after.js' import { runBefore } from './before.js' @@ -18,19 +16,9 @@ import { executeTestSuite } from './execute-test-suite.js' import { values } from './main-config.js' import { outputDir } from './output-dir.js' import { readSave } from './save-list.js' +import { testArgv } from './test-argv.js' import { testIsSerial } from './test-is-serial.js' -const piLoaderURL = await resolveImport( - '@tapjs/processinfo/esm', - import.meta.url -) -/* c8 ignore start */ -if (!piLoaderURL) { - throw new Error('could not get @tapjs/processinfo loader') -} -/* c8 ignore stop */ -const piLoader = String(piLoaderURL) - const node = process.execPath export const run = async (args: string[], config: LoadedConfig) => { @@ -44,7 +32,6 @@ export const run = async (args: string[], config: LoadedConfig) => { /* c8 ignore start */ const testArgs: string[] = values['test-arg'] || [] const testEnv: string[] = values['test-env'] || [] - const nodeArgs: string[] = values['node-arg'] || [] /* c8 ignore stop */ process.env._TAPJS_PROCESSINFO_CWD_ = config.globCwd process.env._TAPJS_PROCESSINFO_EXCLUDE_ = String( @@ -55,13 +42,7 @@ export const run = async (args: string[], config: LoadedConfig) => { /(^(node|tapmock):|[\\\/](tap-testdir-[^\\\/]+|tap-snapshots)[\\\/])/ ) - const argv = [ - '--no-warnings=ExperimentalLoader', - ...loaders.map(l => `--loader=${l}`), - '--enable-source-maps', - `--loader=${piLoader}`, - ...nodeArgs, - ] + const argv = testArgv(config) let env: NodeJS.ProcessEnv | undefined = undefined @@ -78,7 +59,6 @@ export const run = async (args: string[], config: LoadedConfig) => { ReturnType & ReturnType & ReturnType => { - // generate the env here, in case any plugins updated it env = { ...process.env } for (const e of testEnv) { diff --git a/src/run/src/test-argv.ts b/src/run/src/test-argv.ts new file mode 100644 index 000000000..42a7b4197 --- /dev/null +++ b/src/run/src/test-argv.ts @@ -0,0 +1,31 @@ +// the arguments when running test files, abstracted from run.ts for testing +import type { LoadedConfig } from '@tapjs/config' +import { importLoaders, loaderFallbacks, loaders } from '@tapjs/test' +import module from 'node:module' + +// if we have Module.register(), then use --import wherever possible +const useImport = !!(module as { register?: (...a: any) => any }) + .register + +const importScripts = useImport ? importLoaders : [] +const loaderScripts = useImport ? loaders : loaderFallbacks + +const pi = useImport + ? '--import=@tapjs/processinfo/import' + : '--loader=@tapjs/processinfo/loader' + +const always = [ + ...importScripts.map(l => `--import=${l}`), + ...loaderScripts.map(l => `--loader=${l}`), + ...(useImport && !loaderScripts.length + ? [] + : ['--no-warnings=ExperimentalLoader']), + '--enable-source-maps', + // ensure this always comes last in the list + pi, +] + +export const testArgv = (config: LoadedConfig) => [ + ...always, + ...(config.get('node-arg') || []), +] diff --git a/src/run/test/list.ts b/src/run/test/list.ts index 9754d273e..bf7d83355 100644 --- a/src/run/test/list.ts +++ b/src/run/test/list.ts @@ -194,14 +194,14 @@ t.test('list some test files', async t => { }) t.test('no files found', async t => { - const exitCode = t.intercept(process, 'exitCode') + const { exitCode } = process const errs = t.capture(console, 'error') await list(['asdfasfsdf'], mainConfig.config) - t.match(exitCode(), [{ type: 'set', value: 1 }]) t.strictSame(errs.args(), [['No files found.']]) await list(['plugins'], mainConfig.config) - t.match(exitCode(), [{ type: 'set', value: 1 }]) + t.equal(process.exitCode, 1) + if (t.passing()) process.exitCode = exitCode t.strictSame(errs.args(), [ ['No files found.'], ["(Did you mean 'tap plugin list'?)"], diff --git a/src/run/test/plugin.ts b/src/run/test/plugin.ts index 6887b2720..97cc2bb40 100644 --- a/src/run/test/plugin.ts +++ b/src/run/test/plugin.ts @@ -212,7 +212,6 @@ t.test('remove plugin', async t => { t.test('adding plugins', async t => { const logs = t.capture(console, 'log') const errs = t.capture(console, 'error') - const exitCode = t.intercept(process, 'exitCode') t.test('fail if no name provided', async t => { const config = new MockConfig(t) @@ -385,7 +384,8 @@ t.test('adding plugins', async t => { t.strictSame(config.edited, undefined) t.matchSnapshot(logs.args()) t.matchSnapshot(errs.args()) - t.match(exitCode(), [{ type: 'set', value: 1 }]) + t.equal(process.exitCode, 1) + process.exitCode = 0 }) t.test('fail to build installed plugin, with cleanup', async t => { @@ -432,12 +432,12 @@ t.test('adding plugins', async t => { t.strictSame(config.edited, undefined) t.matchSnapshot(logs.args()) t.matchSnapshot(errs.args()) - t.match(exitCode(), [{ type: 'set', value: 1 }]) + t.equal(process.exitCode, 1) + process.exitCode = 0 }) t.test('fail to install plugin', async t => { const errs = t.capture(console, 'error') - const exitCode = t.intercept(process, 'exitCode') let buildRan = false let installRan = false const p = 'dep-plugin' @@ -473,12 +473,12 @@ t.test('adding plugins', async t => { t.strictSame(config.edited, undefined) t.matchSnapshot(logs.args()) t.matchSnapshot(errs.args()) - t.match(exitCode(), [{ type: 'set', value: 1 }]) + t.equal(process.exitCode, 1) + process.exitCode = 0 }) t.test(`fail uninstall after build fail`, async t => { const errs = t.capture(console, 'error') - const exitCode = t.intercept(process, 'exitCode') let buildRan = false let installRan = false let uninstallRan = false @@ -523,6 +523,7 @@ t.test('adding plugins', async t => { t.strictSame(config.edited, undefined) t.strictSame(logs(), []) t.matchSnapshot(errs.args()) - t.match(exitCode(), [{ type: 'set', value: 1 }]) + t.equal(process.exitCode, 1) + process.exitCode = 0 }) }) diff --git a/src/run/test/repl.ts b/src/run/test/repl.ts index 8fe026121..701817475 100644 --- a/src/run/test/repl.ts +++ b/src/run/test/repl.ts @@ -11,13 +11,13 @@ t.beforeEach(t => t.test('do not run if already in the repl', async t => { process.env._TAP_REPL = '1' const errs = t.capture(console, 'error') - const code = t.intercept(process, 'exitCode') const { repl } = (await t.mockImport( '../dist/repl.js' )) as typeof import('../dist/repl.js') await repl([], {} as unknown as LoadedConfig) t.strictSame(errs.args(), [['you are already in the tap repl']]) - t.match(code(), [{ type: 'set', value: 1 }]) + t.equal(process.exitCode, 1) + process.exitCode = 0 }) class MockRepl { diff --git a/src/run/test/report.ts b/src/run/test/report.ts index 4e9fbb280..dd02841f3 100644 --- a/src/run/test/report.ts +++ b/src/run/test/report.ts @@ -235,7 +235,6 @@ t.test('run an html report', async t => { const comments = t.capture(mockTap, 'comment') let openerRan = false const htmlReport = resolve(globCwd, '.tap/report', file) - const exitCode = t.intercept(process, 'exitCode') const { report } = (await t.mockImport('../dist/report.js', { c8: { Report: MockReport }, '@tapjs/core': mockCore, @@ -259,7 +258,6 @@ t.test('run an html report', async t => { t.strictSame(comments.args(), []) t.equal(openerRan, true) t.equal(readFileSync(htmlReport, 'utf8'), 'report') - t.strictSame(exitCode(), []) }) } }) @@ -278,11 +276,11 @@ t.test('no coverage files generated', async t => { })) as typeof import('../dist/report.js') const config = new MockConfig([]) const logs = t.capture(console, 'log') - const exitCode = t.intercept(process, 'exitCode') await report([], config as unknown as LoadedConfig) t.strictSame(logs.args(), []) t.strictSame(comments.args(), [['No coverage generated']]) - t.match(exitCode(), [{ type: 'set', value: 1, success: true }]) + t.equal(process.exitCode, 1) + process.exitCode = 0 }) t.test('no coverage summary generated', async t => { @@ -299,11 +297,11 @@ t.test('no coverage summary generated', async t => { })) as typeof import('../dist/report.js') const config = new MockConfig([]) const logs = t.capture(console, 'log') - const exitCode = t.intercept(process, 'exitCode') await report([], config as unknown as LoadedConfig) t.strictSame(logs.args(), []) t.strictSame(comments.args(), [['No coverage generated']]) - t.match(exitCode(), [{ type: 'set', value: 1, success: true }]) + t.equal(process.exitCode, 1) + process.exitCode = 0 }) t.test('not full coverage', async t => { @@ -326,7 +324,6 @@ t.test('not full coverage', async t => { })) as typeof import('../dist/report.js') const config = new MockConfig([]) const logs = t.capture(console, 'log') - const exitCode = t.intercept(process, 'exitCode') await report(['html'], config as unknown as LoadedConfig) t.strictSame(logs.args(), []) t.strictSame(comments.args(), [ @@ -337,5 +334,6 @@ t.test('not full coverage', async t => { ]) t.equal(openerRan, true) t.equal(readFileSync(htmlReport, 'utf8'), 'report') - t.match(exitCode(), [{ type: 'set', value: 1, success: true }]) + t.equal(process.exitCode, 1) + process.exitCode = 0 }) diff --git a/src/run/test/test-argv.ts b/src/run/test/test-argv.ts new file mode 100644 index 000000000..91cf14ae4 --- /dev/null +++ b/src/run/test/test-argv.ts @@ -0,0 +1,80 @@ +import {LoadedConfig} from '@tapjs/config' +import t from 'tap' + +const mocks = { + '@tapjs/test': { + importLoaders: ['blah/import'], + loaders: ['no-import/loader'], + loaderFallbacks: ['blah/loader', 'no-import/loader'], + }, + module: { + register: () => {} + } +} + +const mocksNoImport = { + '@tapjs/test': { + importLoaders: ['blah/import'], + loaders: ['no-import/loader'], + loaderFallbacks: ['blah/loader', 'no-import/loader'], + }, + module: { + register: null + } +} + +const mocksAllImport = { + '@tapjs/test': { + importLoaders: ['blah/import', 'has-import/import'], + loaders: [], + loaderFallbacks: ['blah/loader', 'has-import/loader'], + }, + module: { + register: () => {} + } +} + +t.test('mix of loaders and imports', async t => { + const { testArgv } = await t.mockImport('../dist/test-argv.js', mocks) as typeof import('../dist/test-argv.js') + t.strictSame(testArgv({ get: () => {}} as unknown as LoadedConfig), [ + '--import=blah/import', + '--loader=no-import/loader', + '--no-warnings=ExperimentalLoader', + '--enable-source-maps', + '--import=@tapjs/processinfo/import', + ]) +}) + +t.test('with --node-arg', async t => { + const { testArgv } = await t.mockImport('../dist/test-argv.js', mocks) as typeof import('../dist/test-argv.js') + t.strictSame(testArgv({ get: () => ['a', 'b']} as unknown as LoadedConfig), [ + '--import=blah/import', + '--loader=no-import/loader', + '--no-warnings=ExperimentalLoader', + '--enable-source-maps', + '--import=@tapjs/processinfo/import', + 'a', + 'b', + ]) +}) + +t.test('all imports, no loader', async t => { + const { testArgv } = await t.mockImport('../dist/test-argv.js', mocksAllImport) as typeof import('../dist/test-argv.js') + t.strictSame(testArgv({ get: () => []} as unknown as LoadedConfig), [ + '--import=blah/import', + '--import=has-import/import', + '--enable-source-maps', + '--import=@tapjs/processinfo/import', + ]) +}) + +t.test('no import support, only loader', async t => { + const { testArgv } = await t.mockImport('../dist/test-argv.js', mocksNoImport) as typeof import('../dist/test-argv.js') + t.strictSame(testArgv({ get: () => []} as unknown as LoadedConfig), [ + '--loader=blah/loader', + '--loader=no-import/loader', + '--no-warnings=ExperimentalLoader', + '--enable-source-maps', + '--loader=@tapjs/processinfo/loader', + ]) +})