Skip to content

Commit 8cd0f5e

Browse files
boneskullclaude
boneskull
and
claude
committed
fix: add Node.js 20 support for CI
- Add runtime check for profiling limitation on Node.js 20 (--cpu-prof not allowed in NODE_OPTIONS for security reasons) - Show helpful error message directing users to Node.js 22+ - Fix run-tests-v20.js to use process.execPath instead of 'node' - Add isNode20 helper to test/util.ts for version-specific test skipping - Split profiling tests into Node 20 and Node 22+ groups 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 8c42f3a commit 8cd0f5e

File tree

5 files changed

+262
-156
lines changed

5 files changed

+262
-156
lines changed

‎scripts/run-tests-v20.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ if (files.length === 0) {
1818
}
1919

2020
execFileSync(
21-
'node',
21+
process.execPath,
2222
['--import', 'tsx', '--test', '--test-reporter=spec', ...files],
2323
{ stdio: 'inherit' },
2424
);

‎src/services/profiler/profile-runner.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,32 @@ interface RunOptions {
2626
timeout?: number;
2727
}
2828

29+
/**
30+
* Node.js major version number
31+
*/
32+
const nodeMajorVersion = Number(process.versions.node.split('.')[0]);
33+
2934
/**
3035
* Run a command with Node.js profiling enabled
3136
*
3237
* @param command - Command to run (e.g., "npm test")
3338
* @param options - Execution options
3439
* @returns Path to generated *.cpuprofile file
40+
* @throws Error if running on Node.js 20 (--cpu-prof not allowed in
41+
* NODE_OPTIONS)
3542
*/
3643
export const runWithProfiling = async (
3744
command: string,
3845
options: RunOptions = {},
3946
): Promise<string> => {
47+
// Node.js 20 blocks --cpu-prof in NODE_OPTIONS for security reasons
48+
if (nodeMajorVersion === 20) {
49+
throw new Error(
50+
'CPU profiling requires Node.js 22 or later. ' +
51+
'Node.js 20 blocks --cpu-prof in NODE_OPTIONS for security reasons.',
52+
);
53+
}
54+
4055
const cwd = options.cwd || process.cwd();
4156

4257
// Create profiles directory

‎test/integration/profile-command.test.ts‎

Lines changed: 134 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -1,99 +1,152 @@
11
import assert from 'node:assert/strict';
2-
import { execSync } from 'node:child_process';
2+
import { execSync, spawnSync } from 'node:child_process';
33
import { mkdtemp, rm, writeFile } from 'node:fs/promises';
44
import { tmpdir } from 'node:os';
55
import { dirname, join } from 'node:path';
66
import { describe, it } from 'node:test';
77
import { fileURLToPath } from 'node:url';
88

9+
import { isNode20 } from '../util.js';
910
import { fixtures } from './fixture-paths.js';
1011

1112
const __filename = fileURLToPath(import.meta.url);
1213
const __dirname = dirname(__filename);
1314

15+
// Node.js v20 doesn't allow --cpu-prof in NODE_OPTIONS for security reasons
16+
// Tests are split into two groups: Node 20 tests and Node 22+ tests
17+
const node20Test = isNode20 ? it : it.skip;
18+
const node22PlusTest = isNode20 ? it.skip : it;
19+
1420
describe('Profile Command Integration', () => {
15-
it('should profile a simple script and generate report', async () => {
16-
const tmpDir = await mkdtemp(join(tmpdir(), 'modestbench-profile-'));
17-
18-
try {
19-
// Create package.json
20-
await writeFile(
21-
join(tmpDir, 'package.json'),
22-
JSON.stringify({ name: 'test-project' }),
23-
);
24-
25-
// Run profile command using fixture
26-
const output = execSync(
27-
`node ${join(__dirname, '../../dist/cli/index.js')} analyze "node ${fixtures.profileHotFunction}"`,
28-
{
29-
cwd: tmpDir,
30-
encoding: 'utf-8',
31-
},
32-
);
33-
34-
// Verify output contains expected sections
35-
assert.ok(output.includes('Profile Analysis'), 'Should contain header');
36-
assert.ok(
37-
output.includes('Benchmark Candidates'),
38-
'Should contain candidates section',
39-
);
40-
assert.ok(
41-
output.includes('hotFunction') || output.includes('ticks'),
42-
'Should contain function data',
43-
);
44-
} finally {
45-
await rm(tmpDir, { force: true, recursive: true });
46-
}
47-
});
21+
describe('on Node.js 20', () => {
22+
node20Test(
23+
'should show helpful error when profiling is unavailable',
24+
async () => {
25+
const tmpDir = await mkdtemp(join(tmpdir(), 'modestbench-profile-'));
26+
27+
try {
28+
await writeFile(
29+
join(tmpDir, 'package.json'),
30+
JSON.stringify({ name: 'test-project' }),
31+
);
32+
33+
// Use process.execPath to ensure we run with the same Node version
34+
const result = spawnSync(
35+
process.execPath,
36+
[
37+
join(__dirname, '../../dist/cli/index.js'),
38+
'analyze',
39+
`node ${fixtures.profileHotFunction}`,
40+
],
41+
{
42+
cwd: tmpDir,
43+
encoding: 'utf-8',
44+
},
45+
);
4846

49-
it('should support --group-by-file option', async () => {
50-
const tmpDir = await mkdtemp(join(tmpdir(), 'modestbench-profile-'));
51-
52-
try {
53-
await writeFile(
54-
join(tmpDir, 'package.json'),
55-
JSON.stringify({ name: 'test-project' }),
56-
);
57-
58-
const output = execSync(
59-
`node ${join(__dirname, '../../dist/cli/index.js')} analyze "node ${fixtures.profileMultiFunction}" --group-by-file`,
60-
{
61-
cwd: tmpDir,
62-
encoding: 'utf-8',
63-
},
64-
);
65-
66-
assert.ok(
67-
output.includes('Grouped by File'),
68-
'Should contain grouped by file header',
69-
);
70-
} finally {
71-
await rm(tmpDir, { force: true, recursive: true });
72-
}
47+
assert.notEqual(result.status, 0, 'Should exit with non-zero code');
48+
assert.ok(
49+
result.stderr.includes('Node.js 22 or later') ||
50+
result.stderr.includes('CPU profiling requires'),
51+
'Should show helpful error message about Node.js version',
52+
);
53+
} finally {
54+
await rm(tmpDir, { force: true, recursive: true });
55+
}
56+
},
57+
);
7358
});
7459

75-
it('should respect --min-percent threshold', async () => {
76-
const tmpDir = await mkdtemp(join(tmpdir(), 'modestbench-profile-'));
77-
78-
try {
79-
await writeFile(
80-
join(tmpDir, 'package.json'),
81-
JSON.stringify({ name: 'test-project' }),
82-
);
83-
84-
// Run with high threshold - should filter out most functions
85-
const output = execSync(
86-
`node ${join(__dirname, '../../dist/cli/index.js')} analyze "node ${fixtures.profileHotFunction}" --min-percent 50`,
87-
{
88-
cwd: tmpDir,
89-
encoding: 'utf-8',
90-
},
91-
);
92-
93-
// Should still have header but fewer functions
94-
assert.ok(output.includes('Profile Analysis'), 'Should have header');
95-
} finally {
96-
await rm(tmpDir, { force: true, recursive: true });
97-
}
60+
describe('on Node.js 22+', () => {
61+
node22PlusTest(
62+
'should profile a simple script and generate report',
63+
async () => {
64+
const tmpDir = await mkdtemp(join(tmpdir(), 'modestbench-profile-'));
65+
66+
try {
67+
// Create package.json
68+
await writeFile(
69+
join(tmpDir, 'package.json'),
70+
JSON.stringify({ name: 'test-project' }),
71+
);
72+
73+
// Run profile command using fixture
74+
const output = execSync(
75+
`node ${join(__dirname, '../../dist/cli/index.js')} analyze "node ${fixtures.profileHotFunction}"`,
76+
{
77+
cwd: tmpDir,
78+
encoding: 'utf-8',
79+
},
80+
);
81+
82+
// Verify output contains expected sections
83+
assert.ok(
84+
output.includes('Profile Analysis'),
85+
'Should contain header',
86+
);
87+
assert.ok(
88+
output.includes('Benchmark Candidates'),
89+
'Should contain candidates section',
90+
);
91+
assert.ok(
92+
output.includes('hotFunction') || output.includes('ticks'),
93+
'Should contain function data',
94+
);
95+
} finally {
96+
await rm(tmpDir, { force: true, recursive: true });
97+
}
98+
},
99+
);
100+
101+
node22PlusTest('should support --group-by-file option', async () => {
102+
const tmpDir = await mkdtemp(join(tmpdir(), 'modestbench-profile-'));
103+
104+
try {
105+
await writeFile(
106+
join(tmpDir, 'package.json'),
107+
JSON.stringify({ name: 'test-project' }),
108+
);
109+
110+
const output = execSync(
111+
`node ${join(__dirname, '../../dist/cli/index.js')} analyze "node ${fixtures.profileMultiFunction}" --group-by-file`,
112+
{
113+
cwd: tmpDir,
114+
encoding: 'utf-8',
115+
},
116+
);
117+
118+
assert.ok(
119+
output.includes('Grouped by File'),
120+
'Should contain grouped by file header',
121+
);
122+
} finally {
123+
await rm(tmpDir, { force: true, recursive: true });
124+
}
125+
});
126+
127+
node22PlusTest('should respect --min-percent threshold', async () => {
128+
const tmpDir = await mkdtemp(join(tmpdir(), 'modestbench-profile-'));
129+
130+
try {
131+
await writeFile(
132+
join(tmpDir, 'package.json'),
133+
JSON.stringify({ name: 'test-project' }),
134+
);
135+
136+
// Run with high threshold - should filter out most functions
137+
const output = execSync(
138+
`node ${join(__dirname, '../../dist/cli/index.js')} analyze "node ${fixtures.profileHotFunction}" --min-percent 50`,
139+
{
140+
cwd: tmpDir,
141+
encoding: 'utf-8',
142+
},
143+
);
144+
145+
// Should still have header but fewer functions
146+
assert.ok(output.includes('Profile Analysis'), 'Should have header');
147+
} finally {
148+
await rm(tmpDir, { force: true, recursive: true });
149+
}
150+
});
98151
});
99152
});

0 commit comments

Comments
 (0)