Skip to content

Commit 0da50ea

Browse files
committed
Fix npm test on Node 24+
The test script passed --experimental-async-context-frame unconditionally. Node 22/23 need it, but Node 24 turned AsyncContextFrame on by default and removed the flag, so from Node 24 on `npm test` failed before running anything: node: bad option: --experimental-async-context-frame Replace it with a small runner in the style of build.js, which computes the flag list from the running version. --disable-warning is conditional for the same reason: it only exists from Node 21.3. The computation lives in test/node-flags.js rather than inline in the runner because tests that spawn a child need the same flag list. npm test now runs on Node 22, 24 and 26; previously 24 and 26 could not run it at all.
1 parent 9f06caf commit 0da50ea

3 files changed

Lines changed: 53 additions & 1 deletion

File tree

js/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"description": "Node.js writer for the OpenTelemetry thread-context record (OTEP-4947), discoverable via AsyncLocalStorage / AsyncContextFrame.",
55
"main": "index.js",
66
"scripts": {
7-
"test": "node --experimental-async-context-frame --disable-warning=ExperimentalWarning --test test/test.js",
7+
"test": "node test/run.js",
88
"test:docker": "./test/run-in-docker.sh",
99
"install": "node build.js"
1010
},

js/test/node-flags.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
'use strict';
2+
3+
// Node flags needed to make AsyncContextFrame — the writer's discovery
4+
// substrate — available on the running Node.
5+
//
6+
// Node 22/23 need the --experimental-async-context-frame opt-in. Node 24
7+
// turned ACF on by default and *removed* the flag, so passing it there is a
8+
// hard `node: bad option` failure rather than a no-op. Anything that spawns a
9+
// Node process for these tests has to compute the list rather than hardcode
10+
// it; that includes `npm test` itself (see run.js).
11+
function acfFlags() {
12+
const major = Number(process.versions.node.split('.')[0]);
13+
return major >= 22 && major < 24 ? ['--experimental-async-context-frame'] : [];
14+
}
15+
16+
module.exports = { acfFlags };

js/test/run.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
'use strict';
2+
3+
// `npm test` entry point.
4+
//
5+
// The script used to pass --experimental-async-context-frame unconditionally,
6+
// which works on Node 22/23 and fails outright from Node 24 on, where the flag
7+
// was removed:
8+
//
9+
// node: bad option: --experimental-async-context-frame
10+
//
11+
// so `npm test` was broken on every current Node. Compute the flags instead.
12+
13+
const { spawnSync } = require('node:child_process');
14+
const path = require('node:path');
15+
16+
const { acfFlags } = require('./node-flags');
17+
18+
const major = Number(process.versions.node.split('.')[0]);
19+
const flags = [...acfFlags()];
20+
// --disable-warning landed in Node 21.3; on older Node it would itself be a
21+
// bad option. Nothing below 22 can run these tests anyway (test.js bails).
22+
if (major >= 22) {
23+
flags.push('--disable-warning=ExperimentalWarning');
24+
}
25+
26+
const res = spawnSync(
27+
process.execPath,
28+
[...flags, '--test', path.join(__dirname, 'test.js')],
29+
{ stdio: 'inherit' },
30+
);
31+
32+
if (res.error) {
33+
console.error(res.error);
34+
process.exit(1);
35+
}
36+
process.exit(res.status === null ? 1 : res.status);

0 commit comments

Comments
 (0)