Skip to content

Commit

Permalink
run: use --import, do not intercept exitCode
Browse files Browse the repository at this point in the history
If plugins export an `importLoader`, and Module.register exists, then
use that instead of their `loader` export.

process.exitCode became {configurable:false} in
nodejs/node#44711

Can bring back exitCode interception when/if it becomes configurable
again, re nodejs/node#49579

For now, just set it, and then verify it's the expected value, and put
it back to 0 if so.
  • Loading branch information
isaacs committed Sep 10, 2023
1 parent 5ee2ddd commit 410a1d6
Show file tree
Hide file tree
Showing 8 changed files with 135 additions and 44 deletions.
5 changes: 3 additions & 2 deletions src/run/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -69,6 +69,7 @@
"which": "^4.0.0"
},
"tap": {
"typecheck": false,
"coverage-map": "map.js"
},
"engines": {
Expand Down
24 changes: 2 additions & 22 deletions src/run/src/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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) => {
Expand All @@ -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(
Expand All @@ -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

Expand All @@ -78,7 +59,6 @@ export const run = async (args: string[], config: LoadedConfig) => {
ReturnType<typeof SpawnPlugin> &
ReturnType<typeof BeforePlugin> &
ReturnType<typeof StdinPlugin> => {

// generate the env here, in case any plugins updated it
env = { ...process.env }
for (const e of testEnv) {
Expand Down
31 changes: 31 additions & 0 deletions src/run/src/test-argv.ts
Original file line number Diff line number Diff line change
@@ -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') || []),
]
6 changes: 3 additions & 3 deletions src/run/test/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'?)"],
Expand Down
15 changes: 8 additions & 7 deletions src/run/test/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
})
})
4 changes: 2 additions & 2 deletions src/run/test/repl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 6 additions & 8 deletions src/run/test/report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(), [])
})
}
})
Expand All @@ -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 => {
Expand All @@ -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 => {
Expand All @@ -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(), [
Expand All @@ -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
})
80 changes: 80 additions & 0 deletions src/run/test/test-argv.ts
Original file line number Diff line number Diff line change
@@ -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',
])
})

0 comments on commit 410a1d6

Please sign in to comment.