Skip to content

Commit f45f068

Browse files
OrKoNOrKoN
authored
fix: make sure env variables are consistently applied when parsing args (#1994)
This PR moves the checks for CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS and CI env vars to parseArguments making sure it is correctly applied by the daemon and in tests. It also updates the tests to set explicit CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS as it might not always be inherited. This change is likely to affect the metrics collected and might explain some fluctuations we observed.
1 parent da0441d commit f45f068

5 files changed

Lines changed: 152 additions & 101 deletions

File tree

‎src/bin/chrome-devtools-mcp-cli-options.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,11 @@ export const cliOptions = {
292292

293293
export type ParsedArguments = ReturnType<typeof parseArguments>;
294294

295-
export function parseArguments(version: string, argv = process.argv) {
295+
export function parseArguments(
296+
version: string,
297+
argv = process.argv,
298+
env = process.env,
299+
) {
296300
const yargsInstance = yargs(hideBin(argv))
297301
.scriptName('npx chrome-devtools-mcp@latest')
298302
.options(cliOptions)
@@ -307,6 +311,12 @@ export function parseArguments(version: string, argv = process.argv) {
307311
) {
308312
args.channel = 'stable';
309313
}
314+
if (env['CI'] || env['CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS']) {
315+
console.error(
316+
"turning off usage statistics. process.env['CI'] || process.env['CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS'] is set.",
317+
);
318+
args.usageStatistics = false;
319+
}
310320
return true;
311321
})
312322
.example([

‎src/bin/chrome-devtools-mcp-main.ts‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,6 @@ await checkForUpdates(
2424
export const args = parseArguments(VERSION);
2525

2626
const logFile = args.logFile ? saveLogsToFile(args.logFile) : undefined;
27-
if (
28-
process.env['CI'] ||
29-
process.env['CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS']
30-
) {
31-
console.error(
32-
"turning off usage statistics. process.env['CI'] || process.env['CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS'] is set.",
33-
);
34-
args.usageStatistics = false;
35-
}
3627

3728
if (process.env['CHROME_DEVTOOLS_MCP_CRASH_ON_UNCAUGHT'] !== 'true') {
3829
process.on('unhandledRejection', (reason, promise) => {

‎tests/cli.test.ts‎

Lines changed: 137 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ describe('cli args parsing', () => {
3232
};
3333

3434
it('parses with default args', async () => {
35-
const args = parseArguments('1.0.0', ['node', 'main.js']);
35+
const args = parseArguments('1.0.0', ['node', 'main.js'], {});
3636
assert.deepStrictEqual(args, {
3737
...defaultArgs,
3838
_: [],
@@ -43,12 +43,11 @@ describe('cli args parsing', () => {
4343
});
4444

4545
it('parses with browser url', async () => {
46-
const args = parseArguments('1.0.0', [
47-
'node',
48-
'main.js',
49-
'--browserUrl',
50-
'http://localhost:3000',
51-
]);
46+
const args = parseArguments(
47+
'1.0.0',
48+
['node', 'main.js', '--browserUrl', 'http://localhost:3000'],
49+
{},
50+
);
5251
assert.deepStrictEqual(args, {
5352
...defaultArgs,
5453
_: [],
@@ -61,12 +60,11 @@ describe('cli args parsing', () => {
6160
});
6261

6362
it('parses with user data dir', async () => {
64-
const args = parseArguments('1.0.0', [
65-
'node',
66-
'main.js',
67-
'--user-data-dir',
68-
'/tmp/chrome-profile',
69-
]);
63+
const args = parseArguments(
64+
'1.0.0',
65+
['node', 'main.js', '--user-data-dir', '/tmp/chrome-profile'],
66+
{},
67+
);
7068
assert.deepStrictEqual(args, {
7169
...defaultArgs,
7270
_: [],
@@ -79,12 +77,11 @@ describe('cli args parsing', () => {
7977
});
8078

8179
it('parses an empty browser url', async () => {
82-
const args = parseArguments('1.0.0', [
83-
'node',
84-
'main.js',
85-
'--browserUrl',
86-
'',
87-
]);
80+
const args = parseArguments(
81+
'1.0.0',
82+
['node', 'main.js', '--browserUrl', ''],
83+
{},
84+
);
8885
assert.deepStrictEqual(args, {
8986
...defaultArgs,
9087
_: [],
@@ -98,12 +95,11 @@ describe('cli args parsing', () => {
9895
});
9996

10097
it('parses with executable path', async () => {
101-
const args = parseArguments('1.0.0', [
102-
'node',
103-
'main.js',
104-
'--executablePath',
105-
'/tmp/test 123/chrome',
106-
]);
98+
const args = parseArguments(
99+
'1.0.0',
100+
['node', 'main.js', '--executablePath', '/tmp/test 123/chrome'],
101+
{},
102+
);
107103
assert.deepStrictEqual(args, {
108104
...defaultArgs,
109105
_: [],
@@ -116,12 +112,11 @@ describe('cli args parsing', () => {
116112
});
117113

118114
it('parses viewport', async () => {
119-
const args = parseArguments('1.0.0', [
120-
'node',
121-
'main.js',
122-
'--viewport',
123-
'888x777',
124-
]);
115+
const args = parseArguments(
116+
'1.0.0',
117+
['node', 'main.js', '--viewport', '888x777'],
118+
{},
119+
);
125120
assert.deepStrictEqual(args, {
126121
...defaultArgs,
127122
_: [],
@@ -136,12 +131,16 @@ describe('cli args parsing', () => {
136131
});
137132

138133
it('parses chrome args', async () => {
139-
const args = parseArguments('1.0.0', [
140-
'node',
141-
'main.js',
142-
`--chrome-arg='--no-sandbox'`,
143-
`--chrome-arg='--disable-setuid-sandbox'`,
144-
]);
134+
const args = parseArguments(
135+
'1.0.0',
136+
[
137+
'node',
138+
'main.js',
139+
`--chrome-arg='--no-sandbox'`,
140+
`--chrome-arg='--disable-setuid-sandbox'`,
141+
],
142+
{},
143+
);
145144
assert.deepStrictEqual(args, {
146145
...defaultArgs,
147146
_: [],
@@ -154,12 +153,16 @@ describe('cli args parsing', () => {
154153
});
155154

156155
it('parses ignore chrome args', async () => {
157-
const args = parseArguments('1.0.0', [
158-
'node',
159-
'main.js',
160-
`--ignore-default-chrome-arg='--disable-extensions'`,
161-
`--ignore-default-chrome-arg='--disable-cancel-all-touches'`,
162-
]);
156+
const args = parseArguments(
157+
'1.0.0',
158+
[
159+
'node',
160+
'main.js',
161+
`--ignore-default-chrome-arg='--disable-extensions'`,
162+
`--ignore-default-chrome-arg='--disable-cancel-all-touches'`,
163+
],
164+
{},
165+
);
163166
assert.deepStrictEqual(args, {
164167
...defaultArgs,
165168
_: [],
@@ -178,12 +181,16 @@ describe('cli args parsing', () => {
178181
});
179182

180183
it('parses wsEndpoint with ws:// protocol', async () => {
181-
const args = parseArguments('1.0.0', [
182-
'node',
183-
'main.js',
184-
'--wsEndpoint',
185-
'ws://127.0.0.1:9222/devtools/browser/abc123',
186-
]);
184+
const args = parseArguments(
185+
'1.0.0',
186+
[
187+
'node',
188+
'main.js',
189+
'--wsEndpoint',
190+
'ws://127.0.0.1:9222/devtools/browser/abc123',
191+
],
192+
{},
193+
);
187194
assert.deepStrictEqual(args, {
188195
...defaultArgs,
189196
_: [],
@@ -196,12 +203,16 @@ describe('cli args parsing', () => {
196203
});
197204

198205
it('parses wsEndpoint with wss:// protocol', async () => {
199-
const args = parseArguments('1.0.0', [
200-
'node',
201-
'main.js',
202-
'--wsEndpoint',
203-
'wss://example.com:9222/devtools/browser/abc123',
204-
]);
206+
const args = parseArguments(
207+
'1.0.0',
208+
[
209+
'node',
210+
'main.js',
211+
'--wsEndpoint',
212+
'wss://example.com:9222/devtools/browser/abc123',
213+
],
214+
{},
215+
);
205216
assert.deepStrictEqual(args, {
206217
...defaultArgs,
207218
_: [],
@@ -214,26 +225,30 @@ describe('cli args parsing', () => {
214225
});
215226

216227
it('parses wsHeaders with valid JSON', async () => {
217-
const args = parseArguments('1.0.0', [
218-
'node',
219-
'main.js',
220-
'--wsEndpoint',
221-
'ws://127.0.0.1:9222/devtools/browser/abc123',
222-
'--wsHeaders',
223-
'{"Authorization":"Bearer token","X-Custom":"value"}',
224-
]);
228+
const args = parseArguments(
229+
'1.0.0',
230+
[
231+
'node',
232+
'main.js',
233+
'--wsEndpoint',
234+
'ws://127.0.0.1:9222/devtools/browser/abc123',
235+
'--wsHeaders',
236+
'{"Authorization":"Bearer token","X-Custom":"value"}',
237+
],
238+
{},
239+
);
225240
assert.deepStrictEqual(args.wsHeaders, {
226241
Authorization: 'Bearer token',
227242
'X-Custom': 'value',
228243
});
229244
});
230245

231246
it('parses disabled category', async () => {
232-
const args = parseArguments('1.0.0', [
233-
'node',
234-
'main.js',
235-
'--no-category-emulation',
236-
]);
247+
const args = parseArguments(
248+
'1.0.0',
249+
['node', 'main.js', '--no-category-emulation'],
250+
{},
251+
);
237252
assert.deepStrictEqual(args, {
238253
...defaultArgs,
239254
_: [],
@@ -245,7 +260,11 @@ describe('cli args parsing', () => {
245260
});
246261
});
247262
it('parses auto-connect', async () => {
248-
const args = parseArguments('1.0.0', ['node', 'main.js', '--auto-connect']);
263+
const args = parseArguments(
264+
'1.0.0',
265+
['node', 'main.js', '--auto-connect'],
266+
{},
267+
);
249268
assert.deepStrictEqual(args, {
250269
...defaultArgs,
251270
_: [],
@@ -259,23 +278,51 @@ describe('cli args parsing', () => {
259278

260279
it('parses usage statistics flag', async () => {
261280
// Test default (should be true).
262-
const defaultArgs = parseArguments('1.0.0', ['node', 'main.js']);
281+
const defaultArgs = parseArguments('1.0.0', ['node', 'main.js'], {});
263282
assert.strictEqual(defaultArgs.usageStatistics, true);
264283

265284
// Test enabling it
266-
const enabledArgs = parseArguments('1.0.0', [
267-
'node',
268-
'main.js',
269-
'--usage-statistics',
270-
]);
285+
const enabledArgs = parseArguments(
286+
'1.0.0',
287+
['node', 'main.js', '--usage-statistics'],
288+
{},
289+
);
271290
assert.strictEqual(enabledArgs.usageStatistics, true);
272291

273292
// Test disabling it
274-
const disabledArgs = parseArguments('1.0.0', [
275-
'node',
276-
'main.js',
277-
'--no-usage-statistics',
278-
]);
293+
const disabledArgs = parseArguments(
294+
'1.0.0',
295+
['node', 'main.js', '--no-usage-statistics'],
296+
{},
297+
);
298+
assert.strictEqual(disabledArgs.usageStatistics, false);
299+
});
300+
301+
it('respects env variable', async () => {
302+
// Test default (should be true).
303+
const defaultArgs = parseArguments('1.0.0', ['node', 'main.js'], {
304+
CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS: 'true',
305+
});
306+
assert.strictEqual(defaultArgs.usageStatistics, false);
307+
308+
// Test enabling it
309+
const enabledArgs = parseArguments(
310+
'1.0.0',
311+
['node', 'main.js', '--usage-statistics'],
312+
{
313+
CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS: 'true',
314+
},
315+
);
316+
assert.strictEqual(enabledArgs.usageStatistics, false);
317+
318+
// Test disabling it
319+
const disabledArgs = parseArguments(
320+
'1.0.0',
321+
['node', 'main.js', '--no-usage-statistics'],
322+
{
323+
CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS: 'true',
324+
},
325+
);
279326
assert.strictEqual(disabledArgs.usageStatistics, false);
280327
});
281328

@@ -284,18 +331,18 @@ describe('cli args parsing', () => {
284331
assert.strictEqual(defaultArgs.performanceCrux, true);
285332

286333
// force enable
287-
const enabledArgs = parseArguments('1.0.0', [
288-
'node',
289-
'main.js',
290-
'--performance-crux',
291-
]);
334+
const enabledArgs = parseArguments(
335+
'1.0.0',
336+
['node', 'main.js', '--performance-crux'],
337+
{},
338+
);
292339
assert.strictEqual(enabledArgs.performanceCrux, true);
293340

294-
const disabledArgs = parseArguments('1.0.0', [
295-
'node',
296-
'main.js',
297-
'--no-performance-crux',
298-
]);
341+
const disabledArgs = parseArguments(
342+
'1.0.0',
343+
['node', 'main.js', '--no-performance-crux'],
344+
{},
345+
);
299346
assert.strictEqual(disabledArgs.performanceCrux, false);
300347
});
301348
});

‎tests/index.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ describe('e2e', () => {
4040
executablePath(),
4141
...extraArgs,
4242
],
43+
env: {...process.env, CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS: 'true'},
4344
});
4445
const client = new Client(
4546
{

0 commit comments

Comments
 (0)