Skip to content

Commit e29c02d

Browse files
authored
Clean up the test harness a bit (#55)
Primarily, this gets rid of the `test/runtest` executable, preventing the need for spawning an additional subprocess for each test. The current approach handles all the things that `test/runtest` would in a `--require` and in a loader that switches whether or not to use the IITM loader via the same filename check that was originally done in `test/runtest`. One interesting thing that happened while making this change is that we need some way of passing the entrypoint filename to the loader. Since newer versions of Node.js run the loader in a separate thread, `process.argv[1]` no longer works. The approved alternative is to use inter-thread messages via the `register()` method, but that's not available on some versions of Node.js that also use loader threads. To get around this, an environment variable is set in the `--require` that's then propagated to the loader thread. This is a bit hacky, but probably the only thing that will work across the supported version range. An attempt was made to use the more popular `tap` test package, rather than `imhotap`, but there's no version of `tap` that supports both the entire Node.js version range that IITM supports _and_ also supports ESM. For now, this means sticking with `imhotap`, but were it not for the incompatibility, `tap` would be just as suitable here, and is a much more popular test package.
1 parent 6f4aa45 commit e29c02d

5 files changed

Lines changed: 57 additions & 54 deletions

File tree

package.json

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
"description": "Intercept imports in Node.js",
55
"main": "index.js",
66
"scripts": {
7-
"test": "c8 --reporter lcov --check-coverage --lines 50 imhotap --runner 'node test/runtest' --files test/{hook,low-level,other,get-esm-exports}/*",
8-
"test:ts": "c8 --reporter lcov imhotap --runner 'node test/runtest' --files test/typescript/*.test.mts",
9-
"coverage": "c8 --reporter html imhotap --runner 'node test/runtest' --files test/{hook,low-level,other,get-esm-exports}/* && echo '\nNow open coverage/index.html\n'",
7+
"test": "c8 --reporter lcov --check-coverage --lines 50 imhotap --files test/{hook,low-level,other,get-esm-exports}/*",
8+
"test:ts": "c8 --reporter lcov imhotap --files test/typescript/*.test.mts",
9+
"coverage": "c8 --reporter html imhotap --files test/{hook,low-level,other,get-esm-exports}/* && echo '\nNow open coverage/index.html\n'",
1010
"lint": "eslint .",
1111
"lint:fix": "eslint . --fix"
1212
},
@@ -28,6 +28,10 @@
2828
"url": "https://github.com/DataDog/import-in-the-middle/issues"
2929
},
3030
"homepage": "https://github.com/DataDog/import-in-the-middle#readme",
31+
"imhotap": {
32+
"runner": "node",
33+
"test-env": "NODE_OPTIONS=--no-warnings --require ./test/version-check.js --experimental-loader ./test/generic-loader.mjs"
34+
},
3135
"devDependencies": {
3236
"@babel/core": "^7.23.7",
3337
"@babel/eslint-parser": "^7.23.3",

test/README.md

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,15 @@ These tests are organized as follows:
33
* Located in the `hook` directory if they use the `Hook` class.
44
* Located in the `low-level` directory if they use the "low-level" API,
55
`addHook` and `removeHook`.
6+
* Other tests are in other adjacent directories.
67

7-
The tests should be run with the `runtest` command found in this directory. If
8-
the command exits with a non-zero code, then it's a test failure.
8+
The tests can be run individually as Node.js programs with non-zero exit codes
9+
upon failures. They should be run with the following Node.js command-line
10+
options (assuming they're run from the project root):
911

10-
Running of all the tests can be done with `npm test`.
12+
```
13+
--require ./test/version-check.js
14+
--experimental-loader ./test/generic-loader.mmjs
15+
```
1116

12-
Coverage must be 100% according to `c8`. If you don't have 100% coverage, you
13-
can run `npm run coverage` to get coverage data in HTML form.
17+
The entire test suite can be run with `npm test`.

test/generic-loader.mjs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2.0 License.
2+
//
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2024 Datadog, Inc.
4+
5+
import * as tsLoader from './typescript/iitm-ts-node-loader.mjs'
6+
import * as regularLoader from '../hook.mjs'
7+
import path from 'path'
8+
9+
const filename = process.env.IITM_TEST_FILE
10+
11+
export const { load, resolve, getFormat, getSource } =
12+
filename.includes('disabled')
13+
? {}
14+
: (path.extname(filename).slice(-2) === 'ts' ? tsLoader : regularLoader)

test/runtest

Lines changed: 0 additions & 46 deletions
This file was deleted.

test/version-check.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2.0 License.
2+
//
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2024 Datadog, Inc.
4+
5+
const wt = require('worker_threads')
6+
7+
// Do not want this running on loader thread
8+
if (!wt.isMainThread) return
9+
10+
const filename = process.argv[1]
11+
process.env.IITM_TEST_FILE = filename
12+
13+
const [processMajor, processMinor] = process.versions.node.split('.').map(Number)
14+
15+
const match = filename.match(/v([0-9]+)(?:\.([0-9]+))?/)
16+
17+
const majorRequirement = match ? match[1] : 0
18+
const minorRequirement = match && match[2]
19+
20+
if (processMajor < majorRequirement) {
21+
console.log(`skipping ${filename} as this is Node.js v${processMajor} and test wants v${majorRequirement}`)
22+
process.exit(0)
23+
}
24+
if (processMajor <= majorRequirement && processMinor < minorRequirement) {
25+
console.log(`skipping ${filename} as this is Node.js v${processMajor}.${processMinor} and test wants >=v${majorRequirement}.${minorRequirement}`)
26+
process.exit(0)
27+
}

0 commit comments

Comments
 (0)